All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: Richard Guy Briggs <rgb@redhat.com>
Cc: Florian Westphal <fw@strlen.de>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux-Audit Mailing List <linux-audit@redhat.com>,
	netfilter-devel@vger.kernel.org, twoerner@redhat.com,
	eparis@parisplace.org, tgraf@infradead.org
Subject: Re: [PATCH ghak124 v1] audit: log nftables configuration change events
Date: Wed, 27 May 2020 22:06:01 +0200	[thread overview]
Message-ID: <20200527200601.GJ2915@breakpoint.cc> (raw)
In-Reply-To: <20200527152443.7axktc2im3zpvk37@madcap2.tricolour.ca>

Richard Guy Briggs <rgb@redhat.com> wrote:
> Well, we are only logging "some change", so is it necessary to log the
> generation count to show that?  Is the generation count of specific
> interest?

No, its of no specific interest.  I just worded this poorly.
If the generation id increments, then something has been changed by the
batch, thats all.

> > (After that, kernel can't back down anymore, i.e. all errors are
> >  caught/handled beforehand).
> 
> I did think of recording all failed attempts too, but coding that would
> be more effort.  It is worth doing if it is deemed important,
> particularly for permission issues (as opposed to resource limits or
> packet format errors.  This would be more of interest to a security
> officer rather than a network technician, but the latter may find it
> useful for debugging.

The permission check is done early, in nfnetlink_rcv() (search for
EPERM), you would need to add an audit call there if thats relevant
for audit purposes.

> > If its 'any config change', then you also need to handle adds
> > or delete from sets/maps, since that may allow something that wasn't
> > allowed before, e.g. consider
> > 
> > ip saddr @trused accept
> > 
> > and then, later on,
> > nft add element ip filter @trusted { 10.0.0.0/8, 192.168.0.1 }
> > 
> > This would not add a table, or chain, or set, but it does implicitly
> > alter the ruleset.
> 
> Ah, ok, so yes, we would need that too.  I see family and table in
> there, op is evident.  Is there a useful value we can use in the
> "entries" field?

Maybe the handle of the set that the element was added to.
Each set, rule, chain, ... has a kernel-assigned number that
serves as a unique identifier.

> > Is that record format expected to emit the current number of chains?
> 
> I was aiming for a relevant value such as perhaps the new rule number or
> the rule number being deleted.

In that case, use the handle, which is a u64 with a unique value (for a
given table).

> > Since table names can be anything in nf_tables (they have no special
> > properties anymore), the table name is interesting from a informational
> > pov, but not super interesting.
> 
> I don't think we need to be able to completely reconstruct the
> tables/chains/rules from the information in the audit log, but be aware
> of who is changing what when.

Ok.  Have a look at nf_tables_fill_gen_info() in that case, you probably
want to emit at least the pid and task info, unless audit doesn't add
that already anyway.

> > Consider a batch update that commits 100 new rules in chain x,
> > this would result in 100 audit_log_nfcfg() calls, each with the
> > same information.
> 
> So rule number would be a useful differentiator here.

Ok.  Yes, that is available (rule->handle).

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


WARNING: multiple messages have this Message-ID (diff)
From: Florian Westphal <fw@strlen.de>
To: Richard Guy Briggs <rgb@redhat.com>
Cc: Florian Westphal <fw@strlen.de>,
	Linux-Audit Mailing List <linux-audit@redhat.com>,
	LKML <linux-kernel@vger.kernel.org>,
	netfilter-devel@vger.kernel.org, Paul Moore <paul@paul-moore.com>,
	sgrubb@redhat.com, omosnace@redhat.com, twoerner@redhat.com,
	eparis@parisplace.org, tgraf@infradead.org
Subject: Re: [PATCH ghak124 v1] audit: log nftables configuration change events
Date: Wed, 27 May 2020 22:06:01 +0200	[thread overview]
Message-ID: <20200527200601.GJ2915@breakpoint.cc> (raw)
In-Reply-To: <20200527152443.7axktc2im3zpvk37@madcap2.tricolour.ca>

Richard Guy Briggs <rgb@redhat.com> wrote:
> Well, we are only logging "some change", so is it necessary to log the
> generation count to show that?  Is the generation count of specific
> interest?

No, its of no specific interest.  I just worded this poorly.
If the generation id increments, then something has been changed by the
batch, thats all.

> > (After that, kernel can't back down anymore, i.e. all errors are
> >  caught/handled beforehand).
> 
> I did think of recording all failed attempts too, but coding that would
> be more effort.  It is worth doing if it is deemed important,
> particularly for permission issues (as opposed to resource limits or
> packet format errors.  This would be more of interest to a security
> officer rather than a network technician, but the latter may find it
> useful for debugging.

The permission check is done early, in nfnetlink_rcv() (search for
EPERM), you would need to add an audit call there if thats relevant
for audit purposes.

> > If its 'any config change', then you also need to handle adds
> > or delete from sets/maps, since that may allow something that wasn't
> > allowed before, e.g. consider
> > 
> > ip saddr @trused accept
> > 
> > and then, later on,
> > nft add element ip filter @trusted { 10.0.0.0/8, 192.168.0.1 }
> > 
> > This would not add a table, or chain, or set, but it does implicitly
> > alter the ruleset.
> 
> Ah, ok, so yes, we would need that too.  I see family and table in
> there, op is evident.  Is there a useful value we can use in the
> "entries" field?

Maybe the handle of the set that the element was added to.
Each set, rule, chain, ... has a kernel-assigned number that
serves as a unique identifier.

> > Is that record format expected to emit the current number of chains?
> 
> I was aiming for a relevant value such as perhaps the new rule number or
> the rule number being deleted.

In that case, use the handle, which is a u64 with a unique value (for a
given table).

> > Since table names can be anything in nf_tables (they have no special
> > properties anymore), the table name is interesting from a informational
> > pov, but not super interesting.
> 
> I don't think we need to be able to completely reconstruct the
> tables/chains/rules from the information in the audit log, but be aware
> of who is changing what when.

Ok.  Have a look at nf_tables_fill_gen_info() in that case, you probably
want to emit at least the pid and task info, unless audit doesn't add
that already anyway.

> > Consider a batch update that commits 100 new rules in chain x,
> > this would result in 100 audit_log_nfcfg() calls, each with the
> > same information.
> 
> So rule number would be a useful differentiator here.

Ok.  Yes, that is available (rule->handle).

  reply	other threads:[~2020-05-27 20:31 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-27 13:53 [PATCH ghak124 v1] audit: log nftables configuration change events Richard Guy Briggs
2020-05-27 13:53 ` Richard Guy Briggs
2020-05-27 14:53 ` Florian Westphal
2020-05-27 14:53   ` Florian Westphal
2020-05-27 15:24   ` Richard Guy Briggs
2020-05-27 15:24     ` Richard Guy Briggs
2020-05-27 20:06     ` Florian Westphal [this message]
2020-05-27 20:06       ` Florian Westphal

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=20200527200601.GJ2915@breakpoint.cc \
    --to=fw@strlen.de \
    --cc=eparis@parisplace.org \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=rgb@redhat.com \
    --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.