Re: [RFC][PATCH] audit: log join and part events to the read-only multicast log socket

Steve Grubb
2014-10-22T21:18:37Z

On Wednesday, October 22, 2014 05:00:03 PM Paul Moore wrote:
> On Wednesday, October 22, 2014 04:39:49 PM Steve Grubb wrote:
> > On Wednesday, October 22, 2014 04:06:47 PM Paul Moore wrote:
> > > On Wednesday, October 22, 2014 01:56:13 PM Steve Grubb wrote:
> > > > On Wednesday, October 22, 2014 11:28:46 AM Paul Moore wrote:
> > > > > On Wednesday, October 22, 2014 10:25:35 AM Steve Grubb wrote:
> > > > > > On Tuesday, October 21, 2014 06:30:24 PM Paul Moore wrote:
> > > > > > > This is getting back to my earlier concerns/questions about
> > > > > > > field
> > > > > > > ordering, or at the very least I'm going to hijack this
> > > > > > > conversation and steer it towards field ordering ;)
> > > > > >
> > > > > > Field ordering is important. For example, suppose we decide to
> > > > > > make
> > > > > > ordering changes to the AUDIT_AVC record to bring it in line with
> > > > > > current standards. Would anyone care?
> > > > >
> > > > > That is an interesting example record considering everyone
> > > > > recognizes
> > > > > it to be an oddly formed, special case.
> > > >
> > > > But it illustrates the point. There are tools that depend on an
> > > > ordering
> > > > and format. There are more programs that just ausearch that needs to
> > > > be
> > > > considered if the fields change. For example, Someone could do things
> > > > like this:
> > > >
> > > > retval = auparse_find_field(au, "auid");
> > > > retval = auparse_next_field(au);
> > > > retval = auparse_next_field(au);
> > > > retval = auparse_find_field(au, res");
> > > >
> > > > Where, if the field ordering can't be guaranteed, the code becomes:
> > > >
> > > > retval = auparse_find_field(au, "auid");
> > > > retval = auparse_first_field(au);
> > > > retval = auparse_find_field(au, "pid");
> > > > retval = auparse_first_field(au);
> > > > retval = auparse_find_field(au, "uid");
> > > > retval = auparse_first_field(au);
> > > > retval = auparse_find_field(au, res");
> > >
> > > In my mind the latter code is more robust and preferable.
> >
> > Except you can have problems when the event is like this
> > auid= pid= old uid= new uid= res=
>
> I honestly don't see the problem here.

You'll never get new uid which is really the one you want.

> > > I disagree about the priority. Eric disagrees about the priority.
> > > Richard hasn't explicitly stated he disagrees with the priority but he
> > > has made several comments on this list about ordering being an issue
> > > (Richard, my apologies if I am putting words in your mouth).

I thought it was a question of what to put in an event. As for ordering all
you have to do is check the event with ausearch-test to see if its well
formed.

> > What events do people need to change and why? There's not been any
> > discussion that I know of saying we need to add fields or change them
> > around.
>
> See the earlier comments on Richard's patch.

He's making a new event. Its not changing things around.

> > > What would we need to change in the userspace to eliminate the reliance
> > > on field ordering?
> >
> > Many of the utilities. ausyscall & autrace might be the only ones not
> > affected.
>
> So we would need to change ausyscall and autrace, possibly others.

Exactly the opposite, those are about the only ones clean because they are the
only ones not parsing logs.

> Do you expect to need any changes to the deamon or audit libraries?

Not the daemon or library directly for this. But if you want to look into
this, you'll need some really big logs for testing. You'll need at least 100MB
to see performance variations. If we can keep performance reasonably close,
I'd take patches. I know it will be slower.

> > > I understand if you don't have a well developed list of items,
> > > but surely you must have some idea?
> > >
> > > I'm willing to help here, and I suspect there might be some others as
> > > well, just let us know what the pressing issues are in the audit
> > > userspace.
> >
> > People have been asking for ... {snip} ...
>
> Right now I was asking for input on what would need to change in userspace
> to eliminate the reliance on field ordering, not a full TODO list.

Well, its this TODO list that makes it a bad time for me to even consider re-
doing something that is working fine. We can't have nice things because we keep
mucking in the bottom layers.

> > > > We shouldn't be doing much modifying of records. They really should be
> > > > pretty static unless requirements change. For new records, there is no
> > > > guarantee that the function created before is the right information
> > > > for
> > > > the new event. It just depends on what it is as to what needs to be
> > > > collected. Then due to all the misused fields, we still need to have
> > > > review to make sure there's no typo. Speaking of which, I just found a
> > > > typo in AUDIT_FEATURE_CHANGE events.
> > >
> > > We're seeing at least one case where our inability to change the
> > > ordering
> > > of the audit fields is causing problems.
> >
> > What field ordering problem is preventing kernel work?
>
> It is making Richard's patch undesirable.

I thought there was some agreement to write a function and use it. User space
doesn't need re-writing for that.

-Steve