nvme-auth: Include SC_C in RVAL controller hash

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

Posted by alistair23@gmail.com 2 weeks, 6 days ago

From: Alistair Francis <alistair.francis@wdc.com>

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@wdc.com>
---
 drivers/nvme/host/auth.c   | 2 +-
 drivers/nvme/target/auth.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index 4e8e4ddd6c8d..4ea1a46bad94 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -581,7 +581,7 @@ static int nvme_auth_dhchap_setup_ctrl_response(struct nvme_ctrl *ctrl,
 	ret = crypto_shash_update(shash, buf, 2);
 	if (ret)
 		goto out;
-	memset(buf, 0, 4);
+	*buf = chap->sc_c;
 	ret = crypto_shash_update(shash, buf, 1);
 	if (ret)
 		goto out;
diff --git a/drivers/nvme/target/auth.c b/drivers/nvme/target/auth.c
index 839239fe877a..965690afd1bf 100644
--- a/drivers/nvme/target/auth.c
+++ b/drivers/nvme/target/auth.c
@@ -477,7 +477,7 @@ int nvmet_auth_ctrl_hash(struct nvmet_req *req, u8 *response,
 	ret = crypto_shash_update(shash, buf, 2);
 	if (ret)
 		goto out;
-	memset(buf, 0, 4);
+	*buf = req->sq->sc_c;
 	ret = crypto_shash_update(shash, buf, 1);
 	if (ret)
 		goto out;
-- 
2.53.0

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

Posted by Chris Leech 2 weeks, 5 days ago

On Tue, Mar 17, 2026 at 01:13:30PM +1000, alistair23@gmail.com wrote:
> From: Alistair Francis <alistair.francis@wdc.com>
> 
> 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.

Am I correct in reading this, the current kernel implementation is out
of spec when doing secure channel concatenation (SC_C != 0) with
bidirectional authentication (R1 takes SC_C into account, but not R2)?

- Chris

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

Posted by Alistair Francis 2 weeks, 4 days ago

On Thu, Mar 19, 2026 at 2:40 AM Chris Leech <cleech@redhat.com> wrote:
>
> On Tue, Mar 17, 2026 at 01:13:30PM +1000, alistair23@gmail.com wrote:
> > From: Alistair Francis <alistair.francis@wdc.com>
> >
> > 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.
>
> Am I correct in reading this, the current kernel implementation is out
> of spec when doing secure channel concatenation (SC_C != 0) with
> bidirectional authentication (R1 takes SC_C into account, but not R2)?

Correct, SC_C isn't included in the RVAL. So if SC_C is 0 it works,
but if it's 1 it doesn't work

Alistair

>
> - Chris
>

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

Posted by Hannes Reinecke 2 weeks, 5 days ago

On 3/18/26 17:40, Chris Leech wrote:
> On Tue, Mar 17, 2026 at 01:13:30PM +1000, alistair23@gmail.com wrote:
>> From: Alistair Francis <alistair.francis@wdc.com>
>>
>> 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.
> 
> Am I correct in reading this, the current kernel implementation is out
> of spec when doing secure channel concatenation (SC_C != 0) with
> bidirectional authentication (R1 takes SC_C into account, but not R2)?
> 
I certainly never tested secure concatenation with bi-directional 
authentication (seem to have fallen into the same trap as Martin
George). So the chance of regression is rather small.
When you fix it, pleas add a test-case for blktests, too, such
that we extend test coverage for this particular corner-case.

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-auth: Include SC_C in RVAL controller hash

Posted by Alistair Francis 2 weeks, 4 days ago

On Thu, Mar 19, 2026 at 2:48 AM Hannes Reinecke <hare@suse.de> wrote:
>
> On 3/18/26 17:40, Chris Leech wrote:
> > On Tue, Mar 17, 2026 at 01:13:30PM +1000, alistair23@gmail.com wrote:
> >> From: Alistair Francis <alistair.francis@wdc.com>
> >>
> >> 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.
> >
> > Am I correct in reading this, the current kernel implementation is out
> > of spec when doing secure channel concatenation (SC_C != 0) with
> > bidirectional authentication (R1 takes SC_C into account, but not R2)?
> >
> I certainly never tested secure concatenation with bi-directional
> authentication (seem to have fallen into the same trap as Martin
> George). So the chance of regression is rather small.
> When you fix it, pleas add a test-case for blktests, too, such
> that we extend test coverage for this particular corner-case.

Ok, I'll add a blktest test-case

Alistair

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