Re: [PATCH ghak25 v5] audit: add subj creds to NETFILTER_CFG record to cover async unregister

[Richard Guy Briggs]

On 2020-05-19 15:18, Paul Moore wrote:
> On Tue, May 19, 2020 at 11:31 AM Richard Guy Briggs <rgb@xxxxxxxxxx> wrote:
> > Some table unregister actions seem to be initiated by the kernel to
> > garbage collect unused tables that are not initiated by any userspace
> > actions.  It was found to be necessary to add the subject credentials to
> > cover this case to reveal the source of these actions.  A sample record:
> >
> > The tty, ses and exe fields have not been included since they are in the
> > SYSCALL record and contain nothing useful in the non-user context.
> >
> >   type=NETFILTER_CFG msg=audit(2020-03-11 21:25:21.491:269) : table=nat family=bridge entries=0 op=unregister pid=153 uid=root auid=unset subj=system_u:system_r:kernel_t:s0 comm=kworker/u4:2
> 
> Based on where things were left in the discussion on the previous
> draft, I think it would be good if you could explain a bit why the uid
> and auid fields are useful here.

They aren't really useful here.  I was hoping to remove them given your
reasoning, but I was having trouble guessing what you wanted even after
asking for clarity.  Can you clarify what you would prefer to see in
this patch?  I was hoping to skip this extra patch revision which took
longer than hoped due to trying to guess what you wanted while working
yesterday during a public holiday to get this patch out in time for the
merge window.

A UID of 0="root" is really a bit misleading since while it is the most
trusted user running the most privileged level, the event wasn't
triggered by a user.  It is the default value of that field.  I did
think aloud that uid could be set by the kernel to run under a
particular user's id (like a daemon dropping capabilities and switching
user after setup to limit abuse), but the kernel is just a tracker for
these IDs and doesn't really know what they mean other than root.  I saw
no reply to that idea.  It was set to "root" which isn't unset or
unexpected, but granted is useless in this case.

You had offered that keeping auid was a concession to Steve so I kept it
in since I had the impression that is what you wanted to see.  That
explanation seems pretty thin to include in a patch description if what
you are getting at in your sentence above.

I am willing to purge both if that is what you would prefer to accept in
the patch.

> 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