nvme: implement secure concatenation
Subsystems (5)
v10 cover nvme: implement secure concatenation
From: Hannes Reinecke <[email protected]>
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.
A blktest submission is in
https://github.com/osandov/blktests/pull/147
in case anyone want to run their own tests.
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.v10
Changes to v9:
- Include reviews from Eric Biggers
- Fixup secure concatenation after reset
- Rebased to nvme-6.12
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 | 6 + crypto/Makefile | 1 + crypto/hkdf.c | 402 +++++++++++++++++++++++++ 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 | 85 +----- include/crypto/hkdf.h | 18 ++ include/linux/nvme-auth.h | 7 + include/linux/nvme-keyring.h | 9 + include/linux/nvme.h | 7 + 24 files changed, 1287 insertions(+), 107 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h
Signature · 1 line
2.35.3
PATCH v10 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 | 6 + crypto/Makefile | 1 + crypto/hkdf.c | 402 ++++++++++++++++++++++++++++++++++++++++++ fs/crypto/Kconfig | 1 + fs/crypto/hkdf.c | 85 ++------- include/crypto/hkdf.h | 18 ++ 6 files changed, 443 insertions(+), 70 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h
crypto/Kconfig +6 -0
@@ -141,6 +141,12 @@ config CRYPTO_ACOMP select CRYPTO_ALGAPI select CRYPTO_ACOMP2 +config CRYPTO_HKDF + tristate + select CRYPTO_SHA1 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_SHA256 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + 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 +402 -0
402 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 -70
124 lines not on this page · raw message
include/crypto/hkdf.h new file +18 -0
@@ -0,0 +1,18 @@ +/* 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 + +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); +#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
crypto/Kconfig config CRYPTO_ACOMP · line 148
Quoted 5 lines
crypto/Kconfig +5
-0+config CRYPTO_HKDF + tristate + select CRYPTO_SHA1 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_SHA256 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_HASH2
Any thoughts on including SHA512 tests instead of SHA1, given that SHA1 is obsolete and should not be used?
Quoted 4 lines
crypto/hkdf.c new file +4
-0+int hkdf_expand (struct crypto_shash *hmac_tfm, +const u8 *info,unsigned int infolen, +u8 *okm,unsigned int okmlen) +{
crypto/hkdf.c line 87
Quoted 10 lines
crypto/hkdf.c new file +10
-0+SHASH_DESC_ON_STACK (desc, hmac_tfm); +unsigned int i, hashlen =crypto_shash_digestsize (hmac_tfm); +int err; +const u8 *prev =NULL ; +u8 counter =1 ; +u8 tmp[HASH_MAX_DIGESTSIZE ]; + +if (WARN_ON (okmlen >255 * hashlen || + hashlen >HASH_MAX_DIGESTSIZE )) +return -EINVAL ;
The crypto API guarantees HASH_MAX_DIGESTSIZE, so checking that again is not very useful.
Review
Quoted 2 lines
(quoted diff) +2
-0+ + memzero_explicit(tmp, HASH_MAX_DIGESTSIZE);
The zeroization above is unnecessary. If it's done anyway, it is just an initialization, so it should use an initializer '= {}' instead of memzero_explicit() which is intended for "destruction".
Quoted 1 line
+MODULE_ALIAS_CRYPTO("hkdf");
This alias does not make sense and is unnecessary. These are library functions that are not exposed by name through the crypto API, so there is no need to wire them up to the module autoloading accordingly.
Quoted 14 lines
include/crypto/hkdf.h new file +8
-0@@ -0,0 +1,18 @@ +/* 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 +*/ +
include/crypto/hkdf.h line 18
Quoted 10 lines
include/crypto/hkdf.h new file +10
-0+#ifndef _CRYPTO_HKDF_H +#define _CRYPTO_HKDF_H + +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); +#endif
This needs to include <crypto/hash.h>.
Otherwise there will be errors if someone includes this as their first header.
- Eric
Hannes Reinecke
maintainerRe: [PATCH 1/9] crypto,fs: Separate out hkdf_extract() and hkdf_expand()
Quoted 8 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.integrity => correctness
Okay.
Quoted 9 lines
crypto/Kconfig config CRYPTO_ACOMP · line 148
Quoted 5 lines
crypto/Kconfig +5
-0+config CRYPTO_HKDF + tristate + select CRYPTO_SHA1 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_SHA256 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_HASH2Any thoughts on including SHA512 tests instead of SHA1, given that SHA1 is obsolete and should not be used?
Hmm. The original implementation did use SHA1, so I used that. But sure I can look into changing that.
Quoted 9 lines
Quoted 9 lines
crypto/hkdf.c new file +9
-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; +const u8 *prev =NULL ; +u8 counter =1 ;
Quoted 9 lines
crypto/hkdf.c line 87
Quoted 5 lines
crypto/hkdf.c new file +5
-0+u8 tmp[HASH_MAX_DIGESTSIZE ]; + +if (WARN_ON (okmlen >255 * hashlen || + hashlen >HASH_MAX_DIGESTSIZE )) +return -EINVAL ;The crypto API guarantees
HASH_MAX_DIGESTSIZE, so checking that again is not very useful.
Okay, will be removing that check.
Quoted 7 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ + memzero_explicit(tmp, HASH_MAX_DIGESTSIZE);The zeroization above is unnecessary. If it's done anyway, it is just an initialization, so it should use an initializer '= {}' instead of
memzero_explicit()which is intended for "destruction".
Ok.
Quoted 6 lines
Quoted 1 line
+MODULE_ALIAS_CRYPTO("hkdf");
This alias does not make sense and is unnecessary. These are library functions that are not exposed by name through the crypto API, so there is no need to wire them up to the module autoloading accordingly.
Ok.
Quoted 27 lines
Quoted 14 lines
include/crypto/hkdf.h new file +8
-0@@ -0,0 +1,18 @@ +/* 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 +*/ +include/crypto/hkdf.h line 18
Quoted 10 lines
include/crypto/hkdf.h new file +10
-0+#ifndef _CRYPTO_HKDF_H +#define _CRYPTO_HKDF_H + +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); +#endif This needs to include <
crypto/hash.h>.
Quoted 2 lines
Otherwise there will be errors if someone includes this as their first header.
Will do.
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 20 lines · Hannes Reinecke
Quoted 8 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.integrity => correctness
Okay.
Quoted 9 lines
crypto/Kconfig config CRYPTO_ACOMP · line 148
Quoted 5 lines
crypto/Kconfig +5
-0+config CRYPTO_HKDF + tristate + select CRYPTO_SHA1 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_SHA256 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_HASH2Any thoughts on including SHA512 tests instead of SHA1, given that SHA1 is obsolete and should not be used?
Quoted 2 lines
Hmm. The original implementation did use SHA1, so I used that. But sure I can look into changing that.
If you're talking about fs/crypto/hkdf.c which is where you're borrowing the code from, that uses SHA512.
- Eric
Hannes Reinecke
maintainerRe: [PATCH 1/9] crypto,fs: Separate out hkdf_extract() and hkdf_expand()
Quoted 21 lines · Eric Biggers
Quoted 20 lines · Hannes Reinecke
Quoted 8 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.integrity => correctness
Okay.
Quoted 9 lines
crypto/Kconfig config CRYPTO_ACOMP · line 148
Quoted 5 lines
crypto/Kconfig +5
-0+config CRYPTO_HKDF + tristate + select CRYPTO_SHA1 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_SHA256 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_HASH2Any thoughts on including SHA512 tests instead of SHA1, given that SHA1 is obsolete and should not be used?
Quoted 6 lines
Quoted 2 lines
Hmm. The original implementation did use SHA1, so I used that. But sure I can look into changing that.
If you're talking about
fs/crypto/hkdf.cwhich is where you're borrowing the code from, that uses SHA512.
Actually, I was talking about the test vectors themselves. RFC 5869 only gives test vectors for SHA1 and SHA256, so that's what I've used. I've found additional test vectors for the other functions at
https://github.com/brycx/Test-Vector-Generation/blob/master/HKDF/hkdf-hmac-sha2-test-vectors.md
so I'll be using them for adding tests for SHA384 and SHA512 (TLS on NVMe-over-TCP is using SHA384, too) and delete the SHA1 ones.
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 28 lines · Hannes Reinecke
Quoted 21 lines · Eric Biggers
Quoted 20 lines · Hannes Reinecke
Quoted 8 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.integrity => correctness
Okay.
Quoted 9 lines
crypto/Kconfig config CRYPTO_ACOMP · line 148
Quoted 5 lines
crypto/Kconfig +5
-0+config CRYPTO_HKDF + tristate + select CRYPTO_SHA1 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_SHA256 if !CONFIG_CRYPTO_MANAGER_DISABLE_TESTS + select CRYPTO_HASH2Any thoughts on including SHA512 tests instead of SHA1, given that SHA1 is obsolete and should not be used?
Quoted 6 lines
Quoted 2 lines
Hmm. The original implementation did use SHA1, so I used that. But sure I can look into changing that.
If you're talking about
fs/crypto/hkdf.cwhich is where you're borrowing the code from, that uses SHA512.
Quoted 6 lines
Actually, I was talking about the test vectors themselves. RFC 5869 only gives test vectors for SHA1 and SHA256, so that's what I've used. I've found additional test vectors for the other functions at
https://github.com/brycx/Test-Vector-Generation/blob/master/HKDF/hkdf-hmac-sha2-test-vectors.md
so I'll be using them for adding tests for SHA384 and SHA512 (TLS on NVMe-over-TCP is using SHA384, too) and delete the SHA1 ones.
Right, it's an old spec, so that's why it has SHA1. Using SHA512 test vectors from elsewhere, or generating your own test vectors using a known-good implementation, would be fine though.
- Eric
PATCH v10 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 v10 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
PATCH v10 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 v10 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
kernel test robot
build botRe: [PATCH 5/9] nvme-keyring: add nvme_tls_psk_refresh()
kernel test robot noticed the following build warnings:
Hi Hannes,
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 brauner-vfs/vfs.all linus/master v6.12-rc3 next-20241014] [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/Hannes-Reinecke/crypto-fs-Separate-out-hkdf_extract-and-hkdf_expand/20241011-235704 base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master patch link: https://lore.kernel.org/r/20241011155430.43450-6-hare%40kernel.org patch subject: [PATCH 5/9] nvme-keyring: add nvme_tls_psk_refresh() config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241015/[email protected]/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241015/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
In file included from drivers/nvme/host/fabrics.c:15:
>> include/linux/nvme-keyring.h:21:20: warning: 'nvme_tls_psk_refresh' defined but not used [-Wunused-function]
21 | static struct key *nvme_tls_psk_refresh(struct key *keyring,
| ^~~~~~~~~~~~~~~~~~~~
vim +/nvme_tls_psk_refresh +21 include/linux/nvme-keyring.h
20 > 21 static struct key *nvme_tls_psk_refresh(struct key *keyring,
Signature · 2 lines
0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
PATCH v10 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
@@ -1132,6 +1132,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) {
@@ -1154,6 +1155,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
@@ -780,10 +780,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
Re: Re: [PATCH 6/9] nvme-tcp: request secure channel concatenation
Quoted 10 lines
(quoted diff) +1
-0+ tls_key = nvme_tls_psk_refresh(ctrl->opts->keyring, ctrl-Quoted 1 line
opts->host->nqn,(quoted diff) +1
-0+ ctrl->opts->subsysnqn, chap-Quoted 1 line
hash_id,drivers/nvme/host/auth.c +5
-0+ 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",
drivers/nvme/host/auth.c static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap) · line 757
Quoted 4 lines
drivers/nvme/host/auth.c +4
-0+ __func__, chap->qid, ret); + tls_key =NULL ; +kfree_sensitive (tls_psk); + }
Should tls_psk be freed in the successful case also?
That is what happens in a similar situation in integrity_load_x509()
Hannes Reinecke
maintainerRe: [PATCH 6/9] nvme-tcp: request secure channel concatenation
Quoted 17 lines · Mark O'Donovan
Quoted 10 lines
(quoted diff) +1
-0+ tls_key = nvme_tls_psk_refresh(ctrl->opts->keyring, ctrl-Quoted 1 line
opts->host->nqn,(quoted diff) +1
-0+ ctrl->opts->subsysnqn, chap-Quoted 1 line
hash_id,drivers/nvme/host/auth.c +5
-0+ 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",
drivers/nvme/host/auth.c static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap) · line 757
Quoted 4 lines
drivers/nvme/host/auth.c +4
-0+ __func__, chap->qid, ret); + tls_key =NULL ; +kfree_sensitive (tls_psk); + }Should
tls_pskbe freed in the successful case also?
Quoted 1 line
That is what happens in a similar situation in
integrity_load_x509()
You are correct; tls_psk needs to be freed in all cases.
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 v10 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 (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
PATCH v10 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"
@@ -138,7 +139,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;
@@ -164,6 +165,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" );
@@ -232,6 +238,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);
@@ -260,6 +269,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)
@@ -541,3 +556,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 v10 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