All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Thu, 18 Feb 2021 16:20:01 -0500	[thread overview]
Message-ID: <20210218212001.GQ3141668@madcap2.tricolour.ca> (raw)
In-Reply-To: <20210218125248.GB22944@breakpoint.cc>

On 2021-02-18 13:52, Florian Westphal wrote:
> Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2021-02-18 09:22, Florian Westphal wrote:
> > > > It seems I'd need to filter out the NFT_MSG_GET_* ops.
> > > 
> > > No need, the GET ops do not cause changes and will not trigger a
> > > generation id change.
> > 
> > Ah, so it could trigger on generation change.  Would GET ops be included
> > in any other change
> 
> No, GET ops are standalone, they cannot be part of a transaction.
> If you look at
> 
> static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
> 
> array in nf_tables_api.c, then those ops with a '.call_batch' can
> appear in transaction (i.e., can cause modification).
> 
> The other ones (.call_rcu) are read-only.
> 
> If they appear in a batch tehy will be ignored, if the batch consists of
> such non-modifying ops only then nf_tables_commit() returns early
> because the transaction list is empty (nothing to do/change).

Ok, one little inconvenient question: what about GETOBJ_RESET?  That
looks like a hybrid that modifies kernel table counters and reports
synchronously.  That could be a special case call in
nf_tables_dump_obj() and nf_tables_getobj().  Will that cause a storm
per commit?

> > such that it would be desirable to filter them out
> > to reduce noise in that single log line if it is attempted to list all
> > the change ops?  It almost sounds like it would be better to do one
> > audit log line for each table for each family, and possibly for each op
> > to avoid the need to change userspace.  This would already be a
> > significant improvement picking the highest ranking op.
> 
> I think i understand what you'd like to do.  Yes, that would reduce
> the log output a lot.

Coded, testing...

- 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


WARNING: multiple messages have this Message-ID (diff)
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
Date: Thu, 18 Feb 2021 16:20:01 -0500	[thread overview]
Message-ID: <20210218212001.GQ3141668@madcap2.tricolour.ca> (raw)
In-Reply-To: <20210218125248.GB22944@breakpoint.cc>

On 2021-02-18 13:52, Florian Westphal wrote:
> Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2021-02-18 09:22, Florian Westphal wrote:
> > > > It seems I'd need to filter out the NFT_MSG_GET_* ops.
> > > 
> > > No need, the GET ops do not cause changes and will not trigger a
> > > generation id change.
> > 
> > Ah, so it could trigger on generation change.  Would GET ops be included
> > in any other change
> 
> No, GET ops are standalone, they cannot be part of a transaction.
> If you look at
> 
> static const struct nfnl_callback nf_tables_cb[NFT_MSG_MAX] = {
> 
> array in nf_tables_api.c, then those ops with a '.call_batch' can
> appear in transaction (i.e., can cause modification).
> 
> The other ones (.call_rcu) are read-only.
> 
> If they appear in a batch tehy will be ignored, if the batch consists of
> such non-modifying ops only then nf_tables_commit() returns early
> because the transaction list is empty (nothing to do/change).

Ok, one little inconvenient question: what about GETOBJ_RESET?  That
looks like a hybrid that modifies kernel table counters and reports
synchronously.  That could be a special case call in
nf_tables_dump_obj() and nf_tables_getobj().  Will that cause a storm
per commit?

> > such that it would be desirable to filter them out
> > to reduce noise in that single log line if it is attempted to list all
> > the change ops?  It almost sounds like it would be better to do one
> > audit log line for each table for each family, and possibly for each op
> > to avoid the need to change userspace.  This would already be a
> > significant improvement picking the highest ranking op.
> 
> I think i understand what you'd like to do.  Yes, that would reduce
> the log output a lot.

Coded, testing...

- 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

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


  parent reply	other threads:[~2021-02-18 21:21 UTC|newest]

Thread overview: 51+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-04 13:20 [PATCH ghak124 v3] audit: log nftables configuration change events Richard Guy Briggs
2020-06-04 13:20 ` Richard Guy Briggs
2020-06-04 17:03 ` Steve Grubb
2020-06-04 17:03   ` Steve Grubb
2020-06-04 17:57   ` Richard Guy Briggs
2020-06-04 17:57     ` Richard Guy Briggs
2020-06-04 18:51     ` Steve Grubb
2020-06-04 18:51       ` Steve Grubb
2020-06-24  0:34 ` Paul Moore
2020-06-24  0:34   ` Paul Moore
2020-06-24 10:03 ` Pablo Neira Ayuso
2020-06-24 10:03   ` Pablo Neira Ayuso
2020-06-24 12:34   ` Richard Guy Briggs
2020-06-24 12:34     ` Richard Guy Briggs
2020-06-24 13:03     ` Pablo Neira Ayuso
2020-06-24 13:03       ` Pablo Neira Ayuso
2020-06-24 13:26       ` Richard Guy Briggs
2020-06-24 13:26         ` Richard Guy Briggs
2021-02-11 15:16 ` Phil Sutter
2021-02-11 16:29   ` Paul Moore
2021-02-11 16:29     ` Paul Moore
2021-02-11 20:26     ` Richard Guy Briggs
2021-02-11 20:26       ` Richard Guy Briggs
2021-02-11 22:09       ` Florian Westphal
2021-02-17 23:41         ` Richard Guy Briggs
2021-02-17 23:41           ` Richard Guy Briggs
2021-02-18  8:22           ` Florian Westphal
2021-02-18  8:22             ` Florian Westphal
2021-02-18 12:42             ` Richard Guy Briggs
2021-02-18 12:42               ` Richard Guy Briggs
2021-02-18 12:52               ` Florian Westphal
2021-02-18 12:52                 ` Florian Westphal
2021-02-18 13:28                 ` Richard Guy Briggs
2021-02-18 13:28                   ` Richard Guy Briggs
2021-02-18 13:41                   ` Florian Westphal
2021-02-18 13:41                     ` Florian Westphal
2021-02-18 21:20                 ` Richard Guy Briggs [this message]
2021-02-18 21:20                   ` Richard Guy Briggs
2021-02-18 22:42                   ` Florian Westphal
2021-02-18 22:42                     ` Florian Westphal
2021-02-19  6:26                     ` Richard Guy Briggs
2021-02-19  6:26                       ` Richard Guy Briggs
2021-02-19 19:25                       ` Richard Guy Briggs
2021-02-19 19:25                         ` Richard Guy Briggs
2021-02-12 20:48       ` Richard Guy Briggs
2021-02-12 20:48         ` Richard Guy Briggs
2021-02-11 21:02     ` Steve Grubb
2021-02-11 21:02       ` Steve Grubb
2021-02-12 12:11       ` Phil Sutter
2021-02-12 20:54         ` Richard Guy Briggs
2021-02-12 20:54           ` Richard Guy Briggs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210218212001.GQ3141668@madcap2.tricolour.ca \
    --to=rgb@redhat.com \
    --cc=eparis@parisplace.org \
    --cc=fw@strlen.de \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    --cc=tgraf@infradead.org \
    --cc=twoerner@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.