Re: [PATCH v32 26/28] Audit: Add record for multiple object security contexts

[Casey Schaufler]

On 3/3/2022 3:36 PM, Paul Moore wrote:
> On Wed, Feb 2, 2022 at 7:23 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx>
> > ---
> >   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);
> > +