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

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

[Martin George]

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;
-- 
2.43.0

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

[Hannes Reinecke]

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.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Martin George]

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

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

[Hannes Reinecke]

On 9/11/25 15:07, Martin George wrote:
> 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.
> 
Right you are.
I stand corrected.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

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

[Keith Busch]

On Tue, Sep 09, 2025 at 04:05:09PM +0530, 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.

Thanks, applied to nvme-6.18.