[PATCH 1/2] audit: log binding and unbinding to netlink multicast socket
Paul Moore
pmoore at redhat.com
Tue Jul 28 15:39:29 UTC 2015
On Tuesday, July 28, 2015 10:31:54 AM Steve Grubb wrote:
> 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 at 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.
If we can't use _task_simple() inside of _task_info() then just use
audit_log_task_info(). Yes, it probably wastes a few extra bytes each time
these records are generated, but these records aren't likely to be frequent.
> 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.
Where can we use _task_simple() beyond these new records? Show me this has
some reuse in the existing code base and I'll reconsider keeping
_task_simple(), but right now it just looks like code duplication to me.
> > ... 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.
See above; we're not going to hand code things, just use _task_info().
Long term we are going to be ditching this awful fixed string format.
> > > 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:
I was talking about some of the scalar format specifiers, e.g. "%u" vs "%d",
but it doesn't matter so much anymore as it looks like we'll need to use
_task_info().
--
paul moore
security @ redhat
More information about the Linux-audit
mailing list