Re: [PATCH] nvme-tcp: send only permitted commands for secure concat

[Martin George <martinus.gpy@gmail.com>]

On Wed, 2025-09-10 at 14:58 +0200, Hannes Reinecke wrote:
> On 9/9/25 12:35, Martin George wrote:
> > In addition to sending permitted commands such as connect/auth
> > over the initial unencrypted admin connection as part of secure
> > channel concatenation, the host also sends commands such as
> > Property Get and Identify on the same. This is a spec violation
> > leading to secure concat failures. Fix this by ensuring these
> > additional commands are avoided on this connection.
> > 
> > Fixes: 104d0e2f6222 ("nvme-fabrics: reset admin connection for
> > secure concatenation")
> > Signed-off-by: Martin George <marting@netapp.com>
> > ---
> >   drivers/nvme/host/tcp.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
> > index c0fe8cfb7229..1413788ca7d5 100644
> > --- a/drivers/nvme/host/tcp.c
> > +++ b/drivers/nvme/host/tcp.c
> > @@ -2250,6 +2250,9 @@ static int
> > nvme_tcp_configure_admin_queue(struct nvme_ctrl *ctrl, bool new)
> >   	if (error)
> >   		goto out_cleanup_tagset;
> >   
> > +	if (ctrl->opts->concat && !ctrl->tls_pskid)
> > +		return 0;
> > +
> >   	error = nvme_enable_ctrl(ctrl);
> >   	if (error)
> >   		goto out_stop_queue;
> 
> Hmm. Not sure. While section 8.3.4.3 'NVMe In-band Authentication'
> states:
> 
>   If one or more of the bits in the AUTHREQ field are set to ‘1’,
> then
>   the controller requires that the host authenticate on that queue in
>   order to proceed with Fabrics, Admin, and I/O commands.
> 
>  From which one could assume that none of the commands are allowed.
> But it then goes on to state:
> 
>   The state of an in-progress authentication transaction is soft-
> state.
>   If the subsequent command in an authentication transaction is not
>   received by the controller within a timeout equal to:
>    * the Keep Alive Timeout value (refer to Figure 546), if the Keep
>      Alive Timer is enabled; or
>    * the default Keep Alive Timeout value (i.e., two minutes), if the
>      Keep Alive Timer is disabled;
> 
> one can imply that KATO is running during authentication
> transactions.
> But that might require additional commands, so really I'm not sure.
> Let me ask FMDS for clarification.
> 

This is specific to secure concat alone, and not otherwise. 

NVMe base spec 2.1 section 8.3.4.1 Fabric Secure Channel states that
"An NVM subsystem that requires use of a fabric secure channel (i.e.,
as indicated by the TSC field in the associated Discovery Log Page
Entry) shall not allow capsules to be transferred until a secure
channel has been established for the NVMe Transport connection."

And again in section 8.3.4.3 Secure Channel Concatenation, "This PSK is
generated by an authentication transaction on an Admin Queue over an
unsecure channel. Once the authentication transaction is completed,
that Admin Queue transport connection shall be disconnected by the
host. The generated PSK may then be used to set up secure channels for
subsequent Admin Queue(s) and I/O Queues."

So once authentication is completed on the unencrypted admin connection
as part of secure concat, the host is expected to immediately
disconnect from that and then proceed with setting up subsequent
encrypted admin & I/O connections i.e. no reason for the host to
continue with nvme_enable_ctrl() & nvme_init_ctrl_finish() on the
initial unsecure connection as that's futile and doesn't serve any
purpose. And that's what this patch is attempting to do above.

-Martin