Re: [PATCH 1/2] audit: log binding and unbinding to netlink multicast socket
On Friday, July 24, 2015 06:54:27 PM Paul Moore wrote:
On Thursday, July 23, 2015 04:45:10 PM Steve Grubb wrote:
> The audit subsystem could use a function that logs the commonly needed
> fields for a typical audit event. This logs less that audit_log_task_info
> and reduces the need to hand code individual fields.
>
> Signed-off-by: Steve Grubb <sgrubb(a)redhat.com>
> ---
>
> include/linux/audit.h | 5 +++++
> kernel/audit.c | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+)
Additional comments below, but I'd like to see this patch change
audit_log_task_info() to call audit_log_task_simple()
They really can't without messing up parsers. The order is different for a
reason. The audit_log_task_info records all kinds of stuff that is really not
needed. It does pids, current credentials, extended uid, extended gid, and
then tty and session, comm, exe, and then context. This wastes disk space.
The new function is what should be used for most cases because it sticks to
what is necessary for "hardwired" events - those that are not dictated by
syscall or file watches. It provides pid, uid, auid, tty, session, context,
comm, exe. Because it jettisons all the stuff that doesn't matter, one cannot
call the other.
... or, why not just call audit_log_task_info() if the audit
bind/unbind is
going to be the only one to benefit from audit_log_task_simple()? Yes, I
know that audit_log_task_info() records more than you need, but this
duplication of code because of the record format mess makes me very grumpy.
I'd rather see us move some other things to audit_log_task_simple over the
long term than hand code things. This is really for "hardwired" events such as
config changes, anomalies, and access decisions by frameworks. Group ids are
only applicable to file access, so it belongs in the syscall event and nowhere
else. Ppid is useless without a mapping back to that process's name. Extended
credentials are used on file access, so we get them in syscall records. We
don't need to waste disk space.
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 1c13e42..29fb38b 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1100,6 +1100,41 @@ static void audit_receive(struct sk_buff *skb)
>
> mutex_unlock(&audit_cmd_mutex);
>
> }
>
> +/* This function logs the essential information needed to understand
> + * what or who is causing the event */
> +void audit_log_task_simple(struct audit_buffer *ab, struct task_struct
> *tsk)
...
> + audit_log_format(ab, "pid=%u uid=%u auid=%u tty=%s ses=%u",
> + task_pid_nr(tsk),
> + from_kuid(&init_user_ns, cred->uid),
> + from_kuid(&init_user_ns, audit_get_loginuid(tsk)),
> + tty, audit_get_sessionid(tsk));
You should check the format string against audit_log_task_info(); they don't
match.
That is correct. It mostly matches the order of just about everything else.
For example, user space originating events get this:
pid=16430 uid=0 auid=4325 ses=3 subj=unconfined
anom_abend: auid=4325 uid=4325 gid=4325 ses=1 subj=unconfined pid=1680
config_change: auid=4325 ses=1
integrity: pid=1 uid=0 auid=4294967295 ses=4294967295 subj=kernel
The new order is:
pid=10068 uid=0 auid=4325 tty=pts0 ses=1 subj=unconfined
comm= exe=
This is an easy switch to make in searching and reporting because things are
mostly in the same order but with extra fields or start with a different field
making it easy to decide between old and new order.
-Steve