Re: [PATCH 06/19] nvmet: check the host NQN against the TLS PSK identity

[Christoph Hellwig <hch@lst.de>]

On Thu, Oct 29, 2026 at 08:52:39AM -0700, Keith Busch wrote:
> From: Josef Bacik <josef@toxicpanda.com>
> 
> The identity of a TLS PSK names the host and subsystem the key was
> generated for. The target remembers which key a handshake ended with,
> but never compares it with the host NQN in the Connect that follows. So
> a host holding any PSK in the port's keyring can connect as another
> host, sees what that host may see, and isn't asked for DH-HMAC-CHAP
> since TLS queues skip it. It can add an I/O queue to that host's TLS
> controller the same way.
> 
> Refuse a Connect whose host NQN isn't the one in the PSK identity of the
> queue. The subsystem NQN isn't compared, a discovery subsystem goes by
> two names.
> 
> Fixes: 675b453e0241 ("nvmet-tcp: enable TLS handshake upcall")
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Sagi Grimberg <sagi@grimberg.me>
> Cc: Chaitanya Kulkarni <kch@nvidia.com>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/nvme/target/fabrics-cmd.c | 33 +++++++++++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
> 
> diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
> index fc626c2fd2c92..b950000696c2a 100644
> --- a/drivers/nvme/target/fabrics-cmd.c
> +++ b/drivers/nvme/target/fabrics-cmd.c
> @@ -267,6 +267,30 @@ static u32 nvmet_connect_result(struct nvmet_ctrl *ctrl, struct nvmet_sq *sq)
>  		(needs_auth ? NVME_CONNECT_AUTHREQ_ATR : 0);
>  }
>  
> +/*
> + * The identity of a TLS PSK names the host that the key was made for:
> + * "NVMe<version><type><hash> <host NQN> <subsystem NQN> ...".
> + * Whoever has proven to hold the key is that host, and no other one.
> + */
> +static u16 nvmet_check_tls_host(struct nvmet_sq *sq, const char *hostnqn)
> +{
> +#ifdef CONFIG_NVME_TARGET_TCP_TLS
> +	size_t len = strlen(hostnqn);
> +	const char *id;
> +
> +	if (!sq->tls_key)
> +		return 0;
> +
> +	id = strchr(sq->tls_key->description, ' ');
> +	if (!id || strncmp(id + 1, hostnqn, len) || id[len + 1] != ' ') {
> +		pr_warn("TLS PSK %08x does not belong to host %s\n",
> +			key_serial(sq->tls_key), hostnqn);
> +		return NVME_SC_CONNECT_INVALID_HOST | NVME_STATUS_DNR;
> +	}
> +#endif
> +	return 0;
> +}

#ifdef for an entire function body is an anti-pattern.  Either stub
out the entire function, or use IS_ENABLED depending on which one
works better here.

The rest looks good.