netdev - Re: [PATCHv3 net-next 2/3] bonding: Add ESN support to IPSec HW offload

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: <ZsbkdzvjVf3GiYHa@gauss3.secunet.de>
Date: Thu, 22 Aug 2024 09:10:47 +0200
From: Steffen Klassert <steffen.klassert@...unet.com>
To: Hangbin Liu <liuhangbin@...il.com>
CC: Sabrina Dubroca <sd@...asysnail.net>, <netdev@...r.kernel.org>, "Jay
 Vosburgh" <j.vosburgh@...il.com>, "David S . Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, "Eric
 Dumazet" <edumazet@...gle.com>, Nikolay Aleksandrov <razor@...ckwall.org>,
	Tariq Toukan <tariqt@...dia.com>, Jianbo Liu <jianbol@...dia.com>, "Simon
 Horman" <horms@...nel.org>
Subject: Re: [PATCHv3 net-next 2/3] bonding: Add ESN support to IPSec HW
 offload

On Thu, Aug 22, 2024 at 08:33:17AM +0800, Hangbin Liu wrote:
> On Wed, Aug 21, 2024 at 03:39:48PM +0200, Sabrina Dubroca wrote:
> > > > > > +	if (!real_dev->xfrmdev_ops ||
> > > > > > +	    !real_dev->xfrmdev_ops->xdo_dev_state_advance_esn) {
> > > > > > +		pr_warn("%s: %s doesn't support xdo_dev_state_advance_esn\n", __func__, real_dev->name);
> > > > > 
> > > > > xdo_dev_state_advance_esn is called on the receive path for every
> > > > > packet when ESN is enabled (xfrm_input -> xfrm_replay_advance ->
> > > > > xfrm_replay_advance_esn -> xfrm_dev_state_advance_esn), this needs to
> > > > > be ratelimited.
> > > > 
> > > > How does xfrm_state offload work on bonding?
> > > > Does every slave have its own negotiated SA?
> > > 
> > > Yes and no. Bonding only supports xfrm offload with active-backup mode. So only
> > > current active slave keep the SA. When active slave changes, the sa on
> > > previous slave is deleted and re-added on new active slave.
> > 
> > It's the same SA, there's no DELSA+NEWSA when we change the active
> > slave (but we call xdo_dev_state_delete/xdo_dev_state_add to inform
> > the driver/HW), and only a single NEWSA to install the offloaded SA on
> > the bond device (which calls the active slave's xdo_dev_state_add).
> 
> Yes, thanks for the clarification. The SA is not changed, we just delete it
> on old active slave
> 
> slave->dev->xfrmdev_ops->xdo_dev_state_delete(ipsec->xs);
> 
> And add to now one.
> 
> ipsec->xs->xso.real_dev = slave->dev;
> slave->dev->xfrmdev_ops->xdo_dev_state_add(ipsec->xs, NULL)

Using the same key on two different devices is very dangerous.
Counter mode algorithms have the requirement that the IV
must not repeat. If you use the same key on two devices,
you can't guarantee that. If both devices use an internal
counter (initialized to one) to generate the IV, then the
IV repeats for the mumber of packets that were already
sent on the old device. The algorithm is cryptographically
broken in that case.

Instead of moving the existing state, it is better to
request a rekey. Maybe by setting the old state to
'expired' and then send a km_state_expired() message.