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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200315132836.cj36ape6rpw33iqb@salvia>
Date:   Sun, 15 Mar 2020 14:28:36 +0100
From:   Pablo Neira Ayuso <pablo@...filter.org>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Lukas Wunner <lukas@...ner.de>,
        Jozsef Kadlecsik <kadlec@...filter.org>,
        Florian Westphal <fw@...len.de>,
        netfilter-devel@...r.kernel.org, coreteam@...filter.org,
        netdev@...r.kernel.org, Martin Mares <mj@....cz>,
        Dmitry Safonov <0x7f454c46@...il.com>,
        Thomas Graf <tgraf@...g.ch>,
        Alexei Starovoitov <ast@...nel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH nf-next 3/3] netfilter: Introduce egress hook

Hello Daniel,

On Sat, Mar 14, 2020 at 01:12:02AM +0100, Daniel Borkmann wrote:
> On 3/13/20 3:55 PM, Pablo Neira Ayuso wrote:
[...]
> > We have plans to support for NAT64 and NAT46, this is the right spot
> > to do this mangling. There is already support for the tunneling
> 
> But why is existing local-out or post-routing hook _not_ sufficient for
> NAT64 given it being IP based?

Those hooks are not coming at the end of the IP processing. There is
very relevant IP code after those hooks that cannot be bypassed such
as fragmentation, tunneling and neighbour output. Such transformation
needs to happen after the IP processing, exactly from where Lukas is
proposing.

[...]
> > infrastructure in netfilter from ingress, this spot from egress will
> > allow us to perform the tunneling from here. There is also no way to
> > drop traffic generated by dhclient, this also allow for filtering such
> > locally generated traffic. And many more.
> 
> This is a known fact for ~17 years [0] or probably more by now and noone
> from netfilter folks cared to address it in all the years, so I presume
> it cannot be important enough, and these days it can be filtered through
> other means already. Tbh, it's a bit laughable that you bring this up as
> an argument ...
> 
>   [0] https://www.spinics.net/lists/netfilter/msg19488.html

Look: ip6tables, arptables and ebtables are a copy and paste from the
original iptables.

At that time, the only way one way to add support for ingress/egress
classification in netfilter: add "devtables", yet another copy and
past from iptables, that was a no-go.

This is not a problem anymore since there is a consolidated netfilter
framework to achieve ingress/egress classification.

> > Performance impact is negligible, Lukas already provided what you
> > asked for.
> 
> Sure, and the claimed result was "as said the fast-path gets faster, not
> slower" without any explanation or digging into details on why this might
> be, especially since it appears counter-intuitive as was stated by the
> author ... and later demonstrated w/ measurements that show the opposite.

I remember one of your collegues used this same argument against new
hooks back in 2015 [0], and the introduction of this hook was proven
to be negligible. This patchset introduces code that looks very much
the same.

I can make a list of recent updates to the output path, several of
them very are targeted to very specific usecases. You did not care at
all about performance impact of those at all, however, you care about
netfilter for some unknown reason.

In my opinion, your original feedback has been addressed, it's time to
move on.

Thank you.

[0] https://lwn.net/Articles/642414/