Re: [PATCH v28 22/25] Audit: Add record for multiple process LSM attributes

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

On 8/24/2021 7:45 AM, Paul Moore wrote:
> On Fri, Aug 20, 2021 at 7:48 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>>> On 8/20/2021 12:06 PM, Paul Moore wrote:
>>>> Unless you explicitly enable audit on the kernel cmdline, e.g.
>>>> "audit=1", processes started before userspace enables audit will not
>>>> have a properly allocated audit_context; see the "if
>>>> (likely(!audit_ever_enabled))" check at the top of audit_alloc() for
>>>> the reason why.
>> I found a hack-around that no one will like. I changed that check to be
>>
>> (likely(!audit_ever_enabled) && !lsm_multiple_contexts())
>>
>> It probably introduces a memory leak and/or performance degradation,
>> but it has the desired affect.
> I can't speak for everyone, but I know I don't like that as a solution
> ;)  I imagine such a change would also draw the ire of the never-audit
> crowd and the distros themselves.  However, I understand the need to
> just get *something* in place so you can continue to test/develop;
> it's fine to keep that for now, but I'm going to be very disappointed
> if that line finds its way into the next posted patchset revision.

As I said, it's a hack-around that demonstrates the scope of the
problem. Had you expressed enthusiastic approval for it I'd have
been very surprised.

> I'm very much open to ideas but my gut feeling is that the end
> solution is going to be changes to audit_log_start() and
> audit_log_end().  In my mind the primary reason for this hunch is that
> support for multiple LSMs[*] needs to be transparent to the various
> callers in the kernel; this means the existing audit pattern of ...
>
>   audit_log_start(...);
>   audit_log_format(...);
>   audit_log_end(...);
>
> ... should be preserved and be unchanged from what it is now.  We've
> already talked in some general terms about what such changes might
> look like, but to summarize the previous discussions, I think we would
> need to think about the following things:

I will give this a shot.

>
> * Adding a timestamp/serial field to the audit_buffer struct, it could
> even be in a union with the audit_context pointer as we would never
> need both at the same time: if the audit_context ptr is non-NULL you
> use that, otherwise you use the timestamp.  The audit_buffer timestamp
> would not eliminate the need for the timestamp info in the
> audit_context struct for what I hope are obvious reasons.
>
> * Additional logic in audit_log_end() to generate additional ancillary
> records for LSM labels.  This will likely mean storing the message
> "type" passed to audit_log_start() in the audit_record struct and
> using that information in audit_log_end() to decide if a LSM ancillary
> record is needed.  Logistically this would likely mean converting the
> existing audit_log_end() function into a static/private
> __audit_log_end() and converting audit_log_end() into something like
> this:
>
>   void audit_log_end(ab)
>   {
>     __audit_log_end(ab); // rm the ab cleanup from __audit_log_end
>     if (ab->anc_mlsm) {
>       // generate the multiple lsm record
>     }
>     audit_buffer_free(ab);
>   }
>
> * Something else that I'm surely forgetting :)
>
> At the end of all this we may find that the "local" audit_context
> concept is no longer needed.  It was originally created to deal with
> grouping ancillary records with non-syscall records into a single
> event; while what we are talking about above is different, I believe
> that our likely solution will also work to solve the original "local"
> audit_context use case as well.  We'll have to see how this goes.

I'll keep that in mind as I fiddle about.

> [*] I expect that the audit container ID work will have similar issues
> and need a similar solution, I'm surprised it hasn't come up yet.

Hmm. That effort has been quiet lately. Too quiet.



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