On 5/25/2021 11:23 AM, Richard Guy Briggs wrote:

On 2021-05-25 10:28, Casey Schaufler wrote:

quoted

On 5/21/2021 1:19 PM, Paul Moore wrote:

<snip> and trim the CC list.

quoted

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

quoted

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

quoted

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

quoted

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 [off-list ref]
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