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;
Date:   Wed, 17 Feb 2021 18:41:31 -0500
From:   Richard Guy Briggs <rgb@redhat.com>
To:     Florian Westphal <fw@strlen.de>
Cc:     Phil Sutter <phil@nwl.cc>, LKML <linux-kernel@vger.kernel.org>,
        Linux-Audit Mailing List <linux-audit@redhat.com>,
        netfilter-devel@vger.kernel.org, twoerner@redhat.com,
        Eric Paris <eparis@parisplace.org>, tgraf@infradead.org
Subject: Re: [PATCH ghak124 v3] audit: log nftables configuration change
 events
Message-ID: <20210217234131.GN3141668@madcap2.tricolour.ca>
References: <f9da8b5dbf2396b621c77c17b5b1123be5aa484e.1591275439.git.rgb@redhat.com>
 <20210211151606.GX3158@orbyte.nwl.cc>
 <CAHC9VhTNQW9d=8GCW-70vAEMh8-LXviP+JHFC2-YkuitokLLMQ@mail.gmail.com>
 <20210211202628.GP2015948@madcap2.tricolour.ca>
 <20210211220930.GC2766@breakpoint.cc>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210211220930.GC2766@breakpoint.cc>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

On 2021-02-11 23:09, Florian Westphal wrote:
> Richard Guy Briggs <rgb@redhat.com> wrote:
> > > > I personally would notify once per transaction. This is easy and quick.
> > 
> > This was the goal.  iptables was atomic.  nftables appears to no longer
> > be so.  If I have this wrong, please show how that works.
> 
> nftables transactions are atomic, either the entire batch takes effect or not
> at all.
> 
> The audit_log_nfcfg() calls got added to the the nft monitor infra which
> is designed to allow userspace to follow the entire content of the
> transaction log.
> 
> So, if its just a 'something was changed' event that is needed all of
> them can be removed. ATM the audit_log_nfcfg() looks like this:
> 
>         /* step 3. Start new generation, rules_gen_X now in use. */
>         net->nft.gencursor = nft_gencursor_next(net);
> 
>         list_for_each_entry_safe(trans, next, &net->nft.commit_list, list) {
>                 switch (trans->msg_type) {
>                 case NFT_MSG_NEWTABLE:
> 			audit_log_nfcfg();
> 			...
> 		case NFT_MSG_...
> 			audit_log_nfcfg();
> 	..
> 	       	}
> 
> which gives an audit_log for every single change in the batch.
> 
> So, if just a summary is needed a single audit_log_nfcfg()
> after 'step 3' and outside of the list_for_each_entry_safe() is all
> that is needed.

Ok, so it should not matter if it is before or after that
list_for_each_entry_safe(), which could be used to collect that summary.

> If a summary is wanted as well one could fe. count the number of
> transaction types in the batch, e.g. table adds, chain adds, rule
> adds etc. and then log a summary count instead.

The current fields are "table", "family", "entries", "op".

Could one batch change more than one table?  (I think it could?)

It appears it can change more than one family.
"family" is currently a single integer, so that might need to be changed
to a list, or something to indicate multi-family.

A batch can certainly change more than one chain, but that was already a
bonus.

"entries" would be the obvious place for the summary count.

Listing all the ops seems a bit onerous.  Is there a hierarchy to the
ops and if so, are they in that order in a batch or in nf_tables_commit()?
It seems I'd need to filter out the NFT_MSG_GET_* ops.


- RGB

--
Richard Guy Briggs <rgb@redhat.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