Re: [PATCH v15 21/23] Audit: Include object data for all security modules

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

On 3/6/2020 6:31 PM, Paul Moore wrote:
> On Fri, Feb 21, 2020 at 7:06 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>> When there is more than one context displaying security
>> module extend what goes into the audit record by supplimenting
>> the "obj=" with an "obj_<lsm>=" for each such security
>> module.
>>
>> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> Cc:linux-audit@redhat.com
>> ---
>>  kernel/audit.h   |   4 +-
>>  kernel/auditsc.c | 106 ++++++++++++++++++++++++-----------------------
>>  2 files changed, 56 insertions(+), 54 deletions(-)
> ...
>
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index 68ae5fa843c1..7dab48661e31 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -956,13 +953,57 @@ static inline void audit_free_context(struct audit_context *context)
>>         kfree(context);
>>  }
>>
>> +static int audit_log_object_context(struct audit_buffer *ab,
>> +                                   struct lsmblob *blob)
>> +{
>> +       struct lsmcontext context;
>> +       const char *lsm;
>> +       int i;
>> +
>> +       /*
>> +        * None of the installed modules have object labels.
>> +        */
>> +       if (security_lsm_slot_name(0) == NULL)
>> +               return 0;
>> +
>> +       if (blob->secid[0] != 0) {
>> +               if (security_secid_to_secctx(blob, &context, 0)) {
>> +                       audit_log_format(ab, " obj=?");
>> +                       return 1;
>> +               }
>> +               audit_log_format(ab, " obj=%s", context.context);
>> +               security_release_secctx(&context);
>> +       }
>> +
>> +       /*
>> +        * Don't do anything more unless there is more than one LSM
>> +        * with a security context to report.
>> +        */
>> +       if (security_lsm_slot_name(1) == NULL)
>> +               return 0;
>> +
>> +       for (i = 0; i < LSMBLOB_ENTRIES; i++) {
>> +               lsm = security_lsm_slot_name(i);
>> +               if (lsm == NULL)
>> +                       break;
>> +               if (blob->secid[i] == 0)
>> +                       continue;
>> +               if (security_secid_to_secctx(blob, &context, i)) {
>> +                       audit_log_format(ab, " obj_%s=?", lsm);
>> +                       continue;
>> +               }
>> +               audit_log_format(ab, " obj_%s=%s", lsm, context.context);
>> +               security_release_secctx(&context);
>> +       }
>> +       return 0;
>> +}
>> +
>>  static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>>                                  kuid_t auid, kuid_t uid,
>>                                  unsigned int sessionid,
>>                                  struct lsmblob *blob, char *comm)
>>  {
>>         struct audit_buffer *ab;
>> -       struct lsmcontext lsmctx;
>>         int rc = 0;
>>
>>         ab = audit_log_start(context, GFP_KERNEL, AUDIT_OBJ_PID);
>> @@ -972,15 +1013,7 @@ static int audit_log_pid_context(struct audit_context *context, pid_t pid,
>>         audit_log_format(ab, "opid=%d oauid=%d ouid=%d oses=%d", pid,
>>                          from_kuid(&init_user_ns, auid),
>>                          from_kuid(&init_user_ns, uid), sessionid);
>> -       if (lsmblob_is_set(blob)) {
>> -               if (security_secid_to_secctx(blob, &lsmctx, LSMBLOB_FIRST)) {
>> -                       audit_log_format(ab, " obj=(none)");
>> -                       rc = 1;
>> -               } else {
>> -                       audit_log_format(ab, " obj=%s", lsmctx.context);
>> -                       security_release_secctx(&lsmctx);
>> -               }
>> -       }
>> +       rc = audit_log_object_context(ab, blob);
>>         audit_log_format(ab, " ocomm=");
>>         audit_log_untrustedstring(ab, comm);
>>         audit_log_end(ab);
> I realized you don't hang around linux-audit

I do, but having implemented audit systems in the past
I try not to interfere with someone else's approach for
fear of getting sucked in to working on it.

>  (why would anyone want to do that?!)

Keeping an eye on trends or possible Smack impact.

> so let me tell you one of my most hated mantras: "new audit
> fields MUST go at the end of the audit record".  The "MUST" is in all
> caps because either I'm being clever and reusing some IETF RFC
> concepts, or I'm tired of arguing this point and feel like
> capitalization is the best I can do for stress relief; maybe it is a
> combination of the two.  Feel free to pick whichever reason you find
> most pleasing.

I'll go with stress relief. Glad to be helpful. ;)

> Either way, the "obj=" field should stay where it is, but the
> "obj_XXX=" fields need to find their way to the end of the record.

As Steve pointed out, there may be a bigger issue here. If the additional
fields aren't going to fit in MAX_AUDIT_MESSAGE_LENGTH bytes another
format may be required. I had hoped that perhaps obj_selinux= might count
as a refinement to obj= and hence not be considered a new field, but
it looks like that's not flying.



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