netdev - RE: [RFC net-next 00/15] add basic PSP encryption for TCP connections

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: Tue, 25 Jun 2024 22:05:13 +0000
From: "Singhai, Anjali" <anjali.singhai@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, Paolo Abeni
	<pabeni@...hat.com>, Boris Pismenny <borisp@...dia.com>, "gal@...dia.com"
	<gal@...dia.com>, "cratiu@...dia.com" <cratiu@...dia.com>,
	"rrameshbabu@...dia.com" <rrameshbabu@...dia.com>,
	"steffen.klassert@...unet.com" <steffen.klassert@...unet.com>,
	"tariqt@...dia.com" <tariqt@...dia.com>, "Samudrala, Sridhar"
	<sridhar.samudrala@...el.com>, "Acharya, Arun Kumar"
	<arun.kumar.acharya@...el.com>
Subject: RE: [RFC net-next 00/15] add basic PSP encryption for TCP connections


On Thu, 20 Jun 2024 21:32:14 +0000 Singhai, Anjali wrote:
> > > > 1. Why do we need  ndo_op set_config() at device level which is setting only one version, instead the description above the psp_dev struct which had a mask for enabled versions at a  device level is better and device lets the stack know at psp_dev create time what all versions it is capable of.  Later on, version is negotiated with the peer and set per session.
> > > > Even the Mellanox driver does not implement this set_config ndo_op. 
> > >  >
> > Can you or Kuba comment on this?

> For now the only action the driver can perform is to disable PSP Rx
> handling:

> https://github.com/kuba-moo/linux/blob/psp/drivers/net/ethernet/mellanox/mlx5/core/en_accel/psp.c#L18

Okay, can move into kernel as the device already fills its capabilities, but it doesn't matter for now, will do the same.

> > > > > 2. Where is the association_index/handle returned to the stack to be used with the packet on TX by the driver and device? ( if an SADB is in use on Tx side in the device), what we understand from Mellanox driver is, its not doing an SADB in TX in HW, but passing the key directly into the Tx descriptor? Is that right, but other devices may not support this and will have an SADB on TX and this allowed as per PSP protocol. Of course on RX there is no SADB for any device.
> > > > In our device we have 2 options, 
> > > >             1. Using SADB on TX and just passing SA_Index in the descriptor (trade off between performance and memory. 
> > > >             As  passing key in descriptor makes for a much larger TX descriptor which will have perf penalty.)
> > > >            2. Passing key in the descriptor.
> > > >             For us we need both these options, so please allow for enhancements.
> > > >  
> > Can you or Kuba comment on this? This is critical, also in the fast path, skb needs to carry the SA_index/handle (like the tls case) instead of the key or both so that either method can be used by the device driver/device.

> The ID should go into the driver state of the association, specify how much space you need by setting this:
https://github.com/kuba-moo/linux/blob/psp/include/net/psp/types.h#L110C6-L110C19
Then you can access it via psp_assoc_drv_data()

Is this already posted as new patch series on netdev mailing list, maybe I missed it. This works for us, will follow the same.

> AFAICT Willem answered all the other points.

We still have two more that are not addressed
1. Master key rotation, this needs to be not just from top but also from device side because of the master key sharing in some cases and a common SPI space that can roll over. So need to have an event mechanism from device driver back to kernel for key rotation. Please add this.

2. Header additions in the driver, for now we can do this, but ideally it should move to the stack, so that it can be common for all devices.

Anjali