netdev - Re: [PATCH 1/2] Revert "xfrm: interface with if

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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 3 Mar 2022 08:54:27 +0100
From:   Steffen Klassert <steffen.klassert@...unet.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     Kai Lueke <kailueke@...ux.microsoft.com>,
        Paul Chaignon <paul@...ium.io>,
        Eyal Birger <eyal.birger@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH 1/2] Revert "xfrm: interface with if_id 0 should return
 error"

On Wed, Mar 02, 2022 at 09:33:49PM -0800, Jakub Kicinski wrote:
> On Wed, 2 Mar 2022 19:11:06 +0100 Kai Lueke wrote:
> > > Agreed. FWIW would be great if patch #2 started flowing towards Linus'es
> > > tree separately if the discussion on #1 is taking longer.  
> > 
> > to preserve the initial goal of helping to uncover id 0 usage I think it
> > would be best to have the revert be accompanied by a patch that instead
> > creates a kernel log warning (or whatever).
> 
> extack would be best, but that would mean a little bit of plumbing 
> so more likely net-next material. Which would have to come after.

We need something that a user directly sees that the configuration is
invalid. That the xfrm interface acts as a blackhole whith IF_ID 0
is one thing (revert #1).

The other thing is that IF_ID 0 for a policy/state internally means
that these are not assigned to a xfrm interface. So a policy/state
with IF_ID 0 will match flows that are not targeted to the xfrm
interface. This means that confidential stuff could be sent to the
wrong peer (revert #2).

> 
> > Since I never did that I suggest to not wait for me.
> > Also, feel free to do the revert yourself with a different commit
> > message if mine didn't capture the things appropriately.
> 
> TBH I'm not 100% clear on the nature of the regression. Does Cilium
> update the configuration later to make if_id be non-zero? Or the broken 
> interface is not used but not being able to create it fails the whole
> configuration?

As far I unerstood Pauls mail, only 68ac0f3810e7 ("xfrm: state and policy
should fail if XFRMA_IF_ID 0") caused issues in Cilium.

So maybe just revert that one and document that IF_ID 0 on a policy/state
is the same as configuring the policy/state without IF_ID.