netdev - Re: Regression in add xfrm interface

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] [day] [month] [year] [list]
Date:   Tue, 8 Mar 2022 23:08:20 -0800
From:   Stephen Hemminger <stephen@...workplumber.org>
To:     Antony Antony <antony.antony@...unet.com>
Cc:     Steffen Klassert <steffen.klassert@...unet.com>,
        David Ahern <dsahern@...il.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        <netdev@...r.kernel.org>, Eyal Birger <eyal.birger@...il.com>,
        Matt Ellison <matt@...oyo.io>
Subject: Re: Regression in add xfrm interface

On Tue, 8 Mar 2022 14:51:30 +0100
Antony Antony <antony.antony@...unet.com> wrote:

> Hi Stephen,
> 
> As Steffen explained bellow if_id = 0 is likely to cause problems in the long
> term. Should we revert the commit because it broke userspace tools?
> 
> I notice the Debian bug is in a iproute2 testsuite, also it is in Debian testing! How about fixing test case than reverting the kernel commit?
> 
> Another option is revert the commit in current kernel development cycle.
> And send the same fix to ipsec-next without "Fixes" tag.
> Would that be acceptable for Debian testsuite usecase?
> 
> regards,
> -antony
> 
> On Tue, Mar 08, 2022 at 08:50:13 +0100, Steffen Klassert wrote:
> > On Mon, Mar 07, 2022 at 12:11:23PM -0800, Stephen Hemminger wrote:  
> > > There appears to be a regression between 5.10 (Debian 11) and 5.16 (Debian testing)
> > > kernel in handling of ip link xfrm create. This shows up in the iproute2 testsuite
> > > which now fails. This is kernel (not iproute2) regression.
> > > 
> > > 
> > > Running ip/link/add_type_xfrm.t [iproute2-this/5.16.0-1-amd64]: FAILED
> > > 
> > > 
> > > Good log:
> > > ::::::::::::::
> > > link/add_type_xfrm.t.iproute2-this.out
> > > ::::::::::::::
> > > [Testing Add XFRM Interface, With IF-ID]
> > > tests/ip/link/add_type_xfrm.t: Add dev-ktyXSm xfrm interface succeeded
> > > tests/ip/link/add_type_xfrm.t: Show dev-ktyXSm xfrm interface succeeded with output:
> > > 2: dev-ktyXSm@lo: <NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> > >     link/none  promiscuity 0 minmtu 68 maxmtu 65535 
> > >     xfrm if_id 0xf addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 
> > > test on: "dev-ktyXSm" [SUCCESS]
> > > test on: "if_id 0xf" [SUCCESS]
> > > tests/ip/link/add_type_xfrm.t: Del dev-ktyXSm xfrm interface succeeded
> > > [Testing Add XFRM Interface, No IF-ID]
> > > tests/ip/link/add_type_xfrm.t: Add dev-tkUDaA xfrm interface succeeded
> > > tests/ip/link/add_type_xfrm.t: Show dev-tkUDaA xfrm interface succeeded with output:
> > > 3: dev-tkUDaA@lo: <NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> > >     link/none  promiscuity 0 minmtu 68 maxmtu 65535 
> > >     xfrm if_id 0 addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 
> > > test on: "dev-tkUDaA" [SUCCESS]
> > > test on: "if_id 0xf" [SUCCESS]
> > > tests/ip/link/add_type_xfrm.t: Del dev-tkUDaA xfrm interface succeeded
> > > 
> > > Failed log:
> > > 
> > > [Testing Add XFRM Interface, With IF-ID]
> > > tests/ip/link/add_type_xfrm.t: Add dev-pxNsUc xfrm interface succeeded
> > > tests/ip/link/add_type_xfrm.t: Show dev-pxNsUc xfrm interface succeeded with output:
> > > 2: dev-pxNsUc@lo: <NOARP,M-DOWN> mtu 1500 qdisc noop state DOWN mode DEFAULT group default qlen 1000
> > >     link/none  promiscuity 0 minmtu 68 maxmtu 65535 
> > >     xfrm if_id 0xf addrgenmode eui64 numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535 
> > > test on: "dev-pxNsUc" [SUCCESS]
> > > test on: "if_id 0xf" [SUCCESS]
> > > tests/ip/link/add_type_xfrm.t: Del dev-pxNsUc xfrm interface succeeded
> > > [Testing Add XFRM Interface, No IF-ID]  
> > 
> > No IF-ID is an invalid configuration, the interface does not work
> > with IF-IF 0. Such an interface will blackhole all packets routed
> > to it. That is because policies and states with no IF-ID are meant
> > for a setup without xfrm interfaces, they will not match the interface.
> > 
> > Unfortunately we did not catch this invalid configuration from the
> > beginning and userspace seems to use (or do some tests tests with)
> > xfrm interfaces with IF-ID 0. In that case, I fear we eventually
> > have to revert the cange that catches the invalid configuration.
> >   

The test is here please update as appropriate:

https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/tree/testsuite/tests/ip/link/add_type_xfrm.t

And was added by:

commit 286446c1e8c7f5f6eca4959015aa9e482b7adb11
Author: Matt Ellison <matt@...oyo.io>
Date:   Thu Apr 4 10:08:45 2019 -0400

    ip: support for xfrm interfaces
    
    Interfaces take a 'if_id' which is an interface id which can be set on
    an xfrm policy as its interface lookup key (XFRMA_IF_ID).
    
    Signed-off-by: Matt Ellison <matt@...oyo.io>
    Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>