Re: [PATCH v15 20/23] Audit: Add subj_LSM fields when necessary

On Fri, Feb 21, 2020 at 7:06 PM Casey Schaufler <casey(a)schaufler-ca.com&gt; wrote: > Add record entries to identify subject data for all of the > security modules when there is more than one. > > Acked-by: Stephen Smalley <sds(a)tycho.nsa.gov&gt; > Signed-off-by: Casey Schaufler <casey(a)schaufler-ca.com&gt; > Cc: netdev(a)vger.kernel.org > Cc: linux-audit(a)redhat.com > --- > 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.