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

Paul Moore
2020-03-07T02:24:58Z

On Fri, Feb 21, 2020 at 7:06 PM Casey Schaufler <casey@schaufler-ca.com> 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@tycho.nsa.gov>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-audit@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.

--
paul moore