[PATCH v26 22/25] Audit: Add new record for multiple process LSM attributes

[Casey Schaufler]

On 5/25/2021 11:23 AM, Richard Guy Briggs wrote:
> On 2021-05-25 10:28, Casey Schaufler wrote:
>> On 5/21/2021 1:19 PM, Paul Moore wrote:
>>
>> <snip> and trim the CC list.
>>
>>> Okay, we've got a disconnect here regarding "audit contexts" and
>>> "local contexts", skip down below where I attempt to explain things a
>>> little more but basically if there is a place that uses this pattern:
>>>
>>>   audit_log_start(audit_context(), ...);
>> This pattern isn't helpful. audit_context() returns NULL in about 1 of 4 calls.
> Ok, this rings a bell...  I think we talked about this on an earlier
> revision...
>
>> All of these callers of audit_context() get a NULL result some of the time.
>>
>> getname_kernel
>> debugfs_create_dir
>> tracefs_create_file
>> vfs_fchown
>> do_settimeofday64
>> bprm_execve
>> ksys_mmap_pgoff
>> move_addr_to_kernel
>> __do_pipe_flags
>> __do_sys_capset
>> syscall_trace_enter
>> cap_bprm_creds_from_file
>> load_module
>> __x64_sys_fsetxattr
>> bpf_prog_load
>> audit_signal_info_syscall
>> sel_write_enforce
>> common_lsm_audit
>> audit_set_loginuid
>> __dev_set_promiscuity
>> ipcperms
>> devpts_pty_new
>>
>>> ... you don't need, or want, a "local context".  You might need a
>>> local context if you see the following pattern:
>>>
>>>   audit_log_start(NULL, ...);
>>>
>>> The "local context" idea is a hack and should be avoided whenever
>>> possible; if you have an existing audit context from a syscall, or
>>> something else, you *really* should use it ... or have a *really* good
>>> explanation as to why you can not.
>> Almost all audit events want to record the subject context by calling
>> audit_log_task_context(). If multiple contexts need to be recorded
>> there has to be a separate record, which means there has to be an
>> audit context. The only case where an audit context is reliably available
>> is in syscalls. Hence, a "local context" for many of the cases where the
>> first argument to audit_log_start() would otherwise be NULL, either
>> explicitly or because audit_context() returns NULL.
> Ok, so in that case, I think I'd test audit_context() and if it is
> indeed NULL then create a local context, otherwise use the one that is
> available.

audit_alloc_for_lsm() returns the value of audit_context() if it is
not NULL. Otherwise it checks to see if a separate record will
be required. If it is the value from audit_alloc_local() is returned.
Otherwise, it returns NULL.

>   I should really go back and look carefully again at your
> code to see if it is in fact doing this, but unilaterally creating a
> local context if a context already exists is going to cause confusion
> because there will be two events generated for one event.

Indeed. I had discovered that.

> Is it possible these functions are not actually generating records if
> the context is NULL?

There are definitely cases where records are generated when audit_context()
is NULL.

> I'd be digging to find out why the context is NULL in these cases and if
> any record is even being produced in those cases?

Context is NULL because they're not coming out of syscalls.

>   Perhaps there is an
> active filter that indicates that logging is not of interest for that
> process/task/file/syscall/perm/etc...
>
>> Is there some other way to synchronize the "timestamp" without use of
>> an audit context? My understanding is that this is the primary purpose
>> of the audit context. 
> What has been done is to call it with a NULL context and it would assign
> a timestamp and serial number, but those are all single records that
> don't need synchronization (obviously).  This was why I'd come up with
> this mechanism to solve the need to attach a contid record to a single
> record associated with a network event (or user record that should not
> be associated with a syscall).  Those were the only two use cases I had
> up until now.

Right. Adding the contid record is a special case. Adding the lsmcontext
record is the common case. Thus Paul's lament.

>
>
> - RGB
>
> --
> Richard Guy Briggs <rgb at redhat.com>
> Sr. S/W Engineer, Kernel Security, Base Operating Systems
> Remote, Ottawa, Red Hat Canada
> IRC: rgb, SunRaycer
> Voice: +1.647.777.2635, Internal: (81) 32635
>