Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201207134309.16762-1-phil@nwl.cc> <CAHsH6Gupw7o96e5hOmaLBCZtqgoV0LZ4L7h-Y+2oROtXSXvTxw@mail.gmail.com>
 <20201208185139.GZ4647@orbyte.nwl.cc> <CAHsH6GvT=Af-BAWK0z_CdrYWPn0qt+C=BRjy10MLRNhLWfH0rQ@mail.gmail.com>
 <9fc5cbb8-26c7-c1c2-2018-3c0cd8c805f4@6wind.com>
In-Reply-To: <9fc5cbb8-26c7-c1c2-2018-3c0cd8c805f4@6wind.com>
From:   Eyal Birger <eyal.birger@gmail.com>
Date:   Thu, 10 Dec 2020 13:48:53 +0200
Message-ID: <CAHsH6GsoavW+435MOTKy33iznMc_-JZ-kndr+G=YxuW7DWLNPA@mail.gmail.com>
Subject: Re: [PATCH v2] xfrm: interface: Don't hide plain packets from netfilter
To:     Nicolas Dichtel <nicolas.dichtel@6wind.com>
Cc:     Phil Sutter <phil@nwl.cc>,
        Steffen Klassert <steffen.klassert@secunet.com>,
        linux-crypto@vger.kernel.org, netfilter-devel@vger.kernel.org,
        Linux Kernel Network Developers <netdev@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Nicolas,

On Thu, Dec 10, 2020 at 1:10 PM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> Le 09/12/2020 =C3=A0 15:40, Eyal Birger a =C3=A9crit :
> > Hi Phil,
> >
> > On Tue, Dec 8, 2020 at 8:51 PM Phil Sutter <phil@nwl.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@nwl.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 make=
s
> >>>> behaviour consistent again from netfilter's point of view.
> >>>
> >>> When an XFRM interface is used when forwarding, why would it be corre=
ct
> >>> 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 understandi=
ng.

> >>
> >> 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 matc=
h
> >> on ipsec context data when using any of those devices is just by desig=
n.
> 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 h=
andle
> 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.