linux-nvme.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] crypto,nvme: fixup HKDF-Expand-Label implementation
@ 2025-08-20  9:12 hare
  2025-08-20  9:12 ` [PATCH 1/2] crypto: hkdf: add hkdf_expand_label() hare
  2025-08-20  9:12 ` [PATCH 2/2] nvme-auth: use hkdf_expand_label() hare
  0 siblings, 2 replies; 7+ messages in thread
From: hare @ 2025-08-20  9:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Keith Busch, Sagi Grimberg, Chris Leech, linux-nvme, Herbert Xu,
	David S . Miller, linux-crypto, Hannes Reinecke

From: Hannes Reinecke <hare@kernel.org>

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.

Chris Leech (1):
  crypto: hkdf: add hkdf_expand_label()

Hannes Reinecke (1):
  nvme-auth: use hkdf_expand_label()

 crypto/hkdf.c              | 55 ++++++++++++++++++++++++++++++++++++++
 drivers/nvme/common/auth.c | 33 +++++++++--------------
 include/crypto/hkdf.h      |  4 +++
 3 files changed, 72 insertions(+), 20 deletions(-)

-- 
2.43.0



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] crypto: hkdf: add hkdf_expand_label()
  2025-08-20  9:12 [PATCH 0/2] crypto,nvme: fixup HKDF-Expand-Label implementation hare
@ 2025-08-20  9:12 ` hare
  2025-08-20 18:46   ` Eric Biggers
  2025-08-20 21:50   ` kernel test robot
  2025-08-20  9:12 ` [PATCH 2/2] nvme-auth: use hkdf_expand_label() hare
  1 sibling, 2 replies; 7+ messages in thread
From: hare @ 2025-08-20  9:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Keith Busch, Sagi Grimberg, Chris Leech, linux-nvme, Herbert Xu,
	David S . Miller, linux-crypto, Eric Biggers, Hannes Reinecke

From: Chris Leech <cleech@redhat.com>

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

Cc: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 crypto/hkdf.c         | 55 +++++++++++++++++++++++++++++++++++++++++++
 include/crypto/hkdf.h |  4 ++++
 2 files changed, 59 insertions(+)

diff --git a/crypto/hkdf.c b/crypto/hkdf.c
index 82d1b32ca6ce..465bad6e6c93 100644
--- a/crypto/hkdf.c
+++ b/crypto/hkdf.c
@@ -11,6 +11,7 @@
 #include <crypto/sha2.h>
 #include <crypto/hkdf.h>
 #include <linux/module.h>
+#include <linux/unaligned.h>
 
 /*
  * HKDF consists of two steps:
@@ -129,6 +130,60 @@ int hkdf_expand(struct crypto_shash *hmac_tfm,
 }
 EXPORT_SYMBOL_GPL(hkdf_expand);
 
+/**
+ * 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
+ * @label_len: 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.
+ */
+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;
+	static const char tls13_prefix[] = "tls13 ";
+	unsigned int prefixlen = sizeof(tls13_prefix) - 1; /* exclude NUL */
+
+	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;
+}
+EXPORT_SYMBOL_GPL(hkdf_expand_label);
+
 struct hkdf_testvec {
 	const char *test;
 	const u8 *ikm;
diff --git a/include/crypto/hkdf.h b/include/crypto/hkdf.h
index 6a9678f508f5..5e75d17a58ab 100644
--- a/include/crypto/hkdf.h
+++ b/include/crypto/hkdf.h
@@ -17,4 +17,8 @@ int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm,
 int hkdf_expand(struct crypto_shash *hmac_tfm,
 		const u8 *info, unsigned int infolen,
 		u8 *okm, unsigned int okmlen);
+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);
 #endif
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] nvme-auth: use hkdf_expand_label()
  2025-08-20  9:12 [PATCH 0/2] crypto,nvme: fixup HKDF-Expand-Label implementation hare
  2025-08-20  9:12 ` [PATCH 1/2] crypto: hkdf: add hkdf_expand_label() hare
@ 2025-08-20  9:12 ` hare
  1 sibling, 0 replies; 7+ messages in thread
From: hare @ 2025-08-20  9:12 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Keith Busch, Sagi Grimberg, Chris Leech, linux-nvme, Herbert Xu,
	David S . Miller, linux-crypto, Hannes Reinecke

From: Hannes Reinecke <hare@kernel.org>

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 91e273b89fea..5ea4d6d9a394 100644
--- a/drivers/nvme/common/auth.c
+++ b/drivers/nvme/common/auth.c
@@ -715,10 +715,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;
 
@@ -758,36 +758,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.43.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] crypto: hkdf: add hkdf_expand_label()
  2025-08-20  9:12 ` [PATCH 1/2] crypto: hkdf: add hkdf_expand_label() hare
@ 2025-08-20 18:46   ` Eric Biggers
  2025-08-20 19:48     ` Chris Leech
  2025-08-20 21:50   ` kernel test robot
  1 sibling, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2025-08-20 18:46 UTC (permalink / raw)
  To: hare
  Cc: Christoph Hellwig, Keith Busch, Sagi Grimberg, Chris Leech,
	linux-nvme, Herbert Xu, David S . Miller, linux-crypto

On Wed, Aug 20, 2025 at 11:12:10AM +0200, hare@kernel.org wrote:
> From: Chris Leech <cleech@redhat.com>
> 
> Provide an implementation of RFC 8446 (TLS 1.3) HKDF-Expand-Label
> 
> Cc: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Chris Leech <cleech@redhat.com>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
>  crypto/hkdf.c         | 55 +++++++++++++++++++++++++++++++++++++++++++
>  include/crypto/hkdf.h |  4 ++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/crypto/hkdf.c b/crypto/hkdf.c
> index 82d1b32ca6ce..465bad6e6c93 100644
> --- a/crypto/hkdf.c
> +++ b/crypto/hkdf.c
> @@ -11,6 +11,7 @@
>  #include <crypto/sha2.h>
>  #include <crypto/hkdf.h>
>  #include <linux/module.h>
> +#include <linux/unaligned.h>
>  
>  /*
>   * HKDF consists of two steps:
> @@ -129,6 +130,60 @@ int hkdf_expand(struct crypto_shash *hmac_tfm,
>  }
>  EXPORT_SYMBOL_GPL(hkdf_expand);
>  
> +/**
> + * 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
> + * @label_len: 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.
> + */
> +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;
> +	static const char tls13_prefix[] = "tls13 ";
> +	unsigned int prefixlen = sizeof(tls13_prefix) - 1; /* exclude NUL */
> +
> +	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;
> +}
> +EXPORT_SYMBOL_GPL(hkdf_expand_label);

Does this belong in crypto/hkdf.c?  It seems to be specific to a
particular user of HKDF.

- Eric


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] crypto: hkdf: add hkdf_expand_label()
  2025-08-20 18:46   ` Eric Biggers
@ 2025-08-20 19:48     ` Chris Leech
  2025-08-21  6:44       ` Hannes Reinecke
  0 siblings, 1 reply; 7+ messages in thread
From: Chris Leech @ 2025-08-20 19:48 UTC (permalink / raw)
  To: Eric Biggers
  Cc: hare, Christoph Hellwig, Keith Busch, Sagi Grimberg, linux-nvme,
	Herbert Xu, David S . Miller, linux-crypto

On Wed, Aug 20, 2025 at 11:46:33AM -0700, Eric Biggers wrote:
> On Wed, Aug 20, 2025 at 11:12:10AM +0200, hare@kernel.org wrote:
> > From: Chris Leech <cleech@redhat.com>
> > 
> > Provide an implementation of RFC 8446 (TLS 1.3) HKDF-Expand-Label
> > 
> > Cc: Eric Biggers <ebiggers@kernel.org>
> > Signed-off-by: Chris Leech <cleech@redhat.com>
> > Signed-off-by: Hannes Reinecke <hare@kernel.org>
> > ---
> >  crypto/hkdf.c         | 55 +++++++++++++++++++++++++++++++++++++++++++
> >  include/crypto/hkdf.h |  4 ++++
> >  2 files changed, 59 insertions(+)
> > 
> > ...
>
> Does this belong in crypto/hkdf.c?  It seems to be specific to a
> particular user of HKDF.

While this is needed for NVMe/TLS, it's a case of the NVMe
specifications referencing a function defined in the TLS 1.3 RFC to be
used.  I though it would be clearest to fix the open-coded implemenation
by creating an RFC complient function, which is now no-longer specific
to NVMe so I moved it out to crypto/hkdf.c

I don't know that there will be other users, it just seemed to make the
most sense there.

- Chris



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] crypto: hkdf: add hkdf_expand_label()
  2025-08-20  9:12 ` [PATCH 1/2] crypto: hkdf: add hkdf_expand_label() hare
  2025-08-20 18:46   ` Eric Biggers
@ 2025-08-20 21:50   ` kernel test robot
  1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2025-08-20 21:50 UTC (permalink / raw)
  To: hare, Christoph Hellwig
  Cc: oe-kbuild-all, Keith Busch, Sagi Grimberg, Chris Leech,
	linux-nvme, Herbert Xu, David S . Miller, linux-crypto,
	Eric Biggers, Hannes Reinecke

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on herbert-cryptodev-2.6/master]
[also build test WARNING on herbert-crypto-2.6/master linus/master v6.17-rc2 next-20250820]
[cannot apply to hch-configfs/for-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/hare-kernel-org/crypto-hkdf-add-hkdf_expand_label/20250820-172750
base:   https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link:    https://lore.kernel.org/r/20250820091211.25368-2-hare%40kernel.org
patch subject: [PATCH 1/2] crypto: hkdf: add hkdf_expand_label()
config: loongarch-randconfig-001-20250821 (https://download.01.org/0day-ci/archive/20250821/202508210523.p8BdHsR7-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 14.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250821/202508210523.p8BdHsR7-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508210523.p8BdHsR7-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: crypto/hkdf.c:151 function parameter 'labellen' not described in 'hkdf_expand_label'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] crypto: hkdf: add hkdf_expand_label()
  2025-08-20 19:48     ` Chris Leech
@ 2025-08-21  6:44       ` Hannes Reinecke
  0 siblings, 0 replies; 7+ messages in thread
From: Hannes Reinecke @ 2025-08-21  6:44 UTC (permalink / raw)
  To: Chris Leech, Eric Biggers
  Cc: hare, Christoph Hellwig, Keith Busch, Sagi Grimberg, linux-nvme,
	Herbert Xu, David S . Miller, linux-crypto

On 8/20/25 21:48, Chris Leech wrote:
> On Wed, Aug 20, 2025 at 11:46:33AM -0700, Eric Biggers wrote:
>> On Wed, Aug 20, 2025 at 11:12:10AM +0200, hare@kernel.org wrote:
>>> From: Chris Leech <cleech@redhat.com>
>>>
>>> Provide an implementation of RFC 8446 (TLS 1.3) HKDF-Expand-Label
>>>
>>> Cc: Eric Biggers <ebiggers@kernel.org>
>>> Signed-off-by: Chris Leech <cleech@redhat.com>
>>> Signed-off-by: Hannes Reinecke <hare@kernel.org>
>>> ---
>>>   crypto/hkdf.c         | 55 +++++++++++++++++++++++++++++++++++++++++++
>>>   include/crypto/hkdf.h |  4 ++++
>>>   2 files changed, 59 insertions(+)
>>>
>>> ...
>>
>> Does this belong in crypto/hkdf.c?  It seems to be specific to a
>> particular user of HKDF.
> 
> While this is needed for NVMe/TLS, it's a case of the NVMe
> specifications referencing a function defined in the TLS 1.3 RFC to be
> used.  I though it would be clearest to fix the open-coded implemenation
> by creating an RFC complient function, which is now no-longer specific
> to NVMe so I moved it out to crypto/hkdf.c
> 
> I don't know that there will be other users, it just seemed to make the
> most sense there.
> 
But having said that, we can easily move it into the nvme code, and let
others move it into crypto if there is a need.
Will be updating the patchset.

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-08-21  9:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20  9:12 [PATCH 0/2] crypto,nvme: fixup HKDF-Expand-Label implementation hare
2025-08-20  9:12 ` [PATCH 1/2] crypto: hkdf: add hkdf_expand_label() hare
2025-08-20 18:46   ` Eric Biggers
2025-08-20 19:48     ` Chris Leech
2025-08-21  6:44       ` Hannes Reinecke
2025-08-20 21:50   ` kernel test robot
2025-08-20  9:12 ` [PATCH 2/2] nvme-auth: use hkdf_expand_label() hare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).