netdev - Re: [devel-ipsec] [PATCH ipsec-next v6] 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 for Android: free password hash cracker in your pocket

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

Message-ID: <ZhesGJtMXk-PPtzz@hog>
Date: Thu, 11 Apr 2024 11:23:36 +0200
From: Sabrina Dubroca <sd@...asysnail.net>
To: Paul Wouters <paul@...ats.ca>,
	Steffen Klassert <steffen.klassert@...unet.com>
Cc: Antony Antony <antony@...nome.org>,
	Nicolas Dichtel <nicolas.dichtel@...nd.com>,
	Antony Antony <antony.antony@...unet.com>,
	Herbert Xu <herbert@...dor.apana.org.au>, netdev@...r.kernel.org,
	devel@...ux-ipsec.org, Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [devel-ipsec] [PATCH ipsec-next v6] xfrm: Add Direction to the
 SA in or out

2024-04-10, 20:58:33 -0400, Paul Wouters wrote:
> On Wed, 10 Apr 2024, Antony Antony via Devel wrote:
> > > > Though supporting 0 is higly desired
> > > > feature and probably a hard to implement feature in xfrm code.
> > > 
> > > Why would it be hard for outgoing SAs? The replay window should never
> > > be used on those. And xfrm_replay_check_esn and xfrm_replay_check_bmp
> > > already have checks for 0-sized replay window.
> > 
> > That information comes from hall way talks with Steffen. I can't explain
> > it:) May be he can elaborate why 0 is not allowed with ESN.
> 
> With ESN, you use a 64 bit number but only send a 32 bit number over the
> wire. So you need to "track" the parts not being sent to do the proper
> packet authentication that uses the full 64bit number. The
> authentication bit is needed for encrypting and decrypting, so on both
> the incoming and outgoing SA.
> 
> AFAIK, this 64 bit number tracking is done using the replay-window code.
> That is why replay-window cannot be 0 when ESN is enabled in either
> direction of the SA.

It's in the replay-window code, but AFAICT it doesn't use the
replay_window variable at all (xfrm_output calls into the
xfrm_replay_overflow_* functions which only look at oseq, xfrm_input
calls the *check and *advance functions of xfrm_replay.c). So I think
we could accept an unset replay_window for an output SA.

> I have already poked Steffen it would be good to decouple ESN code from
> replay-window code, as often people want to benchmark highspeed links
> by disabling replay protection completely, but then they are also
> unwittingly disabling ESN and causing needing a rekey ever 2 minutes
> or so on a modern 100gbps ipsec link.
> 
> > strongSwan sets ESN and replay-window 1 on "out" SA.
> 
> It has to set a replay-window of non-zero or else ESN won't work.
> It is not related to migration AFAIK.
> 
> > For instance, there isn't a validation for unused XFRMA_SA_EXTRA_FLAGS in
> > DELSA; if set, it's simply ignored. Similarly, if XFRMA_SA_DIR were set in
> > DELSA, it would also be disregarded. Attempting to introduce validations for
> > DELSA and other methods seems like an extensive cleanup task. Do we consider
> > this level of validation within the scope of our current patch? It feels
> > like we are going too far.
> 
> Is there a way where rate limited logging can be introduced, so that
> userlands will clean up their use and after a few years change the API
> to not allow setting bogus values?

Yes, this is doable. Steffen, does that seem reasonable? (for example,
when XFRMA_REPLAY_THRESH is passed to NEWSA, or XFRMA_ALG_AEAD to
DELSA, etc)

(as part of a separate patchset of course)

-- 
Sabrina