Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1584480281.git.rgb@redhat.com> <13ef49b2f111723106d71c1bdeedae09d9b300d8.1584480281.git.rgb@redhat.com>
 <20200318131128.axyddgotzck7cit2@madcap2.tricolour.ca> <CAHC9VhTdLZop0eT11H4uSXRj5M=kBet=GkA8taDwGN_BVMyhrQ@mail.gmail.com>
 <20200318213327.ow22q6nnjn3ijq6v@madcap2.tricolour.ca>
In-Reply-To: <20200318213327.ow22q6nnjn3ijq6v@madcap2.tricolour.ca>
From:   Paul Moore <paul@paul-moore.com>
Date:   Fri, 17 Apr 2020 17:53:47 -0400
Message-ID: <CAHC9VhSbbjFbF0A_-saquZ8B85XaF7SWD2e1QcWsXhFSQrUAbQ@mail.gmail.com>
Subject: Re: [PATCH ghak25 v3 3/3] audit: add subj creds to NETFILTER_CFG
 record to cover async unregister
To:     Richard Guy Briggs <rgb@redhat.com>
Cc:     Linux-Audit Mailing List <linux-audit@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>,
        netfilter-devel@vger.kernel.org, twoerner@redhat.com,
        Eric Paris <eparis@parisplace.org>, fw@strlen.de,
        ebiederm@xmission.com, tgraf@infradead.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, Mar 18, 2020 at 5:33 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2020-03-18 17:22, Paul Moore wrote:
> > On Wed, Mar 18, 2020 at 9:12 AM Richard Guy Briggs <rgb@redhat.com> wrote:
> > > On 2020-03-17 17:30, Richard Guy Briggs 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:
> > > >
> > > >   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 tty=(none) ses=unset subj=system_u:system_r:kernel_t:s0 comm=kworker/u4:2 exe=(null)
> > >
> > > Given the precedent set by bpf unload, I'd really rather drop this patch
> > > that adds subject credentials.
> > >
> > > Similarly with ghak25's subject credentials, but they were already
> > > present and that would change an existing record format, so it isn't
> > > quite as justifiable in that case.
> >
> > Your comments have me confused - do you want this patch (v3 3/3)
> > considered for merging or no?
>
> I would like it considered for merging if you think it will be required
> to provide enough information about the event that happenned.  In the
> bpf unload case, there is a program number to provide a link to a
> previous load action.  In this case, we won't know for sure what caused
> the table to be unloaded if the number of entries was empty.  I'm still
> trying to decide if it matters.  For the sake of caution I think it
> should be included.  I don't like it, but I think it needs to be
> included.

I'm in the middle of building patches 1/3 and 2/3, assuming all goes
well I'll merge them into audit/next (expect mail soon), however I'm
going back and forth on this patch.  Like you I kinda don't like it,
and with both of us not in love with this patch I have to ask if there
is certification requirement for this?  I know about the generic
subj/obj requirements, but in the case where there is no associated
task/syscall/etc. information it isn't like the extra fields supplied
in this patch are going to have much information in that regard; it's
really the *absence* of that information which is telling.  Which
brings me to wonder if simply the lack of any associated records in
this event is enough?  Before when we weren't associating records into
a single event it would have been a problem, but the way things
currently are, if there are no other records (and you have configured
that) then I think you have everything you need to know.

Thoughts?

-- 
paul moore
www.paul-moore.com