Re: [PATCH v26 22/25] Audit: Add new record for multiple process LSM attributes
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(a)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