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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0dad94cc-2f4a-536a-94a9-c74e99c2f4ef@schaufler-ca.com>
Date:   Thu, 3 Mar 2022 17:26:54 -0800
From:   Casey Schaufler <casey@...aufler-ca.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     casey.schaufler@...el.com, jmorris@...ei.org,
        linux-security-module@...r.kernel.org, selinux@...r.kernel.org,
        linux-audit@...hat.com, keescook@...omium.org,
        john.johansen@...onical.com, penguin-kernel@...ove.sakura.ne.jp,
        sds@...ho.nsa.gov, linux-kernel@...r.kernel.org,
        Casey Schaufler <casey@...aufler-ca.com>
Subject: 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@...aufler-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@...aufler-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);
>> +