netdev - Re: [PATCH v2] xfrm: interface: Don't hide plain packets from netfilter

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: <CAHsH6GsoavW+435MOTKy33iznMc_-JZ-kndr+G=YxuW7DWLNPA@mail.gmail.com>
Date:   Thu, 10 Dec 2020 13:48:53 +0200
From:   Eyal Birger <eyal.birger@...il.com>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>
Cc:     Phil Sutter <phil@....cc>,
        Steffen Klassert <steffen.klassert@...unet.com>,
        linux-crypto@...r.kernel.org, netfilter-devel@...r.kernel.org,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH v2] xfrm: interface: Don't hide plain packets from netfilter

Hi Nicolas,

On Thu, Dec 10, 2020 at 1:10 PM Nicolas Dichtel
<nicolas.dichtel@...nd.com> wrote:
>
> Le 09/12/2020 à 15:40, Eyal Birger a écrit :
> > Hi Phil,
> >
> > On Tue, Dec 8, 2020 at 8:51 PM Phil Sutter <phil@....cc> wrote:
> >>
> >> Hi Eyal,
> >>
> >> On Tue, Dec 08, 2020 at 04:47:02PM +0200, Eyal Birger wrote:
> >>> On Mon, Dec 7, 2020 at 4:07 PM Phil Sutter <phil@....cc> wrote:
> [snip]
> >>
> >> The packet appears twice being sent to eth1, the second time as ESP
> >> packet. I understand xfrm interface as a collector of to-be-xfrmed
> >> packets, dropping those which do not match a policy.
> >>
> >>>> Fix this by looping packets transmitted from xfrm_interface through
> >>>> NF_INET_LOCAL_OUT before passing them on to dst_output(), which makes
> >>>> behaviour consistent again from netfilter's point of view.
> >>>
> >>> When an XFRM interface is used when forwarding, why would it be correct
> >>> for NF_INET_LOCAL_OUT to observe the inner packet?
> I think it is valid because:
>  - it would be consistent with ip tunnels (see iptunnel_xmit())

Are you referring to the flow:
  iptunnel_xmit()
    ip_local_out()
      __ip_local_out()
        nf_hook(.., NF_INET_LOCAL_OUT, ...)

If I understand that flow correctly it operates on the outer packet
as it is called after all the header had been pushed already. no?
Or are you referring to a different flow?

>  - it would be consistent with the standard xfrm path see [1]

In the regular path as well I understand the OUTPUT hooks are called
after xfrm encoding in the forwarding case, so they can't see the inner
packet.

>  - from the POV of the forwarder, the packet is locally emitted, the src @ is
>    owned by the forwarder.

The inner IP source address is not owned by the forwarder to my understanding.

> >>
> >> A valid question, indeed. One could interpret packets being forwarded by
> >> those tunneling devices emit the packets one feeds them from the local
> >> host. I just checked and ip_vti behaves identical to xfrm_interface
> >> prior to my patch, so maybe my patch is crap and the inability to match
> >> on ipsec context data when using any of those devices is just by design.
> There was no real design for vti[6] interfaces, it's why xfrmi interfaces have
> been added. But they should be consistent I think, so this patch should handle
> xfrmi and vti[6] together.

I also think they should be consistent. But it'd still be confusing to me
to get an OUTPUT hook on the inner packet in the forwarding case.

Thanks,
Eyal.