linux-kernel - Re: [PATCH ghak25 v5] audit: add subj creds to NETFILTER

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 May 2020 15:44:57 -0400
From:   Richard Guy Briggs <rgb@...hat.com>
To:     Paul Moore <paul@...l-moore.com>
Cc:     Linux-Audit Mailing List <linux-audit@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        netfilter-devel@...r.kernel.org, sgrubb@...hat.com,
        Ondrej Mosnacek <omosnace@...hat.com>, fw@...len.de,
        twoerner@...hat.com, Eric Paris <eparis@...isplace.org>,
        tgraf@...radead.org
Subject: Re: [PATCH ghak25 v5] audit: add subj creds to NETFILTER_CFG record
 to cover async unregister

On 2020-05-19 15:18, Paul Moore wrote:
> On Tue, May 19, 2020 at 11:31 AM Richard Guy Briggs <rgb@...hat.com> 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@...hat.com>
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