Re: [PATCH v32 26/28] Audit: Add record for multiple object security contexts
On 3/3/2022 3:36 PM, Paul Moore wrote:
On Wed, Feb 2, 2022 at 7:23 PM Casey Schaufler
<casey(a)schaufler-ca.com> wrote:
> Create a new audit record AUDIT_MAC_OBJ_CONTEXTS.
> An example of the MAC_OBJ_CONTEXTS (1421) record is:
>
> type=MAC_OBJ_CONTEXTS[1421]
> msg=audit(1601152467.009:1050):
> obj_selinux=unconfined_u:object_r:user_home_t:s0
>
> When an audit event includes a AUDIT_MAC_OBJ_CONTEXTS record
> the "obj=" field in other records in the event will be "obj=?".
> An AUDIT_MAC_OBJ_CONTEXTS record is supplied when the system has
> multiple security modules that may make access decisions based
> on an object security context.
>
> Signed-off-by: Casey Schaufler <casey(a)schaufler-ca.com>
> ---
> include/linux/audit.h | 5 ++++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 59 ++++++++++++++++++++++++++++++++++++++
> kernel/auditsc.c | 37 ++++--------------------
> 4 files changed, 70 insertions(+), 32 deletions(-)
...
> diff --git a/kernel/audit.c b/kernel/audit.c
> index e8744e80ef21..3b9ce617b150 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2199,6 +2200,43 @@ int audit_log_task_context(struct audit_buffer *ab)
> }
> EXPORT_SYMBOL(audit_log_task_context);
>
> +void audit_log_object_context(struct audit_buffer *ab, struct lsmblob *blob)
> +{
> + struct audit_context_entry *ace;
> + struct lsmcontext context;
> + int error;
> +
> + if (!lsm_multiple_contexts()) {
> + error = security_secid_to_secctx(blob, &context, LSMBLOB_FIRST);
> + if (error) {
> + if (error != -EINVAL)
> + goto error_path;
> + return;
> + }
> + audit_log_format(ab, " obj=%s", context.context);
> + security_release_secctx(&context);
> + } else {
> + /*
> + * If there is more than one security module that has a
> + * object "context" it's necessary to put the object
data
> + * into a separate record to maintain compatibility.
> + */
I know this is nitpicky, but I'm going to say it anyway ... the
separate record isn't purely for compatibility reasons, it's for size
reasons. There is a fear that multiple LSM labels could blow past the
record size limit when combined with other fields, so putting them in
their own dedicated record gives us more room. If that wasn't the
case we could just tack them on the end of existing records.
Fair enough. I have no objection to adding commentary that will
help the next developer who comes into this code.
However, converting the existing "obj=" field into "obj=?" when
multiple LSM labels are present *is* a compatibility nod as it allows
existing userspace tooling that expects a single "obj=" field to
continue to work.
Likewise here.
> + audit_log_format(ab, " obj=?");
> + ace = kzalloc(sizeof(*ace), ab->gfp_mask);
> + if (!ace)
> + goto error_path;
> + INIT_LIST_HEAD(&ace->list);
> + ace->type = AUDIT_MAC_OBJ_CONTEXTS;
> + ace->lsm_objs = *blob;
> + list_add(&ace->list, &ab->aux_records);
> + }
> + return;
> +
> +error_path:
> + audit_panic("error in audit_log_object_context");
> +}
> +EXPORT_SYMBOL(audit_log_object_context);
> +