| Message ID | 20200626223900.253615-10-tyhicks@linux.microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support | expand |
On 6/26/20 3:38 PM, Tyler Hicks wrote: > Use ima_validate_rule() to ensure that the combination of a hook > function and the keyrings conditional is valid and that the keyrings > conditional is not specified without an explicit KEY_CHECK func > conditional. This is a code cleanup and has no user-facing change. In addition to checking for func=KEY_CHECK and the keyrings conditional, the patch also validates the flags for other IMA hooks (such as KEXEC_KERNEL_CHECK, POLICY_CHECK, etc.) Would be good to mention that in the patch description. -lakshmi > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > --- > > * v2 > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be > present in the rule entry flags for non-buffer hook functions. > > security/integrity/ima/ima_policy.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 8cdca2399d59..43d49ad958fb 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > case KEXEC_KERNEL_CHECK: > case KEXEC_INITRAMFS_CHECK: > case POLICY_CHECK: > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | > + IMA_UID | IMA_FOWNER | IMA_FSUUID | > + IMA_INMASK | IMA_EUID | IMA_PCR | > + IMA_FSNAME | IMA_DIGSIG_REQUIRED | > + IMA_PERMIT_DIRECTIO | > + IMA_MODSIG_ALLOWED | > + IMA_CHECK_BLACKLIST)) > + return false; > + > break; > case KEXEC_CMDLINE: > if (entry->action & ~(MEASURE | DONT_MEASURE)) > @@ -1027,7 +1036,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > default: > return false; > } > - } > + } else if (entry->flags & IMA_KEYRINGS) > + return false; > > return true; > } > @@ -1209,7 +1219,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > keyrings_len = strlen(args[0].from) + 1; > > if ((entry->keyrings) || > - (entry->func != KEY_CHECK) || > (keyrings_len < 2)) { > result = -EINVAL; > break; >
On 2020-06-27 16:49:46, Lakshmi Ramasubramanian wrote: > On 6/26/20 3:38 PM, Tyler Hicks wrote: > > > Use ima_validate_rule() to ensure that the combination of a hook > > function and the keyrings conditional is valid and that the keyrings > > conditional is not specified without an explicit KEY_CHECK func > > conditional. This is a code cleanup and has no user-facing change. > > In addition to checking for func=KEY_CHECK and the keyrings conditional, the > patch also validates the flags for other IMA hooks (such as > KEXEC_KERNEL_CHECK, POLICY_CHECK, etc.) Would be good to mention that in the > patch description. It actually doesn't do any additional validation of other IMA hooks at this time. That check on entry->flags is an allowlist of every possible conditional flag except IMA_KEYRINGS. The ima_parse_rule() function is already validating all of these conditional flags. Tyler > > -lakshmi > > > > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > > --- > > > > * v2 > > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, > > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be > > present in the rule entry flags for non-buffer hook functions. > > > > security/integrity/ima/ima_policy.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > index 8cdca2399d59..43d49ad958fb 100644 > > --- a/security/integrity/ima/ima_policy.c > > +++ b/security/integrity/ima/ima_policy.c > > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > > case KEXEC_KERNEL_CHECK: > > case KEXEC_INITRAMFS_CHECK: > > case POLICY_CHECK: > > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | > > + IMA_UID | IMA_FOWNER | IMA_FSUUID | > > + IMA_INMASK | IMA_EUID | IMA_PCR | > > + IMA_FSNAME | IMA_DIGSIG_REQUIRED | > > + IMA_PERMIT_DIRECTIO | > > + IMA_MODSIG_ALLOWED | > > + IMA_CHECK_BLACKLIST)) > > + return false; > > + > > break; > > case KEXEC_CMDLINE: > > if (entry->action & ~(MEASURE | DONT_MEASURE)) > > @@ -1027,7 +1036,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > > default: > > return false; > > } > > - } > > + } else if (entry->flags & IMA_KEYRINGS) > > + return false; > > return true; > > } > > @@ -1209,7 +1219,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > > keyrings_len = strlen(args[0].from) + 1; > > if ((entry->keyrings) || > > - (entry->func != KEY_CHECK) || > > (keyrings_len < 2)) { > > result = -EINVAL; > > break; > >
On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote: > Use ima_validate_rule() to ensure that the combination of a hook > function and the keyrings conditional is valid and that the keyrings > conditional is not specified without an explicit KEY_CHECK func > conditional. This is a code cleanup and has no user-facing change. > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > --- > > * v2 > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be > present in the rule entry flags for non-buffer hook functions. > > security/integrity/ima/ima_policy.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 8cdca2399d59..43d49ad958fb 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > case KEXEC_KERNEL_CHECK: > case KEXEC_INITRAMFS_CHECK: > case POLICY_CHECK: > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | > + IMA_UID | IMA_FOWNER | IMA_FSUUID | > + IMA_INMASK | IMA_EUID | IMA_PCR | > + IMA_FSNAME | IMA_DIGSIG_REQUIRED | > + IMA_PERMIT_DIRECTIO | > + IMA_MODSIG_ALLOWED | > + IMA_CHECK_BLACKLIST)) Other than KEYRINGS, this patch should continue to behave the same. However, this list gives the impressions that all of these flags are permitted on all of the above flags, which isn't true. For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK. Both should only be allowed on APPRAISE action rules. IMA_PCR should be limited to MEASURE action rules. IMA_DIGSIG_REQUIRED should be limited to APPRAISE action rules. > + return false; > + > break; > case KEXEC_CMDLINE: > if (entry->action & ~(MEASURE | DONT_MEASURE)) > @@ -1027,7 +1036,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > default: > return false; > } > - } > + } else if (entry->flags & IMA_KEYRINGS) > + return false; IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST need to be added here as well. Mimi > > return true; > } > @@ -1209,7 +1219,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > keyrings_len = strlen(args[0].from) + 1; > > if ((entry->keyrings) || > - (entry->func != KEY_CHECK) || > (keyrings_len < 2)) { > result = -EINVAL; > break;
On 2020-06-30 19:07:29, Mimi Zohar wrote: > On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote: > > Use ima_validate_rule() to ensure that the combination of a hook > > function and the keyrings conditional is valid and that the keyrings > > conditional is not specified without an explicit KEY_CHECK func > > conditional. This is a code cleanup and has no user-facing change. > > > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > > --- > > > > * v2 > > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, > > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be > > present in the rule entry flags for non-buffer hook functions. > > > > security/integrity/ima/ima_policy.c | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > index 8cdca2399d59..43d49ad958fb 100644 > > --- a/security/integrity/ima/ima_policy.c > > +++ b/security/integrity/ima/ima_policy.c > > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > > case KEXEC_KERNEL_CHECK: > > case KEXEC_INITRAMFS_CHECK: > > case POLICY_CHECK: > > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | > > + IMA_UID | IMA_FOWNER | IMA_FSUUID | > > + IMA_INMASK | IMA_EUID | IMA_PCR | > > + IMA_FSNAME | IMA_DIGSIG_REQUIRED | > > + IMA_PERMIT_DIRECTIO | > > + IMA_MODSIG_ALLOWED | > > + IMA_CHECK_BLACKLIST)) > > Other than KEYRINGS, this patch should continue to behave the same. > However, this list gives the impressions that all of these flags are > permitted on all of the above flags, which isn't true. > > For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited > to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK. Just to clarify, are both IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST limited to KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK, and MODULE_CHECK? That's what ima_hook_supports_modsig() suggests. > Both should only be allowed on APPRAISE action rules. For completeness, it looks like DONT_APPRAISE should not be allowed. > IMA_PCR should be limited to MEASURE action rules. It looks like DONT_MEASURE should not be allowed. > IMA_DIGSIG_REQUIRED should be limited to APPRAISE action rules. It looks like DONT_APPRAISE should not be allowed. > > > + return false; > > + > > break; > > case KEXEC_CMDLINE: > > if (entry->action & ~(MEASURE | DONT_MEASURE)) > > @@ -1027,7 +1036,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > > default: > > return false; > > } > > - } > > + } else if (entry->flags & IMA_KEYRINGS) > > + return false; > > IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST need to be added here as > well. That makes sense. Tyler > > Mimi > > > > > return true; > > } > > @@ -1209,7 +1219,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > > keyrings_len = strlen(args[0].from) + 1; > > > > if ((entry->keyrings) || > > - (entry->func != KEY_CHECK) || > > (keyrings_len < 2)) { > > result = -EINVAL; > > break;
On Thu, 2020-07-02 at 17:16 -0500, Tyler Hicks wrote: > On 2020-06-30 19:07:29, Mimi Zohar wrote: > > On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote: > > > Use ima_validate_rule() to ensure that the combination of a hook > > > function and the keyrings conditional is valid and that the keyrings > > > conditional is not specified without an explicit KEY_CHECK func > > > conditional. This is a code cleanup and has no user-facing change. > > > > > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > > > --- > > > > > > * v2 > > > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, > > > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be > > > present in the rule entry flags for non-buffer hook functions. > > > > > > security/integrity/ima/ima_policy.c | 13 +++++++++++-- > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > > index 8cdca2399d59..43d49ad958fb 100644 > > > --- a/security/integrity/ima/ima_policy.c > > > +++ b/security/integrity/ima/ima_policy.c > > > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > > > case KEXEC_KERNEL_CHECK: > > > case KEXEC_INITRAMFS_CHECK: > > > case POLICY_CHECK: > > > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | > > > + IMA_UID | IMA_FOWNER | IMA_FSUUID | > > > + IMA_INMASK | IMA_EUID | IMA_PCR | > > > + IMA_FSNAME | IMA_DIGSIG_REQUIRED | > > > + IMA_PERMIT_DIRECTIO | > > > + IMA_MODSIG_ALLOWED | > > > + IMA_CHECK_BLACKLIST)) > > > > Other than KEYRINGS, this patch should continue to behave the same. > > However, this list gives the impressions that all of these flags are > > permitted on all of the above flags, which isn't true. > > > > For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited > > to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK. > > Just to clarify, are both IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST > limited to KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK, and MODULE_CHECK? > That's what ima_hook_supports_modsig() suggests. Theoretically that is true, but I have no idea how you would append a signature to the kexec boot command line. The only users of appended signatures are currently kernel modules and the kexec'ed kernel image. > > > Both should only be allowed on APPRAISE action rules. > > For completeness, it looks like DONT_APPRAISE should not be allowed. Good point. > > > IMA_PCR should be limited to MEASURE action rules. > > It looks like DONT_MEASURE should not be allowed. The TPM PCR isn't a file attribute. > > > IMA_DIGSIG_REQUIRED should be limited to APPRAISE action rules. > > It looks like DONT_APPRAISE should not be allowed. Right, in all of these cases the DONT_XXXX isn't applicable. Mimi
On 2020-07-03 10:15:32, Mimi Zohar wrote: > On Thu, 2020-07-02 at 17:16 -0500, Tyler Hicks wrote: > > On 2020-06-30 19:07:29, Mimi Zohar wrote: > > > On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote: > > > > Use ima_validate_rule() to ensure that the combination of a hook > > > > function and the keyrings conditional is valid and that the keyrings > > > > conditional is not specified without an explicit KEY_CHECK func > > > > conditional. This is a code cleanup and has no user-facing change. > > > > > > > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > > > > --- > > > > > > > > * v2 > > > > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, > > > > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be > > > > present in the rule entry flags for non-buffer hook functions. > > > > > > > > security/integrity/ima/ima_policy.c | 13 +++++++++++-- > > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > > > index 8cdca2399d59..43d49ad958fb 100644 > > > > --- a/security/integrity/ima/ima_policy.c > > > > +++ b/security/integrity/ima/ima_policy.c > > > > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > > > > case KEXEC_KERNEL_CHECK: > > > > case KEXEC_INITRAMFS_CHECK: > > > > case POLICY_CHECK: > > > > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | > > > > + IMA_UID | IMA_FOWNER | IMA_FSUUID | > > > > + IMA_INMASK | IMA_EUID | IMA_PCR | > > > > + IMA_FSNAME | IMA_DIGSIG_REQUIRED | > > > > + IMA_PERMIT_DIRECTIO | > > > > + IMA_MODSIG_ALLOWED | > > > > + IMA_CHECK_BLACKLIST)) > > > > > > Other than KEYRINGS, this patch should continue to behave the same. > > > However, this list gives the impressions that all of these flags are > > > permitted on all of the above flags, which isn't true. > > > > > > For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited > > > to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK. > > > > Just to clarify, are both IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST > > limited to KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK, and MODULE_CHECK? > > That's what ima_hook_supports_modsig() suggests. > > Theoretically that is true, but I have no idea how you would append a > signature to the kexec boot command line. The only users of appended > signatures are currently kernel modules and the kexec'ed kernel image. The discrepancy was with KEXEC_INITRAMFS_CHECK, not KEXEC_CMDLINE. I now see that there's no support for initramfs signature verification in the kexec code so I'll assume that ima_hook_supports_modsig() is wrong and limit IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST to the KEXEC_KERNEL_CHECK and MODULE_CHECK actions, as you originally suggested. Tyler > > > > > > Both should only be allowed on APPRAISE action rules. > > > > For completeness, it looks like DONT_APPRAISE should not be allowed. > > Good point. > > > > > > IMA_PCR should be limited to MEASURE action rules. > > > > It looks like DONT_MEASURE should not be allowed. > > The TPM PCR isn't a file attribute. > > > > > > IMA_DIGSIG_REQUIRED should be limited to APPRAISE action rules. > > > > It looks like DONT_APPRAISE should not be allowed. > > Right, in all of these cases the DONT_XXXX isn't applicable. > > Mimi
On Mon, 2020-07-06 at 08:18 -0500, Tyler Hicks wrote: > On 2020-07-03 10:15:32, Mimi Zohar wrote: > > On Thu, 2020-07-02 at 17:16 -0500, Tyler Hicks wrote: > > > On 2020-06-30 19:07:29, Mimi Zohar wrote: > > > > On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote: > > > > > Use ima_validate_rule() to ensure that the combination of a hook > > > > > function and the keyrings conditional is valid and that the keyrings > > > > > conditional is not specified without an explicit KEY_CHECK func > > > > > conditional. This is a code cleanup and has no user-facing change. > > > > > > > > > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > > > > > --- > > > > > > > > > > * v2 > > > > > - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, > > > > > IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be > > > > > present in the rule entry flags for non-buffer hook functions. > > > > > > > > > > security/integrity/ima/ima_policy.c | 13 +++++++++++-- > > > > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > > > > index 8cdca2399d59..43d49ad958fb 100644 > > > > > --- a/security/integrity/ima/ima_policy.c > > > > > +++ b/security/integrity/ima/ima_policy.c > > > > > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > > > > > case KEXEC_KERNEL_CHECK: > > > > > case KEXEC_INITRAMFS_CHECK: > > > > > case POLICY_CHECK: > > > > > + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | > > > > > + IMA_UID | IMA_FOWNER | IMA_FSUUID | > > > > > + IMA_INMASK | IMA_EUID | IMA_PCR | > > > > > + IMA_FSNAME | IMA_DIGSIG_REQUIRED | > > > > > + IMA_PERMIT_DIRECTIO | > > > > > + IMA_MODSIG_ALLOWED | > > > > > + IMA_CHECK_BLACKLIST)) > > > > > > > > Other than KEYRINGS, this patch should continue to behave the same. > > > > However, this list gives the impressions that all of these flags are > > > > permitted on all of the above flags, which isn't true. > > > > > > > > For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited > > > > to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK. > > > > > > Just to clarify, are both IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST > > > limited to KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK, and MODULE_CHECK? > > > That's what ima_hook_supports_modsig() suggests. > > > > Theoretically that is true, but I have no idea how you would append a > > signature to the kexec boot command line. The only users of appended > > signatures are currently kernel modules and the kexec'ed kernel image. > > The discrepancy was with KEXEC_INITRAMFS_CHECK, not KEXEC_CMDLINE. I now > see that there's no support for initramfs signature verification in the > kexec code so I'll assume that ima_hook_supports_modsig() is wrong and > limit IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST to the > KEXEC_KERNEL_CHECK and MODULE_CHECK actions, as you originally > suggested. My mistake. Yes, both the kexec kernel image and the initramfs read the respective file into memory and can be signed either with an imasig or modsig. Refer to kernel_read_file_from_fd(). Mimi
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8cdca2399d59..43d49ad958fb 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) case KEXEC_KERNEL_CHECK: case KEXEC_INITRAMFS_CHECK: case POLICY_CHECK: + if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC | + IMA_UID | IMA_FOWNER | IMA_FSUUID | + IMA_INMASK | IMA_EUID | IMA_PCR | + IMA_FSNAME | IMA_DIGSIG_REQUIRED | + IMA_PERMIT_DIRECTIO | + IMA_MODSIG_ALLOWED | + IMA_CHECK_BLACKLIST)) + return false; + break; case KEXEC_CMDLINE: if (entry->action & ~(MEASURE | DONT_MEASURE)) @@ -1027,7 +1036,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) default: return false; } - } + } else if (entry->flags & IMA_KEYRINGS) + return false; return true; } @@ -1209,7 +1219,6 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) keyrings_len = strlen(args[0].from) + 1; if ((entry->keyrings) || - (entry->func != KEY_CHECK) || (keyrings_len < 2)) { result = -EINVAL; break;
Use ima_validate_rule() to ensure that the combination of a hook function and the keyrings conditional is valid and that the keyrings conditional is not specified without an explicit KEY_CHECK func conditional. This is a code cleanup and has no user-facing change. Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> --- * v2 - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO, IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be present in the rule entry flags for non-buffer hook functions. security/integrity/ima/ima_policy.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)