| Message ID | 20210126164108.1958-5-casey@schaufler-ca.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v24,01/25] LSM: Infrastructure management of the sock security | expand |
Hi Casey, On Tue, 2021-01-26 at 08:40 -0800, Casey Schaufler wrote: > Integrity measurement may filter on security module information > and needs to be clear in the case of multiple active security > modules which applies. Provide a boot option ima_rules_lsm= to > allow the user to specify an active securty module to apply > filters to. If not specified, use the first registered module > that supports the audit_rule_match() LSM hook. Allow the user > to specify in the IMA policy an lsm= option to specify the > security module to use for a particular rule. Thanks, Casey. (This patch description line length seems short.) > > Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> > To: Mimi Zohar <zohar@linux.ibm.com> > To: linux-integrity@vger.kernel.org > --- > Documentation/ABI/testing/ima_policy | 8 +++- > security/integrity/ima/ima_policy.c | 64 ++++++++++++++++++++++------ > 2 files changed, 57 insertions(+), 15 deletions(-) > > diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy > index e35263f97fc1..a7943d40466f 100644 > --- a/Documentation/ABI/testing/ima_policy > +++ b/Documentation/ABI/testing/ima_policy > @@ -25,7 +25,7 @@ Description: > base: [[func=] [mask=] [fsmagic=] [fsuuid=] [uid=] > [euid=] [fowner=] [fsname=]] > lsm: [[subj_user=] [subj_role=] [subj_type=] > - [obj_user=] [obj_role=] [obj_type=]] > + [obj_user=] [obj_role=] [obj_type=] [lsm=]] "[lsm=]" either requires all LSM rules types (e.g. {subj/obj}_user, role, type) to be exactly the same for multiple LSMs or all of the LSM rule types are applicable to only a single LSM. Supporting multiple LSMs with exactly the same LSM labels doesn't seem worth the effort. Keep it simple - a single rule, containing any LSM rule types, is applicable to a single LSM. > option: [[appraise_type=]] [template=] [permit_directio] > [appraise_flag=] [keyrings=] > base: > @@ -114,6 +114,12 @@ Description: > > measure subj_user=_ func=FILE_CHECK mask=MAY_READ > > + It is possible to explicitly specify which security > + module a rule applies to using lsm=. If the security > + modules specified is not active on the system the rule > + will be rejected. If lsm= is not specified the first > + security module registered on the system will be assumed. > + > Example of measure rules using alternate PCRs:: > > measure func=KEXEC_KERNEL_CHECK pcr=4 > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 8002683003e6..de72b719c90c 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -82,6 +82,7 @@ struct ima_rule_entry { > void *rules[LSMBLOB_ENTRIES]; /* LSM file metadata specific */ > char *args_p; /* audit value */ > int type; /* audit type */ > + int which_lsm; /* which of the rules to use */ > } lsm[MAX_LSM_RULES]; Even if we wanted to support multiple LSMs within the same rule having both "rules[LSMBLOB_ENTRIES]" and "which_lsm" shouldn't be necessary. The LSMBLOB_ENTRIES should already identify the LSM. To support a single LSM per policy rule, "which_lsm" should be defined outside of lsm[MAX_LSM_RULES]. This will simplify the rest of the code (e.g. matching/freeing rules). int which_lsm; /* which of the rules to use */ struct { void *rule; /* LSM file metadata specific */ char *args_p; /* audit value */ int type; /* audit type */ } lsm[MAX_LSM_RULES]; > char *fsname; > struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */ > @@ -90,17 +91,15 @@ struct ima_rule_entry { > > /** > * ima_lsm_isset - Is a rule set for any of the active security modules > - * @rules: The set of IMA rules to check > + * @entry: the rule entry to examine > + * @lsm_rule: the specific rule type in question > * > - * If a rule is set for any LSM return true, otherwise return false. > + * If a rule is set return true, otherwise return false. > */ > -static inline bool ima_lsm_isset(void *rules[]) > +static inline bool ima_lsm_isset(struct ima_rule_entry *entry, int lsm_rule) > { > - int i; > - > - for (i = 0; i < LSMBLOB_ENTRIES; i++) > - if (rules[i]) > - return true; > + if (entry->lsm[lsm_rule].rules[entry->lsm[lsm_rule].which_lsm]) > + return true; If each IMA policy rule is limited to a specific LSM, then the test would be "entry->which_lsm". > return false; > } > > @@ -273,6 +272,20 @@ static int __init default_appraise_policy_setup(char *str) > } > __setup("ima_appraise_tcb", default_appraise_policy_setup); > > +static int ima_rule_lsm __ro_after_init; > + > +static int __init ima_rule_lsm_init(char *str) > +{ > + ima_rule_lsm = lsm_name_to_slot(str); > + if (ima_rule_lsm < 0) { > + ima_rule_lsm = 0; > + pr_err("rule lsm \"%s\" not registered", str); > + } > + > + return 1; > +} > +__setup("ima_rule_lsm=", ima_rule_lsm_init); The patch description refers to "ima_rules_lsm=". Please update one or the other. thanks, Mimi
On 2/14/2021 10:21 AM, Mimi Zohar wrote: > Hi Casey, > > On Tue, 2021-01-26 at 08:40 -0800, Casey Schaufler wrote: >> Integrity measurement may filter on security module information >> and needs to be clear in the case of multiple active security >> modules which applies. Provide a boot option ima_rules_lsm= to >> allow the user to specify an active securty module to apply >> filters to. If not specified, use the first registered module >> that supports the audit_rule_match() LSM hook. Allow the user >> to specify in the IMA policy an lsm= option to specify the >> security module to use for a particular rule. > Thanks, Casey. > > (This patch description line length seems short.) > >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> To: Mimi Zohar <zohar@linux.ibm.com> >> To: linux-integrity@vger.kernel.org >> --- >> Documentation/ABI/testing/ima_policy | 8 +++- >> security/integrity/ima/ima_policy.c | 64 ++++++++++++++++++++++------ >> 2 files changed, 57 insertions(+), 15 deletions(-) >> >> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy >> index e35263f97fc1..a7943d40466f 100644 >> --- a/Documentation/ABI/testing/ima_policy >> +++ b/Documentation/ABI/testing/ima_policy >> @@ -25,7 +25,7 @@ Description: >> base: [[func=] [mask=] [fsmagic=] [fsuuid=] [uid=] >> [euid=] [fowner=] [fsname=]] >> lsm: [[subj_user=] [subj_role=] [subj_type=] >> - [obj_user=] [obj_role=] [obj_type=]] >> + [obj_user=] [obj_role=] [obj_type=] [lsm=]] > "[lsm=]" either requires all LSM rules types (e.g. {subj/obj}_user, > role, type) to be exactly the same for multiple LSMs or all of the LSM > rule types are applicable to only a single LSM. Supporting multiple > LSMs with exactly the same LSM labels doesn't seem worth the effort. > Keep it simple - a single rule, containing any LSM rule types, is > applicable to a single LSM. Thank you. I will add this. > >> option: [[appraise_type=]] [template=] [permit_directio] >> [appraise_flag=] [keyrings=] >> base: >> @@ -114,6 +114,12 @@ Description: >> >> measure subj_user=_ func=FILE_CHECK mask=MAY_READ >> >> + It is possible to explicitly specify which security >> + module a rule applies to using lsm=. If the security >> + modules specified is not active on the system the rule >> + will be rejected. If lsm= is not specified the first >> + security module registered on the system will be assumed. >> + >> Example of measure rules using alternate PCRs:: >> >> measure func=KEXEC_KERNEL_CHECK pcr=4 >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >> index 8002683003e6..de72b719c90c 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -82,6 +82,7 @@ struct ima_rule_entry { >> void *rules[LSMBLOB_ENTRIES]; /* LSM file metadata specific */ >> char *args_p; /* audit value */ >> int type; /* audit type */ >> + int which_lsm; /* which of the rules to use */ >> } lsm[MAX_LSM_RULES]; > Even if we wanted to support multiple LSMs within the same rule having > both "rules[LSMBLOB_ENTRIES]" and "which_lsm" shouldn't be necessary. > The LSMBLOB_ENTRIES should already identify the LSM. > > To support a single LSM per policy rule, "which_lsm" should be defined > outside of lsm[MAX_LSM_RULES]. This will simplify the rest of the code > (e.g. matching/freeing rules). > > int which_lsm; /* which of the rules to use */ > struct { > void *rule; /* LSM file metadata specific */ > char *args_p; /* audit value */ > int type; /* audit type */ > } lsm[MAX_LSM_RULES]; You're right, that is better. I'll incorporate the change. > > >> char *fsname; >> struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */ >> @@ -90,17 +91,15 @@ struct ima_rule_entry { >> >> /** >> * ima_lsm_isset - Is a rule set for any of the active security modules >> - * @rules: The set of IMA rules to check >> + * @entry: the rule entry to examine >> + * @lsm_rule: the specific rule type in question >> * >> - * If a rule is set for any LSM return true, otherwise return false. >> + * If a rule is set return true, otherwise return false. >> */ >> -static inline bool ima_lsm_isset(void *rules[]) >> +static inline bool ima_lsm_isset(struct ima_rule_entry *entry, int lsm_rule) >> { >> - int i; >> - >> - for (i = 0; i < LSMBLOB_ENTRIES; i++) >> - if (rules[i]) >> - return true; >> + if (entry->lsm[lsm_rule].rules[entry->lsm[lsm_rule].which_lsm]) >> + return true; > If each IMA policy rule is limited to a specific LSM, then the test > would be "entry->which_lsm". Which would be an improvement. > >> return false; >> } >> >> @@ -273,6 +272,20 @@ static int __init default_appraise_policy_setup(char *str) >> } >> __setup("ima_appraise_tcb", default_appraise_policy_setup); >> >> +static int ima_rule_lsm __ro_after_init; >> + >> +static int __init ima_rule_lsm_init(char *str) >> +{ >> + ima_rule_lsm = lsm_name_to_slot(str); >> + if (ima_rule_lsm < 0) { >> + ima_rule_lsm = 0; >> + pr_err("rule lsm \"%s\" not registered", str); >> + } >> + >> + return 1; >> +} >> +__setup("ima_rule_lsm=", ima_rule_lsm_init); > The patch description refers to "ima_rules_lsm=". Please update one or > the other. ima_rules_lsm seem to be more accurate. I'll fix it. > > thanks, > > Mimi Thanks for the review and recommendations.
On 2/14/2021 10:21 AM, Mimi Zohar wrote: > Hi Casey, > > On Tue, 2021-01-26 at 08:40 -0800, Casey Schaufler wrote: >> Integrity measurement may filter on security module information >> and needs to be clear in the case of multiple active security >> modules which applies. Provide a boot option ima_rules_lsm= to >> allow the user to specify an active securty module to apply >> filters to. If not specified, use the first registered module >> that supports the audit_rule_match() LSM hook. Allow the user >> to specify in the IMA policy an lsm= option to specify the >> security module to use for a particular rule. > Thanks, Casey. > > (This patch description line length seems short.) > >> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> >> To: Mimi Zohar <zohar@linux.ibm.com> >> To: linux-integrity@vger.kernel.org >> --- >> Documentation/ABI/testing/ima_policy | 8 +++- >> security/integrity/ima/ima_policy.c | 64 ++++++++++++++++++++++------ >> 2 files changed, 57 insertions(+), 15 deletions(-) >> >> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy >> index e35263f97fc1..a7943d40466f 100644 >> --- a/Documentation/ABI/testing/ima_policy >> +++ b/Documentation/ABI/testing/ima_policy >> @@ -25,7 +25,7 @@ Description: >> base: [[func=] [mask=] [fsmagic=] [fsuuid=] [uid=] >> [euid=] [fowner=] [fsname=]] >> lsm: [[subj_user=] [subj_role=] [subj_type=] >> - [obj_user=] [obj_role=] [obj_type=]] >> + [obj_user=] [obj_role=] [obj_type=] [lsm=]] > "[lsm=]" either requires all LSM rules types (e.g. {subj/obj}_user, > role, type) to be exactly the same for multiple LSMs or all of the LSM > rule types are applicable to only a single LSM. Supporting multiple > LSMs with exactly the same LSM labels doesn't seem worth the effort. > Keep it simple - a single rule, containing any LSM rule types, is > applicable to a single LSM. > >> option: [[appraise_type=]] [template=] [permit_directio] >> [appraise_flag=] [keyrings=] >> base: >> @@ -114,6 +114,12 @@ Description: >> >> measure subj_user=_ func=FILE_CHECK mask=MAY_READ >> >> + It is possible to explicitly specify which security >> + module a rule applies to using lsm=. If the security >> + modules specified is not active on the system the rule >> + will be rejected. If lsm= is not specified the first >> + security module registered on the system will be assumed. >> + >> Example of measure rules using alternate PCRs:: >> >> measure func=KEXEC_KERNEL_CHECK pcr=4 >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >> index 8002683003e6..de72b719c90c 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -82,6 +82,7 @@ struct ima_rule_entry { >> void *rules[LSMBLOB_ENTRIES]; /* LSM file metadata specific */ >> char *args_p; /* audit value */ >> int type; /* audit type */ >> + int which_lsm; /* which of the rules to use */ >> } lsm[MAX_LSM_RULES]; > Even if we wanted to support multiple LSMs within the same rule having > both "rules[LSMBLOB_ENTRIES]" and "which_lsm" shouldn't be necessary. > The LSMBLOB_ENTRIES should already identify the LSM. > > To support a single LSM per policy rule, "which_lsm" should be defined > outside of lsm[MAX_LSM_RULES]. This will simplify the rest of the code > (e.g. matching/freeing rules). > > int which_lsm; /* which of the rules to use */ > struct { > void *rule; /* LSM file metadata specific */ > char *args_p; /* audit value */ > int type; /* audit type */ > } lsm[MAX_LSM_RULES]; > > >> char *fsname; >> struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */ >> @@ -90,17 +91,15 @@ struct ima_rule_entry { >> >> /** >> * ima_lsm_isset - Is a rule set for any of the active security modules >> - * @rules: The set of IMA rules to check >> + * @entry: the rule entry to examine >> + * @lsm_rule: the specific rule type in question >> * >> - * If a rule is set for any LSM return true, otherwise return false. >> + * If a rule is set return true, otherwise return false. >> */ >> -static inline bool ima_lsm_isset(void *rules[]) >> +static inline bool ima_lsm_isset(struct ima_rule_entry *entry, int lsm_rule) >> { >> - int i; >> - >> - for (i = 0; i < LSMBLOB_ENTRIES; i++) >> - if (rules[i]) >> - return true; >> + if (entry->lsm[lsm_rule].rules[entry->lsm[lsm_rule].which_lsm]) >> + return true; > If each IMA policy rule is limited to a specific LSM, then the test > would be "entry->which_lsm". > >> return false; >> } >> >> @@ -273,6 +272,20 @@ static int __init default_appraise_policy_setup(char *str) >> } >> __setup("ima_appraise_tcb", default_appraise_policy_setup); >> >> +static int ima_rule_lsm __ro_after_init; >> + >> +static int __init ima_rule_lsm_init(char *str) >> +{ >> + ima_rule_lsm = lsm_name_to_slot(str); >> + if (ima_rule_lsm < 0) { >> + ima_rule_lsm = 0; >> + pr_err("rule lsm \"%s\" not registered", str); >> + } >> + >> + return 1; >> +} >> +__setup("ima_rule_lsm=", ima_rule_lsm_init); > The patch description refers to "ima_rules_lsm=". Please update one or > the other. > > thanks, > > Mimi Would these changes match your suggestion? security/integrity/ima/ima_policy.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 9ac673472781..e80956548243 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -78,11 +78,11 @@ struct ima_rule_entry { bool (*uid_op)(kuid_t, kuid_t); /* Handlers for operators */ bool (*fowner_op)(kuid_t, kuid_t); /* uid_eq(), uid_gt(), uid_lt() */ int pcr; + int which_lsm; /* which of the rules to use */ struct { void *rules[LSMBLOB_ENTRIES]; /* LSM file metadata specific */ char *args_p; /* audit value */ int type; /* audit type */ - int which_lsm; /* which of the rules to use */ } lsm[MAX_LSM_RULES]; char *fsname; struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */ @@ -98,7 +98,7 @@ struct ima_rule_entry { */ static inline bool ima_lsm_isset(struct ima_rule_entry *entry, int lsm_rule) { - if (entry->lsm[lsm_rule].rules[entry->lsm[lsm_rule].which_lsm]) + if (entry->lsm[lsm_rule].rules[entry->which_lsm]) return true; return false; } @@ -272,19 +272,19 @@ static int __init default_appraise_policy_setup(char *str) } __setup("ima_appraise_tcb", default_appraise_policy_setup); -static int ima_rule_lsm __ro_after_init; +static int ima_rules_lsm __ro_after_init; -static int __init ima_rule_lsm_init(char *str) +static int __init ima_rules_lsm_init(char *str) { - ima_rule_lsm = lsm_name_to_slot(str); - if (ima_rule_lsm < 0) { - ima_rule_lsm = 0; + ima_rules_lsm = lsm_name_to_slot(str); + if (ima_rules_lsm < 0) { + ima_rules_lsm = 0; pr_err("rule lsm \"%s\" not registered", str); } return 1; } -__setup("ima_rule_lsm=", ima_rule_lsm_init); +__setup("ima_rules_lsm=", ima_rules_lsm_init); static struct ima_rule_opt_list *ima_alloc_rule_opt_list(const substring_t *src) { @@ -1515,7 +1515,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) result = -EINVAL; break; } - entry->lsm->which_lsm = result; + entry->which_lsm = result; result = 0; break; case Opt_err: @@ -1573,7 +1573,7 @@ ssize_t ima_parse_add_rule(char *rule) INIT_LIST_HEAD(&entry->list); for (i = 0; i < MAX_LSM_RULES; i++) - entry->lsm[i].which_lsm = ima_rule_lsm; + entry->which_lsm = ima_rules_lsm; result = ima_parse_rule(p, entry); if (result) { @@ -1827,9 +1827,9 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "appraise_flag=check_blacklist "); if (entry->flags & IMA_PERMIT_DIRECTIO) seq_puts(m, "permit_directio "); - if (entry->lsm->which_lsm >= 0) + if (entry->which_lsm >= 0) seq_printf(m, pt(Opt_lsm), - lsm_slot_to_name(entry->lsm->which_lsm)); + lsm_slot_to_name(entry->which_lsm)); rcu_read_unlock(); seq_puts(m, "\n"); return 0;
On Mon, 2021-02-22 at 15:45 -0800, Casey Schaufler wrote: > On 2/14/2021 10:21 AM, Mimi Zohar wrote: > > Would these changes match your suggestion? > > security/integrity/ima/ima_policy.c | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 9ac673472781..e80956548243 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -78,11 +78,11 @@ struct ima_rule_entry { > bool (*uid_op)(kuid_t, kuid_t); /* Handlers for operators */ > bool (*fowner_op)(kuid_t, kuid_t); /* uid_eq(), uid_gt(), uid_lt() */ > int pcr; > + int which_lsm; /* which of the rules to use */ > struct { > void *rules[LSMBLOB_ENTRIES]; /* LSM file metadata specific */ If each IMA policy rule may only contain a single LSM specific LSM_OBJ_{USER | ROLE | TYPE} and LSM_SUBJ_{USER | ROLE | TYPE}, then there is no need for rules[LSMBLOB_ENTRIES]. Leave it as "*rule". Otherwise it looks good. Mimi > char *args_p; /* audit value */ > int type; /* audit type */ > - int which_lsm; /* which of the rules to use */ > } lsm[MAX_LSM_RULES]; > char *fsname; > struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index e35263f97fc1..a7943d40466f 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -25,7 +25,7 @@ Description: base: [[func=] [mask=] [fsmagic=] [fsuuid=] [uid=] [euid=] [fowner=] [fsname=]] lsm: [[subj_user=] [subj_role=] [subj_type=] - [obj_user=] [obj_role=] [obj_type=]] + [obj_user=] [obj_role=] [obj_type=] [lsm=]] option: [[appraise_type=]] [template=] [permit_directio] [appraise_flag=] [keyrings=] base: @@ -114,6 +114,12 @@ Description: measure subj_user=_ func=FILE_CHECK mask=MAY_READ + It is possible to explicitly specify which security + module a rule applies to using lsm=. If the security + modules specified is not active on the system the rule + will be rejected. If lsm= is not specified the first + security module registered on the system will be assumed. + Example of measure rules using alternate PCRs:: measure func=KEXEC_KERNEL_CHECK pcr=4 diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8002683003e6..de72b719c90c 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -82,6 +82,7 @@ struct ima_rule_entry { void *rules[LSMBLOB_ENTRIES]; /* LSM file metadata specific */ char *args_p; /* audit value */ int type; /* audit type */ + int which_lsm; /* which of the rules to use */ } lsm[MAX_LSM_RULES]; char *fsname; struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */ @@ -90,17 +91,15 @@ struct ima_rule_entry { /** * ima_lsm_isset - Is a rule set for any of the active security modules - * @rules: The set of IMA rules to check + * @entry: the rule entry to examine + * @lsm_rule: the specific rule type in question * - * If a rule is set for any LSM return true, otherwise return false. + * If a rule is set return true, otherwise return false. */ -static inline bool ima_lsm_isset(void *rules[]) +static inline bool ima_lsm_isset(struct ima_rule_entry *entry, int lsm_rule) { - int i; - - for (i = 0; i < LSMBLOB_ENTRIES; i++) - if (rules[i]) - return true; + if (entry->lsm[lsm_rule].rules[entry->lsm[lsm_rule].which_lsm]) + return true; return false; } @@ -273,6 +272,20 @@ static int __init default_appraise_policy_setup(char *str) } __setup("ima_appraise_tcb", default_appraise_policy_setup); +static int ima_rule_lsm __ro_after_init; + +static int __init ima_rule_lsm_init(char *str) +{ + ima_rule_lsm = lsm_name_to_slot(str); + if (ima_rule_lsm < 0) { + ima_rule_lsm = 0; + pr_err("rule lsm \"%s\" not registered", str); + } + + return 1; +} +__setup("ima_rule_lsm=", ima_rule_lsm_init); + static struct ima_rule_opt_list *ima_alloc_rule_opt_list(const substring_t *src) { struct ima_rule_opt_list *opt_list; @@ -346,7 +359,8 @@ static void ima_lsm_free_rule(struct ima_rule_entry *entry) for (i = 0; i < MAX_LSM_RULES; i++) { for (r = 0; r < LSMBLOB_ENTRIES; r++) - ima_filter_rule_free(entry->lsm[i].rules[r]); + if (entry->lsm[i].rules[r]) + ima_filter_rule_free(entry->lsm[i].rules[r]); kfree(entry->lsm[i].args_p); } } @@ -398,7 +412,7 @@ static struct ima_rule_entry *ima_lsm_copy_rule(struct ima_rule_entry *entry) ima_filter_rule_init(nentry->lsm[i].type, Audit_equal, nentry->lsm[i].args_p, &nentry->lsm[i].rules[0]); - if (!ima_lsm_isset(nentry->lsm[i].rules)) + if (!ima_lsm_isset(nentry, i)) pr_warn("rule for LSM \'%s\' is undefined\n", nentry->lsm[i].args_p); } @@ -563,7 +577,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, int rc = 0; u32 osid; - if (!ima_lsm_isset(rule->lsm[i].rules)) { + if (!ima_lsm_isset(rule, i)) { if (!rule->lsm[i].args_p) continue; else @@ -925,6 +939,7 @@ enum { Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt, Opt_appraise_type, Opt_appraise_flag, Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings, + Opt_lsm, Opt_err }; @@ -962,6 +977,7 @@ static const match_table_t policy_tokens = { {Opt_pcr, "pcr=%s"}, {Opt_template, "template=%s"}, {Opt_keyrings, "keyrings=%s"}, + {Opt_lsm, "lsm=%s"}, {Opt_err, NULL} }; @@ -970,7 +986,7 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry, { int result; - if (ima_lsm_isset(entry->lsm[lsm_rule].rules)) + if (ima_lsm_isset(entry, lsm_rule)) return -EINVAL; entry->lsm[lsm_rule].args_p = match_strdup(args); @@ -981,7 +997,7 @@ static int ima_lsm_rule_init(struct ima_rule_entry *entry, result = ima_filter_rule_init(entry->lsm[lsm_rule].type, Audit_equal, entry->lsm[lsm_rule].args_p, &entry->lsm[lsm_rule].rules[0]); - if (!ima_lsm_isset(entry->lsm[lsm_rule].rules)) { + if (!ima_lsm_isset(entry, lsm_rule)) { pr_warn("rule for LSM \'%s\' is undefined\n", entry->lsm[lsm_rule].args_p); @@ -1488,6 +1504,19 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) &(template_desc->num_fields)); entry->template = template_desc; break; + case Opt_lsm: + result = lsm_name_to_slot(args[0].from); + if (result == LSMBLOB_INVALID) { + int i; + + for (i = 0; i < MAX_LSM_RULES; i++) + entry->lsm[i].args_p = NULL; + result = -EINVAL; + break; + } + entry->lsm->which_lsm = result; + result = 0; + break; case Opt_err: ima_log_string(ab, "UNKNOWN", p); result = -EINVAL; @@ -1524,6 +1553,7 @@ ssize_t ima_parse_add_rule(char *rule) struct ima_rule_entry *entry; ssize_t result, len; int audit_info = 0; + int i; p = strsep(&rule, "\n"); len = strlen(p) + 1; @@ -1541,6 +1571,9 @@ ssize_t ima_parse_add_rule(char *rule) INIT_LIST_HEAD(&entry->list); + for (i = 0; i < MAX_LSM_RULES; i++) + entry->lsm[i].which_lsm = ima_rule_lsm; + result = ima_parse_rule(p, entry); if (result) { ima_free_rule(entry); @@ -1751,7 +1784,7 @@ int ima_policy_show(struct seq_file *m, void *v) } for (i = 0; i < MAX_LSM_RULES; i++) { - if (ima_lsm_isset(entry->lsm[i].rules)) { + if (ima_lsm_isset(entry, i)) { switch (i) { case LSM_OBJ_USER: seq_printf(m, pt(Opt_obj_user), @@ -1793,6 +1826,9 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "appraise_flag=check_blacklist "); if (entry->flags & IMA_PERMIT_DIRECTIO) seq_puts(m, "permit_directio "); + if (entry->lsm->which_lsm >= 0) + seq_printf(m, pt(Opt_lsm), + lsm_slot_to_name(entry->lsm->which_lsm)); rcu_read_unlock(); seq_puts(m, "\n"); return 0;
Integrity measurement may filter on security module information and needs to be clear in the case of multiple active security modules which applies. Provide a boot option ima_rules_lsm= to allow the user to specify an active securty module to apply filters to. If not specified, use the first registered module that supports the audit_rule_match() LSM hook. Allow the user to specify in the IMA policy an lsm= option to specify the security module to use for a particular rule. Signed-off-by: Casey Schaufler <casey@schaufler-ca.com> To: Mimi Zohar <zohar@linux.ibm.com> To: linux-integrity@vger.kernel.org --- Documentation/ABI/testing/ima_policy | 8 +++- security/integrity/ima/ima_policy.c | 64 ++++++++++++++++++++++------ 2 files changed, 57 insertions(+), 15 deletions(-)