Messages in this thread | | | Subject | Re: [PATCH v5 1/4] IMA: Add func to measure LSM state and policy | From | Lakshmi Ramasubramanian <> | Date | Thu, 30 Jul 2020 08:15:34 -0700 |
| |
On 7/30/20 8:02 AM, Tyler Hicks wrote:
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >> index 07f033634b27..a0f5c39d9084 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -442,13 +442,20 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, >> { >> int i; >> >> - if (func == KEY_CHECK) { >> - return (rule->flags & IMA_FUNC) && (rule->func == func) && >> - ima_match_keyring(rule, keyring, cred); >> - } >> if ((rule->flags & IMA_FUNC) && >> (rule->func != func && func != POST_SETATTR)) >> return false; >> + >> + switch (func) { >> + case KEY_CHECK: >> + return ima_match_keyring(rule, keyring, cred); >> + case LSM_STATE: >> + case LSM_POLICY: >> + return true; >> + default: >> + break; >> + } >> + >> if ((rule->flags & IMA_MASK) && >> (rule->mask != mask && func != POST_SETATTR)) >> return false; >> @@ -1044,6 +1051,18 @@ static bool ima_validate_rule(struct ima_rule_entry *entry) >> if (ima_rule_contains_lsm_cond(entry)) >> return false; >> >> + break; >> + case LSM_STATE: >> + case LSM_POLICY: >> + 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; >> default: >> return false; >> @@ -1176,6 +1195,10 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) >> entry->func = KEXEC_CMDLINE; >> else if (strcmp(args[0].from, "KEY_CHECK") == 0) >> entry->func = KEY_CHECK; >> + else if (strcmp(args[0].from, "LSM_STATE") == 0) >> + entry->func = LSM_STATE; >> + else if (strcmp(args[0].from, "LSM_POLICY") == 0) >> + entry->func = LSM_POLICY; > > This patch generally looks really good to me with the exception of one > thing... > > We should only accept rules with these specified hook functions when an > LSM that has measurement support is enabled. This messes up the ordering > of your patch series but it could be as simple as doing this: > > else if (IS_ENABLED(CONFIG_SECURITY_SELINUX) && > strcmp(args[0].from, "LSM_STATE") == 0) > entry->func = LSM_STATE; > > Or you could do something a little more complex, like what's done with > CONFIG_IMA_LSM_RULES. You could create a CONFIG_IMA_MEASURE_LSM option > that's default enabled but depends on CONFIG_SECURITY_SELINUX and then > check for IS_ENABLED(CONFIG_IMA_MEASURE_LSM) in ima_parse_rule(). > > I'd personally opt for just placing the > IS_ENABLED(CONFIG_SECURITY_SELINUX) check directly into > ima_parse_rule(). >
The LSM hook can be used by any security module (not just SELinux) to measure their data.
I have implemented measurement in SELinux to illustrate the usage. Maybe, I can add the check you have suggested for now and when more security modules start using this IMA policy additional checks can be added as appropriate.
thanks, -lakshmi
| |