Re: [PATCH v2] nvme-auth: Include SC_C in RVAL controller hash

[Hannes Reinecke]

On 4/16/26 01:08, alistair23@xxxxxxxxx wrote:
> From: Alistair Francis <alistair.francis@xxxxxxx>
>
> Section 8.3.4.5.5 of the NVMe Base Specification 2.1 describes what is
> included in the Response Value (RVAL) hash and SC_C should be included.
> Currently we are hardcoding 0 instead of using the correct SC_C value.
>
> Update the host and target code to use the SC_C when calculating the
> RVAL instead of using 0.
>
> Fixes: f50fff73d620 ("nvme: implement In-Band authentication")
> Signed-off-by: Alistair Francis <alistair.francis@xxxxxxx>
> ---
> v2:
>   - Rebase using new nvme_auth_hmac_update() functions
>
>   drivers/nvme/host/auth.c   | 3 ++-
>   drivers/nvme/target/auth.c | 3 ++-
>   2 files changed, 4 insertions(+), 2 deletions(-)
I would argue that this is a fix to secure concatenation, as the
original implementation (ie the quoted commit) did not support
secure concatenation (which sets 'sc_c' to a non-zero value).

Can you adapt the 'Fixes' line?

> diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
> index bbedbe181c8a6..63f543e809985 100644
> --- a/drivers/nvme/host/auth.c
> +++ b/drivers/nvme/host/auth.c
> @@ -535,11 +535,12 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
>   	put_unaligned_le16(chap->transaction, buf);
>   	nvme_auth_hmac_update(&hmac, buf, 2);
>   
> -	memset(buf, 0, 4);
> +	*buf = chap->sc_c;
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, "Controller", 10);
>   	nvme_auth_hmac_update(&hmac, ctrl->opts->subsysnqn,
>   			      strlen(ctrl->opts->subsysnqn));
> +	memset(buf, 0, 4);
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, ctrl->opts->host->nqn,
>   			      strlen(ctrl->opts->host->nqn));
> diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
> index b34610e2f19d4..f032855b21477 100644
> --- a/drivers/nvme/target/auth.c
> +++ b/drivers/nvme/target/auth.c
> @@ -402,11 +402,12 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
>   	put_unaligned_le16(req->sq->dhchap_tid, buf);
>   	nvme_auth_hmac_update(&hmac, buf, 2);
>   
> -	memset(buf, 0, 4);
> +	*buf = req->sq->sc_c;
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, "Controller", 10);
>   	nvme_auth_hmac_update(&hmac, ctrl->subsys->subsysnqn,
>   			      strlen(ctrl->subsys->subsysnqn));
> +	memset(buf, 0, 4);
>   	nvme_auth_hmac_update(&hmac, buf, 1);
>   	nvme_auth_hmac_update(&hmac, ctrl->hostnqn, strlen(ctrl->hostnqn));
>   	nvme_auth_hmac_final(&hmac, response);

Otherwise looks good.

Cheers,

Hannes
--
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@xxxxxxx                                +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