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

[Richard Guy Briggs]

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.  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.

Is it possible these functions are not actually generating records if
the 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?  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.


- 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