[PATCH] nvme-auth: Don't propose NVME_AUTH_DHGROUP_NULL with SC_C

[PATCH] nvme-auth: Don't propose NVME_AUTH_DHGROUP_NULL with SC_C

[alistair23]

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

Section 8.3.4.5.2 of the NVMe 2.1 base spec states that

"""
The 00h identifier shall not be proposed in an AUTH_Negotiate message
that requests secure channel concatenation (i.e., with the SC_C field
set to a non-zero value).
"""

We need to ensure that we don't set the NVME_AUTH_DHGROUP_NULL idlist if
SC_C is set.

Signed-off-by: Kamaljit Singh <kamaljit.singh@opensource.wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 drivers/nvme/host/auth.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c
index 405e7c03b1cf..40ef6f3fb970 100644
--- a/drivers/nvme/host/auth.c
+++ b/drivers/nvme/host/auth.c
@@ -125,6 +125,7 @@ static int nvme_auth_set_dhchap_negotiate_data(struct nvme_ctrl *ctrl,
 {
 	struct nvmf_auth_dhchap_negotiate_data *data = chap->buf;
 	size_t size = sizeof(*data) + sizeof(union nvmf_auth_protocol);
+	u8 dh_list_offset = 30;
 
 	if (size > CHAP_BUF_SIZE) {
 		chap->status = NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
@@ -144,16 +145,17 @@ static int nvme_auth_set_dhchap_negotiate_data(struct nvme_ctrl *ctrl,
 	data->napd = 1;
 	data->auth_protocol[0].dhchap.authid = NVME_AUTH_DHCHAP_AUTH_ID;
 	data->auth_protocol[0].dhchap.halen = 3;
-	data->auth_protocol[0].dhchap.dhlen = 6;
 	data->auth_protocol[0].dhchap.idlist[0] = NVME_AUTH_HASH_SHA256;
 	data->auth_protocol[0].dhchap.idlist[1] = NVME_AUTH_HASH_SHA384;
 	data->auth_protocol[0].dhchap.idlist[2] = NVME_AUTH_HASH_SHA512;
-	data->auth_protocol[0].dhchap.idlist[30] = NVME_AUTH_DHGROUP_NULL;
-	data->auth_protocol[0].dhchap.idlist[31] = NVME_AUTH_DHGROUP_2048;
-	data->auth_protocol[0].dhchap.idlist[32] = NVME_AUTH_DHGROUP_3072;
-	data->auth_protocol[0].dhchap.idlist[33] = NVME_AUTH_DHGROUP_4096;
-	data->auth_protocol[0].dhchap.idlist[34] = NVME_AUTH_DHGROUP_6144;
-	data->auth_protocol[0].dhchap.idlist[35] = NVME_AUTH_DHGROUP_8192;
+	if (chap->sc_c == NVME_AUTH_SECP_NOSC)
+		data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_NULL;
+	data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_2048;
+	data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_3072;
+	data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_4096;
+	data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_6144;
+	data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_8192;
+	data->auth_protocol[0].dhchap.dhlen = dh_list_offset - 30;
 
 	chap->sc_c = data->sc_c;
 
-- 
2.53.0

Re: [PATCH] nvme-auth: Don't propose NVME_AUTH_DHGROUP_NULL with SC_C

[Christoph Hellwig]

On Fri, Mar 06, 2026 at 02:18:06PM +1000, alistair23@gmail.com wrote:
> +	u8 dh_list_offset = 30;

Can you add a symbolic name for this while we're at it and reuse it for
the dhchap.dhlen assignment.

> +	if (chap->sc_c == NVME_AUTH_SECP_NOSC)
> +		data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_NULL;
> +	data->auth_protocol[0].dhchap.idlist[dh_list_offset++] = NVME_AUTH_DHGROUP_2048;

And please add a local variable pointing data->auth_protocol[0].dhchap.idlist
to make this somewhat readable.