Re: [PATCH nf-next v3 3/3] netfilter: Introduce egress hook

From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Daniel Borkmann <daniel@iogearbox.net>
Cc: "Laura García Liébana" <nevola@gmail.com>,
	"Lukas Wunner" <lukas@wunner.de>,
	"John Fastabend" <john.fastabend@gmail.com>,
	"Jozsef Kadlecsik" <kadlec@netfilter.org>,
	"Florian Westphal" <fw@strlen.de>,
	"Netfilter Development Mailing list"
	<netfilter-devel@vger.kernel.org>,
	coreteam@netfilter.org, netdev@vger.kernel.org,
	"Alexei Starovoitov" <ast@kernel.org>,
	"Eric Dumazet" <edumazet@google.com>,
	"Thomas Graf" <tgraf@suug.ch>,
	"David Miller" <davem@davemloft.net>
Subject: Re: [PATCH nf-next v3 3/3] netfilter: Introduce egress hook
Date: Sat, 19 Sep 2020 17:52:06 +0200	[thread overview]
Message-ID: <20200919155206.GB28865@salvia> (raw)
In-Reply-To: <b0989f93-e708-4a68-1622-ab3de629be77@iogearbox.net>

Hi Daniel,

Long time no see, unfortunately this complicated situation is keeping
us away from personal reach, that's unfortunate. Now, looking into
this topic...

On Fri, Sep 18, 2020 at 10:31:09PM +0200, Daniel Borkmann wrote:
> [...] That is if there is an opt-in to such data path being used, then it also
> needs to continue to work, which gets me back to the earlier mentioned example
> with the interaction on the egress side with that hook that it needs to
> /interoperate/ with tc to avoid breakage of existing use cases in the wild.
> Reuse of skb flag could be one option to move forward, or as mentioned in
> earlier mails overall rework of ingress/egress side to be a more flexible
> pipeline (think of cont/ok actions as with tc filters or stackable LSMs to
> process & delegate).

The netfilter ingress hook was introduced many years after the tc
ingress "qdisc" (in the 4.2 kernel series), and I have absolutely no
records of one single complain from users in the netdev and netfilter
mailing lists regarding this being an issue / breaking anything. The
ingress hook needs to be *explicitly* registered by the user, so an
explicit user action to register the hook is required to register this
hook. As for this egress hook, it will be disabled by default too, since
egress chains are only registered on demand.

Assuming that preventing Netfilter to operate will *not* break things
makes no sense. It's the user that make sure that policies are
consistent across the datapath. I don't think there is any mechanism
that ensures that user policy fully makes sense.

Note that:

- The user can easily inspect if someone registered an egress hook.
- Your software can just report a warning to your user if there is an
  interaction with other subsystems makes no sense, it's just a bit of
  Netlink code from userspace if you don't want to wait for the user to
  notice.

You mentioned there is a real issue at this moment since AF_PACKET might
bypass dev_queue_xmit(), I think we can just ask Lukas to extend his
patch to include a hook there, so you can also follow up to fix this
issue for you too.

Thank you Daniel, stay safe.