Re: [PATCH v3 00/12] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support
[Cc'ing Sasha]
On Thu, 2020-07-09 at 01:18 -0500, Tyler Hicks wrote:
> I envision patches 1-7 going to stable. The series is ordered in a way
> that has all the fixes up front, followed by cleanups, followed by the
> feature patch. The breakdown of patches looks like so:
>
> Memory leak fixes: 1-3
> Parser strictness fixes: 4-7
> Code cleanups made possible by the fixes: 8-11
> Extend KEXEC_CMDLINE rule support: 12
I agree they should be backported, but they don't apply cleanly before
linux-5.6. The changes aren't that major. Some patch hunks apply
cleanly, but won't compile, while others patch hunks need to be
dropped based on when the feature was upstreamed. For these reasons,
I'm not Cc'ing stable.
Feature upstreamed:
- LSM policy update: linux 5.3
- key command line: linux 5.3
- blacklist: linux 5.5
- keyrings: linux 5.6
For Linux 5.3:
- Dependency on backporting commit 483ec26eed42 ("ima: ima/lsm policy
rule loading logic bug fixes") to apply " ima: Free the entire rule if
it fails to parse".
Mimi
Re: [PATCH v3 00/12] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support
On 2020-07-17 00:31:33, Mimi Zohar wrote: > On Thu, 2020-07-09 at 01:18 -0500, Tyler Hicks wrote: > > This series ultimately extends the supported IMA rule conditionals for > > the KEXEC_CMDLINE hook function. As of today, there's an imbalance in > > IMA language conditional support for KEXEC_CMDLINE rules in comparison > > to KEXEC_KERNEL_CHECK and KEXEC_INITRAMFS_CHECK rules. The KEXEC_CMDLINE > > rules do not support *any* conditionals so you cannot have a sequence of > > rules like this: > > > > dont_measure func=KEXEC_KERNEL_CHECK obj_type=foo_t > > dont_measure func=KEXEC_INITRAMFS_CHECK obj_type=foo_t > > dont_measure func=KEXEC_CMDLINE obj_type=foo_t > > measure func=KEXEC_KERNEL_CHECK > > measure func=KEXEC_INITRAMFS_CHECK > > measure func=KEXEC_CMDLINE > > > > Instead, KEXEC_CMDLINE rules can only be measured or not measured and > > there's no additional flexibility in today's implementation of the > > KEXEC_CMDLINE hook function. > > > > With this series, the above sequence of rules becomes valid and any > > calls to kexec_file_load() with a kernel and initramfs inode type of > > foo_t will not be measured (that includes the kernel cmdline buffer) > > while all other objects given to a kexec_file_load() syscall will be > > measured. There's obviously not an inode directly associated with the > > kernel cmdline buffer but this patch series ties the inode based > > decision making for KEXEC_CMDLINE to the kernel's inode. I think this > > will be intuitive to policy authors. > > > > While reading IMA code and preparing to make this change, I realized > > that the buffer based hook functions (KEXEC_CMDLINE and KEY_CHECK) are > > quite special in comparison to longer standing hook functions. These > > buffer based hook functions can only support measure actions and there > > are some restrictions on the conditionals that they support. However, > > the rule parser isn't enforcing any of those restrictions and IMA policy > > authors wouldn't have any immediate way of knowing that the policy that > > they wrote is invalid. For example, the sequence of rules above parses > > successfully in today's kernel but the > > "dont_measure func=KEXEC_CMDLINE ..." rule is incorrectly handled in > > ima_match_rules(). The dont_measure rule is *always* considered to be a > > match so, surprisingly, no KEXEC_CMDLINE measurements are made. > > > > While making the rule parser more strict, I realized that the parser > > does not correctly free all of the allocated memory associated with an > > ima_rule_entry when going down some error paths. Invalid policy loaded > > by the policy administrator could result in small memory leaks. > > > > I envision patches 1-7 going to stable. The series is ordered in a way > > that has all the fixes up front, followed by cleanups, followed by the > > feature patch. The breakdown of patches looks like so: > > > > Memory leak fixes: 1-3 > > Parser strictness fixes: 4-7 > > Code cleanups made possible by the fixes: 8-11 > > Extend KEXEC_CMDLINE rule support: 12 > > Thanks, Tyler. This is a really nice patch set. The patches are now > in the "next-integrity-testing" branch. Thank you for all the helpful review comments. You know where to find me if any bugs pop up during testing. :) Tyler > > Mimi
Re: [PATCH v3 00/12] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support
On Thu, 2020-07-09 at 01:18 -0500, Tyler Hicks wrote: > This series ultimately extends the supported IMA rule conditionals for > the KEXEC_CMDLINE hook function. As of today, there's an imbalance in > IMA language conditional support for KEXEC_CMDLINE rules in comparison > to KEXEC_KERNEL_CHECK and KEXEC_INITRAMFS_CHECK rules. The KEXEC_CMDLINE > rules do not support *any* conditionals so you cannot have a sequence of > rules like this: > > dont_measure func=KEXEC_KERNEL_CHECK obj_type=foo_t > dont_measure func=KEXEC_INITRAMFS_CHECK obj_type=foo_t > dont_measure func=KEXEC_CMDLINE obj_type=foo_t > measure func=KEXEC_KERNEL_CHECK > measure func=KEXEC_INITRAMFS_CHECK > measure func=KEXEC_CMDLINE > > Instead, KEXEC_CMDLINE rules can only be measured or not measured and > there's no additional flexibility in today's implementation of the > KEXEC_CMDLINE hook function. > > With this series, the above sequence of rules becomes valid and any > calls to kexec_file_load() with a kernel and initramfs inode type of > foo_t will not be measured (that includes the kernel cmdline buffer) > while all other objects given to a kexec_file_load() syscall will be > measured. There's obviously not an inode directly associated with the > kernel cmdline buffer but this patch series ties the inode based > decision making for KEXEC_CMDLINE to the kernel's inode. I think this > will be intuitive to policy authors. > > While reading IMA code and preparing to make this change, I realized > that the buffer based hook functions (KEXEC_CMDLINE and KEY_CHECK) are > quite special in comparison to longer standing hook functions. These > buffer based hook functions can only support measure actions and there > are some restrictions on the conditionals that they support. However, > the rule parser isn't enforcing any of those restrictions and IMA policy > authors wouldn't have any immediate way of knowing that the policy that > they wrote is invalid. For example, the sequence of rules above parses > successfully in today's kernel but the > "dont_measure func=KEXEC_CMDLINE ..." rule is incorrectly handled in > ima_match_rules(). The dont_measure rule is *always* considered to be a > match so, surprisingly, no KEXEC_CMDLINE measurements are made. > > While making the rule parser more strict, I realized that the parser > does not correctly free all of the allocated memory associated with an > ima_rule_entry when going down some error paths. Invalid policy loaded > by the policy administrator could result in small memory leaks. > > I envision patches 1-7 going to stable. The series is ordered in a way > that has all the fixes up front, followed by cleanups, followed by the > feature patch. The breakdown of patches looks like so: > > Memory leak fixes: 1-3 > Parser strictness fixes: 4-7 > Code cleanups made possible by the fixes: 8-11 > Extend KEXEC_CMDLINE rule support: 12 Thanks, Tyler. This is a really nice patch set. The patches are now in the "next-integrity-testing" branch. Mimi
