nvme: implement secure concatenation
Subsystems (5)
v9 cover nvme: implement secure concatenation
Hi all,
here's my attempt to implement secure concatenation for NVMe-of TCP as outlined in TP8018.
The original (v5) patchset had been split in two, the first part of which has already been merged with nvme-6.11, and this is the second part which actually implements secure concatenation.
Secure concatenation means that a TLS PSK is generated from the key material negotiated by the DH-HMAC-CHAP protocol, and the TLS PSK is then used for a subsequent TLS connection.
The difference between the original definition of secure concatenation and the method outlined in TP8018 is that with TP8018 the connection is reset after DH-HMAC-CHAP negotiation, and a new connection is setup with the generated TLS PSK.
To implement that Sagi came up with the idea to directly reset the admin queue once the DH-CHAP negotiation has completed; that way it will be transparent to the upper layers and we don't have to worry about exposing queues which should not be used.
As usual, comments and reviews are welcome.
Patchset can be found at git.kernel.org:/pub/scm/linux/kernel/git/hare/nvme.git branch secure-concat.v9
Changes to v8:
- Include reviews from Eric Biggers
- Make hkdf a proper module
- Add testcases for hkdf
Changes to v7:
- Add patch to display nvme target TLS status in debugfs
- Include reviews from Sagi
Changes to v6:
- Rebase to nvme-6.11
Changes to v5:
- Include reviews from Sagi
- Split patchset in two parts
Changes to v4:
- Rework reset admin queue functionality based on an idea from Sagi (thanks!)
- kbuild robot fixes
- Fixup dhchap negotiation with non-empty C2 value
Changes to v3:
- Include reviews from Sagi
- Do not start I/O queues after DH-HMAC-CHAP negotiation
- Use bool to indicate TLS has been enabled on a queue
- Add 'tls_keyring' sysfs attribute
- Add 'tls_configured_key' sysfs attribute
Changes to v2:
- Fixup reset after dhchap negotiation
- Disable namespace scanning on I/O queues after
dhchap negotiation
- Reworked TLS key handling (again)
Changes to the original submission:
- Sanitize TLS key handling
- Fixup modconfig compilation
Hannes Reinecke (9):
crypto,fs: Separate out hkdf_extract() and hkdf_expand()
nvme: add nvme_auth_generate_psk()
nvme: add nvme_auth_generate_digest()
nvme: add nvme_auth_derive_tls_psk()
nvme-keyring: add nvme_tls_psk_refresh()
nvme-tcp: request secure channel concatenation
nvme-fabrics: reset admin connection for secure concatenation
nvmet-tcp: support secure channel concatenation
nvmet: add tls_concat and tls_key debugfs entries
crypto/Kconfig | 4 + crypto/Makefile | 1 + crypto/hkdf.c | 404 +++++++++++++++++++++++++ drivers/nvme/common/Kconfig | 1 + drivers/nvme/common/auth.c | 344 +++++++++++++++++++++ drivers/nvme/common/keyring.c | 64 ++++ drivers/nvme/host/auth.c | 108 ++++++- drivers/nvme/host/fabrics.c | 34 ++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/sysfs.c | 4 +- drivers/nvme/host/tcp.c | 56 +++- drivers/nvme/target/auth.c | 72 ++++- drivers/nvme/target/debugfs.c | 27 ++ drivers/nvme/target/fabrics-cmd-auth.c | 49 ++- drivers/nvme/target/fabrics-cmd.c | 33 +- drivers/nvme/target/nvmet.h | 38 ++- drivers/nvme/target/tcp.c | 23 +- fs/crypto/Kconfig | 1 + fs/crypto/hkdf.c | 87 +----- include/crypto/hkdf.h | 34 +++ include/linux/nvme-auth.h | 7 + include/linux/nvme-keyring.h | 9 + include/linux/nvme.h | 7 + 24 files changed, 1303 insertions(+), 109 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h
Signature · 1 line
2.35.3
PATCH v9 1/9 crypto,fs: Separate out hkdf_extract() and hkdf_expand()
Separate out the HKDF functions into a separate module to to make them available to other callers.
And add a testsuite to the module with test vectors from RFC 5869 to ensure the integrity of the algorithm.
--- crypto/Kconfig | 4 + crypto/Makefile | 1 + crypto/hkdf.c | 404 ++++++++++++++++++++++++++++++++++++++++++ fs/crypto/Kconfig | 1 + fs/crypto/hkdf.c | 87 ++------- include/crypto/hkdf.h | 34 ++++ 6 files changed, 459 insertions(+), 72 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h
crypto/Kconfig +4 -0
@@ -141,6 +141,10 @@ config CRYPTO_ACOMP select CRYPTO_ALGAPI select CRYPTO_ACOMP2 +config CRYPTO_HKDF + tristate + select CRYPTO_HASH2 + config CRYPTO_MANAGER tristate "Cryptographic algorithm manager" select CRYPTO_MANAGER2
crypto/Makefile +1 -0
@@ -34,6 +34,7 @@ obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o obj-$(CONFIG_CRYPTO_SIG2) += sig.o obj-$(CONFIG_CRYPTO_KPP2) += kpp.o +obj-$(CONFIG_CRYPTO_HKDF) += hkdf.o dh_generic-y := dh.o dh_generic-y += dh_helper.o
crypto/hkdf.c new file +404 -0
404 lines not on this page · raw message
fs/crypto/Kconfig +1 -0
@@ -3,6 +3,7 @@ config FS_ENCRYPTION bool "FS Encryption (Per-file encryption)" select CRYPTO select CRYPTO_HASH + select CRYPTO_HKDF select CRYPTO_SKCIPHER select CRYPTO_LIB_SHA256 select KEYS
fs/crypto/hkdf.c +15 -72
129 lines not on this page · raw message
include/crypto/hkdf.h new file +34 -0
@@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* HKDF: HMAC-based Key Derivation Function (HKDF), RFC 5869 +* +* Extracted from fs/crypto/hkdf.c, which has +* Copyright 2019 Google LLC +*/ + +#ifndef _CRYPTO_HKDF_H +#define _CRYPTO_HKDF_H + +#ifdef CONFIG_CRYPTO_HKDF +int hkdf_extract (struct crypto_shash *hmac_tfm,const u8 *ikm, +unsigned int ikmlen,const u8 *salt,unsigned int saltlen, +u8 *prk); +int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen); +#else +static inline int hkdf_extract (struct crypto_shash *hmac_tfm, +const u8 *ikm,unsigned int ikmlen, +const u8 *salt,unsigned int saltlen, +u8 *prk) +{ +return -ENOTSUP ; +} +static inline int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen) +{ +return -ENOTSUP ; +} +#endif +#endif
Signature · 1 line
2.35.3
Eric Biggers
maintainerRe: [PATCH 1/9] crypto,fs: Separate out hkdf_extract() and hkdf_expand()
Quoted 4 lines · Hannes Reinecke
Separate out the HKDF functions into a separate module to to make them available to other callers.
And add a testsuite to the module with test vectors from RFC 5869 to ensure the integrity of the algorithm.
integrity => correctness
Quoted 5 lines
crypto/hkdf.c new file +0
-0
[...]
Quoted 3 lines
+/**
+ *hkdf_extract- HKDF-Extract (RFC 5869 section 2.2)
+ * @hmac_tfm: hash context
Should document hmac_tfm properly. Maybe something like:
@hmac_tfm: an HMAC transform using the hash function desired for HKDF. The
caller is responsible for setting the @prk afterwards.
Quoted 11 lines
+ * @ikm: input keying material
+ * @ikmlen: length of @ikm
+ * @salt: input salt value
+ * @saltlen: length of @salt
+ * @prk: resulting pseudorandom key
+ *
+ * Extracts a pseudorandom key @prk from the input keying material
+ * @ikm with length @ikmlen and salt @salt with length @saltlen.
+ * The length of @prk is given by the digest size of @hmac_tfm.
+ * For an 'unsalted' version of HKDF-Extract @salt should be set
+ * to all zeroes and @saltlen should be set to the length of @prk.
should => must (both times above)
Quoted 1 line
+ * otherwise a negative error.
negative errno value (not just any negative error)
Quoted 2 lines
+/*
+ *hkdf_expand- HKDF-Expand (RFC 5869 section 2.3)
Needs to be /** to make it a kerneldoc comment
Quoted 4 lines
+ * @hmac_tfm: hash context keyed with pseudorandom key
+ * @info: application-specific information
+ * @infolen: length of @info
+ * @okm: output keying material
Missing documentation for @okmlen parameter
Quoted 16 lines
+ *
+ * This expands the pseudorandom key, which was already keyed into @hmac_tfm,
+ * into @okmlen bytes of output keying material parameterized by the
+ * application-specific @info of length @infolen bytes.
+ * This is thread-safe and may be called by multiple threads in parallel.
+ *
+ * Returns 0 on success with output keying material stored in @okm,
+ * negative error number otherwise.
+ */crypto/hkdf.c new file +7
-0+int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen) +{ +SHASH_DESC_ON_STACK (desc, hmac_tfm); +unsigned int i, hashlen =crypto_shash_digestsize (hmac_tfm); +int err;
crypto/hkdf.c line 88
Quoted 10 lines
crypto/hkdf.c new file +10
-0+const u8 *prev =NULL ; +u8 counter =1 ; +u8 *tmp; + +if (WARN_ON (okmlen >255 * hashlen)) +return -EINVAL ; + + tmp =kzalloc (hashlen,GFP_KERNEL ); +if (!tmp) +return -ENOMEM ;
tmp can be allocated on the stack since it has maximum length HASH_MAX_DIGESTSIZE.
Also, either way, it needs to be zeroized at the end of the function.
Quoted 5 lines
crypto/hkdf.c new file +5
-0+ desc->tfm = hmac_tfm; + +for (i =0 ; i < okmlen; i += hashlen) { + + err =crypto_shash_init (desc);
crypto/hkdf.c line 104
Quoted 10 lines
crypto/hkdf.c new file +10
-0+if (err) + goto out; + +if (prev) { + err =crypto_shash_update (desc, prev, hashlen); +if (err) + goto out; + } + +if (info && infolen) {
'if (infolen)' instead of 'if (info && infolen)'. The latter is a bad practice because it can hide bugs.
crypto/hkdf.c line 147
Quoted 13 lines
crypto/hkdf.c new file +13
-0+struct hkdf_testvec { +const char *test; +const unsigned char *ikm; +const unsigned char *salt; +const unsigned char *info; +const unsigned char *prk; +const unsigned char *okm; +unsigned short ikm_size; +unsigned short salt_size; +unsigned short info_size; +unsigned short prk_size; +unsigned short okm_size; +};
u8 and u16 instead of unsigned char and unsigned short, please.
Quoted 18 lines
crypto/hkdf.c new file +18
-0+static int hkdf_test (const char *shash,const struct hkdf_testvec *tv) +{struct crypto_shash *tfm =NULL ; +u8 *prk =NULL , *okm =NULL ; +unsigned int prk_size; +const char *driver; +int err; + + tfm =crypto_alloc_shash (shash,0 ,0 ); +if (IS_ERR (tfm)) { +pr_err ("%s(%s): failed to allocate transform: %ld\n" , + tv->test, shash,PTR_ERR (tfm)); +return PTR_ERR (tfm); + } + driver =crypto_shash_driver_name (tfm); + + prk_size =crypto_shash_digestsize (tfm); + prk =kzalloc (prk_size,GFP_KERNEL ); +if (!prk) {
crypto/hkdf.c line 327
Quoted 10 lines
crypto/hkdf.c new file +10
-0+ err = -ENOMEM ; + goto out_free_shash; + } + +if (tv->prk_size != prk_size) { +pr_err ("%s(%s): prk size mismatch (vec %u, digest %u\n" , + tv->test, driver, tv->prk_size, prk_size); + err = -EINVAL ; + goto out_free_prk; + }
Since freeing NULL is a no-op, this function only really needs one label.
crypto/hkdf.c line 353
Quoted 4 lines
crypto/hkdf.c new file +4
-0+ err =crypto_shash_setkey (tfm, tv->prk, tv->prk_size); +if (err) { +pr_err ("%s(%s): failed to hash key to PRK, error %d\n" , + tv->test, driver, err);
Not sure what "failed to hash key to PRK" means here. It failed to set the PRK, so a better error message might be "failed to set PRK".
Quoted 7 lines
crypto/hkdf.c new file +7
-0+static int __init crypto_hkdf_module_init (void ) +{ +int ret =0 , i; + +if (IS_ENABLED (CONFIG_CRYPTO_MANAGER_DISABLE_TESTS )) +return 0 ; +
crypto/hkdf.c line 393
Quoted 10 lines
crypto/hkdf.c new file +10
-0+for (i =0 ; i <ARRAY_SIZE (hkdf_sha256_tv); i++) { + ret =hkdf_test ("hmac(sha256)" , &hkdf_sha256_tv[i]); +if (ret) +return ret; + } +for (i =0 ; i <ARRAY_SIZE (hkdf_sha1_tv); i++) { + ret =hkdf_test ("hmac(sha1)" , &hkdf_sha1_tv[i]); +if (ret) +return ret; + }
It's surprising that there are tests for sha1 which is obsolete, but none for sha512 which is what is actually being used.
In any case, nothing guarantees that sha256 and sha1 are available here, as they aren't selected by CRYPTO_HKDF. They would need to be selected, at least if CRYPTO_MANAGER_DISABLE_TESTS=n, or else the tests would need to be skipped if the underlying hash algorithm is not available in the crypto API.
Quoted 1 line
+MODULE_DESCRIPTION("Hashed-Key derivation functions");
This may be a misunderstanding about what HKDF stands for.
How about: MODULE_DESCRIPTION("HMAC-based Key Derivation Function (HKDF)");
Quoted 4 lines
fs/crypto/hkdf.c +0
-0
[...]
fs/crypto/hkdf.c int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key, · line 85
Quoted 8 lines
fs/crypto/hkdf.c +0
-4@@ -79,12 +63,8 @@ int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,return PTR_ERR (hmac_tfm); } -if (WARN_ON_ONCE (crypto_shash_digestsize (hmac_tfm) !=sizeof (prk))) { - err = -EINVAL ; - goto err_free_tfm; - }
Why is this WARN_ON_ONCE being removed? It doesn't seem to be replaced by anything.
Quoted 11 lines
fs/crypto/hkdf.c +2
-2- - err =hkdf_extract (hmac_tfm, master_key, master_key_size, prk); + err =hkdf_extract (hmac_tfm, master_key, master_key_size, + default_salt,HKDF_HASHLEN , prk);if (err) goto err_free_tfm;@@ -118,61 +98,24 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context, u8 *okm, unsigned int okmlen) { SHASH_DESC_ON_STACK(desc, hkdf->hmac_tfm);
fs/crypto/hkdf.c int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context, · line 105
Quoted 10 lines
fs/crypto/hkdf.c +1
-5-u8 prefix[9 ]; -unsigned int i; +u8 *full_info;int err; -const u8 *prev =NULL ; -u8 counter =1 ; -u8 tmp[HKDF_HASHLEN ];if (WARN_ON_ONCE (okmlen >255 *HKDF_HASHLEN ))return -EINVAL ;
... but this WARN_ON_ONCE is no longer needed since hkdf_expand() does it.
Quoted 47 lines
fs/crypto/hkdf.c +3
-41+ full_info =kzalloc (infolen +9 ,GFP_KERNEL ); +if (!full_info) +return -ENOMEM ; desc->tfm = hkdf->hmac_tfm; -memcpy (prefix,"fscrypt\0" ,8 ); - prefix[8 ] = context; - -for (i =0 ; i < okmlen; i +=HKDF_HASHLEN ) { - - err =crypto_shash_init (desc); -if (err) - goto out; - -if (prev) { - err =crypto_shash_update (desc, prev,HKDF_HASHLEN ); -if (err) - goto out; - } - - err =crypto_shash_update (desc, prefix,sizeof (prefix)); -if (err) - goto out; - - err =crypto_shash_update (desc, info, infolen); -if (err) - goto out; - -BUILD_BUG_ON (sizeof (counter) !=1 ); -if (okmlen - i <HKDF_HASHLEN ) { - err =crypto_shash_finup (desc, &counter,1 , tmp); -if (err) - goto out; -memcpy (&okm[i], tmp, okmlen - i); -memzero_explicit (tmp,sizeof (tmp)); - }else { - err =crypto_shash_finup (desc, &counter,1 , &okm[i]); -if (err) - goto out; - } - counter++; - prev = &okm[i]; - } - err =0 ; -out: -if (unlikely (err))
fs/crypto/hkdf.c int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context, · line 119
Quoted 10 lines
fs/crypto/hkdf.c +7
-2-memzero_explicit (okm, okmlen);/* so caller doesn't need to */ -shash_desc_zero (desc); +memcpy (full_info,"fscrypt\0" ,8 ); + full_info[8 ] = context; +memcpy (full_info +9 , info, infolen); + + err =hkdf_expand (hkdf->hmac_tfm, full_info, infolen +8 , + okm, okmlen); +kfree_sensitive (full_info);return err;
infolen + 9, not infolen + 8. Otherwise this patch breaks fscrypt.
Quoted 30 lines
include/crypto/hkdf.h new file +24
-0@@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* HKDF: HMAC-based Key Derivation Function (HKDF), RFC 5869 +* +* Extracted from fs/crypto/hkdf.c, which has +* Copyright 2019 Google LLC +*/ + +#ifndef _CRYPTO_HKDF_H +#define _CRYPTO_HKDF_H + +#ifdef CONFIG_CRYPTO_HKDF +int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm, + unsigned int ikmlen, const u8 *salt, unsigned int saltlen, + u8 *prk); +int hkdf_expand(struct crypto_shash *hmac_tfm, + const u8 *info, unsigned int infolen, + u8 *okm, unsigned int okmlen); +#else +static inline int hkdf_extract(struct crypto_shash *hmac_tfm, + const u8 *ikm, unsigned int ikmlen, + const u8 *salt, unsigned int saltlen, + u8 *prk) +{
include/crypto/hkdf.h line 34
Quoted 10 lines
include/crypto/hkdf.h new file +10
-0+return -ENOTSUP ; +} +static inline int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen) +{ +return -ENOTSUP ; +} +#endif +#endif
This header is missing <crypto/hash.h> which it depends on.
Also the !CONFIG_CRYPTO_HKDF stubs are unnecessary and should not be included.
- Eric
Hannes Reinecke
maintainerRe: [PATCH 1/9] crypto,fs: Separate out hkdf_extract() and hkdf_expand()
Quoted 5 lines · Eric Biggers
Quoted 4 lines · Hannes Reinecke
Separate out the HKDF functions into a separate module to to make them available to other callers.
And add a testsuite to the module with test vectors from RFC 5869 to ensure the integrity of the algorithm.
[ .. ]
Quoted 10 lines
Quoted 10 lines
crypto/hkdf.c new file +10
-0+ desc->tfm = hmac_tfm; + +for (i =0 ; i < okmlen; i += hashlen) { + + err =crypto_shash_init (desc); +if (err) + goto out; + +if (prev) { + err =crypto_shash_update (desc, prev, hashlen);
Quoted 9 lines
crypto/hkdf.c line 104
Quoted 5 lines
crypto/hkdf.c new file +5
-0+if (err) + goto out; + } + +if (info && infolen) {'if (infolen)' instead of 'if (info && infolen)'. The latter is a bad practice because it can hide bugs.
Do I need to set a 'WARN_ON(!info)' (or something) in this case? Or are the '->update' callbacks expected to handle it themselves?
[ .. ]
Quoted 43 lines
Quoted 30 lines
include/crypto/hkdf.h new file +24
-0@@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* +* HKDF: HMAC-based Key Derivation Function (HKDF), RFC 5869 +* +* Extracted from fs/crypto/hkdf.c, which has +* Copyright 2019 Google LLC +*/ + +#ifndef _CRYPTO_HKDF_H +#define _CRYPTO_HKDF_H + +#ifdef CONFIG_CRYPTO_HKDF +int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm, + unsigned int ikmlen, const u8 *salt, unsigned int saltlen, + u8 *prk); +int hkdf_expand(struct crypto_shash *hmac_tfm, + const u8 *info, unsigned int infolen, + u8 *okm, unsigned int okmlen); +#else +static inline int hkdf_extract(struct crypto_shash *hmac_tfm, + const u8 *ikm, unsigned int ikmlen, + const u8 *salt, unsigned int saltlen, + u8 *prk) +{include/crypto/hkdf.h line 34
Quoted 10 lines
include/crypto/hkdf.h new file +10
-0+return -ENOTSUP ; +} +static inline int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen) +{ +return -ENOTSUP ; +} +#endif +#endif This header is missing <
crypto/hash.h> which it depends on.
Quoted 2 lines
Also the !CONFIG_CRYPTO_HKDF stubs are unnecessary and should not be included.
But that would mean that every call to '#include <crypto/hkdf.h>' would need to be encapsulated by 'CONFIG_CRYPTO_HKDF' (or the file itself is conditionally compiled on that symbol).
Is that the direction you want to head?
Thanks for the review!
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +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
Eric Biggers
maintainerRe: [PATCH 1/9] crypto,fs: Separate out hkdf_extract() and hkdf_expand()
Quoted 26 lines · Hannes Reinecke
Quoted 5 lines · Eric Biggers
Quoted 4 lines · Hannes Reinecke
Separate out the HKDF functions into a separate module to to make them available to other callers.
And add a testsuite to the module with test vectors from RFC 5869 to ensure the integrity of the algorithm.[ .. ]
Quoted 10 lines
Quoted 10 lines
crypto/hkdf.c new file +10
-0+ desc->tfm = hmac_tfm; + +for (i =0 ; i < okmlen; i += hashlen) { + + err =crypto_shash_init (desc); +if (err) + goto out; + +if (prev) { + err =crypto_shash_update (desc, prev, hashlen);Quoted 9 lines
crypto/hkdf.c line 104
Quoted 5 lines
crypto/hkdf.c new file +5
-0+if (err) + goto out; + } + +if (info && infolen) {'if (infolen)' instead of 'if (info && infolen)'. The latter is a bad practice because it can hide bugs.
Quoted 2 lines
Do I need to set a '
WARN_ON(!info)' (or something) in this case? Or are the '->update' callbacks expected to handle it themselves?
No, if someone does pass NULL with a nonzero length there will be a crash. But the same will happen with another invalid pointer that is not NULL. It's just a bad practice to insert random NULL checks like this because it can hide bugs. Really a call like info=NULL, infolen=10 is ambiguous --- you've made it silently override infolen to 0 but how do you know the caller wanted that?
Quoted 26 lines
Quoted 26 lines
Quoted 13 lines
include/crypto/hkdf.h new file +13
-0+#ifdef CONFIG_CRYPTO_HKDF +int hkdf_extract (struct crypto_shash *hmac_tfm,const u8 *ikm, +unsigned int ikmlen,const u8 *salt,unsigned int saltlen, +u8 *prk); +int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen); +#else +static inline int hkdf_extract (struct crypto_shash *hmac_tfm, +const u8 *ikm,unsigned int ikmlen, +const u8 *salt,unsigned int saltlen, +u8 *prk) +{include/crypto/hkdf.h line 34
Quoted 10 lines
include/crypto/hkdf.h new file +10
-0+return -ENOTSUP ; +} +static inline int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen) +{ +return -ENOTSUP ; +} +#endif +#endif This header is missing <
crypto/hash.h> which it depends on.
Quoted 5 lines
Quoted 2 lines
Also the !CONFIG_CRYPTO_HKDF stubs are unnecessary and should not be included.
But that would mean that every call to '#include <
crypto/hkdf.h>' would need to be encapsulated by 'CONFIG_CRYPTO_HKDF' (or the file itself is conditionally compiled on that symbol).
No, it doesn't mean that. As long as the functions are not called when !CONFIG_CRYPTO_HKDF, it doesn't hurt to have declarations of them.
- Eric
Hannes Reinecke
maintainerRe: [PATCH 1/9] crypto,fs: Separate out hkdf_extract() and hkdf_expand()
Quoted 27 lines · Eric Biggers
Quoted 26 lines · Hannes Reinecke
Quoted 5 lines · Eric Biggers
Quoted 4 lines · Hannes Reinecke
Separate out the HKDF functions into a separate module to to make them available to other callers.
And add a testsuite to the module with test vectors from RFC 5869 to ensure the integrity of the algorithm.[ .. ]
Quoted 19 lines
Quoted 5 lines
crypto/hkdf.c new file +5
-0+ desc->tfm = hmac_tfm; + +for (i =0 ; i < okmlen; i += hashlen) { + + err =crypto_shash_init (desc);crypto/hkdf.c line 104
Quoted 10 lines
crypto/hkdf.c new file +10
-0+if (err) + goto out; + +if (prev) { + err =crypto_shash_update (desc, prev, hashlen); +if (err) + goto out; + } + +if (info && infolen) {'if (infolen)' instead of 'if (info && infolen)'. The latter is a bad practice because it can hide bugs.
Quoted 9 lines
Quoted 2 lines
Do I need to set a '
WARN_ON(!info)' (or something) in this case? Or are the '->update' callbacks expected to handle it themselves?No, if someone does pass NULL with a nonzero length there will be a crash. But the same will happen with another invalid pointer that is not NULL. It's just a bad practice to insert random NULL checks like this because it can hide bugs. Really a call like info=NULL, infolen=10 is ambiguous --- you've made it silently override infolen to 0 but how do you know the caller wanted that?
Just wanted to clarify; different maintainers have different styles ...
Quoted 26 lines
Quoted 26 lines
Quoted 26 lines
Quoted 13 lines
include/crypto/hkdf.h new file +13
-0+#ifdef CONFIG_CRYPTO_HKDF +int hkdf_extract (struct crypto_shash *hmac_tfm,const u8 *ikm, +unsigned int ikmlen,const u8 *salt,unsigned int saltlen, +u8 *prk); +int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen); +#else +static inline int hkdf_extract (struct crypto_shash *hmac_tfm, +const u8 *ikm,unsigned int ikmlen, +const u8 *salt,unsigned int saltlen, +u8 *prk) +{include/crypto/hkdf.h line 34
Quoted 10 lines
include/crypto/hkdf.h new file +10
-0+return -ENOTSUP ; +} +static inline int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen) +{ +return -ENOTSUP ; +} +#endif +#endif This header is missing <
crypto/hash.h> which it depends on.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
Also the !CONFIG_CRYPTO_HKDF stubs are unnecessary and should not be included.
But that would mean that every call to '#include <
crypto/hkdf.h>' would need to be encapsulated by 'CONFIG_CRYPTO_HKDF' (or the file itself is conditionally compiled on that symbol).No, it doesn't mean that. As long as the functions are not called when !CONFIG_CRYPTO_HKDF, it doesn't hurt to have declarations of them.
Guess that is correct. Will be reposting the series.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +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
PATCH v9 2/9 nvme: add nvme_auth_generate_psk()
In reply to Hannes ReineckeAdd a function to generate a NVMe PSK from the shared credentials negotiated by DH-HMAC-CHAP.
--- drivers/nvme/common/auth.c | 98 ++++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 3 ++ 2 files changed, 101 insertions(+)
drivers/nvme/common/auth.c +98 -0
@@ -11,6 +11,7 @@#include <asm/unaligned.h> #include <crypto/hash.h> #include <crypto/dh.h> +#include <crypto/hkdf.h> #include <linux/nvme.h> #include <linux/nvme-auth.h>
@@ -471,5 +472,102 @@ int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key) }EXPORT_SYMBOL_GPL (nvme_auth_generate_key); +/** +* nvme_auth_generate_psk - Generate a PSK for TLS +* @hmac_id: Hash function identifier +* @skey: Session key +* @skey_len: Length of @skey +* @c1: Value of challenge C1 +* @c2: Value of challenge C2 +* @hash_len: Hash length of the hash algorithm +* @ret_psk: Pointer too the resulting generated PSK +* @ret_len: length of @ret_psk +* +* Generate a PSK for TLS as specified in NVMe base specification, section 8.13.5.9: +* Generated PSK for TLS +* +* The generated PSK for TLS shall be computed applying the HMAC function using the +* hash function H( ) selected by the HashID parameter in the DH-HMAC-CHAP_Challenge +* message with the session key KS as key to the concatenation of the two challenges +* C1 and C2 (i.e., generated PSK = HMAC(KS, C1 || C2)). +* +* Returns 0 on success with a valid generated PSK pointer in @ret_psk and the length +* of @ret_psk in @ret_len, or a negative error number otherwise. +*/ +int nvme_auth_generate_psk (u8 hmac_id,u8 *skey,size_t skey_len, +u8 *c1,u8 *c2,size_t hash_len,u8 **ret_psk,size_t *ret_len) +{ +struct crypto_shash *tfm; +struct shash_desc *shash; +u8 *psk; +const char *hmac_name; +int ret, psk_len; + +if (!c1 || !c2) { +pr_warn ("%s: invalid parameter\n" , __func__); +return -EINVAL ; + } + + hmac_name =nvme_auth_hmac_name (hmac_id); +if (!hmac_name) { +pr_warn ("%s: invalid hash algoritm %d\n" , + __func__, hmac_id); +return -EINVAL ; + } + + tfm =crypto_alloc_shash (hmac_name,0 ,0 ); +if (IS_ERR (tfm)) +return PTR_ERR (tfm); + + psk_len =crypto_shash_digestsize (tfm); + psk =kzalloc (psk_len,GFP_KERNEL ); +if (!psk) { + ret = -ENOMEM ; + goto out_free_tfm; + } + + shash =kmalloc (sizeof (struct shash_desc ) + +crypto_shash_descsize (tfm), +GFP_KERNEL ); +if (!shash) { + ret = -ENOMEM ; + goto out_free_psk; + } + + shash->tfm = tfm; + ret =crypto_shash_setkey (tfm, skey, skey_len); +if (ret) + goto out_free_shash; + + ret =crypto_shash_init (shash); +if (ret) + goto out_free_shash; + + ret =crypto_shash_update (shash, c1, hash_len); +if (ret) + goto out_free_shash; + + ret =crypto_shash_update (shash, c2, hash_len); +if (ret) + goto out_free_shash; + + ret =crypto_shash_final (shash, psk); +if (!ret) { + *ret_psk = psk; + *ret_len = psk_len; + } + +out_free_shash: +kfree_sensitive (shash); +out_free_psk: +if (ret) +kfree_sensitive (psk); +out_free_tfm: +crypto_free_shash (tfm); + +return ret; +} +EXPORT_SYMBOL_GPL (nvme_auth_generate_psk); +MODULE_DESCRIPTION ("NVMe Authentication framework" );MODULE_LICENSE ("GPL v2" );
include/linux/nvme-auth.h +3 -0
@@ -40,5 +40,8 @@ int nvme_auth_gen_pubkey(struct crypto_kpp *dh_tfm,int nvme_auth_gen_shared_secret (struct crypto_kpp *dh_tfm,u8 *ctrl_key,size_t ctrl_key_len,u8 *sess_key,size_t sess_key_len); +int nvme_auth_generate_psk (u8 hmac_id,u8 *skey,size_t skey_len, +u8 *c1,u8 *c2,size_t hash_len, +u8 **ret_psk,size_t *ret_len);#endif /* _NVME_AUTH_H */
Signature · 1 line
2.35.3
PATCH v9 3/9 nvme: add nvme_auth_generate_digest()
In reply to Hannes ReineckeAdd a function to calculate the PSK digest as specified in TP8018.
--- drivers/nvme/common/auth.c | 137 +++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 2 + 2 files changed, 139 insertions(+)
drivers/nvme/common/auth.c +137 -0
@@ -569,5 +569,142 @@ int nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, }EXPORT_SYMBOL_GPL (nvme_auth_generate_psk); +/** +* nvme_auth_generate_digest - Generate TLS PSK digest +* @hmac_id: Hash function identifier +* @psk: Generated input PSK +* @psk_len: Lenght of @psk +* @subsysnqn: NQN of the subsystem +* @hostnqn: NQN of the host +* @ret_digest: Pointer to the returned digest +* +* Generate a TLS PSK digest as specified in TP8018 Section 3.6.1.3: +* TLS PSK and PSK identity Derivation +* +* The PSK digest shall be computed by encoding in Base64 (refer to RFC 4648) +* the result of the application of the HMAC function using the hash function +* specified in item 4 above (ie the hash function of the cipher suite associated +* with the PSK identity) with the PSK as HMAC key to the concatenation of: +* - the NQN of the host (i.e., NQNh) not including the null terminator; +* - a space character; +* - the NQN of the NVM subsystem (i.e., NQNc) not including the null terminator; +* - a space character; and +* - the seventeen ASCII characters "NVMe-over-Fabrics" +* (i.e., <PSK digest> = Base64(HMAC(PSK, NQNh || " " || NQNc || " " || "NVMe-over-Fabrics"))). +* The length of the PSK digest depends on the hash function used to compute +* it as follows: +* - If the SHA-256 hash function is used, the resulting PSK digest is 44 characters long; or +* - If the SHA-384 hash function is used, the resulting PSK digest is 64 characters long. +* +* Returns 0 on success with a valid digest pointer in @ret_digest, or a negative +* error number on failure. +*/ +int nvme_auth_generate_digest (u8 hmac_id,u8 *psk,size_t psk_len, +char *subsysnqn,char *hostnqn,u8 **ret_digest) +{ +struct crypto_shash *tfm; +struct shash_desc *shash; +u8 *digest, *hmac; +const char *hmac_name; +size_t digest_len, hmac_len; +int ret; + +if (WARN_ON (!subsysnqn || !hostnqn)) +return -EINVAL ; + + hmac_name =nvme_auth_hmac_name (hmac_id); +if (!hmac_name) { +pr_warn ("%s: invalid hash algoritm %d\n" , + __func__, hmac_id); +return -EINVAL ; + } + +switch (nvme_auth_hmac_hash_len (hmac_id)) { +case 32 : + hmac_len =44 ; +break ; +case 48 : + hmac_len =64 ; +break ; +default : +pr_warn ("%s: invalid hash algorithm '%s'\n" , + __func__, hmac_name); +return -EINVAL ; + } + + hmac =kzalloc (hmac_len +1 ,GFP_KERNEL ); +if (!hmac) +return -ENOMEM ; + + tfm =crypto_alloc_shash (hmac_name,0 ,0 ); +if (IS_ERR (tfm)) { + ret =PTR_ERR (tfm); + goto out_free_hmac; + } + + digest_len =crypto_shash_digestsize (tfm); + digest =kzalloc (digest_len,GFP_KERNEL ); +if (!digest) { + ret = -ENOMEM ; + goto out_free_tfm; + } + + shash =kmalloc (sizeof (struct shash_desc ) + +crypto_shash_descsize (tfm), +GFP_KERNEL ); +if (!shash) { + ret = -ENOMEM ; + goto out_free_digest; + } + + shash->tfm = tfm; + ret =crypto_shash_setkey (tfm, psk, psk_len); +if (ret) + goto out_free_shash; + + ret =crypto_shash_init (shash); +if (ret) + goto out_free_shash; + + ret =crypto_shash_update (shash, hostnqn,strlen (hostnqn)); +if (ret) + goto out_free_shash; + + ret =crypto_shash_update (shash," " ,1 ); +if (ret) + goto out_free_shash; + + ret =crypto_shash_update (shash, subsysnqn,strlen (subsysnqn)); +if (ret) + goto out_free_shash; + + ret =crypto_shash_update (shash," NVMe-over-Fabrics" ,18 ); +if (ret) + goto out_free_shash; + + ret =crypto_shash_final (shash, digest); +if (ret) + goto out_free_shash; + + ret =base64_encode (digest, digest_len, hmac); +if (ret < hmac_len) + ret = -ENOKEY ; + *ret_digest = hmac; + ret =0 ; + +out_free_shash: +kfree_sensitive (shash); +out_free_digest: +kfree_sensitive (digest); +out_free_tfm: +crypto_free_shash (tfm); +out_free_hmac: +if (ret) +kfree_sensitive (hmac); + +return ret; +} +EXPORT_SYMBOL_GPL (nvme_auth_generate_digest); +MODULE_DESCRIPTION ("NVMe Authentication framework" );MODULE_LICENSE ("GPL v2" );
include/linux/nvme-auth.h +2 -0
@@ -43,5 +43,7 @@ int nvme_auth_gen_shared_secret(struct crypto_kpp *dh_tfm,int nvme_auth_generate_psk (u8 hmac_id,u8 *skey,size_t skey_len,u8 *c1,u8 *c2,size_t hash_len,u8 **ret_psk,size_t *ret_len); +int nvme_auth_generate_digest (u8 hmac_id,u8 *psk,size_t psk_len, +char *subsysnqn,char *hostnqn,u8 **ret_digest);#endif /* _NVME_AUTH_H */
Signature · 1 line
2.35.3
Re: Re: [PATCH 3/9] nvme: add nvme_auth_generate_digest()
+ hmac_name = nvme_auth_hmac_name(hmac_id);
+ if (!hmac_name) {
+ pr_warn("%s: invalid hash algoritm %d\n",
algoritm should be algorithm.
This typo is also present in other patches.
+ ret = base64_encode(digest, digest_len, hmac);
+ if (ret < hmac_len)
+ ret = -ENOKEY;
+ *ret_digest = hmac;
+ ret = 0;
The value assigned to ret in the if statement will be overwritten. It looks like a goto is missing.
Regards,
Mark
Sagi Grimberg
maintainerRe: [PATCH 3/9] nvme: add nvme_auth_generate_digest()
Your patch is not formatted correctly.
Please create a proper patch and use git-send-email to send it to the mailing list.
Quoted 18 lines · Mark O'Donovan
drivers/nvme/common/auth.c +3
-0+ hmac_name =nvme_auth_hmac_name (hmac_id); +if (!hmac_name) { + pr_warn("%s: invalid hash algoritm %d\n",algoritm should be algorithm.
This typo is also present in other patches.drivers/nvme/common/auth.c +5
-0+ ret =base64_encode (digest, digest_len, hmac); +if (ret < hmac_len) + ret = -ENOKEY ; + *ret_digest = hmac; + ret =0 ;The value assigned to ret in the if statement will be overwritten. It looks like a goto is missing.
Regards,
Mark
Re: [PATCH 3/9] nvme: add nvme_auth_generate_digest()
Sorry, it is not a patch. It is a section of Hannes' patch. I used the In-Reply-To / Reply-To / mailto header from the web archive as I am not currently subscribed to the list.
I guess I should have added the > indent characters manually.
Ar Domh, 2024-08-25 ag 11:55 +0300, Sagi Grimberg scríobh:
Quoted 26 lines
Quoted 26 lines
Your patch is not formatted correctly.
Please create a proper patch and use git-send-email to send it to the mailing list.Quoted 19 lines · Mark O'Donovan
Quoted 19 lines
+ hmac_name =
nvme_auth_hmac_name(hmac_id); + if (!hmac_name) {
+ pr_warn("%s: invalid hash algoritm %d\n", algoritm should be algorithm.
This typo is also present in other patches.+ ret =
base64_encode(digest, digest_len, hmac); + if (ret <hmac_len)
+ ret =-ENOKEY;
+ *ret_digest = hmac;
+ ret = 0;
The value assigned to ret in the if statement will be overwritten.
It looks like a goto is missing.Regards,
Mark
Re: Re: [PATCH 3/9] nvme: add nvme_auth_generate_digest()
In reply to Mark O'DonovanQuoted 1 line
pr_warn("%s: invalid hash algoritm %d\n",
algoritm -> algorithm (also present in other patches)
Quoted 4 lines
if (ret < hmac_len) ret = -ENOKEY; *ret_digest = hmac; ret = 0;
Setting ret to -ENOKEY has no effect as it gets set to 0 after.
PATCH v9 4/9 nvme: add nvme_auth_derive_tls_psk()
In reply to Hannes ReineckeAdd a function to derive the TLS PSK as specified TP8018.
--- drivers/nvme/common/Kconfig | 1 + drivers/nvme/common/auth.c | 109 ++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 2 + 3 files changed, 112 insertions(+)
drivers/nvme/common/Kconfig +1 -0
@@ -12,3 +12,4 @@ config NVME_AUTH select CRYPTO_SHA512 select CRYPTO_DH select CRYPTO_DH_RFC7919_GROUPS + select CRYPTO_HKDF
drivers/nvme/common/auth.c +109 -0
120 lines not on this page · raw message
include/linux/nvme-auth.h +2 -0
@@ -45,5 +45,7 @@ int nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, u8 **ret_psk, size_t *ret_len);int nvme_auth_generate_digest (u8 hmac_id,u8 *psk,size_t psk_len,char *subsysnqn,char *hostnqn,u8 **ret_digest); +int nvme_auth_derive_tls_psk (int hmac_id,u8 *psk,size_t psk_len, +u8 *psk_digest,u8 **ret_psk);#endif /* _NVME_AUTH_H */
Signature · 1 line
2.35.3
PATCH v9 5/9 nvme-keyring: add nvme_tls_psk_refresh()
In reply to Hannes ReineckeAdd a function to refresh a generated PSK in the specified keyring.
--- drivers/nvme/common/keyring.c | 64 +++++++++++++++++++++++++++++++++++ include/linux/nvme-keyring.h | 9 +++++ 2 files changed, 73 insertions(+)
drivers/nvme/common/keyring.c +64 -0
@@ -124,6 +124,70 @@ static struct key *nvme_tls_psk_lookup(struct key *keyring,return key_ref_to_ptr (keyref); } +/** +* nvme_tls_psk_refresh - Refresh TLS PSK +* @keyring: Keyring holding the TLS PSK +* @hostnqn: Host NQN to use +* @subnqn: Subsystem NQN to use +* @hmac_id: Hash function identifier +* @data: TLS PSK key material +* @data_len: Length of @data +* @digest: TLS PSK digest +* +* Refresh a generated version 1 TLS PSK with the identity generated +* from @hmac_id, @hostnqn, @subnqn, and @digest in the keyring given +* by @keyring. +* +* Returns the updated key success or an error pointer otherwise. +*/ +struct key *nvme_tls_psk_refresh (struct key *keyring, +const char *hostnqn,const char *subnqn,u8 hmac_id, +u8 *data,size_t data_len,const char *digest) +{ +key_perm_t keyperm = +KEY_POS_SEARCH |KEY_POS_VIEW |KEY_POS_READ | +KEY_POS_WRITE |KEY_POS_LINK |KEY_POS_SETATTR | +KEY_USR_SEARCH |KEY_USR_VIEW |KEY_USR_READ ; +char *identity; +key_ref_t keyref; +key_serial_t keyring_id; +struct key *key; + +if (!hostnqn || !subnqn || !data || !data_len) +return ERR_PTR (-EINVAL ); + + identity =kasprintf (GFP_KERNEL ,"NVMe1G%02d %s %s %s" , + hmac_id, hostnqn, subnqn, digest); +if (!identity) +return ERR_PTR (-ENOMEM ); + +if (!keyring) + keyring = nvme_keyring; + keyring_id =key_serial (keyring); +pr_debug ("keyring %x refresh tls psk '%s'\n" , + keyring_id, identity); + keyref =key_create_or_update (make_key_ref (keyring, true), +"psk" , identity, data, data_len, + keyperm,KEY_ALLOC_NOT_IN_QUOTA | +KEY_ALLOC_BUILT_IN | +KEY_ALLOC_BYPASS_RESTRICTION ); +if (IS_ERR (keyref)) { +pr_debug ("refresh tls psk '%s' failed, error %ld\n" , + identity,PTR_ERR (keyref)); +kfree (identity); +return ERR_PTR (-ENOKEY ); + } +kfree (identity); +/* +* Set the default timeout to 1 hour +* as suggested in TP8018. +*/ + key =key_ref_to_ptr (keyref); +key_set_timeout (key,3600 ); +return key; +} +EXPORT_SYMBOL_GPL (nvme_tls_psk_refresh); +/* * NVMe PSK priority list *
include/linux/nvme-keyring.h +9 -0
@@ -8,6 +8,9 @@#if IS_ENABLED (CONFIG_NVME_KEYRING ) +struct key *nvme_tls_psk_refresh (struct key *keyring, +const char *hostnqn,const char *subnqn,u8 hmac_id, +u8 *data,size_t data_len,const char *digest);key_serial_t nvme_tls_psk_default (struct key *keyring,const char *hostnqn,const char *subnqn);
@@ -15,6 +18,12 @@ key_serial_t nvme_keyring_id(void);struct key *nvme_tls_key_lookup (key_serial_t key_id);#else +static struct key *nvme_tls_psk_refresh (struct key *keyring, +const char *hostnqn,char *subnqn,u8 hmac_id, +u8 *data,size_t data_len,const char *digest) +{ +return ERR_PTR (-ENOTSUPP ); +}static inline key_serial_t nvme_tls_psk_default (struct key *keyring,const char *hostnqn,const char *subnqn) {
Signature · 1 line
2.35.3
PATCH v9 6/9 nvme-tcp: request secure channel concatenation
In reply to Hannes ReineckeAdd a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset.
--- drivers/nvme/host/auth.c | 108 +++++++++++++++++++++++++++++++++++- drivers/nvme/host/fabrics.c | 34 +++++++++++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/sysfs.c | 4 +- drivers/nvme/host/tcp.c | 47 ++++++++++++++-- include/linux/nvme.h | 7 +++ 7 files changed, 191 insertions(+), 14 deletions(-)
drivers/nvme/host/auth.c +105 -3
@@ -12,6 +12,7 @@#include "nvme.h" #include "fabrics.h" #include <linux/nvme-auth.h> +#include <linux/nvme-keyring.h> #define CHAP_BUF_SIZE 4096static struct kmem_cache *nvme_chap_buf_cache;
@@ -131,7 +132,12 @@ static int nvme_auth_set_dhchap_negotiate_data(struct nvme_ctrl *ctrl, data->auth_type =NVME_AUTH_COMMON_MESSAGES ; data->auth_id =NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE ; data->t_id =cpu_to_le16 (chap->transaction); - data->sc_c =0 ;/* No secure channel concatenation */ +if (!ctrl->opts->concat || chap->qid !=0 ) + data->sc_c =NVME_AUTH_SECP_NOSC ; +else if (ctrl->opts->tls_key) + data->sc_c =NVME_AUTH_SECP_REPLACETLSPSK ; +else + data->sc_c =NVME_AUTH_SECP_NEWTLSPSK ; data->napd =1 ; data->auth_protocol[0 ].dhchap.authid =NVME_AUTH_DHCHAP_AUTH_ID ; data->auth_protocol[0 ].dhchap.halen =3 ;
@@ -311,8 +317,9 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, data->hl = chap->hash_len; data->dhvlen =cpu_to_le16 (chap->host_key_len);memcpy (data->rval, chap->response, chap->hash_len); -if (ctrl->ctrl_key) { +if (ctrl->ctrl_key) chap->bi_directional = true; +if (ctrl->ctrl_key || ctrl->opts->concat) {get_random_bytes (chap->c2, chap->hash_len); data->cvalid =1 ; memcpy(data->rval + chap->hash_len, chap->c2,
@@ -322,7 +329,10 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, }else {memset (chap->c2,0 , chap->hash_len); } - chap->s2 =nvme_auth_get_seqnum (); +if (ctrl->opts->concat) + chap->s2 =0 ; +else + chap->s2 =nvme_auth_get_seqnum (); data->seqnum =cpu_to_le32 (chap->s2);if (chap->host_key_len) { dev_dbg(ctrl->device, "%s: qid %d host public key %*ph\n",
@@ -677,6 +687,84 @@ static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap)crypto_free_kpp (chap->dh_tfm); } +void nvme_auth_revoke_tls_key (struct nvme_ctrl *ctrl) +{ +dev_dbg (ctrl->device,"Wipe generated TLS PSK %08x\n" , +key_serial (ctrl->opts->tls_key)); +key_revoke (ctrl->opts->tls_key); +key_put (ctrl->opts->tls_key); + ctrl->opts->tls_key =NULL ; +} +EXPORT_SYMBOL_GPL (nvme_auth_revoke_tls_key); + +static int nvme_auth_secure_concat (struct nvme_ctrl *ctrl, +struct nvme_dhchap_queue_context *chap) +{ +u8 *psk, *digest, *tls_psk; +struct key *tls_key; +size_t psk_len; +int ret =0 ; + +if (!chap->sess_key) { +dev_warn (ctrl->device, +"%s: qid %d no session key negotiated\n" , + __func__, chap->qid); +return -ENOKEY ; + } + + ret =nvme_auth_generate_psk (chap->hash_id, chap->sess_key, + chap->sess_key_len, + chap->c1, chap->c2, + chap->hash_len, &psk, &psk_len); +if (ret) { +dev_warn (ctrl->device, +"%s: qid %d failed to generate PSK, error %d\n" , + __func__, chap->qid, ret); +return ret; + } +dev_dbg (ctrl->device, +"%s: generated psk %*ph\n" , __func__, (int )psk_len, psk); + + ret =nvme_auth_generate_digest (chap->hash_id, psk, psk_len, + ctrl->opts->subsysnqn, + ctrl->opts->host->nqn, &digest); +if (ret) { +dev_warn (ctrl->device, +"%s: qid %d failed to generate digest, error %d\n" , + __func__, chap->qid, ret); + goto out_free_psk; + }; +dev_dbg (ctrl->device,"%s: generated digest %s\n" , + __func__, digest); + ret =nvme_auth_derive_tls_psk (chap->hash_id, psk, psk_len, digest, &tls_psk); +if (ret) { +dev_warn (ctrl->device, +"%s: qid %d failed to derive TLS psk, error %d\n" , + __func__, chap->qid, ret); + goto out_free_digest; + }; + + tls_key =nvme_tls_psk_refresh (ctrl->opts->keyring, ctrl->opts->host->nqn, + ctrl->opts->subsysnqn, chap->hash_id, + tls_psk, psk_len, digest); +if (IS_ERR (tls_key)) { + ret =PTR_ERR (tls_key); +dev_warn (ctrl->device, +"%s: qid %d failed to insert generated key, error %d\n" , + __func__, chap->qid, ret); + tls_key =NULL ; +kfree_sensitive (tls_psk); + } +if (ctrl->opts->tls_key) +nvme_auth_revoke_tls_key (ctrl); + ctrl->opts->tls_key = tls_key; +out_free_digest: +kfree_sensitive (digest); +out_free_psk: +kfree_sensitive (psk); +return ret; +} +static void nvme_queue_auth_work (struct work_struct *work) { struct nvme_dhchap_queue_context *chap =
@@ -833,6 +921,14 @@ static void nvme_queue_auth_work(struct work_struct *work) }if (!ret) { chap->error =0 ; +/* Secure concatenation can only be enabled on the admin queue */ +if (!chap->qid && ctrl->opts->concat && + (ret =nvme_auth_secure_concat (ctrl, chap))) { +dev_warn (ctrl->device, +"%s: qid %d failed to enable secure concatenation\n" , + __func__, chap->qid); + chap->error = ret; + }return ; }
@@ -912,6 +1008,12 @@ static void nvme_ctrl_auth_work(struct work_struct *work) "qid 0: authentication failed\n");return ; } +/* +* Only run authentication on the admin queue for +* secure concatenation +*/ +if (ctrl->opts->concat) +return ;for (q =1 ; q < ctrl->queue_count; q++) { ret =nvme_auth_negotiate (ctrl, q);
drivers/nvme/host/fabrics.c +31 -3
@@ -472,8 +472,9 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) result =le32_to_cpu (res.u32); ctrl->cntlid = result &0xFFFF ;if (result & (NVME_CONNECT_AUTHREQ_ATR |NVME_CONNECT_AUTHREQ_ASCR )) { -/* Secure concatenation is not implemented */ -if (result &NVME_CONNECT_AUTHREQ_ASCR ) { +/* Check for secure concatenation */ +if ((result &NVME_CONNECT_AUTHREQ_ASCR ) && + !ctrl->opts->concat) {dev_warn (ctrl->device,"qid 0: secure concatenation is not supported\n" ); ret = -EOPNOTSUPP ;
@@ -550,7 +551,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)/* Secure concatenation is not implemented */ if (result &NVME_CONNECT_AUTHREQ_ASCR ) {dev_warn (ctrl->device, -"qid 0: secure concatenation is not supported\n" ); +"qid %d: secure concatenation is not supported\n" , qid); ret = -EOPNOTSUPP ; goto out_free_data; }
@@ -706,6 +707,7 @@ static const match_table_t opt_tokens = {#endif #ifdef CONFIG_NVME_TCP_TLS { NVMF_OPT_TLS, "tls" }, + { NVMF_OPT_CONCAT, "concat" }, #endif { NVMF_OPT_ERR, NULL } };
@@ -735,6 +737,7 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, opts->tls = false; opts->tls_key =NULL ; opts->keyring =NULL ; + opts->concat = false; options = o =kstrdup (buf,GFP_KERNEL );if (!options)
@@ -1053,6 +1056,14 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, } opts->tls = true;break ; +case NVMF_OPT_CONCAT : +if (!IS_ENABLED (CONFIG_NVME_TCP_TLS )) { +pr_err ("TLS is not supported\n" ); + ret = -EINVAL ; + goto out; + } + opts->concat = true; +break ;default :pr_warn ("unknown parameter or missing value '%s' in ctrl creation request\n" , p);
@@ -1079,6 +1090,23 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,pr_warn ("failfast tmo (%d) larger than controller loss tmo (%d)\n" , opts->fast_io_fail_tmo, ctrl_loss_tmo); } +if (opts->concat) { +if (opts->tls) { +pr_err ("Secure concatenation over TLS is not supported\n" ); + ret = -EINVAL ; + goto out; + } +if (opts->tls_key) { +pr_err ("Cannot specify a TLS key for secure concatenation\n" ); + ret = -EINVAL ; + goto out; + } +if (!opts->dhchap_secret) { +pr_err ("Need to enable DH-CHAP for secure concatenation\n" ); + ret = -EINVAL ; + goto out; + } + } opts->host =nvmf_host_add (hostnqn, &hostid);if (IS_ERR (opts->host)) {
drivers/nvme/host/fabrics.h +3 -0
@@ -66,6 +66,7 @@ enum {NVMF_OPT_TLS =1 <<25 ,NVMF_OPT_KEYRING =1 <<26 ,NVMF_OPT_TLS_KEY =1 <<27 , + NVMF_OPT_CONCAT = 1 << 28, };/**
@@ -101,6 +102,7 @@ enum {* @keyring: Keyring to use for key lookups * @tls_key: TLS key for encrypted connections (TCP) * @tls: Start TLS encrypted connections (TCP) +* @concat: Enabled Secure channel concatenation (TCP) * @disable_sqflow: disable controller sq flow control * @hdr_digest: generate/verify header digest (TCP) * @data_digest: generate/verify data digest (TCP)
@@ -130,6 +132,7 @@ struct nvmf_ctrl_options {struct key *keyring;struct key *tls_key;bool tls; +bool concat;bool disable_sqflow;bool hdr_digest;bool data_digest;
drivers/nvme/host/nvme.h +2 -0
@@ -1128,6 +1128,7 @@ void nvme_auth_stop(struct nvme_ctrl *ctrl);int nvme_auth_negotiate (struct nvme_ctrl *ctrl,int qid);int nvme_auth_wait (struct nvme_ctrl *ctrl,int qid);void nvme_auth_free (struct nvme_ctrl *ctrl); +void nvme_auth_revoke_tls_key (struct nvme_ctrl *ctrl);#else static inline int nvme_auth_init_ctrl (struct nvme_ctrl *ctrl) {
@@ -1150,6 +1151,7 @@ static inline int nvme_auth_wait(struct nvme_ctrl *ctrl, int qid)return -EPROTONOSUPPORT ; }static inline void nvme_auth_free (struct nvme_ctrl *ctrl) {}; +static void nvme_auth_revoke_tls_key (struct nvme_ctrl *ctrl) {};#endif u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
drivers/nvme/host/sysfs.c +2 -2
@@ -779,10 +779,10 @@ static umode_t nvme_tls_attrs_are_visible(struct kobject *kobj,return 0 ;if (a == &dev_attr_tls_key.attr && - !ctrl->opts->tls) + !ctrl->opts->tls && !ctrl->opts->concat)return 0 ;if (a == &dev_attr_tls_configured_key.attr && - !ctrl->opts->tls_key) + (!ctrl->opts->tls_key || ctrl->opts->concat))return 0 ;if (a == &dev_attr_tls_keyring.attr && !ctrl->opts->keyring)
drivers/nvme/host/tcp.c +41 -6
89 lines not on this page · raw message
include/linux/nvme.h +7 -0
@@ -1678,6 +1678,13 @@ enum { NVME_AUTH_DHGROUP_INVALID = 0xff, }; +enum { +NVME_AUTH_SECP_NOSC =0x00 , +NVME_AUTH_SECP_SC =0x01 , +NVME_AUTH_SECP_NEWTLSPSK =0x02 , +NVME_AUTH_SECP_REPLACETLSPSK =0x03 , +}; +union nvmf_auth_protocol {struct nvmf_auth_dhchap_protocol_descriptor dhchap; };
Signature · 1 line
2.35.3
Sagi Grimberg
maintainerRe: [PATCH 6/9] nvme-tcp: request secure channel concatenation
Quoted 151 lines · Hannes Reinecke
Add a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset.
Signed-off-by Hannes Reinecke <[email protected]>--- drivers/nvme/host/auth.c | 108 +++++++++++++++++++++++++++++++++++- drivers/nvme/host/fabrics.c | 34 +++++++++++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/sysfs.c | 4 +- drivers/nvme/host/tcp.c | 47 ++++++++++++++-- include/linux/nvme.h | 7 +++ 7 files changed, 191 insertions(+), 14 deletions(-)drivers/nvme/host/auth.c +91
-3@@ -12,6 +12,7 @@#include "nvme.h" #include "fabrics.h" #include <linux/nvme-auth.h> +#include <linux/nvme-keyring.h> #define CHAP_BUF_SIZE 4096static struct kmem_cache *nvme_chap_buf_cache;@@ -131,7 +132,12 @@ static int nvme_auth_set_dhchap_negotiate_data(struct nvme_ctrl *ctrl, data->auth_type =NVME_AUTH_COMMON_MESSAGES ; data->auth_id =NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE ; data->t_id =cpu_to_le16 (chap->transaction); - data->sc_c =0 ;/* No secure channel concatenation */ +if (!ctrl->opts->concat || chap->qid !=0 ) + data->sc_c =NVME_AUTH_SECP_NOSC ; +else if (ctrl->opts->tls_key) + data->sc_c =NVME_AUTH_SECP_REPLACETLSPSK ; +else + data->sc_c =NVME_AUTH_SECP_NEWTLSPSK ; data->napd =1 ; data->auth_protocol[0 ].dhchap.authid =NVME_AUTH_DHCHAP_AUTH_ID ; data->auth_protocol[0 ].dhchap.halen =3 ;@@ -311,8 +317,9 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, data->hl = chap->hash_len; data->dhvlen =cpu_to_le16 (chap->host_key_len);memcpy (data->rval, chap->response, chap->hash_len); -if (ctrl->ctrl_key) { +if (ctrl->ctrl_key) chap->bi_directional = true; +if (ctrl->ctrl_key || ctrl->opts->concat) {get_random_bytes (chap->c2, chap->hash_len); data->cvalid =1 ; memcpy(data->rval + chap->hash_len, chap->c2,@@ -322,7 +329,10 @@ static int nvme_auth_set_dhchap_reply_data(struct nvme_ctrl *ctrl, }else {memset (chap->c2,0 , chap->hash_len); } - chap->s2 =nvme_auth_get_seqnum (); +if (ctrl->opts->concat) + chap->s2 =0 ; +else + chap->s2 =nvme_auth_get_seqnum (); data->seqnum =cpu_to_le32 (chap->s2);if (chap->host_key_len) { dev_dbg(ctrl->device, "%s: qid %d host public key %*ph\n",@@ -677,6 +687,84 @@ static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap)crypto_free_kpp (chap->dh_tfm); } +void nvme_auth_revoke_tls_key (struct nvme_ctrl *ctrl) +{ +dev_dbg (ctrl->device,"Wipe generated TLS PSK %08x\n" , +key_serial (ctrl->opts->tls_key)); +key_revoke (ctrl->opts->tls_key); +key_put (ctrl->opts->tls_key); + ctrl->opts->tls_key =NULL ; +} +EXPORT_SYMBOL_GPL (nvme_auth_revoke_tls_key); + +static int nvme_auth_secure_concat (struct nvme_ctrl *ctrl, +struct nvme_dhchap_queue_context *chap) +{ +u8 *psk, *digest, *tls_psk; +struct key *tls_key; +size_t psk_len; +int ret =0 ; + +if (!chap->sess_key) { +dev_warn (ctrl->device, +"%s: qid %d no session key negotiated\n" , + __func__, chap->qid); +return -ENOKEY ; + } + + ret =nvme_auth_generate_psk (chap->hash_id, chap->sess_key, + chap->sess_key_len, + chap->c1, chap->c2, + chap->hash_len, &psk, &psk_len); +if (ret) { +dev_warn (ctrl->device, +"%s: qid %d failed to generate PSK, error %d\n" , + __func__, chap->qid, ret); +return ret; + } +dev_dbg (ctrl->device, +"%s: generated psk %*ph\n" , __func__, (int )psk_len, psk); + + ret =nvme_auth_generate_digest (chap->hash_id, psk, psk_len, + ctrl->opts->subsysnqn, + ctrl->opts->host->nqn, &digest); +if (ret) { +dev_warn (ctrl->device, +"%s: qid %d failed to generate digest, error %d\n" , + __func__, chap->qid, ret); + goto out_free_psk; + }; +dev_dbg (ctrl->device,"%s: generated digest %s\n" , + __func__, digest); + ret =nvme_auth_derive_tls_psk (chap->hash_id, psk, psk_len, digest, &tls_psk); +if (ret) { +dev_warn (ctrl->device, +"%s: qid %d failed to derive TLS psk, error %d\n" , + __func__, chap->qid, ret); + goto out_free_digest; + }; + + tls_key =nvme_tls_psk_refresh (ctrl->opts->keyring, ctrl->opts->host->nqn, + ctrl->opts->subsysnqn, chap->hash_id, + tls_psk, psk_len, digest); +if (IS_ERR (tls_key)) { + ret =PTR_ERR (tls_key); +dev_warn (ctrl->device, +"%s: qid %d failed to insert generated key, error %d\n" , + __func__, chap->qid, ret); + tls_key =NULL ; +kfree_sensitive (tls_psk); + } +if (ctrl->opts->tls_key) +nvme_auth_revoke_tls_key (ctrl); + ctrl->opts->tls_key = tls_key; +out_free_digest: +kfree_sensitive (digest); +out_free_psk: +kfree_sensitive (psk); +return ret; +} +static void nvme_queue_auth_work (struct work_struct *work) { struct nvme_dhchap_queue_context *chap =@@ -833,6 +921,14 @@ static void nvme_queue_auth_work(struct work_struct *work) }
drivers/nvme/host/auth.c static void nvme_queue_auth_work(struct work_struct *work) · line 931
Quoted 10 lines
drivers/nvme/host/auth.c +8
-0if (!ret) { chap->error =0 ; +/* Secure concatenation can only be enabled on the admin queue */ +if (!chap->qid && ctrl->opts->concat && + (ret =nvme_auth_secure_concat (ctrl, chap))) { +dev_warn (ctrl->device, +"%s: qid %d failed to enable secure concatenation\n" , + __func__, chap->qid); + chap->error = ret; + }
Lets break up the if condition:
if (!chap->qid && ctrl->opts->concat) {
ret = nvme_auth_secure_concat(ctrl, chap);
if (!ret)
return;
dev_warn(ctrl->device,
"%s: qid %d failed to enable secure concatenation\n",
__func__, chap->qid);
chap->error = ret;
}drivers/nvme/host/auth.c static void nvme_queue_auth_work(struct work_struct *work) · line 932
Quoted 1 line
drivers/nvme/host/auth.c +0
-0return ;
No need for the return statement.
Quoted 268 lines
drivers/nvme/host/auth.c +6
-0}@@ -912,6 +1008,12 @@ static void nvme_ctrl_auth_work(struct work_struct *work) "qid 0: authentication failed\n");return ; } +/* +* Only run authentication on the admin queue for +* secure concatenation +*/ +if (ctrl->opts->concat) +return ;for (q =1 ; q < ctrl->queue_count; q++) { ret =nvme_auth_negotiate (ctrl, q);drivers/nvme/host/fabrics.c +31
-3@@ -472,8 +472,9 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl) result =le32_to_cpu (res.u32); ctrl->cntlid = result &0xFFFF ;if (result & (NVME_CONNECT_AUTHREQ_ATR |NVME_CONNECT_AUTHREQ_ASCR )) { -/* Secure concatenation is not implemented */ -if (result &NVME_CONNECT_AUTHREQ_ASCR ) { +/* Check for secure concatenation */ +if ((result &NVME_CONNECT_AUTHREQ_ASCR ) && + !ctrl->opts->concat) {dev_warn (ctrl->device,"qid 0: secure concatenation is not supported\n" ); ret = -EOPNOTSUPP ;@@ -550,7 +551,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)/* Secure concatenation is not implemented */ if (result &NVME_CONNECT_AUTHREQ_ASCR ) {dev_warn (ctrl->device, -"qid 0: secure concatenation is not supported\n" ); +"qid %d: secure concatenation is not supported\n" , qid); ret = -EOPNOTSUPP ; goto out_free_data; }@@ -706,6 +707,7 @@ static const match_table_t opt_tokens = {#endif #ifdef CONFIG_NVME_TCP_TLS { NVMF_OPT_TLS, "tls" }, + { NVMF_OPT_CONCAT, "concat" }, #endif { NVMF_OPT_ERR, NULL } };@@ -735,6 +737,7 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, opts->tls = false; opts->tls_key =NULL ; opts->keyring =NULL ; + opts->concat = false; options = o =kstrdup (buf,GFP_KERNEL );if (!options)@@ -1053,6 +1056,14 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts, } opts->tls = true;break ; +case NVMF_OPT_CONCAT : +if (!IS_ENABLED (CONFIG_NVME_TCP_TLS )) { +pr_err ("TLS is not supported\n" ); + ret = -EINVAL ; + goto out; + } + opts->concat = true; +break ;default :pr_warn ("unknown parameter or missing value '%s' in ctrl creation request\n" , p);@@ -1079,6 +1090,23 @@ static int nvmf_parse_options(struct nvmf_ctrl_options *opts,pr_warn ("failfast tmo (%d) larger than controller loss tmo (%d)\n" , opts->fast_io_fail_tmo, ctrl_loss_tmo); } +if (opts->concat) { +if (opts->tls) { +pr_err ("Secure concatenation over TLS is not supported\n" ); + ret = -EINVAL ; + goto out; + } +if (opts->tls_key) { +pr_err ("Cannot specify a TLS key for secure concatenation\n" ); + ret = -EINVAL ; + goto out; + } +if (!opts->dhchap_secret) { +pr_err ("Need to enable DH-CHAP for secure concatenation\n" ); + ret = -EINVAL ; + goto out; + } + } opts->host =nvmf_host_add (hostnqn, &hostid);if (IS_ERR (opts->host)) {drivers/nvme/host/fabrics.h +3
-0@@ -66,6 +66,7 @@ enum {NVMF_OPT_TLS =1 <<25 ,NVMF_OPT_KEYRING =1 <<26 ,NVMF_OPT_TLS_KEY =1 <<27 , + NVMF_OPT_CONCAT = 1 << 28, };/** @@ -101,6 +102,7 @@ enum {* @keyring: Keyring to use for key lookups * @tls_key: TLS key for encrypted connections (TCP) * @tls: Start TLS encrypted connections (TCP) +* @concat: Enabled Secure channel concatenation (TCP) * @disable_sqflow: disable controller sq flow control * @hdr_digest: generate/verify header digest (TCP) * @data_digest: generate/verify data digest (TCP) @@ -130,6 +132,7 @@ struct nvmf_ctrl_options {struct key *keyring;struct key *tls_key;bool tls; +bool concat;bool disable_sqflow;bool hdr_digest;bool data_digest;drivers/nvme/host/nvme.h +2
-0@@ -1128,6 +1128,7 @@ void nvme_auth_stop(struct nvme_ctrl *ctrl);int nvme_auth_negotiate (struct nvme_ctrl *ctrl,int qid);int nvme_auth_wait (struct nvme_ctrl *ctrl,int qid);void nvme_auth_free (struct nvme_ctrl *ctrl); +void nvme_auth_revoke_tls_key (struct nvme_ctrl *ctrl);#else static inline int nvme_auth_init_ctrl (struct nvme_ctrl *ctrl) {@@ -1150,6 +1151,7 @@ static inline int nvme_auth_wait(struct nvme_ctrl *ctrl, int qid)return -EPROTONOSUPPORT ; }static inline void nvme_auth_free (struct nvme_ctrl *ctrl) {}; +static void nvme_auth_revoke_tls_key (struct nvme_ctrl *ctrl) {};#endif u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,drivers/nvme/host/sysfs.c +2
-2@@ -779,10 +779,10 @@ static umode_t nvme_tls_attrs_are_visible(struct kobject *kobj,return 0 ;if (a == &dev_attr_tls_key.attr && - !ctrl->opts->tls) + !ctrl->opts->tls && !ctrl->opts->concat)return 0 ;if (a == &dev_attr_tls_configured_key.attr && - !ctrl->opts->tls_key) + (!ctrl->opts->tls_key || ctrl->opts->concat))return 0 ;if (a == &dev_attr_tls_keyring.attr && !ctrl->opts->keyring)drivers/nvme/host/tcp.c +41
-6@@ -233,7 +233,7 @@ static inline bool nvme_tcp_tls_configured(struct nvme_ctrl *ctrl)if (!IS_ENABLED (CONFIG_NVME_TCP_TLS ))return 0 ; -return ctrl->opts->tls; +return ctrl->opts->tls || ctrl->opts->concat; }static inline struct blk_mq_tags *nvme_tcp_tagset (struct nvme_tcp_queue *queue)@@ -1948,7 +1948,7 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl)if (nvme_tcp_tls_configured (ctrl)) {if (ctrl->opts->tls_key) pskid =key_serial (ctrl->opts->tls_key); -else { +else if (ctrl->opts->tls) { pskid =nvme_tls_psk_default (ctrl->opts->keyring, ctrl->opts->host->nqn, ctrl->opts->subsysnqn);@@ -1978,9 +1978,25 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) {int i, ret; -if (nvme_tcp_tls_configured (ctrl) && !ctrl->tls_pskid) { -dev_err (ctrl->device,"no PSK negotiated\n" ); -return -ENOKEY ; +if (nvme_tcp_tls_configured (ctrl)) { +if (ctrl->opts->concat) { +/* +* The generated PSK is stored in the +* fabric options +*/ +if (!ctrl->opts->tls_key) { +dev_err (ctrl->device,"no PSK generated\n" ); +return -ENOKEY ; + } +if (ctrl->tls_pskid && + ctrl->tls_pskid !=key_serial (ctrl->opts->tls_key)) { +dev_err (ctrl->device,"Stale PSK id %08x\n" , ctrl->tls_pskid); + ctrl->tls_pskid =0 ; + } + }else if (!ctrl->tls_pskid) { +dev_err (ctrl->device,"no PSK negotiated\n" ); +return -ENOKEY ; + } }for (i =1 ; i < ctrl->queue_count; i++) {@@ -2211,6 +2227,21 @@ static void nvme_tcp_reconnect_or_remove(struct nvme_ctrl *ctrl, } } +/* +* The TLS key needs to be revoked when: +* - concatenation is enabled and +* -> This is a generated key and only valid for this session +* - the generated key is present in ctrl->tls_key and +* -> authentication has completed and the key has been generated +* - tls has been enabled +* -> otherwise we are about to reset the admin queue after authentication +* to enable TLS with the generated key +*/ +static bool nvme_tcp_key_revoke_needed (struct nvme_ctrl *ctrl) +{ +return ctrl->opts->concat && ctrl->opts->tls_key && ctrl->tls_pskid; +} +static int nvme_tcp_setup_ctrl (struct nvme_ctrl *ctrl,bool new) {struct nvmf_ctrl_options *opts = ctrl->opts;@@ -2314,6 +2345,8 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work) struct nvme_tcp_ctrl, err_work);struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl; +if (nvme_tcp_key_revoke_needed (ctrl)) +nvme_auth_revoke_tls_key (ctrl);nvme_stop_keep_alive (ctrl);flush_work (&ctrl->async_event_work);nvme_tcp_teardown_io_queues (ctrl, false);@@ -2354,6 +2387,8 @@ static void nvme_reset_ctrl_work(struct work_struct *work) container_of(work, struct nvme_ctrl, reset_work);int ret; +if (nvme_tcp_key_revoke_needed (ctrl)) +nvme_auth_revoke_tls_key (ctrl);nvme_stop_ctrl (ctrl);nvme_tcp_teardown_ctrl (ctrl, false);@@ -2848,7 +2883,7 @@ static struct nvmf_transport_ops nvme_tcp_transport = {NVMF_OPT_HDR_DIGEST |NVMF_OPT_DATA_DIGEST |NVMF_OPT_NR_WRITE_QUEUES |NVMF_OPT_NR_POLL_QUEUES |NVMF_OPT_TOS |NVMF_OPT_HOST_IFACE |NVMF_OPT_TLS | - NVMF_OPT_KEYRING | NVMF_OPT_TLS_KEY, + NVMF_OPT_KEYRING | NVMF_OPT_TLS_KEY | NVMF_OPT_CONCAT, .create_ctrl = nvme_tcp_create_ctrl, };include/linux/nvme.h +0
-0@@ -1678,6 +1678,13 @@ enum { NVME_AUTH_DHGROUP_INVALID = 0xff, };
include/linux/nvme.h enum { · line 1690
Quoted 10 lines
include/linux/nvme.h +7
-0+enum { +NVME_AUTH_SECP_NOSC =0x00 , +NVME_AUTH_SECP_SC =0x01 , +NVME_AUTH_SECP_NEWTLSPSK =0x02 , +NVME_AUTH_SECP_REPLACETLSPSK =0x03 , +}; +union nvmf_auth_protocol {struct nvmf_auth_dhchap_protocol_descriptor dhchap; };
Wandering if splitting core/fabrics from tcp would be viable here...
Hannes Reinecke
maintainerRe: [PATCH 6/9] nvme-tcp: request secure channel concatenation
Quoted 21 lines · Sagi Grimberg
Quoted 19 lines · Hannes Reinecke
Add a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset.
Signed-off-by Hannes Reinecke <[email protected]>---
drivers/nvme/host/auth.c | 108 +++++++++++++++++++++++++++++++++++-drivers/nvme/host/fabrics.c| 34 +++++++++++-drivers/nvme/host/fabrics.h| 3 + drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/sysfs.c | 4 +- drivers/nvme/host/tcp.c | 47 ++++++++++++++-- include/linux/nvme.h | 7 +++7 files changed, 191 insertions(+), 14 deletions(-)drivers/nvme/host/auth.c +0
-0
[ .. ]
Quoted 17 lines
Quoted 6 lines
drivers/nvme/host/auth.c +1
-0@@ -833,6 +921,14 @@ static void nvme_queue_auth_work(struct work_struct *work) } if (!ret) { chap->error = 0; + /* Secure concatenation can only be enabled on the adminQuoted 8 lines
queue */
+ if (!chap->qid &&ctrl->opts->concat&&
+ (ret =nvme_auth_secure_concat(ctrl, chap))) { + dev_warn(ctrl->device,
+ "%s: qid %d failed to enable secure concatenation\n", +__func__,chap->qid);
+chap->error= ret;
+ }Lets break up the if condition:
Quoted 12 lines
if (!chap->qid &&
ctrl->opts->concat) {
ret =nvme_auth_secure_concat(ctrl, chap);
if (!ret)
return;
dev_warn(ctrl->device,
"%s: qid %d failed to enable secure concatenation\n",__func__,chap->qid);
chap->error= ret;
}
Okay.
Quoted 4 lines
Quoted 1 line
return;
No need for the return statement.
[ .. ]
Quoted 11 lines
Quoted 11 lines
include/linux/nvme.h +4
-0@@ -1678,6 +1678,13 @@ enum { NVME_AUTH_DHGROUP_INVALID = 0xff, }; +enum { + NVME_AUTH_SECP_NOSC = 0x00, + NVME_AUTH_SECP_SC = 0x01, + NVME_AUTH_SECP_NEWTLSPSK = 0x02,
Quoted 8 lines
include/linux/nvme.h enum { · line 1690
Quoted 6 lines
include/linux/nvme.h +3
-0+ NVME_AUTH_SECP_REPLACETLSPSK = 0x03, +}; + union nvmf_auth_protocol { struct nvmf_auth_dhchap_protocol_descriptor dhchap; };Wandering if splitting core/fabrics from tcp would be viable here...
Well, authentication is a base functionality, not a tcp one. Even though it's pretty much TCP-centric.
So I'd rather keep in in nvme.h for now.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +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
PATCH v9 7/9 nvme-fabrics: reset admin connection for secure concatenation
In reply to Hannes ReineckeWhen secure concatenation is requested the connection needs to be reset to enable TLS encryption on the new cnnection. That implies that the original connection used for the DH-CHAP negotiation really shouldn't be used, and we should reset as soon as the DH-CHAP negotiation has succeeded on the admin queue.
Based on an idea from Sagi.
--- drivers/nvme/host/tcp.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/nvme/host/tcp.c +9 -0
@@ -2251,6 +2251,15 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)if (ret)return ret; +if (new && ctrl->opts && ctrl->opts->concat && !ctrl->tls_pskid) { +dev_dbg (ctrl->device,"restart admin queue for secure concatenation\n" ); +nvme_stop_keep_alive (ctrl); +nvme_tcp_teardown_admin_queue (ctrl, false); + ret =nvme_tcp_configure_admin_queue (ctrl, false); +if (ret) +return ret; + } +if (ctrl->icdoff) { ret = -EOPNOTSUPP ;dev_err (ctrl->device,"icdoff is not supported!\n" );
Signature · 1 line
2.35.3
Re: [PATCH 7/9] nvme-fabrics: reset admin connection for secure concatenation
I'm assuming you mean "nvme-tcp: reset admin connection for secure concatenation" ?
Other than that, as already noted:
PATCH v9 8/9 nvmet-tcp: support secure channel concatenation
In reply to Hannes ReineckeEvaluate the SC_C flag during DH-CHAP-HMAC negotiation and insert the generated PSK once negotiation has finished.
--- drivers/nvme/target/auth.c | 72 +++++++++++++++++++++++++- drivers/nvme/target/fabrics-cmd-auth.c | 49 +++++++++++++++--- drivers/nvme/target/fabrics-cmd.c | 33 +++++++++--- drivers/nvme/target/nvmet.h | 38 +++++++++++--- drivers/nvme/target/tcp.c | 23 +++++++- 5 files changed, 192 insertions(+), 23 deletions(-)
drivers/nvme/target/auth.c +71 -1
@@ -15,6 +15,7 @@#include <linux/ctype.h> #include <linux/random.h> #include <linux/nvme-auth.h> +#include <linux/nvme-keyring.h> #include <asm/unaligned.h> #include "nvmet.h"
@@ -126,7 +127,7 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id)return ret; } -u8 nvmet_setup_auth (struct nvmet_ctrl *ctrl) +u8 nvmet_setup_auth (struct nvmet_ctrl *ctrl,struct nvmet_req *req) {int ret =0 ;struct nvmet_host_link *p;
@@ -152,6 +153,11 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl) goto out_unlock; } +if (nvmet_queue_tls_keyid (req->sq)) { +pr_debug ("host %s tls enabled\n" , ctrl->hostnqn); + goto out_unlock; + } + ret =nvmet_setup_dhgroup (ctrl, host->dhchap_dhgroup_id);if (ret <0 ) {pr_warn ("Failed to setup DH group" );
@@ -220,6 +226,9 @@ u8 nvmet_setup_auth(struct nvmet_ctrl *ctrl)void nvmet_auth_sq_free (struct nvmet_sq *sq) {cancel_delayed_work (&sq->auth_expired_work); +#ifdef CONFIG_NVME_TARGET_TCP_TLS + sq->tls_key =0 ; +#endif kfree (sq->dhchap_c1); sq->dhchap_c1 =NULL ;kfree (sq->dhchap_c2);
@@ -248,6 +257,12 @@ void nvmet_destroy_auth(struct nvmet_ctrl *ctrl)nvme_auth_free_key (ctrl->ctrl_key); ctrl->ctrl_key =NULL ; } +#ifdef CONFIG_NVME_TARGET_TCP_TLS +if (ctrl->tls_key) { +key_put (ctrl->tls_key); + ctrl->tls_key =NULL ; + } +#endif }bool nvmet_check_auth_status (struct nvmet_req *req)
@@ -529,3 +544,58 @@ int nvmet_auth_ctrl_sesskey(struct nvmet_req *req,return ret; } + +void nvmet_auth_insert_psk (struct nvmet_sq *sq) +{ +int hash_len =nvme_auth_hmac_hash_len (sq->ctrl->shash_id); +u8 *psk, *digest, *tls_psk; +size_t psk_len; +int ret; +#ifdef CONFIG_NVME_TARGET_TCP_TLS +struct key *tls_key =NULL ; +#endif + + ret =nvme_auth_generate_psk (sq->ctrl->shash_id, + sq->dhchap_skey, + sq->dhchap_skey_len, + sq->dhchap_c1, sq->dhchap_c2, + hash_len, &psk, &psk_len); +if (ret) { +pr_warn ("%s: ctrl %d qid %d failed to generate PSK, error %d\n" , + __func__, sq->ctrl->cntlid, sq->qid, ret); +return ; + } + ret =nvme_auth_generate_digest (sq->ctrl->shash_id, psk, psk_len, + sq->ctrl->subsysnqn, + sq->ctrl->hostnqn, &digest); +if (ret) { +pr_warn ("%s: ctrl %d qid %d failed to generate digest, error %d\n" , + __func__, sq->ctrl->cntlid, sq->qid, ret); + goto out_free_psk; + } + ret =nvme_auth_derive_tls_psk (sq->ctrl->shash_id, psk, psk_len, + digest, &tls_psk); +if (ret) { +pr_warn ("%s: ctrl %d qid %d failed to derive TLS PSK, error %d\n" , + __func__, sq->ctrl->cntlid, sq->qid, ret); + goto out_free_digest; + } +#ifdef CONFIG_NVME_TARGET_TCP_TLS + tls_key =nvme_tls_psk_refresh (NULL , sq->ctrl->hostnqn, sq->ctrl->subsysnqn, + sq->ctrl->shash_id, tls_psk, psk_len, digest); +if (IS_ERR (tls_key)) { +pr_warn ("%s: ctrl %d qid %d failed to refresh key, error %ld\n" , + __func__, sq->ctrl->cntlid, sq->qid,PTR_ERR (tls_key)); + tls_key =NULL ; +kfree_sensitive (tls_psk); + } +if (sq->ctrl->tls_key) +key_put (sq->ctrl->tls_key); + sq->ctrl->tls_key = tls_key; +#endif + +out_free_digest: +kfree_sensitive (digest); +out_free_psk: +kfree_sensitive (psk); +}
drivers/nvme/target/fabrics-cmd-auth.c +43 -6
87 lines not on this page · raw message
drivers/nvme/target/fabrics-cmd.c +25 -8
@@ -199,10 +199,26 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)return ret; } -static u32 nvmet_connect_result (struct nvmet_ctrl *ctrl) +static u32 nvmet_connect_result (struct nvmet_ctrl *ctrl,struct nvmet_req *req) { +bool needs_auth =nvmet_has_auth (ctrl, req); +key_serial_t keyid =nvmet_queue_tls_keyid (req->sq); + +/* Do not authenticate I/O queues for secure concatenation */ +if (ctrl->concat && req->sq->qid) + needs_auth = false; + +if (keyid) +pr_debug ("%s: ctrl %d qid %d should %sauthenticate, tls psk %08x\n" , + __func__, ctrl->cntlid, req->sq->qid, + needs_auth ?"" :"not " , keyid); +else +pr_debug ("%s: ctrl %d qid %d should %sauthenticate%s\n" , + __func__, ctrl->cntlid, req->sq->qid, + needs_auth ?"" :"not " , + ctrl->concat ?", secure concatenation" :"" );return (u32 )ctrl->cntlid | - (nvmet_has_auth (ctrl) ?NVME_CONNECT_AUTHREQ_ATR :0 ); + (needs_auth ?NVME_CONNECT_AUTHREQ_ATR :0 ); }static void nvmet_execute_admin_connect (struct nvmet_req *req)
@@ -251,7 +267,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)uuid_copy (&ctrl->hostid, &d->hostid); - dhchap_status =nvmet_setup_auth (ctrl); + dhchap_status =nvmet_setup_auth (ctrl, req);if (dhchap_status) {pr_err ("Failed to setup authentication, dhchap status %u\n" , dhchap_status);
@@ -269,12 +285,13 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req) goto out; } -pr_info ("creating %s controller %d for subsystem %s for NQN %s%s%s.\n" , +pr_info ("creating %s controller %d for subsystem %s for NQN %s%s%s%s.\n" ,nvmet_is_disc_subsys (ctrl->subsys) ?"discovery" :"nvm" , ctrl->cntlid, ctrl->subsys->subsysnqn, ctrl->hostnqn, - ctrl->pi_support ?" T10-PI is enabled" :"" , -nvmet_has_auth (ctrl) ?" with DH-HMAC-CHAP" :"" ); - req->cqe->result.u32 =cpu_to_le32 (nvmet_connect_result (ctrl)); + ctrl->pi_support ?", T10-PI" :"" , +nvmet_has_auth (ctrl, req) ?", DH-HMAC-CHAP" :"" , +nvmet_queue_tls_keyid (req->sq) ?", TLS" :"" ); + req->cqe->result.u32 =cpu_to_le32 (nvmet_connect_result (ctrl, req)); out:kfree (d); complete:
@@ -330,7 +347,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req) goto out_ctrl_put;pr_debug ("adding queue %d to ctrl %d.\n" , qid, ctrl->cntlid); - req->cqe->result.u32 =cpu_to_le32 (nvmet_connect_result (ctrl)); + req->cqe->result.u32 =cpu_to_le32 (nvmet_connect_result (ctrl, req)); out:kfree (d); complete:
drivers/nvme/target/nvmet.h +32 -6
@@ -121,6 +121,9 @@ struct nvmet_sq {u32 dhchap_s2;u8 *dhchap_skey;int dhchap_skey_len; +#endif +#ifdef CONFIG_NVME_TARGET_TCP_TLS +struct key *tls_key;#endif struct completion free_done;struct completion confirm_done;
@@ -237,6 +240,7 @@ struct nvmet_ctrl {u64 err_counter;struct nvme_error_slot slots[NVMET_ERROR_LOG_SLOTS ];bool pi_support; +bool concat;#ifdef CONFIG_NVME_TARGET_AUTH struct nvme_dhchap_key *host_key;struct nvme_dhchap_key *ctrl_key;
@@ -246,6 +250,9 @@ struct nvmet_ctrl {u8 *dh_key;size_t dh_keysize;#endif +#ifdef CONFIG_NVME_TARGET_TCP_TLS +struct key *tls_key; +#endif };struct nvmet_subsys {
@@ -716,13 +723,29 @@ static inline void nvmet_req_bio_put(struct nvmet_req *req, struct bio *bio)bio_put (bio); } +#ifdef CONFIG_NVME_TARGET_TCP_TLS +static inline key_serial_t nvmet_queue_tls_keyid (struct nvmet_sq *sq) +{ +return sq->tls_key ?key_serial (sq->tls_key) :0 ; +} +static inline void nvmet_sq_put_tls_key (struct nvmet_sq *sq) +{ +if (sq->tls_key) { +key_put (sq->tls_key); + sq->tls_key =NULL ; + } +} +#else +static inline key_serial_t nvmet_queue_tls_keyid (struct nvmet_sq *sq) {return 0 ; } +static inline void nvmet_sq_put_tls_key (struct nvmet_sq *sq) {} +#endif #ifdef CONFIG_NVME_TARGET_AUTH void nvmet_execute_auth_send (struct nvmet_req *req);void nvmet_execute_auth_receive (struct nvmet_req *req);int nvmet_auth_set_key (struct nvmet_host *host,const char *secret,bool set_ctrl);int nvmet_auth_set_host_hash (struct nvmet_host *host,const char *hash); -u8 nvmet_setup_auth (struct nvmet_ctrl *ctrl); +u8 nvmet_setup_auth (struct nvmet_ctrl *ctrl,struct nvmet_req *req);void nvmet_auth_sq_init (struct nvmet_sq *sq);void nvmet_destroy_auth (struct nvmet_ctrl *ctrl);void nvmet_auth_sq_free (struct nvmet_sq *sq);
@@ -732,16 +755,18 @@ int nvmet_auth_host_hash(struct nvmet_req *req, u8 *response, unsigned int hash_len);int nvmet_auth_ctrl_hash (struct nvmet_req *req,u8 *response,unsigned int hash_len); -static inline bool nvmet_has_auth (struct nvmet_ctrl *ctrl) +static inline bool nvmet_has_auth (struct nvmet_ctrl *ctrl,struct nvmet_req *req) { -return ctrl->host_key !=NULL ; +return ctrl->host_key !=NULL && !nvmet_queue_tls_keyid (req->sq); }int nvmet_auth_ctrl_exponential (struct nvmet_req *req,u8 *buf,int buf_size);int nvmet_auth_ctrl_sesskey (struct nvmet_req *req,u8 *buf,int buf_size); +void nvmet_auth_insert_psk (struct nvmet_sq *sq);#else -static inline u8 nvmet_setup_auth (struct nvmet_ctrl *ctrl) +static inline u8 nvmet_setup_auth (struct nvmet_ctrl *ctrl, +struct nvmet_req *req) {return 0 ; }
@@ -754,11 +779,12 @@ static inline bool nvmet_check_auth_status(struct nvmet_req *req) {return true; } -static inline bool nvmet_has_auth (struct nvmet_ctrl *ctrl) +static inline bool nvmet_has_auth (struct nvmet_ctrl *ctrl, +struct nvmet_req *req) {return false; }static inline const char *nvmet_dhchap_dhgroup_name (u8 dhgid) {return NULL ; } +static inline void nvmet_auth_insert_psk (struct nvmet_sq *sq) {};#endif -#endif /* _NVMET_H */
drivers/nvme/target/tcp.c +21 -2
@@ -1073,10 +1073,11 @@ static int nvmet_tcp_done_recv_pdu(struct nvmet_tcp_queue *queue)if (unlikely (!nvmet_req_init (req, &queue->nvme_cq, &queue->nvme_sq, &nvmet_tcp_ops))) { -pr_err ("failed cmd %p id %d opcode %d, data_len: %d\n" , +pr_err ("failed cmd %p id %d opcode %d, data_len: %d, status: %04x\n" , req->cmd, req->cmd->common.command_id, req->cmd->common.opcode, -le32_to_cpu (req->cmd->common.dptr.sgl.length)); +le32_to_cpu (req->cmd->common.dptr.sgl.length), +le16_to_cpu (req->cqe->status));nvmet_tcp_handle_req_failure (queue, queue->cmd, req);return 0 ;
@@ -1602,6 +1603,7 @@ static void nvmet_tcp_release_queue_work(struct work_struct *w)/* stop accepting incoming data */ queue->rcv_state =NVMET_TCP_RECV_ERR ; +nvmet_sq_put_tls_key (&queue->nvme_sq);nvmet_tcp_uninit_data_in_cmds (queue);nvmet_sq_destroy (&queue->nvme_sq);cancel_work_sync (&queue->io_work);
@@ -1807,6 +1809,23 @@ static void nvmet_tcp_tls_handshake_done(void *data, int status,spin_unlock_bh (&queue->state_lock);cancel_delayed_work_sync (&queue->tls_handshake_tmo_work); + +if (!status) { +struct key *tls_key =nvme_tls_key_lookup (peerid); + +if (IS_ERR (tls_key)) { +pr_warn ("%s: queue %d failed to lookup key %x\n" , + __func__, queue->idx, peerid); +spin_lock_bh (&queue->state_lock); + queue->state =NVMET_TCP_Q_FAILED ; +spin_unlock_bh (&queue->state_lock); + status =PTR_ERR (tls_key); + }else { +pr_debug ("%s: queue %d using TLS PSK %x\n" , + __func__, queue->idx, peerid); + queue->nvme_sq.tls_key = tls_key; + } + }if (status)nvmet_tcp_schedule_release_queue (queue);else
Signature · 1 line
2.35.3
PATCH v9 9/9 nvmet: add tls_concat and tls_key debugfs entries
In reply to Hannes ReineckeAdd debugfs entries to display the 'concat' and 'tls_key' controller attributes.
--- drivers/nvme/target/debugfs.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
drivers/nvme/target/debugfs.c +27 -0
@@ -132,6 +132,27 @@ static int nvmet_ctrl_host_traddr_show(struct seq_file *m, void *p) }NVMET_DEBUGFS_ATTR (nvmet_ctrl_host_traddr); +#ifdef CONFIG_NVME_TARGET_TCP_TLS +static int nvmet_ctrl_tls_key_show (struct seq_file *m,void *p) +{ +struct nvmet_ctrl *ctrl = m->private; +key_serial_t keyid =nvmet_queue_tls_keyid (ctrl->sqs[0 ]); + +seq_printf (m,"%08x\n" , keyid); +return 0 ; +} +NVMET_DEBUGFS_ATTR (nvmet_ctrl_tls_key); + +static int nvmet_ctrl_tls_concat_show (struct seq_file *m,void *p) +{ +struct nvmet_ctrl *ctrl = m->private; + +seq_printf (m,"%d\n" , ctrl->concat); +return 0 ; +} +NVMET_DEBUGFS_ATTR (nvmet_ctrl_tls_concat); +#endif +int nvmet_debugfs_ctrl_setup (struct nvmet_ctrl *ctrl) {char name[32 ];
@@ -157,6 +178,12 @@ int nvmet_debugfs_ctrl_setup(struct nvmet_ctrl *ctrl) &nvmet_ctrl_state_fops);debugfs_create_file ("host_traddr" ,S_IRUSR , ctrl->debugfs_dir, ctrl, &nvmet_ctrl_host_traddr_fops); +#ifdef CONFIG_NVME_TARGET_TCP_TLS +debugfs_create_file ("tls_concat" ,S_IRUSR , ctrl->debugfs_dir, ctrl, + &nvmet_ctrl_tls_concat_fops); +debugfs_create_file ("tls_key" ,S_IRUSR , ctrl->debugfs_dir, ctrl, + &nvmet_ctrl_tls_key_fops); +#endif return 0 ; }
Signature · 1 line
2.35.3
Re: [PATCH 9/9] nvmet: add tls_concat and tls_key debugfs entries
Hi Hannes,
Quoted 3 lines
Patchset can be found at git.kernel.org:/pub/scm/linux/kernel/git/hare/nvme.git branch secure-concat.v9
I don't see the .v9 branch under that repo. Did you mean secure-concat.v8 branch?
Thanks,
Kamaljit
________________________________________
From: Linux-nvme <[email protected]> on behalf of Hannes Reinecke <[email protected]>
Sent: Tuesday, August 13, 2024 4:15 AM
To: hch
Cc: Keith Busch; Sagi Grimberg; [email protected]; Eric Biggers; [email protected]; Hannes Reinecke
Subject: [PATCHv9 0/9] nvme: implement secure concatenation
CAUTION: This email originated from outside of Western Digital. Do not click on links or open attachments unless you recognize the sender and know that the content is safe.
Hi all,
here's my attempt to implement secure concatenation for NVMe-of TCP as outlined in TP8018.
The original (v5) patchset had been split in two, the first part of which has already been merged with nvme-6.11, and this is the second part which actually implements secure concatenation.
Secure concatenation means that a TLS PSK is generated from the key material negotiated by the DH-HMAC-CHAP protocol, and the TLS PSK is then used for a subsequent TLS connection.
The difference between the original definition of secure concatenation and the method outlined in TP8018 is that with TP8018 the connection is reset after DH-HMAC-CHAP negotiation, and a new connection is setup with the generated TLS PSK.
To implement that Sagi came up with the idea to directly reset the admin queue once the DH-CHAP negotiation has completed; that way it will be transparent to the upper layers and we don't have to worry about exposing queues which should not be used.
As usual, comments and reviews are welcome.
Patchset can be found at git.kernel.org:/pub/scm/linux/kernel/git/hare/nvme.git branch secure-concat.v9
Changes to v8:
- Include reviews from Eric Biggers
- Make hkdf a proper module
- Add testcases for hkdf
Changes to v7:
- Add patch to display nvme target TLS status in debugfs
- Include reviews from Sagi
Changes to v6:
- Rebase to nvme-6.11
Changes to v5:
- Include reviews from Sagi
- Split patchset in two parts
Changes to v4:
- Rework reset admin queue functionality based on an idea from Sagi (thanks!)
- kbuild robot fixes
- Fixup dhchap negotiation with non-empty C2 value
Changes to v3:
- Include reviews from Sagi
- Do not start I/O queues after DH-HMAC-CHAP negotiation
- Use bool to indicate TLS has been enabled on a queue
- Add 'tls_keyring' sysfs attribute
- Add 'tls_configured_key' sysfs attribute
Changes to v2:
- Fixup reset after dhchap negotiation
- Disable namespace scanning on I/O queues after
dhchap negotiation
- Reworked TLS key handling (again)
Changes to the original submission:
- Sanitize TLS key handling
- Fixup modconfig compilation
Hannes Reinecke (9):
crypto,fs: Separate out hkdf_extract() and hkdf_expand()
nvme: add nvme_auth_generate_psk()
nvme: add nvme_auth_generate_digest()
nvme: add nvme_auth_derive_tls_psk()
nvme-keyring: add nvme_tls_psk_refresh()
nvme-tcp: request secure channel concatenation
nvme-fabrics: reset admin connection for secure concatenation
nvmet-tcp: support secure channel concatenation
nvmet: add tls_concat and tls_key debugfs entries
crypto/Kconfig | 4 + crypto/Makefile | 1 + crypto/hkdf.c | 404 +++++++++++++++++++++++++ drivers/nvme/common/Kconfig | 1 + drivers/nvme/common/auth.c | 344 +++++++++++++++++++++ drivers/nvme/common/keyring.c | 64 ++++ drivers/nvme/host/auth.c | 108 ++++++- drivers/nvme/host/fabrics.c | 34 ++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/nvme.h | 2 + drivers/nvme/host/sysfs.c | 4 +- drivers/nvme/host/tcp.c | 56 +++- drivers/nvme/target/auth.c | 72 ++++- drivers/nvme/target/debugfs.c | 27 ++ drivers/nvme/target/fabrics-cmd-auth.c | 49 ++- drivers/nvme/target/fabrics-cmd.c | 33 +- drivers/nvme/target/nvmet.h | 38 ++- drivers/nvme/target/tcp.c | 23 +- fs/crypto/Kconfig | 1 + fs/crypto/hkdf.c | 87 +----- include/crypto/hkdf.h | 34 +++ include/linux/nvme-auth.h | 7 + include/linux/nvme-keyring.h | 9 + include/linux/nvme.h | 7 + 24 files changed, 1303 insertions(+), 109 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h
--
2.35.3
Hannes Reinecke
maintainerQuoted 7 lines · Kamaljit Singh
Hi Hannes,
Quoted 3 lines
Patchset can be found at git.kernel.org:/pub/scm/linux/kernel/git/hare/nvme.git branch secure-concat.v9
I don't see the .v9 branch under that repo. Did you mean secure-concat.v8 branch?
Sorry. Pushed now.
Cheers,
Hannes
Signature · 4 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +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
Quoted 8 lines
Quoted 7 lines
Hi Hannes,
Quoted 3 lines
Patchset can be found at git.kernel.org:/pub/scm/linux/kernel/git/hare/nvme.git branch secure-concat.v9
I don't see the .v9 branch under that repo. Did you mean secure-concat.v8 branch?
Sorry. Pushed now.
Thanks Hannes! I can access the v9 branch now.
Kamaljit