From: Phil Sutter <phil@nwl.cc>
To: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Florian Westphal <fwestpha@redhat.com>, netfilter-devel@vger.kernel.org
Subject: Re: [nf PATCH v2 1/2] net: nf_tables: Make nft_meta expression more robust
Date: Tue, 23 Jul 2019 17:06:44 +0200 [thread overview]
Message-ID: <20190723150644.GO22661@orbyte.nwl.cc> (raw)
In-Reply-To: <20190722195321.uf2r5lp46bvslvtd@salvia>
Hi Pablo,
On Mon, Jul 22, 2019 at 09:53:21PM +0200, Pablo Neira Ayuso wrote:
> On Sat, Jul 20, 2019 at 05:15:02PM +0200, Phil Sutter wrote:
> > Hi,
> >
> > On Fri, Jul 19, 2019 at 06:35:21PM +0200, Pablo Neira Ayuso wrote:
> > > On Fri, Jul 19, 2019 at 02:39:20PM +0200, Phil Sutter wrote:
> > > > nft_meta_get_eval()'s tendency to bail out setting NFT_BREAK verdict in
> > > > situations where required data is missing breaks inverted checks
> > > > like e.g.:
> > > >
> > > > | meta iifname != eth0 accept
> > > >
> > > > This rule will never match if there is no input interface (or it is not
> > > > known) which is not intuitive and, what's worse, breaks consistency of
> > > > iptables-nft with iptables-legacy.
> > > >
> > > > Fix this by falling back to placing a value in dreg which never matches
> > > > (avoiding accidental matches):
> > > >
> > > > {I,O}IF:
> > > > Use invalid ifindex value zero.
> > > >
> > > > {BRI_,}{I,O}IFNAME, {I,O}IFKIND:
> > > > Use an empty string which is neither a valid interface name nor
> > > > kind.
> > > >
> > > > {I,O}IFTYPE:
> > > > Use ARPHRD_VOID (0xFFFF).
> > >
> > > What could it be done with?
> > >
> > > NFT_META_BRI_IIFPVID
> > > NFT_META_BRI_IIFPVPROTO
> > >
> > > Those will still not work for
> > >
> > > meta ibrpvid != 40
> > >
> > > if interface is not available.
> > >
> > > For VPROTO probably it's possible. I don't have a solution for
> > > IIFPVID.
> >
> > VLAN IDs 0 and 4095 are reserved, we could use those. I refrained from
> > changing bridge VLAN matches because of IIFPVPROTO, no idea if there's
> > an illegal value we could use for that. If you have an idea, I'm all for
> > it. :)
>
> I think we can add something like:
>
> NFT_META_BRI_IIFVLAN
>
> just to check for br_vlan_enabled(), from userspace we can check for
> exists/missing as a boolean, so we don't have to worry on assuming an
> unused value for things like this. This can be added in the next
> release cycle.
Adding existence checks where missing is indeed a good idea, but doesn't
quite solve the problem we're facing here. :)
[...]
> These ones are missing:
>
> NFT_META_IIFGROUP
> NFT_META_OIFGROUP
>
> For these two, the default group (0) should be fine since every
> interface is falling under this category by default.
>
> I can squash this small patch to this one and push it one.
My problem with these "sane defaults" is that we may cause inconsistent
behaviour in rulesets: In prerouting, 'meta oifgroup 0' will match no
matter which interface the packet will be routed to. Yes, prerouting
implies there is no output interface (yet), but I consider this an
implementation detail and there will likely be cases where it is not as
easy to spot why something can't work.
Cheers, Phil
next prev parent reply other threads:[~2019-07-23 15:06 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-19 12:39 [nf PATCH v2 1/2] net: nf_tables: Make nft_meta expression more robust Phil Sutter
2019-07-19 12:39 ` [PATCH 2/2] net: netfilter: nft_meta_bridge: Eliminate 'out' label Phil Sutter
2019-07-19 16:35 ` [nf PATCH v2 1/2] net: nf_tables: Make nft_meta expression more robust Pablo Neira Ayuso
2019-07-20 15:15 ` Phil Sutter
2019-07-22 19:53 ` Pablo Neira Ayuso
2019-07-23 15:06 ` Phil Sutter [this message]
2019-07-23 18:38 ` Pablo Neira Ayuso
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=20190723150644.GO22661@orbyte.nwl.cc \
--to=phil@nwl.cc \
--cc=fwestpha@redhat.com \
--cc=netfilter-devel@vger.kernel.org \
--cc=pablo@netfilter.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).