[PATCH v2 0/2] nvme: fixup HKDF-Expand-Label implementation

[PATCH v2 0/2] nvme: fixup HKDF-Expand-Label implementation

[Chris Leech]

As per RFC 8446 (TLS 1.3) the HKDF-Expand-Label function is using vectors
for the 'label' and 'context' field, but defines these vectors as a string
prefixed with the string length (in binary). The implementation in nvme
is missing the length prefix which was causing interoperability issues
with spec-conformant implementations.

This patchset adds a function 'hkdf_expand_label()' to correctly implement
the HKDF-Expand-Label functionality and modifies the nvme driver to utilize
this function instead of the open-coded implementation.

As usual, comments and reviews are welcome.

Changes from v1:
 - Moved hkdf_expand_label() from crypto/hkdf.c to nvme/common/auth.c.
   It's not really an RFC 5869 HKDF function, it's defined for TLS but
   currently only used by nvme in-kernel.
 - Fixed kdoc label_len -> labellen
 - Replaced "static const char []" with "const char *", it's just
   clearer and generates the same code with a string literal assignment.

(I've left the crypto emails on this version, mostly to make it known
that hkdf_expand_label() has been moved as Eric asked.)

Chris Leech (2):
  nvme-auth: add hkdf_expand_label()
  nvme-auth: use hkdf_expand_label()

 drivers/nvme/common/auth.c | 86 +++++++++++++++++++++++++++++---------
 1 file changed, 66 insertions(+), 20 deletions(-)

-- 
2.50.1

[PATCH v2 1/2] nvme-auth: add hkdf_expand_label()

[Chris Leech]

Provide an implementation of RFC 8446 (TLS 1.3) HKDF-Expand-Label

Signed-off-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/common/auth.c | 53 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index 91e273b89fea3..c6eae8e6b6f99 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -683,6 +683,59 @@ int nvme_auth_generate_digest(u8 hmac_id, u8 *psk, size_t psk_len,
 }
 EXPORT_SYMBOL_GPL(nvme_auth_generate_digest);
 
+/**
+ * hkdf_expand_label - HKDF-Expand-Label (RFC 8846 section 7.1)
+ * @hmac_tfm: hash context keyed with pseudorandom key
+ * @label: ASCII label without "tls13 " prefix
+ * @labellen: length of @label
+ * @context: context bytes
+ * @contextlen: length of @context
+ * @okm: output keying material
+ * @okmlen: length of @okm
+ *
+ * Build the TLS 1.3 HkdfLabel structure and invoke hkdf_expand().
+ *
+ * Returns 0 on success with output keying material stored in @okm,
+ * or a negative errno value otherwise.
+ */
+static int hkdf_expand_label(struct crypto_shash *hmac_tfm,
+		const u8 *label, unsigned int labellen,
+		const u8 *context, unsigned int contextlen,
+		u8 *okm, unsigned int okmlen)
+{
+	int err;
+	u8 *info;
+	unsigned int infolen;
+	const char *tls13_prefix = "tls13 ";
+	unsigned int prefixlen = strlen(tls13_prefix);
+
+	if (WARN_ON(labellen > (255 - prefixlen)))
+		return -EINVAL;
+	if (WARN_ON(contextlen > 255))
+		return -EINVAL;
+
+	infolen = 2 + (1 + prefixlen + labellen) + (1 + contextlen);
+	info = kzalloc(infolen, GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	/* HkdfLabel.Length */
+	put_unaligned_be16(okmlen, info);
+
+	/* HkdfLabel.Label */
+	info[2] = prefixlen + labellen;
+	memcpy(info + 3, tls13_prefix, prefixlen);
+	memcpy(info + 3 + prefixlen, label, labellen);
+
+	/* HkdfLabel.Context */
+	info[3 + prefixlen + labellen] = contextlen;
+	memcpy(info + 4 + prefixlen + labellen, context, contextlen);
+
+	err = hkdf_expand(hmac_tfm, info, infolen, okm, okmlen);
+	kfree_sensitive(info);
+	return err;
+}
+
 /**
  * nvme_auth_derive_tls_psk - Derive TLS PSK
  * @hmac_id: Hash function identifier
-- 
2.50.1

[PATCH v2 2/2] nvme-auth: use hkdf_expand_label()

[Chris Leech]

When generating keying material during an authentication transaction
(secure channel concatenation), the HKDF-Expand-Label function is part
of the specified key derivation process.

The current open-coded implementation misses the length prefix
requirements on the HkdfLabel label and context variable-length vectors
(RFC 8446 Section 3.4).

Instead, use the hkdf_expand_label() function.

Signed-off-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/nvme/common/auth.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/nvme/common/auth.c b/drivers/nvme/common/auth.c
index c6eae8e6b6f99..1f51fbebd9fac 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -768,10 +768,10 @@ int nvme_auth_derive_tls_psk(int hmac_id, u8 *psk, size_t psk_len,
 {
 	struct crypto_shash *hmac_tfm;
 	const char *hmac_name;
-	const char *psk_prefix = "tls13 nvme-tls-psk";
+	const char *label = "nvme-tls-psk";
 	static const char default_salt[HKDF_MAX_HASHLEN];
-	size_t info_len, prk_len;
-	char *info;
+	size_t prk_len;
+	const char *ctx;
 	unsigned char *prk, *tls_key;
 	int ret;
 
@@ -811,36 +811,29 @@ int nvme_auth_derive_tls_psk(int hmac_id, u8 *psk, size_t psk_len,
 	if (ret)
 		goto out_free_prk;
 
-	/*
-	 * 2 additional bytes for the length field from HDKF-Expand-Label,
-	 * 2 additional bytes for the HMAC ID, and one byte for the space
-	 * separator.
-	 */
-	info_len = strlen(psk_digest) + strlen(psk_prefix) + 5;
-	info = kzalloc(info_len + 1, GFP_KERNEL);
-	if (!info) {
+	ctx = kasprintf(GFP_KERNEL, "%02d %s", hmac_id, psk_digest);
+	if (!ctx) {
 		ret = -ENOMEM;
 		goto out_free_prk;
 	}
 
-	put_unaligned_be16(psk_len, info);
-	memcpy(info + 2, psk_prefix, strlen(psk_prefix));
-	sprintf(info + 2 + strlen(psk_prefix), "%02d %s", hmac_id, psk_digest);
-
 	tls_key = kzalloc(psk_len, GFP_KERNEL);
 	if (!tls_key) {
 		ret = -ENOMEM;
-		goto out_free_info;
+		goto out_free_ctx;
 	}
-	ret = hkdf_expand(hmac_tfm, info, info_len, tls_key, psk_len);
+	ret = hkdf_expand_label(hmac_tfm,
+				label, strlen(label),
+				ctx, strlen(ctx),
+				tls_key, psk_len);
 	if (ret) {
 		kfree(tls_key);
-		goto out_free_info;
+		goto out_free_ctx;
 	}
 	*ret_psk = tls_key;
 
-out_free_info:
-	kfree(info);
+out_free_ctx:
+	kfree(ctx);
 out_free_prk:
 	kfree(prk);
 out_free_shash:
-- 
2.50.1

Re: [PATCH v2 0/2] nvme: fixup HKDF-Expand-Label implementation

[Eric Biggers]

On Thu, Aug 21, 2025 at 01:48:14PM -0700, Chris Leech wrote:
> As per RFC 8446 (TLS 1.3) the HKDF-Expand-Label function is using vectors
> for the 'label' and 'context' field, but defines these vectors as a string
> prefixed with the string length (in binary). The implementation in nvme
> is missing the length prefix which was causing interoperability issues
> with spec-conformant implementations.
> 
> This patchset adds a function 'hkdf_expand_label()' to correctly implement
> the HKDF-Expand-Label functionality and modifies the nvme driver to utilize
> this function instead of the open-coded implementation.
> 
> As usual, comments and reviews are welcome.

Well, it's nice that my review comment from last year is finally being
addressed: https://lore.kernel.org/r/20240723014715.GB2319848@google.com

- Eric

Re: [PATCH v2 0/2] nvme: fixup HKDF-Expand-Label implementation

[Hannes Reinecke]

On 8/22/25 03:09, Eric Biggers wrote:
> On Thu, Aug 21, 2025 at 01:48:14PM -0700, Chris Leech wrote:
>> As per RFC 8446 (TLS 1.3) the HKDF-Expand-Label function is using vectors
>> for the 'label' and 'context' field, but defines these vectors as a string
>> prefixed with the string length (in binary). The implementation in nvme
>> is missing the length prefix which was causing interoperability issues
>> with spec-conformant implementations.
>>
>> This patchset adds a function 'hkdf_expand_label()' to correctly implement
>> the HKDF-Expand-Label functionality and modifies the nvme driver to utilize
>> this function instead of the open-coded implementation.
>>
>> As usual, comments and reviews are welcome.
> 
> Well, it's nice that my review comment from last year is finally being
> addressed: https://lore.kernel.org/r/20240723014715.GB2319848@google.com
> 
Yeah, because I misread your comments, and was only focussed on the
'length' field (which is a 16-bit field at the start), and not on the
length fields of the individual vectors.

Reading specs is hard...

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