| Message ID | 20200626223900.253615-6-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: > The KEXEC_CMDLINE hook function only supports the pcr conditional. Make > this clear at policy load so that IMA policy authors don't assume that > other conditionals are supported. > > Since KEXEC_CMDLINE's inception, ima_match_rules() has always returned > true on any loaded KEXEC_CMDLINE rule without any consideration for > other conditionals present in the rule. Make it clear that pcr is the > only supported KEXEC_CMDLINE conditional by returning an error during > policy load. > > An example of why this is a problem can be explained with the following > rule: > > dont_measure func=KEXEC_CMDLINE obj_type=foo_t > > An IMA policy author would have assumed that rule is valid because the > parser accepted it but the result was that measurements for all > KEXEC_CMDLINE operations would be disabled. > > Fixes: b0935123a183 ("IMA: Define a new hook to measure the kexec boot command line arguments") > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> > --- > > * v2 > - Added Mimi's Reviewed-by > > security/integrity/ima/ima_policy.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 166124d67774..676d5557af1a 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -343,6 +343,17 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry) > return 0; > } > > +static bool ima_rule_contains_lsm_cond(struct ima_rule_entry *entry) > +{ > + int i; > + > + for (i = 0; i < MAX_LSM_RULES; i++) > + if (entry->lsm[i].args_p) > + return true; > + > + return false; > +} > + > /* > * The LSM policy can be reloaded, leaving the IMA LSM based rules referring > * to the old, stale LSM policy. Update the IMA LSM based rules to reflect > @@ -993,6 +1004,16 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) > case POLICY_CHECK: > break; > case KEXEC_CMDLINE: > + if (entry->action & ~(MEASURE | DONT_MEASURE)) > + return false; > + > + if (entry->flags & ~(IMA_FUNC | IMA_PCR)) > + return false; > + > + if (ima_rule_contains_lsm_cond(entry)) > + return false; > + > + break; > case KEY_CHECK: > if (entry->action & ~(MEASURE | DONT_MEASURE)) > return false; > Reviewed-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 166124d67774..676d5557af1a 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -343,6 +343,17 @@ static int ima_lsm_update_rule(struct ima_rule_entry *entry) return 0; } +static bool ima_rule_contains_lsm_cond(struct ima_rule_entry *entry) +{ + int i; + + for (i = 0; i < MAX_LSM_RULES; i++) + if (entry->lsm[i].args_p) + return true; + + return false; +} + /* * The LSM policy can be reloaded, leaving the IMA LSM based rules referring * to the old, stale LSM policy. Update the IMA LSM based rules to reflect @@ -993,6 +1004,16 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) case POLICY_CHECK: break; case KEXEC_CMDLINE: + if (entry->action & ~(MEASURE | DONT_MEASURE)) + return false; + + if (entry->flags & ~(IMA_FUNC | IMA_PCR)) + return false; + + if (ima_rule_contains_lsm_cond(entry)) + return false; + + break; case KEY_CHECK: if (entry->action & ~(MEASURE | DONT_MEASURE)) return false;
The KEXEC_CMDLINE hook function only supports the pcr conditional. Make this clear at policy load so that IMA policy authors don't assume that other conditionals are supported. Since KEXEC_CMDLINE's inception, ima_match_rules() has always returned true on any loaded KEXEC_CMDLINE rule without any consideration for other conditionals present in the rule. Make it clear that pcr is the only supported KEXEC_CMDLINE conditional by returning an error during policy load. An example of why this is a problem can be explained with the following rule: dont_measure func=KEXEC_CMDLINE obj_type=foo_t An IMA policy author would have assumed that rule is valid because the parser accepted it but the result was that measurements for all KEXEC_CMDLINE operations would be disabled. Fixes: b0935123a183 ("IMA: Define a new hook to measure the kexec boot command line arguments") Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com> --- * v2 - Added Mimi's Reviewed-by security/integrity/ima/ima_policy.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)