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

[Richard Guy Briggs]

On 2019-01-17 22:26, Paul Moore wrote:
> On Thu, Jan 17, 2019 at 6:19 PM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> > On 2019-01-17 12:58, Paul Moore wrote:
> > > On Thu, Jan 17, 2019 at 10:34 AM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> > > >
> > > > On 2019-01-17 08:21, Paul Moore wrote:
> > > > > On Thu, Jan 17, 2019 at 4:33 AM Steve Grubb <sgrubb@xxxxxxxxxx> wrote:
> > > > > > On Mon, 14 Jan 2019 17:58:58 -0500 Paul Moore <paul@xxxxxxxxxxxxxx> wrote:
> > > > > > > On Mon, Dec 10, 2018 at 5:18 PM Richard Guy Briggs <rgb@xxxxxxxxxx> 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.

Agreed.  They are already inconsistent, so this patch isn't going to make
that worse.

> paul moore

- RGB

--
Richard Guy Briggs <rgb@xxxxxxxxxx>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635