On 8/24/2021 7:45 AM, Paul Moore wrote:
> On Fri, Aug 20, 2021 at 7:48 PM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> 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.
