On Mon, Mar 9, 2020 at 9:25 PM Casey Schaufler <[email protected]> wrote: > On 3/6/2020 6:24 PM, Paul Moore wrote: > > On Fri, Feb 21, 2020 at 7:06 PM Casey Schaufler <[email protected]> > > wrote: > >> Add record entries to identify subject data for all of the > >> security modules when there is more than one. > >> > >> Acked-by: Stephen Smalley <[email protected]> > >> Signed-off-by: Casey Schaufler <[email protected]> > >> Cc: [email protected] > >> Cc: [email protected] > >> --- > >> drivers/android/binder.c | 2 +- > >> include/linux/audit.h | 1 + > >> include/linux/security.h | 9 ++++- > >> include/net/scm.h | 3 +- > >> kernel/audit.c | 40 ++++++++++++++++++- > >> kernel/audit_fsnotify.c | 1 + > >> kernel/auditfilter.c | 1 + > >> kernel/auditsc.c | 10 +++-- > >> net/ipv4/ip_sockglue.c | 2 +- > >> net/netfilter/nf_conntrack_netlink.c | 4 +- > >> net/netfilter/nf_conntrack_standalone.c | 2 +- > >> net/netfilter/nfnetlink_queue.c | 2 +- > >> net/netlabel/netlabel_unlabeled.c | 11 ++++-- > >> net/netlabel/netlabel_user.c | 2 +- > >> net/xfrm/xfrm_policy.c | 2 + > >> net/xfrm/xfrm_state.c | 2 + > >> security/integrity/ima/ima_api.c | 1 + > >> security/integrity/integrity_audit.c | 1 + > >> security/security.c | 51 +++++++++++++++++++++++-- > >> 19 files changed, 124 insertions(+), 23 deletions(-) > > ... > > > >> diff --git a/kernel/audit.c b/kernel/audit.c > >> index a25097cfe623..c3a1d8d2d33c 100644 > >> --- a/kernel/audit.c > >> +++ b/kernel/audit.c > >> @@ -2054,6 +2061,33 @@ void audit_log_key(struct audit_buffer *ab, char > >> *key) > >> audit_log_format(ab, "(null)"); > >> } > >> > >> +void audit_log_task_lsms(struct audit_buffer *ab) > >> +{ > >> + int i; > >> + const char *lsm; > >> + struct lsmblob blob; > >> + struct lsmcontext context; > >> + > >> + /* > >> + * Don't do anything unless there is more than one LSM > >> + * with a security context to report. > >> + */ > >> + if (security_lsm_slot_name(1) == NULL) > >> + return; > >> + > >> + security_task_getsecid(current, &blob); > >> + > >> + for (i = 0; i < LSMBLOB_ENTRIES; i++) { > >> + lsm = security_lsm_slot_name(i); > >> + if (lsm == NULL) > >> + break; > >> + if (security_secid_to_secctx(&blob, &context, i)) > >> + continue; > >> + audit_log_format(ab, " subj_%s=%s", lsm, context.context); > >> + security_release_secctx(&context); > >> + } > >> +} > >> + > >> int audit_log_task_context(struct audit_buffer *ab) > >> { > >> int error; > >> @@ -2064,7 +2098,7 @@ int audit_log_task_context(struct audit_buffer *ab) > >> if (!lsmblob_is_set(&blob)) > >> return 0; > >> > >> - error = security_secid_to_secctx(&blob, &context); > >> + error = security_secid_to_secctx(&blob, &context, LSMBLOB_FIRST); > >> if (error) { > >> if (error != -EINVAL) > >> goto error_path; > > Sorry, please disregard my previous ACK. > > :( > > > We should treat "subj=" similar to how we treat "obj="; if there is > > more than one LSM loaded the "subj=" should be set to "?" with the > > "subj_XXX=" set to the appropriate label for the named LSM. This > > patch looks like it is always using LSMBLOB_FIRST and not "?" when > > multiple LSMs are present. > > I'm fine with that, although I could see someone suggesting that > would constitute breaking backward compatibility.
The argument is the same for both the subject and object fields. I maintain that in the brave new world of LSM stacking if you are using a newly stacked kernel with old userspace, having a "?" for a subject/object label is much safer than only showing the first LSM's information and assuming that was the problem. With a "?" for a subject/object label you have some indication that something is "not quite right" and you can dig into it further. -- paul moore www.paul-moore.com -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
