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

[Casey Schaufler <casey@schaufler-ca.com>]

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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit