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

[Paul Moore <pmoore@redhat.com>]

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.
 
> and yes there are places like that. The performance really is the main
> issue.

So you've said.  As I've said, I'm not convinced.

> > > Which of the two is likely to be faster? Especially when doing realtime
> > > analysis as a plugin and needing to finish because another event is
> > > coming in? Just like a binary struct has to maintain an order of data
> > > members if written to disk, the sequence of fields need to be maintained
> > > in a text record.
> > 
> > What about the speed and performance of the code in the kernel?
> 
> kprintf is the same speed no matter what the order.

Having to duplicate code in the kernel has a negative effect.

> > What about the maintenance burden of having to duplicate code to ensure a
> > fixed format?
> 
> There just isn't a need to be altering events. We've had to add a few things
> here and there, but its only been a couple changes.

See the earlier comments on Richard's patch.

> > > > > > Before we go to much farther, I'd really like us to agree that
> > > > > > ordering is not important, can we do that?
> > > > > 
> > > > > Its kind of doubtful we can do anything like this quickly. Maybe
> > > > > over time.
> > > > 
> > > > Why?  Why can we not do this now?
> > > 
> > > There are more pressing needs on the user space side of this. reordering
> > > fields means I have to drop all my current plans and redo something
> > > that's been working fine. This is why it takes so long to get audit
> > > utilities and reports that are fast, understandable, and the right
> > > information.
> > 
> > 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).
> 
> 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.

> > 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.

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

> > 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.
 
> > > 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.

> > > > > To me, these are the burning issues that I think should be on the
> > > > > table to be solved rather than field ordering:
> > > > > 
> > > > > 1) ... {snip} ...
> > > > 
> > > > Ignoring the priority for a moment, thanks for posting these.  Is
> > > > there an audit TODO list posted somewhere?
> > > 
> > > That is just some kernel issues off the top of my head. Things come up
> > > all the time. Most of the time things are found because someone is
> > > asking questions or I am trying to make sense of the audit trail.
> > > 
> > > As for user space tools, yes there is a TODO file in the audit tarball.
> > > I don't know if the kernel maintainers have a TODO list published
> > > anywhere.
> > 
> > Eric, do you have one published somewhere?
> > 
> > Assuming that Eric doesn't have a TODO list posted somewhere, do you have
> > any objections to my posting and maintaining an audit kernel TODO list on
> > the audit fedorahosted.org wiki?
> 
> That would be nice.

Great, I'll look into that once I hear back from Eric on any old kernel audit 
TODO lists he might have.

-- 
paul moore
security and virtualization @ redhat