netdev - Re: [PATCH ipsec-next v9] xfrm: Add Direction to the SA in or out

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]

Date: Wed, 10 Apr 2024 10:17:32 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Nicolas Dichtel <nicolas.dichtel@...nd.com>
Cc: antony.antony@...unet.com,
	Steffen Klassert <steffen.klassert@...unet.com>,
	Herbert Xu <herbert@...dor.apana.org.au>, netdev@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	devel@...ux-ipsec.org, Leon Romanovsky <leon@...nel.org>,
	Eyal Birger <eyal.birger@...il.com>
Subject: Re: [PATCH ipsec-next v9] xfrm: Add Direction to the SA in or out

2024-04-10, 09:35:08 +0200, Nicolas Dichtel wrote:
> Le 10/04/2024 à 09:26, Sabrina Dubroca a écrit :
> > 2024-04-10, 08:32:20 +0200, Nicolas Dichtel wrote:
> >> Le 09/04/2024 à 19:56, Antony Antony a écrit :
> >>> v6->v7:
> >>>  - add replay-window check non-esn 0 and ESN 1.
> >>>  - remove :XFRMA_SA_DIR only allowed with HW OFFLOAD
> >> Why? I still think that having an 'input' SA used in the output path is wrong
> >> and confusing.
> >> Please, don't drop this check.
> > 
> > Limiting XFRMA_SA_DIR to only HW offload makes no sense. It's
> > completely redundant with an existing property. We should also try to
> > limit the divergence between offload and non-offload configuration. If
> Sure.
> 
> > something is clearly only for offloaded configs, then fine, but
> > otherwise the APIs should be identical.
> But right now, the property is enforced for offload and but not for non-offload.
> In that sense, the api is not identical. I'm only asking to make this explicit.

We can't get rid of the offload-specific way of setting the direction,
because it's a flag (off = out, on = in), but if we add another way of
setting the direction, it should be for all cases (we already have one
for offload, we don't need a 2nd offload-specific flag), and it should
correctly lock down uses (incompatible options, and use of the SA in
the datapath as you said).

> > And based on what Antony says, this is intended in large part for
> > IPTFS, which is not going to be offloaded any time soon (or probably
> > ever), so that restriction would have to be lifted immediately. I'm
> > not sure why Antony accepted your request.
> I don't see the problem with that. The attribute can be relaxed later for IPTFS
> if needed.

Then we would have landed back on v4 (unless we add the checks we're
discussing now)...

> But there are use cases without offload and without IPTFS.

Sure. That's probably the vast majority of IPsec users.

> Why isn't it possible to restrict the use of an input SA to the input path and
> output SA to xmit path?

Because nobody has written a patch for it yet :)

-- 
Sabrina