Re: [PATCH v35 26/29] Audit: Add record for multiple task security contexts

John Johansen
2022-04-26T01:08:40Z

On 4/18/22 07:59, Casey Schaufler wrote:
> Create a new audit record AUDIT_MAC_TASK_CONTEXTS.
> An example of the MAC_TASK_CONTEXTS (1420) record is:
>
> type=MAC_TASK_CONTEXTS[1420]
> msg=audit(1600880931.832:113)
> subj_apparmor=unconfined
> subj_smack=_
>
> When an audit event includes a AUDIT_MAC_TASK_CONTEXTS record
> the "subj=" field in other records in the event will be "subj=?".
> An AUDIT_MAC_TASK_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based
> on a subject security context.
>
> Functions are created to manage the skb list in the audit_buffer.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>

Besides moving the aux fns, and the whining below
Reviewed-by: John Johansen <john.johansen@canonical.com>

> ---
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 93 +++++++++++++++++++++++++++++++++++---
> 2 files changed, 88 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 8eda133ca4c1..af0aaccfaf57 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -143,6 +143,7 @@
> #define AUDIT_MAC_UNLBL_STCDEL 1417 /* NetLabel: del a static label */
> #define AUDIT_MAC_CALIPSO_ADD 1418 /* NetLabel: add CALIPSO DOI entry */
> #define AUDIT_MAC_CALIPSO_DEL 1419 /* NetLabel: del CALIPSO DOI entry */
> +#define AUDIT_MAC_TASK_CONTEXTS 1420 /* Multiple LSM task contexts */
>
> #define AUDIT_FIRST_KERN_ANOM_MSG 1700
> #define AUDIT_LAST_KERN_ANOM_MSG 1799
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 4d44c05053b0..8ed2d717c217 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2175,8 +2175,61 @@ void audit_log_key(struct audit_buffer *ab, char *key)
> audit_log_format(ab, "(null)");
> }
>
> +/**
> + * audit_buffer_aux_new - Add an aux record buffer to the skb list
> + * @ab: audit_buffer
> + * @type: message type
> + *
> + * Aux records are allocated and added to the skb list of
> + * the "main" record. The ab->skb is reset to point to the
> + * aux record on its creation. When the aux record in complete
> + * ab->skb has to be reset to point to the "main" record.
> + * This allows the audit_log_ functions to be ignorant of
> + * which kind of record it is logging to. It also avoids adding
> + * special data for aux records.
> + *
> + * On success ab->skb will point to the new aux record.
> + * Returns 0 on success, -ENOMEM should allocation fail.
> + */
> +static int audit_buffer_aux_new(struct audit_buffer *ab, int type)
> +{
> + WARN_ON(ab->skb != skb_peek(&ab->skb_list));
> +
> + ab->skb = nlmsg_new(AUDIT_BUFSIZ, ab->gfp_mask);
> + if (!ab->skb)
> + goto err;
> + if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0))
> + goto err;
> + skb_queue_tail(&ab->skb_list, ab->skb);
> +
> + audit_log_format(ab, "audit(%llu.%03lu:%u): ",
> + (unsigned long long)ab->stamp.ctime.tv_sec,
> + ab->stamp.ctime.tv_nsec/1000000,
> + ab->stamp.serial);
> +
> + return 0;
> +
> +err:
> + kfree_skb(ab->skb);
> + ab->skb = skb_peek(&ab->skb_list);
> + return -ENOMEM;
> +}
> +
> +/**
> + * audit_buffer_aux_end - Switch back to the "main" record from an aux record
> + * @ab: audit_buffer
> + *
> + * Restores the "main" audit record to ab->skb.
> + */
> +static void audit_buffer_aux_end(struct audit_buffer *ab)
> +{
> + ab->skb = skb_peek(&ab->skb_list);
> +}
> +
> +
> int audit_log_task_context(struct audit_buffer *ab)
> {
> + int i;
> int error;
> struct lsmblob blob;
> struct lsmcontext context;
> @@ -2185,16 +2238,44 @@ int audit_log_task_context(struct audit_buffer *ab)
> if (!lsmblob_is_set(&blob))
> return 0;
>
> - error = security_secid_to_secctx(&blob, &context, LSMBLOB_FIRST);
> + if (!lsm_multiple_contexts()) {
> + error = security_secid_to_secctx(&blob, &context,
> + LSMBLOB_FIRST);
> + if (error) {
> + if (error != -EINVAL)
> + goto error_path;
> + return 0;
> + }
>
> - if (error) {
> - if (error != -EINVAL)
> + audit_log_format(ab, " subj=%s", context.context);
> + security_release_secctx(&context);
> + } else {
> + /* Multiple LSMs provide contexts. Include an aux record. */
> + audit_log_format(ab, " subj=?");

just me whining, you sure we can't just drop subj= here

> + error = audit_buffer_aux_new(ab, AUDIT_MAC_TASK_CONTEXTS);
> + if (error)
> goto error_path;
> - return 0;
> + for (i = 0; i < LSMBLOB_ENTRIES; i++) {
> + if (blob.secid[i] == 0)
> + continue;
> + error = security_secid_to_secctx(&blob, &context, i);
> + if (error) {
> + audit_log_format(ab, "%ssubj_%s=?",
> + i ? " " : "",
> + lsm_slot_to_name(i));
> + if (error != -EINVAL)
> + audit_panic("error in audit_log_task_context");
> + } else {
> + audit_log_format(ab, "%ssubj_%s=%s",
> + i ? " " : "",
> + lsm_slot_to_name(i),
> + context.context);
> + security_release_secctx(&context);
> + }
> + }
> + audit_buffer_aux_end(ab);
> }
>
> - audit_log_format(ab, " subj=%s", context.context);
> - security_release_secctx(&context);
> return 0;
>