Re: [PATCH 1/1] audit: add missing fields to AUDIT_CONFIG_CHANGE event
On Monday, October 16, 2017 11:27:06 AM EDT Richard Guy Briggs wrote:
On 2017-10-13 21:11, Paul Moore wrote:
> On Fri, Oct 13, 2017 at 3:54 PM, Richard Guy Briggs <rgb(a)redhat.com>
wrote:
> > Since these are already standalone records (since the
context passed to
> > audit_log_start() is NULL) this info is necessary.
>
> For the record, I don't have a problem with converting standalone
> records to syscall accompanied records if that makes sense (not all
> audit events can be attributed to a syscall).
I don't either. I think I've fixed a couple like that in the past when
I thought it made sense.
My main objection to this is on wasting disk space and hurting search
performance. We don't need group information or extended uid information or
the actual syscall because it will always be a write nor do we need the arch
or arguments. It also eats more memory when parsing and we have lots of extra
fields that now need strtok to iterate over.
I'd almost rather add a task record because its more conservative, but syscall
is the only kind of event that carries auxiliary records. We'd have to
probably create something to do that if we did go that way. But it sounds like
more trouble than just adding a couple required fields.
> Looking purely at the additional information mentioned in this
thread,
> e.g. pid/uid/session/tty, it would make me believe that these records
> *could* be accompanied by a syscall (what is the point of recording
> that information if it isn't triggered by a syscall?). However, I
> can't say I've followed all the different fsnotify paths to know if
> that is the case ... it may be a mix, and perhaps that would be an
> argument for the logging this information in the accompanied SYSCALL
> record (it's only recorded when it is valid).
Ok, fair enough. There are some records generated by actions that seem
indirect for watches and trees, but I suppose they are all ultimately
triggered by a user action...
They all should be. Rules just don't disappear for no reason at all. Its
because a directory tree got removed or the admin ran auditctl.
The issue I still get stuck with is how do we make sure we put in
rules
to catch all the CONFIG_CHANGE instances without getting flooded by all
sorts of other stuff we don't want?
I don't quite follow you here. Its all working fine. It just needs some extra
fields.
> > I'm fine with the field ordering. If that is not
acceptable, I'd
> > recommend a new record type (AUDIT_TASK) to act as an aux record to this
> > record that lists this information in a standard order that can be used
> > as an aux record for all the standalone records that are missing this
> > information.
>
> As I just said in the GH issue, I'm not a big fan of the aux record at
> the moment (it seems too much of a dup with the SYSCALL record), but
> I'm not going to rule it out.
The advantage of it is its not as wasteful and incurs less performance hit for
ausearch. Although parsing a second record with the memory allocations, strtok
iterations, frees and moving past duplicate timestamp info will slow down
processing.
-Steve