Re: [PATCH net-next 4/6] netdevsim: psp: implement kdf from psp spec

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Daniel Zahka wrote:
> Implement the PSP key derivation function (KDF) per the PSP
> Architecture Spec.
> 
> The kdf is used to generate spi + session key pairs, and will also be

Text is a bit ambiguous here: the kdf does not generate the spi. It
derives a session key from the master key and spi.

> used in the rx path to re-derive the tx key used by the peer.
> 
> Also, remove support for psd->generation, as it is not needed for
> netdevsim after removing the fake authentication hack.

Is psd->generation only used inside driver code, not by the core PSP
stack? Else it should be set to !!(ns->psp.spi & PSP_SPI_KEY_PHASE) on
key rotation. If only used by the driver, no need to reset it on each
rotation.

> Assisted-by: Claude:claude-opus-4.6
> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>

>  enum skb_drop_reason
>  nsim_psp_handle_tx(struct sk_buff *skb, struct netdevsim *ns)
>  {
> @@ -155,7 +189,7 @@ nsim_rx_spi_alloc(struct psp_dev *psd, u32 version,
>  		  struct netlink_ext_ack *extack)
>  {
>  	struct netdevsim *ns = psd->drv_priv;
> -	int i;
> +	unsigned int phase;
>  
>  	if ((ns->psp.spi ^ (ns->psp.spi + 1)) & PSP_SPI_KEY_PHASE) {
>  		NL_SET_ERR_MSG(extack, "SPI space exhausted");
> @@ -163,9 +197,11 @@ nsim_rx_spi_alloc(struct psp_dev *psd, u32 version,
>  	}
>  
>  	assoc->spi = cpu_to_be32(++ns->psp.spi);
> -	assoc->key[0] = psd->generation;
> -	for (i = 1; i < PSP_MAX_KEY; i++)
> -		assoc->key[i] = ns->psp.spi + i;
> +	phase = !!(ns->psp.spi & PSP_SPI_KEY_PHASE);
> +
> +	/* dev_keys_lock not needed because of psd->lock */

Can you elaborate a bit?

Is dev_keys_lock only used to synchronize the writers, then? Which after
device init would only be concurrent invocations of nsim_key_rotate. But
that operation correctly also holds the device lock using
psp_device_get_locked.

> +	nsim_psp_derive_key(ns->psp.dev_keys[phase], assoc->spi, version,
> +			    assoc->key);
>  
>  	return 0;
>  }
> @@ -186,8 +222,15 @@ static int nsim_assoc_add(struct psp_dev *psd, struct psp_assoc *pas,
>  static int nsim_key_rotate(struct psp_dev *psd, struct netlink_ext_ack *extack)
>  {
>  	struct netdevsim *ns = psd->drv_priv;
> +	unsigned int next_phase;
>  
> +	psd->generation = 0;
>  	ns->psp.spi = (ns->psp.spi & PSP_SPI_KEY_PHASE) ^ PSP_SPI_KEY_PHASE;
> +	next_phase = !!(ns->psp.spi & PSP_SPI_KEY_PHASE);
> +
> +	spin_lock_bh(&ns->psp.dev_keys_lock);
> +	get_random_bytes(ns->psp.dev_keys[next_phase], NSIM_PSP_DEV_KEY_SIZE);
> +	spin_unlock_bh(&ns->psp.dev_keys_lock);
>  
>  	return 0;
>  }
> @@ -295,6 +338,10 @@ int nsim_psp_init(struct netdevsim *ns)
>  	struct dentry *ddir = ns->nsim_dev_port->ddir;
>  	struct psp_dev *psd;
>  
> +	spin_lock_init(&ns->psp.dev_keys_lock);
> +	get_random_bytes(ns->psp.dev_keys[0], NSIM_PSP_DEV_KEY_SIZE);
> +	get_random_bytes(ns->psp.dev_keys[1], NSIM_PSP_DEV_KEY_SIZE);
> +
>  	psd = psp_dev_create(ns->netdev, &nsim_psp_ops, &nsim_psp_caps, ns);
>  	if (IS_ERR(psd))
>  		return PTR_ERR(psd);
> 
> -- 
> 2.52.0
>