Re: [PATCH ghak59 V3 2/4] audit: add syscall information to CONFIG_CHANGE records

[Paul Moore <paul@paul-moore.com>]

On Thu, Jan 17, 2019 at 6:19 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2019-01-17 12:58, Paul Moore wrote:
> > On Thu, Jan 17, 2019 at 10:34 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > >
> > > On 2019-01-17 08:21, Paul Moore wrote:
> > > > On Thu, Jan 17, 2019 at 4:33 AM Steve Grubb <sgrubb@redhat.com> wrote:
> > > > > On Mon, 14 Jan 2019 17:58:58 -0500 Paul Moore <paul@paul-moore.com> wrote:
> > > > > > On Mon, Dec 10, 2018 at 5:18 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > > > > Tie syscall information to all CONFIG_CHANGE calls since they are
> > > > > > > all a result of user actions.
> > > > >
> > > > > Please don't tie syscall information to this. The syscall will be
> > > > > sendto. We don't need that information, its implicit. Also, doing this
> > > > > will possibly wreck things in libauparse. Please test the events with
> > > > > ausearch --format csv and --format text. IFF the event looks better or
> > > > > the same should we do this. If stuff disappears, the patch is
> > > > > breaking things
> > > >
> > > > We've discussed this quite a bit already; connecting associated
> > > > records into a single event is something that should happen, needs to
> > > > happen, and will happen.  Conceptually it makes no sense to record the
> > > > syscall (and any other associated records) which triggers the audit
> > > > configuration change, and the configuration change record itself as
> > > > two distinct events - they are the same event.  We've also heard from
> > > > a prominent user that associating records in this way is desirable.
> > > >
> > > > If the ausearch csv and text audit log transformations can't handle
> > > > this particular change, I would consider that a shortcoming of that
> > > > code.  We have multi-record events now, and this is only going to
> > > > increase in the future.
> > > >
> > > > Richard, if you can't make the requested changes to this patch and
> > > > resubmit by ... let's say the middle of next week? that should be
> > > > enough time, yes? ... please let me know and I'll make the changes and
> > > > get this merged.
> > >
> > > I would do the change, which should be very trivial, but I'm dense
> > > enough that I still don't know what you want.  In the last 6 months I've
> > > asked a number of direct questions that have not been directly
> > > addressed.  Perhaps I should be able to figure it out from the more
> > > general or fundamental principles replies I've gotten (which have been
> > > helpful, but perhaps incomplete), but I'm still having some trouble.
> > > Perhaps I'm exposing my limitations.
> >
> > Since code is unambiguous, let me just cut and paste what I was
> > thinking (be warned, this is a cut-n-paste, so the whitespace is
> > probably mangled):
>
> Thank you.  Very clear.  I don't see the point of
> audit_log_user_recv_msg() for reasons already stated but that's ok.

That's a fair comment.  I think there is some value in having the
function dedicated for the user messages since they are fairly unique
in that we don't ever want to associate them with the current task,
but it does result in a single use function (which I generally
dislike).  Who knows, maybe at some future point in time we'll get rid
of audit_log_user_recv_msg(), but let's stick with it for now.

> Since we're looking at these, here are the various field formats of the
> AUDIT_CONFIG_CHANGE records.
>
> Steve and Paul, are they worth attempting to normalize?

Right now, my vote is "no".  Although I'm voting that way pretty much
just because I want to get this patch in during this development cycle
and I'm fairly certain that trying to reach any consensus around
normalization is going to drag this out.  I would strongly encourage
you to just tweak this patch as we've just talked about and leave it
at that for the time being.

-- 
paul moore
www.paul-moore.com