Re: [PATCH 1/1] audit: log binding and unbinding to netlink multicast

[Steve Grubb <sgrubb@redhat.com>]

On Tuesday, October 17, 2017 11:11:31 AM EDT Paul Moore wrote:
> On Mon, Oct 16, 2017 at 6:28 PM, Steve Grubb <sgrubb@redhat.com> wrote:
> > On Monday, October 16, 2017 6:06:47 PM EDT Steve Grubb wrote:
> >> > > +/* Log information about who is connecting to the audit multicast
> >> > > socket
> >> > > */ +static void audit_log_multicast_bind(int group, const char *op,
> >> > > int
> >> > > err) +{
> >> > > +       const struct cred *cred;
> >> > > +       struct tty_struct *tty;
> >> > > +       char comm[sizeof(current->comm)];
> >> > > +       struct audit_buffer *ab;
> >> > > +
> >> > > +       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_EVENT_LISTENER);
> >> > 
> >> > It really seems like this should be associated with the current task,
> >> > e.g. "audit_log_start(current->audit_context, ...)".  After all, the
> >> > whole point of this record is to capture information about the subject
> >> > who is binding to the multicast socket.
> >> 
> >> OK, easy enough.
> > 
> > But wouldn't that make it an auxiliary record (if there happens to be a
> > syscall record) instead of a standalone event?
> 
> I've always found the significance placed on auxiliary vs standalone
> events amusing.  The only significant difference between the two is
> that "auxiliary" records share a timestamp with the other records that
> have been triggered by a given syscall instance whereas standlone
> records always have a new timestamp generated.  I'm personally of the
> opinion that if we are emitting records we should always try to group
> related records into a single event to help reduce the confusion for
> people looking through their audit logs; in other words, whenever
> possible we should pass the audit_context to audit_log_start() and
> friends.

No, actually that introduces confusion. An event should have one specific 
meaning. This subject did something to that. Adding a syscall record takes a 
simple event and makes it incomprehensible to anyone that doesn't have 
familiarity with audit events. All necessary information is in the event that 
I created.

> If syscall auditing is disabled, these records (e.g.
> AUDIT_EVENT_LISTENER) will still be emitted by the kernel as
> "standalone" events.  If syscall auditing is enabled, these records
> will be grouped with the triggering syscall, which just makes sense.
> Look at the recent problem with init/systemd and the MAC_STATUS
> records, that is a great example.

Right. That was a mess because of piggy backing on top of a SYSCALL. I do not 
like events that are sometime simple and sometimes compound. I think that also 
messes up anyone doing json or xml formatting because they need a firm layout 
in their schema. The event is good as is. It has all required fields. Its easy 
to parse and keeps things compact.

-Steve