nvme: implement secure concatenation
Subsystems (5)
v2 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.
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 I have decided on resetting the connection from the nvme-tcp driver after the initial connection has been set up. Another way would have been to offload the connection reset to userspace, and let nvme-cli reset the connection. But that would be a modification to the userspace interface, and hence I didn't go that way. The drawback with this approach is that we'll create all I/O queues before resetting for TLS, even though these queues should never be used. But fixing that requires a larger rewrite of the TCP driver to unify the setup and reconnect paths. So keep it that way for now.
As usual, comments and reviews are welcome.
Changes to the original submission:
- Sanitize TLS key handling
- Fixup modconfig compilation
Hannes Reinecke (13):
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-keyring: restrict match length for version '1' identifiers
nvme-tcp: check for invalidated or revoked key
nvme-fabrics: authentication errors are not retryable
nvme-tcp: sanitize TLS key handling
nvme-tcp: request secure channel concatenation
nvme-tcp: combine reset and recovery
nvme-tcp: reset after recovery for secure concatenation
nvmet-tcp: support secure channel concatenation
crypto/Makefile | 1 + crypto/hkdf.c | 111 +++++++++++ drivers/nvme/common/auth.c | 252 +++++++++++++++++++++++++ drivers/nvme/common/keyring.c | 71 +++++++ drivers/nvme/host/auth.c | 108 ++++++++++- drivers/nvme/host/core.c | 1 - drivers/nvme/host/fabrics.c | 46 ++++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/nvme.h | 1 - drivers/nvme/host/sysfs.c | 9 +- drivers/nvme/host/tcp.c | 106 ++++++++--- drivers/nvme/target/auth.c | 73 ++++++- drivers/nvme/target/fabrics-cmd-auth.c | 46 ++++- drivers/nvme/target/fabrics-cmd.c | 30 ++- drivers/nvme/target/nvmet.h | 30 ++- drivers/nvme/target/tcp.c | 27 +++ fs/crypto/hkdf.c | 68 +------ include/crypto/hkdf.h | 18 ++ include/linux/nvme-auth.h | 5 + include/linux/nvme-keyring.h | 7 + include/linux/nvme.h | 7 + 21 files changed, 898 insertions(+), 122 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h
Signature · 1 line
2.35.3
PATCH v2 1/13 crypto,fs: Separate out hkdf_extract() and hkdf_expand()
From: Hannes Reinecke <[email protected]>
Separate out the HKDF functions into a separate file to make them available to other callers.
--- crypto/Makefile | 1 + crypto/hkdf.c | 111 ++++++++++++++++++++++++++++++++++++++++++ fs/crypto/hkdf.c | 68 ++++---------------------- include/crypto/hkdf.h | 18 +++++++ 4 files changed, 139 insertions(+), 59 deletions(-) create mode 100644 crypto/hkdf.c create mode 100644 include/crypto/hkdf.h
crypto/Makefile +1 -0
@@ -26,6 +26,7 @@ obj-$(CONFIG_CRYPTO_ECHAINIV) += echainiv.o crypto_hash-y += ahash.o crypto_hash-y += shash.o +crypto_hash-y += hkdf.o obj-$(CONFIG_CRYPTO_HASH2) += crypto_hash.o obj-$(CONFIG_CRYPTO_AKCIPHER2) += akcipher.o
crypto/hkdf.c new file +111 -0
111 lines not on this page · raw message
fs/crypto/hkdf.c +9 -59
96 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-or-later */ +/* +* 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,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
PATCH v2 2/13 nvme: add nvme_auth_generate_psk()
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Add a function to generate a NVMe PSK from the shared credentials negotiated by DH-HMAC-CHAP.
--- drivers/nvme/common/auth.c | 76 ++++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 2 + 2 files changed, 78 insertions(+)
drivers/nvme/common/auth.c +76 -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,4 +472,79 @@ int nvme_auth_generate_key(u8 *secret, struct nvme_dhchap_key **ret_key) }EXPORT_SYMBOL_GPL (nvme_auth_generate_key); +u8 *nvme_auth_generate_psk (u8 hmac_id,u8 *skey,size_t skey_len, +u8 *c1,u8 *c2,size_t hash_len,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 ERR_PTR (-EINVAL ); + } + + hmac_name =nvme_auth_hmac_name (hmac_id); +if (!hmac_name) { +pr_warn ("%s: invalid hash algoritm %d\n" , + __func__, hmac_id); +return ERR_PTR (-EINVAL ); + } + + tfm =crypto_alloc_shash (hmac_name,0 ,0 ); +if (IS_ERR (tfm)) +return (u8 *)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_len = psk_len; + +out_free_shash: +kfree_sensitive (shash); +out_free_psk: +if (ret) { +kfree_sensitive (psk); + psk =NULL ; + } +out_free_tfm: +crypto_free_shash (tfm); + +return ret ?ERR_PTR (ret) : psk; +} +EXPORT_SYMBOL_GPL (nvme_auth_generate_psk); +MODULE_LICENSE ("GPL v2" );
include/linux/nvme-auth.h +2 -0
@@ -40,5 +40,7 @@ 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); +u8 *nvme_auth_generate_psk (u8 hmac_id,u8 *skey,size_t skey_len, +u8 *c1,u8 *c2,size_t hash_len,size_t *ret_len);#endif /* _NVME_AUTH_H */
Signature · 1 line
2.35.3
Re: [PATCH 02/13] nvme: add nvme_auth_generate_psk()
PATCH v2 3/13 nvme: add nvme_auth_generate_digest()
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Add a function to calculate the PSK digest as specified in TP8018.
--- drivers/nvme/common/auth.c | 105 +++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 2 + 2 files changed, 107 insertions(+)
drivers/nvme/common/auth.c +105 -0
@@ -547,4 +547,109 @@ u8 *nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, }EXPORT_SYMBOL_GPL (nvme_auth_generate_psk); +u8 *nvme_auth_generate_digest (u8 hmac_id,u8 *psk,size_t psk_len, +char *subsysnqn,char *hostnqn) +{ +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 ERR_PTR (-EINVAL ); + + hmac_name =nvme_auth_hmac_name (hmac_id); +if (!hmac_name) { +pr_warn ("%s: invalid hash algoritm %d\n" , + __func__, hmac_id); +return ERR_PTR (-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 ERR_PTR (-EINVAL ); + } + + hmac =kzalloc (hmac_len +1 ,GFP_KERNEL ); +if (!hmac) +return ERR_PTR (-ENOMEM ); + + tfm =crypto_alloc_shash (hmac_name,0 ,0 ); +if (IS_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 =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); + hmac =NULL ; + } +return ret ?ERR_PTR (ret) : hmac; +} +EXPORT_SYMBOL_GPL (nvme_auth_generate_digest); +MODULE_LICENSE ("GPL v2" );
include/linux/nvme-auth.h +2 -0
@@ -42,5 +42,7 @@ int nvme_auth_gen_shared_secret(struct crypto_kpp *dh_tfm, u8 *sess_key, size_t sess_key_len);u8 *nvme_auth_generate_psk (u8 hmac_id,u8 *skey,size_t skey_len,u8 *c1,u8 *c2,size_t hash_len,size_t *ret_len); +u8 *nvme_auth_generate_digest (u8 hmac_id,u8 *psk,size_t psk_len, +char *subsysnqn,char *hostnqn);#endif /* _NVME_AUTH_H */
Signature · 1 line
2.35.3
Re: [PATCH 03/13] nvme: add nvme_auth_generate_digest()
PATCH v2 4/13 nvme: add nvme_auth_derive_tls_psk()
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Add a function to derive the TLS PSK as specified TP8018.
--- drivers/nvme/common/auth.c | 71 ++++++++++++++++++++++++++++++++++++++ include/linux/nvme-auth.h | 1 + 2 files changed, 72 insertions(+)
drivers/nvme/common/auth.c +71 -0
@@ -652,4 +652,75 @@ u8 *nvme_auth_generate_digest(u8 hmac_id, u8 *psk, size_t psk_len, }EXPORT_SYMBOL_GPL (nvme_auth_generate_digest); +u8 *nvme_auth_derive_tls_psk (int hmac_id,u8 *psk,size_t psk_len,u8 *psk_digest) +{ +struct crypto_shash *hmac_tfm; +const char *hmac_name; +const char *psk_prefix ="tls13 nvme-tls-psk" ; +size_t info_len, prk_len; +char *info; +unsigned char *prk, *tls_key; +int ret; + + hmac_name =nvme_auth_hmac_name (hmac_id); +if (!hmac_name) { +pr_warn ("%s: invalid hash algoritm %d\n" , + __func__, hmac_id); +return ERR_PTR (-EINVAL ); + } +if (hmac_id ==NVME_AUTH_HASH_SHA512 ) { +pr_warn ("%s: unsupported hash algorithm %s\n" , + __func__, hmac_name); +return ERR_PTR (-EINVAL ); + } + + hmac_tfm =crypto_alloc_shash (hmac_name,0 ,0 ); +if (IS_ERR (hmac_tfm)) +return (u8 *)hmac_tfm; + + prk_len =crypto_shash_digestsize (hmac_tfm); + prk =kzalloc (prk_len,GFP_KERNEL ); +if (!prk) { + ret = -ENOMEM ; + goto out_free_shash; + } + + ret =hkdf_extract (hmac_tfm, psk, psk_len, prk); +if (ret) + goto out_free_prk; + + ret =crypto_shash_setkey (hmac_tfm, prk, prk_len); +if (ret) + goto out_free_prk; + + info_len =strlen (psk_digest) +strlen (psk_prefix) +1 ; + info =kzalloc (info_len,GFP_KERNEL ); +if (!info) + goto out_free_prk; + +memcpy (info, psk_prefix,strlen (psk_prefix)); +memcpy (info +strlen (psk_prefix), psk_digest,strlen (psk_digest)); + + tls_key =kzalloc (psk_len,GFP_KERNEL ); +if (!tls_key) { + ret = -ENOMEM ; + goto out_free_info; + } + ret =hkdf_expand (hmac_tfm, info,strlen (info), tls_key, psk_len); +if (ret) + goto out_free_key; + +out_free_key: +kfree (tls_key); +out_free_info: +kfree (info); +out_free_prk: +kfree (prk); +out_free_shash: +crypto_free_shash (hmac_tfm); + +return ret ?ERR_PTR (ret) : tls_key; +} +EXPORT_SYMBOL_GPL (nvme_auth_derive_tls_psk); +MODULE_LICENSE ("GPL v2" );
include/linux/nvme-auth.h +1 -0
@@ -44,5 +44,6 @@ u8 *nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len, u8 *c1, u8 *c2, size_t hash_len, size_t *ret_len);u8 *nvme_auth_generate_digest (u8 hmac_id,u8 *psk,size_t psk_len,char *subsysnqn,char *hostnqn); +u8 *nvme_auth_derive_tls_psk (int hmac_id,u8 *psk,size_t psk_len,u8 *psk_digest);#endif /* _NVME_AUTH_H */
Signature · 1 line
2.35.3
PATCH v2 5/13 nvme-keyring: add nvme_tls_psk_refresh()
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Add a function to refresh a generated PSK in the specified keyring.
--- drivers/nvme/common/keyring.c | 50 +++++++++++++++++++++++++++++++++++ include/linux/nvme-keyring.h | 7 +++++ 2 files changed, 57 insertions(+)
drivers/nvme/common/keyring.c +50 -0
@@ -104,6 +104,56 @@ static struct key *nvme_tls_psk_lookup(struct key *keyring,return key_ref_to_ptr (keyref); } +struct key *nvme_tls_psk_refresh (struct key *keyring,char *hostnqn,char *subnqn, +u8 hmac_id,bool generated,u8 *data,size_t data_len,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; +size_t identity_len = (NVMF_NQN_SIZE ) *2 +77 ; +key_ref_t keyref; +key_serial_t keyring_id; +struct key *key; + +if (!hostnqn || !subnqn || !data || !data_len) +return ERR_PTR (-EINVAL ); + + identity =kzalloc (identity_len,GFP_KERNEL ); +if (!identity) +return ERR_PTR (-ENOMEM ); + +snprintf (identity, identity_len,"NVMe1%c%02d %s %s %s" , + generated ?'G' :'R' , hmac_id, hostnqn, subnqn, digest); + +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 +7 -0
@@ -8,6 +8,8 @@#if IS_ENABLED (CONFIG_NVME_KEYRING ) +struct key *nvme_tls_psk_refresh (struct key *keyring,char *hostnqn,char *subnqn, +u8 hmac_id,bool generated,u8 *data,size_t data_len,char *digest);key_serial_t nvme_tls_psk_default (struct key *keyring,const char *hostnqn,const char *subnqn);
@@ -15,6 +17,11 @@ key_serial_t nvme_keyring_id(void);#else +static struct key *nvme_tls_psk_refresh (struct key *keyring,char *hostnqn,char *subnqn, +u8 hmac_id,bool generated,u8 *data,size_t data_len,char *digest) +{ +return -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 v2 6/13 nvme-keyring: restrict match length for version '1' identifiers
In reply to [email protected]From: Hannes Reinecke <[email protected]>
TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup any version '1' identifiers we need to restrict the match length to exclude the PSK hash value (which we don't have when looking up keys).
--- drivers/nvme/common/keyring.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
drivers/nvme/common/keyring.c +21 -0
@@ -44,6 +44,27 @@ static bool nvme_tls_psk_match(const struct key *key,return false; } match_id = match_data->raw_data; +if (memcmp (match_id,"NVMe1" ,5 )) { +char *e = (char *)match_id; +size_t offset =0 ; +int n =0 ; + +while (*e !=' ' && offset < match_len) { +if (*e ==' ' ) { + n++; +if (n ==3 ) +break ; + } + e++; + offset++; + } +if (n !=3 ) { +pr_debug ("%s: error parsing '%s'\n" , + __func__, match_id); +return false; + } + match_len = offset; + }pr_debug ("%s: match '%s' '%s' len %zd\n" , __func__, match_id, key->description, match_len);return !memcmp (key->description, match_id, match_len);
Signature · 1 line
2.35.3
Sagi Grimberg
maintainerRe: [PATCH 06/13] nvme-keyring: restrict match length for version '1' identifiers
Quoted 31 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup any version '1' identifiers we need to restrict the match length to exclude the PSK hash value (which we don't have when looking up keys).
Signed-off-by Hannes Reinecke <[email protected]>--- drivers/nvme/common/keyring.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)drivers/nvme/common/keyring.c +11
-0@@ -44,6 +44,27 @@ static bool nvme_tls_psk_match(const struct key *key,return false; } match_id = match_data->raw_data; +if (memcmp (match_id,"NVMe1" ,5 )) { +char *e = (char *)match_id; +size_t offset =0 ; +int n =0 ; + +while (*e !=' ' && offset < match_len) { +if (*e ==' ' ) { + n++; +if (n ==3 ) +break ; + }
drivers/nvme/common/keyring.c static bool nvme_tls_psk_match(const struct key *key, · line 67
Quoted 10 lines
drivers/nvme/common/keyring.c +10
-0+ e++; + offset++; + } +if (n !=3 ) { +pr_debug ("%s: error parsing '%s'\n" , + __func__, match_id); +return false; + } + match_len = offset; + }
My eyes hurt... I am failing to understand what this even does...
You have a concrete example?
Hannes Reinecke
maintainerRe: [PATCH 06/13] nvme-keyring: restrict match length for version '1' identifiers
Quoted 47 lines · Sagi Grimberg
Quoted 31 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup any version '1' identifiers we need to restrict the match length to exclude the PSK hash value (which we don't have when looking up keys).
Signed-off-by Hannes Reinecke <[email protected]>---
drivers/nvme/common/keyring.c| 21 +++++++++++++++++++++1 file changed, 21 insertions(+)drivers/nvme/common/keyring.c +11
-0@@ -44,6 +44,27 @@ static bool nvme_tls_psk_match(const struct key *key, return false; } match_id = match_data->raw_data; + if (memcmp(match_id, "NVMe1", 5)) { + char *e = (char *)match_id; + size_t offset = 0; + int n = 0; + + while (*e != ' ' && offset < match_len) { + if (*e == ' ') { + n++; + if (n == 3) + break; + }drivers/nvme/common/keyring.c static bool nvme_tls_psk_match(const struct key *key, · line 67
Quoted 10 lines
drivers/nvme/common/keyring.c +10
-0+ e++; + offset++; + } + if (n != 3) { + pr_debug("%s: error parsing '%s'\n", + __func__, match_id); + return false; + } + match_len = offset; + }My eyes hurt... I am failing to understand what this even does...
Quoted 1 line
You have a concrete example?
Yeah, it's ugly. What I'm trying to do here is to lookup TLS PSK identities which are either in the original format:
NVMe0[RG]0[12] <hostnqn> <subsysnqn>
_or_ the TP8018 TLS PSK format:
NVMe1[RG]0[12] <hostnqn> <subsysnqn> <digest>
given that I only have <hostnqn> and <subsysnqn> as lookup keys. So for the memcmp() I'll have to exclude the ' <digest>' bit, which happens to the after the third blank space in the identity. I'll have a slightly better version queued, but parsing remains ugly.
Cheers,
Hannes
Sagi Grimberg
maintainerRe: [PATCH 06/13] nvme-keyring: restrict match length for version '1' identifiers
Quoted 55 lines · Hannes Reinecke
Quoted 48 lines · Sagi Grimberg
Quoted 32 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup any version '1' identifiers we need to restrict the match length to exclude the PSK hash value (which we don't have when looking up keys).
Signed-off-by Hannes Reinecke <[email protected]>---
drivers/nvme/common/keyring.c| 21 +++++++++++++++++++++1 file changed, 21 insertions(+)drivers/nvme/common/keyring.c +11
-0@@ -44,6 +44,27 @@ static bool nvme_tls_psk_match(const struct key *key, return false; } match_id = match_data->raw_data; + if (memcmp(match_id, "NVMe1", 5)) { + char *e = (char *)match_id; + size_t offset = 0; + int n = 0; + + while (*e != ' ' && offset < match_len) { + if (*e == ' ') { + n++; + if (n == 3) + break; + }drivers/nvme/common/keyring.c static bool nvme_tls_psk_match(const struct key · line 68
Quoted 10 lines
drivers/nvme/common/keyring.c +10
-0+ e++; + offset++; + } + if (n != 3) { + pr_debug("%s: error parsing '%s'\n", + __func__, match_id); + return false; + } + match_len = offset; + }My eyes hurt... I am failing to understand what this even does...
Quoted 1 line
You have a concrete example?
Yeah, it's ugly. What I'm trying to do here is to lookup TLS PSK identities which are either in the original format:
NVMe0[RG]0[12] <hostnqn> <subsysnqn>
Quoted 8 lines
_or_ the TP8018 TLS PSK format:
NVMe1[RG]0[12] <hostnqn> <subsysnqn> <digest>
given that I only have <hostnqn> and <subsysnqn> as lookup keys. So for the
memcmp()I'll have to exclude the ' <digest>' bit, which happens to the after the third blank space in the identity. I'll have a slightly better version queued, but parsing remains ugly.
Shouldn't strtok/strsep simplify what you are trying to do here?
Hannes Reinecke
maintainerRe: [PATCH 06/13] nvme-keyring: restrict match length for version '1' identifiers
Quoted 60 lines · Sagi Grimberg
Quoted 57 lines · Hannes Reinecke
Quoted 48 lines · Sagi Grimberg
Quoted 32 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
TP8018 changed the TLS PSK identifiers to append a PSK hash value, so to lookup any version '1' identifiers we need to restrict the match length to exclude the PSK hash value (which we don't have when looking up keys).
Signed-off-by Hannes Reinecke <[email protected]>---
drivers/nvme/common/keyring.c| 21 +++++++++++++++++++++1 file changed, 21 insertions(+)drivers/nvme/common/keyring.c +11
-0@@ -44,6 +44,27 @@ static bool nvme_tls_psk_match(const struct key *key, return false; } match_id = match_data->raw_data; + if (memcmp(match_id, "NVMe1", 5)) { + char *e = (char *)match_id; + size_t offset = 0; + int n = 0; + + while (*e != ' ' && offset < match_len) { + if (*e == ' ') { + n++; + if (n == 3) + break; + }drivers/nvme/common/keyring.c static bool nvme_tls_psk_match(const struct key · line 68
Quoted 10 lines
drivers/nvme/common/keyring.c +10
-0+ e++; + offset++; + } + if (n != 3) { + pr_debug("%s: error parsing '%s'\n", + __func__, match_id); + return false; + } + match_len = offset; + }My eyes hurt... I am failing to understand what this even does...
Quoted 1 line
You have a concrete example?
Yeah, it's ugly. What I'm trying to do here is to lookup TLS PSK identities which are either in the original format:
NVMe0[RG]0[12] <hostnqn> <subsysnqn>
_or_ the TP8018 TLS PSK format:
Quoted 8 lines
Quoted 6 lines
NVMe1[RG]0[12] <hostnqn> <subsysnqn> <digest>
given that I only have <hostnqn> and <subsysnqn> as lookup keys. So for the
memcmp()I'll have to exclude the ' <digest>' bit, which happens to the after the third blank space in the identity. I'll have a slightly better version queued, but parsing remains ugly.Shouldn't strtok/strsep simplify what you are trying to do here?
Yeah. I'll be reworking that.
Cheers,
Hannes
PATCH v2 7/13 nvme-tcp: check for invalidated or revoked key
In reply to [email protected]From: Hannes Reinecke <[email protected]>
key_lookup() will always return a key, even if that key is revoked or invalidated. So check for invalid keys before continuing.
--- drivers/nvme/host/fabrics.c | 7 ++++++- drivers/nvme/host/sysfs.c | 9 +++++++-- drivers/nvme/host/tcp.c | 8 +++++++- 3 files changed, 20 insertions(+), 4 deletions(-)
drivers/nvme/host/fabrics.c +6 -1
@@ -635,7 +635,12 @@ static struct key *nvmf_parse_key(int key_id) key =key_lookup (key_id);if (!IS_ERR (key))pr_err ("key id %08x not found\n" , key_id); -else +else if (test_bit (KEY_FLAG_REVOKED , &key->flags) || +test_bit (KEY_FLAG_INVALIDATED , &key->flags)) { +pr_err ("key id %08x invalid\n" , key_id); +key_put (key); + key =ERR_PTR (-EKEYREVOKED ); + }else pr_debug ("Using key id %08x\n" , key_id);return key; }
drivers/nvme/host/sysfs.c +7 -2
@@ -617,10 +617,15 @@ static ssize_t tls_key_show(struct device *dev, struct device_attribute *attr, char *buf) { struct nvme_ctrl *ctrl = dev_get_drvdata(dev); +struct key *key = ctrl->tls_key; -if (!ctrl->tls_key) +if (!key)return 0 ; -return sysfs_emit (buf,"%08x" ,key_serial (ctrl->tls_key)); +if (test_bit (KEY_FLAG_REVOKED , &key->flags) || +test_bit (KEY_FLAG_INVALIDATED , &key->flags)) +return -EKEYREVOKED ; + +return sysfs_emit (buf,"%08x" ,key_serial (key)); }static DEVICE_ATTR_RO (tls_key);#endif
drivers/nvme/host/tcp.c +7 -1
@@ -1571,9 +1571,15 @@ static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid) tls_key =key_lookup (pskid);if (IS_ERR (tls_key)) { -dev_warn (ctrl->ctrl.device,"queue %d: Invalid key %x\n" , +dev_warn (ctrl->ctrl.device,"queue %d: key %08x not found\n" , qid, pskid); queue->tls_err = -ENOKEY ; + }else if (test_bit (KEY_FLAG_REVOKED , &tls_key->flags) || +test_bit (KEY_FLAG_INVALIDATED , &tls_key->flags)) { +dev_warn (ctrl->ctrl.device,"queue %d: key %08x invalid\n" , + qid, pskid); +key_put (tls_key); + queue->tls_err = -EKEYREVOKED ; }else { ctrl->ctrl.tls_key = tls_key; queue->tls_err =0 ;
Signature · 1 line
2.35.3
Sagi Grimberg
maintainerRe: [PATCH 07/13] nvme-tcp: check for invalidated or revoked key
Quoted 4 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
key_lookup()will always return a key, even if that key is revoked or invalidated. So check for invalid keys before continuing.
Is this a bug fix? it should go probably either as a standalone patch or as the first in this set (if it is only relevant with this patchset).
Hannes Reinecke
maintainerRe: [PATCH 07/13] nvme-tcp: check for invalidated or revoked key
Quoted 10 lines · Sagi Grimberg
Quoted 4 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
key_lookup()will always return a key, even if that key is revoked or invalidated. So check for invalid keys before continuing.Is this a bug fix? it should go probably either as a standalone patch or as the first in this set (if it is only relevant with this patchset).
Possibly. But I've created them via code review, so I'm not sure how relevant it really is.
Cheers,
Hannes
PATCH v2 8/13 nvme-fabrics: authentication errors are not retryable
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Whenever authentication failed the error is non-retryable as the authentication solely depends on the passed parameters, which do not change during retry.
--- drivers/nvme/host/fabrics.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/nvme/host/fabrics.c +4 -4
@@ -465,7 +465,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)if (result &NVME_CONNECT_AUTHREQ_ASCR ) {dev_warn (ctrl->device,"qid 0: secure concatenation is not supported\n" ); - ret =NVME_SC_AUTH_REQUIRED ; + ret =NVME_SC_AUTH_REQUIRED |NVME_SC_DNR ; goto out_free_data; }/* Authentication required */
@@ -473,7 +473,7 @@ int nvmf_connect_admin_queue(struct nvme_ctrl *ctrl)if (ret) {dev_warn (ctrl->device,"qid 0: authentication setup failed\n" ); - ret =NVME_SC_AUTH_REQUIRED ; + ret =NVME_SC_AUTH_REQUIRED |NVME_SC_DNR ; goto out_free_data; } ret =nvme_auth_wait (ctrl,0 );
@@ -537,7 +537,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)if (result &NVME_CONNECT_AUTHREQ_ASCR ) {dev_warn (ctrl->device,"qid 0: secure concatenation is not supported\n" ); - ret =NVME_SC_AUTH_REQUIRED ; + ret =NVME_SC_AUTH_REQUIRED |NVME_SC_DNR ; goto out_free_data; }/* Authentication required */
@@ -545,7 +545,7 @@ int nvmf_connect_io_queue(struct nvme_ctrl *ctrl, u16 qid)if (ret) {dev_warn (ctrl->device,"qid %d: authentication setup failed\n" , qid); - ret =NVME_SC_AUTH_REQUIRED ; + ret =NVME_SC_AUTH_REQUIRED |NVME_SC_DNR ; }else { ret =nvme_auth_wait (ctrl, qid);if (ret)
Signature · 1 line
2.35.3
Sagi Grimberg
maintainerRe: [PATCH 08/13] nvme-fabrics: authentication errors are not retryable
Quoted 5 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Whenever authentication failed the error is non-retryable as the authentication solely depends on the passed parameters, which do not change during retry.
Does this directly relate to secure concatenation? because if it isn't we should drop it from here.
Hannes Reinecke
maintainerRe: [PATCH 08/13] nvme-fabrics: authentication errors are not retryable
Quoted 10 lines · Sagi Grimberg
Quoted 5 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Whenever authentication failed the error is non-retryable as the authentication solely depends on the passed parameters, which do not change during retry.
Does this directly relate to secure concatenation? because if it isn't we should drop it from here.
No, it does not. Meanwhile it went to the other patch series for authentication errors, and will be dropped from the next iteration.
Cheers,
Hannes
PATCH v2 9/13 nvme-tcp: sanitize TLS key handling
In reply to [email protected]From: Hannes Reinecke <[email protected]>
There is a difference between TLS configured (ie the user has provisioned/requested a key) and TLS enabled (ie the connection is encrypted with TLS).
So to differentiate between those two states store the provisioned key in opts->tls_key (as we're using the same TLS key for all queues) and the key serial of the key negotiated by the TLS handshake in queue->tls_key.
--- drivers/nvme/host/core.c | 1 - drivers/nvme/host/nvme.h | 1 - drivers/nvme/host/sysfs.c | 2 +- drivers/nvme/host/tcp.c | 40 +++++++++++++++++++++++---------------- 4 files changed, 25 insertions(+), 19 deletions(-)
drivers/nvme/host/core.c +0 -1
@@ -4450,7 +4450,6 @@ static void nvme_free_ctrl(struct device *dev)if (!subsys || ctrl->instance != subsys->instance)ida_free (&nvme_instance_ida, ctrl->instance); -key_put (ctrl->tls_key);nvme_free_cels (ctrl);nvme_mpath_uninit (ctrl);nvme_auth_stop (ctrl);
drivers/nvme/host/nvme.h +0 -1
@@ -357,7 +357,6 @@ struct nvme_ctrl {struct nvme_dhchap_key *ctrl_key;u16 transaction;#endif -struct key *tls_key;/* Power saving configuration */ u64 ps_max_latency_us;
drivers/nvme/host/sysfs.c +1 -1
@@ -617,7 +617,7 @@ static ssize_t tls_key_show(struct device *dev, struct device_attribute *attr, char *buf) { struct nvme_ctrl *ctrl = dev_get_drvdata(dev); -struct key *key = ctrl->tls_key; +struct key *key = ctrl->opts->tls_key;if (!key)return 0 ;
drivers/nvme/host/tcp.c +24 -16
@@ -163,6 +163,7 @@ struct nvme_tcp_queue {__le32 recv_ddgst;struct completion tls_complete;int tls_err; +key_serial_t tls_key;struct page_frag_cache pf_cache;void (*state_change)(struct sock *);
@@ -205,7 +206,15 @@ static inline int nvme_tcp_queue_id(struct nvme_tcp_queue *queue)return queue - queue->ctrl->queues; } -static inline bool nvme_tcp_tls (struct nvme_ctrl *ctrl) +static inline bool nvme_tcp_tls_enabled (struct nvme_tcp_queue *queue) +{ +if (!IS_ENABLED (CONFIG_NVME_TCP_TLS )) +return 0 ; + +return (queue->tls_key !=0 ); +} + +static inline bool nvme_tcp_tls_configured (struct nvme_ctrl *ctrl) {if (!IS_ENABLED (CONFIG_NVME_TCP_TLS ))return 0 ;
@@ -1418,7 +1427,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue)memset (&msg,0 ,sizeof (msg)); iov.iov_base = icresp; iov.iov_len =sizeof (*icresp); -if (nvme_tcp_tls (&queue->ctrl->ctrl)) { +if (nvme_tcp_tls_enabled (queue)) { msg.msg_control = cbuf; msg.msg_controllen =sizeof (cbuf); }
@@ -1430,7 +1439,7 @@ static int nvme_tcp_init_connection(struct nvme_tcp_queue *queue) goto free_icresp; } ret = -ENOTCONN ; -if (nvme_tcp_tls (&queue->ctrl->ctrl)) { +if (nvme_tcp_tls_enabled (queue)) { ctype =tls_get_record_type (queue->sock->sk, (struct cmsghdr *)cbuf);if (ctype !=TLS_RECORD_TYPE_DATA ) {
@@ -1581,7 +1590,8 @@ static void nvme_tcp_tls_done(void *data, int status, key_serial_t pskid)key_put (tls_key); queue->tls_err = -EKEYREVOKED ; }else { - ctrl->ctrl.tls_key = tls_key; + queue->tls_key = pskid; +key_put (tls_key); queue->tls_err =0 ; }
@@ -1762,7 +1772,7 @@ static int nvme_tcp_alloc_queue(struct nvme_ctrl *nctrl, int qid, }/* If PSKs are configured try to start TLS */ -if (IS_ENABLED (CONFIG_NVME_TCP_TLS ) && pskid) { +if (nvme_tcp_tls_configured (nctrl) && pskid) { ret =nvme_tcp_start_tls (nctrl, queue, pskid);if (ret) goto err_init_connect;
@@ -1919,16 +1929,17 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl)int ret;key_serial_t pskid =0 ; -if (nvme_tcp_tls (ctrl)) { +if (nvme_tcp_tls_configured (ctrl)) {if (ctrl->opts->tls_key) pskid =key_serial (ctrl->opts->tls_key); -else +else { pskid =nvme_tls_psk_default (ctrl->opts->keyring, ctrl->opts->host->nqn, ctrl->opts->subsysnqn); -if (!pskid) { -dev_err (ctrl->device,"no valid PSK found\n" ); -return -ENOKEY ; +if (!pskid) { +dev_err (ctrl->device,"no valid PSK found\n" ); +return -ENOKEY ; + } } }
@@ -1949,15 +1960,12 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl)static int __nvme_tcp_alloc_io_queues (struct nvme_ctrl *ctrl) { +struct nvme_tcp_ctrl *tcp_ctrl =to_tcp_ctrl (ctrl); +key_serial_t pskid = tcp_ctrl->queues[0 ].tls_key;int i, ret; -if (nvme_tcp_tls (ctrl) && !ctrl->tls_key) { -dev_err (ctrl->device,"no PSK negotiated\n" ); -return -ENOKEY ; - }for (i =1 ; i < ctrl->queue_count; i++) { - ret =nvme_tcp_alloc_queue (ctrl, i, -key_serial (ctrl->tls_key)); + ret =nvme_tcp_alloc_queue (ctrl, i, pskid);if (ret) goto out_free_queues; }
Signature · 1 line
2.35.3
Sagi Grimberg
maintainerRe: [PATCH 09/13] nvme-tcp: sanitize TLS key handling
Quoted 5 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
There is a difference between TLS configured (ie the user has provisioned/requested a key) and TLS enabled (ie the connection is encrypted with TLS).
When would the latter happen without the former?
Quoted 4 lines
So to differentiate between those two states store the provisioned key in
opts->tls_key(as we're using the same TLS key for all queues) and the key serial of the key negotiated by the TLS handshake inqueue->tls_key.
Does nvmet generate a different key for each queue?
Hannes Reinecke
maintainerRe: [PATCH 09/13] nvme-tcp: sanitize TLS key handling
Quoted 11 lines · Sagi Grimberg
Quoted 5 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
There is a difference between TLS configured (ie the user has provisioned/requested a key) and TLS enabled (ie the connection is encrypted with TLS).
When would the latter happen without the former?
No. Difference is that 'TLS configured' is a configuration setting (ie the admin has specified --tls or --tls_key), and 'TLS enabled' is the result once the queue has been established.
Quoted 6 lines
Quoted 4 lines
So to differentiate between those two states store the provisioned key in
opts->tls_key(as we're using the same TLS key for all queues) and the key serial of the key negotiated by the TLS handshake inqueue->tls_key.Does nvmet generate a different key for each queue?
No.
Cheers,
Hannes
PATCH v2 10/13 nvme-tcp: request secure channel concatenation
In reply to [email protected]From: Hannes Reinecke <[email protected]>
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 for the next connection attempt.
--- drivers/nvme/host/auth.c | 108 ++++++++++++++++++++++++++++++++++-- drivers/nvme/host/fabrics.c | 31 ++++++++++- drivers/nvme/host/fabrics.h | 3 + drivers/nvme/host/tcp.c | 9 ++- include/linux/nvme.h | 7 +++ 5 files changed, 148 insertions(+), 10 deletions(-)
drivers/nvme/host/auth.c +104 -4
@@ -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;
@@ -132,7 +133,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) + 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 ;
@@ -312,8 +318,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,
@@ -323,7 +330,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",
@@ -678,6 +688,79 @@ static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap)crypto_free_kpp (chap->dh_tfm); } +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 ; + } + + psk =nvme_auth_generate_psk (chap->hash_id, chap->sess_key, + chap->sess_key_len, + chap->c1, chap->c2, + chap->hash_len, &psk_len); +if (IS_ERR (psk)) { + ret =PTR_ERR (psk); +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); + + digest =nvme_auth_generate_digest (chap->hash_id, psk, psk_len, + ctrl->opts->subsysnqn, + ctrl->opts->host->nqn); +if (IS_ERR (digest)) { + ret =PTR_ERR (digest); +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); + tls_psk =nvme_auth_derive_tls_psk (chap->hash_id, psk, psk_len, digest); +if (IS_ERR (tls_psk)) { + ret =PTR_ERR (tls_psk); +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, + true, 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) { +key_revoke (ctrl->opts->tls_key); +key_put (ctrl->opts->tls_key); + } + 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 =
@@ -832,10 +915,21 @@ static void nvme_queue_auth_work(struct work_struct *work)if (ret) chap->error = ret; } -if (!ret) { +if (ret) + goto fail2; +if (chap->qid || !ctrl->opts->concat) { chap->error =0 ;return ; } + ret =nvme_auth_secure_concat (ctrl, chap); +if (ret) { +dev_warn (ctrl->device, +"%s: qid %d failed to enable secure concatenation\n" , + __func__, chap->qid); + chap->error = ret; + }else + chap->error =0 ; +return ; fail2:if (chap->status ==0 )
@@ -913,6 +1007,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 +28 -3
@@ -461,10 +461,11 @@ 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" ); +"qid 0: secure concatenation is not enabled\n" ); ret =NVME_SC_AUTH_REQUIRED |NVME_SC_DNR ; goto out_free_data; }
@@ -678,6 +679,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 } };
@@ -707,6 +709,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)
@@ -1025,6 +1028,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);
@@ -1051,6 +1062,16 @@ 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 && opts->tls) { +pr_err ("Secure concatenation over TLS is not supported\n" ); + ret = -EINVAL ; + goto out; + } +if (opts->concat && opts->tls_key) { +pr_err ("Cannot specify a TLS key for secure concatenation\n" ); + ret = -EINVAL ; + goto out; + } opts->host =nvmf_host_add (hostnqn, &hostid);if (IS_ERR (opts->host)) {
@@ -1306,6 +1327,10 @@ nvmf_create_ctrl(struct device *dev, const char *buf) goto out_module_put; } +/* Reset controller to start TLS */ +if (opts->concat) +nvme_reset_ctrl (ctrl); +module_put (ops->module);return ctrl;
drivers/nvme/host/fabrics.h +3 -0
@@ -73,6 +73,7 @@ enum {NVMF_OPT_TLS =1 <<25 ,NVMF_OPT_KEYRING =1 <<26 ,NVMF_OPT_TLS_KEY =1 <<27 , + NVMF_OPT_CONCAT = 1 << 28, };/**
@@ -108,6 +109,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)
@@ -137,6 +139,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/tcp.c +6 -3
@@ -219,7 +219,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)
@@ -1932,7 +1932,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);
@@ -2615,6 +2615,9 @@ static int nvme_tcp_get_address(struct nvme_ctrl *ctrl, char *buf, int size) len =nvmf_get_address (ctrl, buf, size); +if (ctrl->state !=NVME_CTRL_LIVE ) +return len; +mutex_lock (&queue->queue_lock);if (!test_bit (NVME_TCP_Q_LIVE , &queue->flags))
@@ -2800,7 +2803,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 +7 -0
@@ -1671,6 +1671,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
PATCH v2 11/13 nvme-tcp: combine reset and recovery
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Call 'connect_work' from reset to reduce duplicate code.
--- drivers/nvme/host/tcp.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
drivers/nvme/host/tcp.c +1 -8
@@ -2329,14 +2329,7 @@ static void nvme_reset_ctrl_work(struct work_struct *work)return ; } -if (nvme_tcp_setup_ctrl (ctrl, false)) - goto out_fail; - -return ; - -out_fail: - ++ctrl->nr_reconnects; -nvme_tcp_reconnect_or_remove (ctrl); +queue_delayed_work (nvme_wq, &to_tcp_ctrl (ctrl)->connect_work,0 ); }static void nvme_tcp_stop_ctrl (struct nvme_ctrl *ctrl)
Signature · 1 line
2.35.3
Sagi Grimberg
maintainerRe: [PATCH 11/13] nvme-tcp: combine reset and recovery
Quoted 17 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Call '
connect_work' from reset to reduce duplicate code.Signed-off-by Hannes Reinecke <[email protected]>--- drivers/nvme/host/tcp.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)drivers/nvme/host/tcp.c +0
-0@@ -2329,14 +2329,7 @@ static void nvme_reset_ctrl_work(struct work_struct *work)return ; }
drivers/nvme/host/tcp.c static void nvme_reset_ctrl_work(struct work_struct *work) · line 2333
Quoted 10 lines
drivers/nvme/host/tcp.c +1
-8-if (nvme_tcp_setup_ctrl (ctrl, false)) - goto out_fail; - -return ; - -out_fail: - ++ctrl->nr_reconnects; -nvme_tcp_reconnect_or_remove (ctrl); +queue_delayed_work (nvme_wq, &to_tcp_ctrl (ctrl)->connect_work,0 ); }
This breaks nvme_reset_ctrl_sync
Quoted 2 lines
drivers/nvme/host/tcp.c +0
-0static void nvme_tcp_stop_ctrl (struct nvme_ctrl *ctrl)
Hannes Reinecke
maintainerRe: [PATCH 11/13] nvme-tcp: combine reset and recovery
Quoted 24 lines · Sagi Grimberg
Quoted 21 lines · On 27/01/2024 11:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Call '
connect_work' from reset to reduce duplicate code.Signed-off-by Hannes Reinecke <[email protected]>---
drivers/nvme/host/tcp.c| 9 +--------1 file changed, 1 insertion(+), 8 deletions(-)drivers/nvme/host/tcp.c +0
-4@@ -2329,14 +2329,7 @@ static void nvme_reset_ctrl_work(struct work_struct *work) return; } - if (nvme_tcp_setup_ctrl(ctrl, false)) - goto out_fail; - - return;
Quoted 11 lines
drivers/nvme/host/tcp.c static void nvme_reset_ctrl_work(struct work_struct *work) · line 2333
Quoted 6 lines
drivers/nvme/host/tcp.c +1
-4- -out_fail: - ++ctrl->nr_reconnects; - nvme_tcp_reconnect_or_remove(ctrl); + queue_delayed_work(nvme_wq, &to_tcp_ctrl(ctrl)->connect_work, 0); }This breaks
nvme_reset_ctrl_syncQuoted 1 line
drivers/nvme/host/tcp.c +0
-0static void nvme_tcp_stop_ctrl(struct nvme_ctrl *ctrl)
Okay, let's drop it. I've got another patchset anyway reshuffling the tcp init sequence.
Cheers,
Hannes
PATCH v2 12/13 nvme-tcp: reset after recovery for secure concatenation
In reply to [email protected]From: Hannes Reinecke <[email protected]>
With TP8018 a new key will be generated from the DH-HMAC-CHAP protocol after reset or recovery, but we need to start over to establish a new TLS connection with the new keys.
--- drivers/nvme/host/tcp.c | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+)
drivers/nvme/host/tcp.c +42 -0
@@ -1834,6 +1834,18 @@ static void nvme_tcp_stop_queue(struct nvme_ctrl *nctrl, int qid)if (test_and_clear_bit (NVME_TCP_Q_LIVE , &queue->flags))__nvme_tcp_stop_queue (queue);mutex_unlock (&queue->queue_lock); +/* +* If the TLS key has not been set +* but the queue PSK serial is valid +* we are in reset, and should invalidate +* the PSK serial for this queue to ensure +* TLS won't be started after reset. +*/ +if (nvme_tcp_tls_enabled (queue) && + nctrl->opts->concat && !nctrl->opts->tls_key) { +dev_dbg (nctrl->device,"queue %d clear TLS PSK\n" , qid); + queue->tls_key =0 ; + } }static void nvme_tcp_setup_sock_ops (struct nvme_tcp_queue *queue)
@@ -2263,6 +2275,21 @@ static void nvme_tcp_reconnect_ctrl_work(struct work_struct *work)if (nvme_tcp_setup_ctrl (ctrl, false)) goto requeue; +/* +* Secure concatenation works in two steps; +* the first connection is not encrypted, and +* authentication generates the new TLS PSK. +* Then the connection needs to be reset, +* the TLS needs to be started with the generated +* TLS PSK. +*/ +if (ctrl->opts->concat && ctrl->opts->tls_key && + !nvme_tcp_tls_enabled (&tcp_ctrl->queues[0 ])) { +dev_info (ctrl->device,"Reset to enable TLS with generated PSK\n" ); +nvme_reset_ctrl (ctrl); +return ; + } +dev_info (ctrl->device,"Successfully reconnected (%d attempt)\n" , ctrl->nr_reconnects);
@@ -2284,6 +2311,13 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work)nvme_stop_keep_alive (ctrl);flush_work (&ctrl->async_event_work); +if (ctrl->opts->concat && ctrl->opts->tls_key) { +dev_dbg (ctrl->device,"Wipe generated TLS PSK %0x8\n" , +key_serial (ctrl->opts->tls_key)); +key_revoke (ctrl->opts->tls_key); +key_put (ctrl->opts->tls_key); + ctrl->opts->tls_key =NULL ; + }nvme_tcp_teardown_io_queues (ctrl, false);/* unquiesce to fail fast pending requests */ nvme_unquiesce_io_queues (ctrl);
@@ -2320,6 +2354,14 @@ static void nvme_reset_ctrl_work(struct work_struct *work) container_of(work, struct nvme_ctrl, reset_work);nvme_stop_ctrl (ctrl); +if (ctrl->opts->concat && ctrl->opts->tls_key && +to_tcp_ctrl (ctrl)->queues[0 ].tls_key) { +dev_dbg (ctrl->device,"Wipe generated TLS PSK %0x8\n" , +key_serial (ctrl->opts->tls_key)); +key_revoke (ctrl->opts->tls_key); +key_put (ctrl->opts->tls_key); + ctrl->opts->tls_key =NULL ; + }nvme_tcp_teardown_ctrl (ctrl, false);if (!nvme_change_ctrl_state (ctrl,NVME_CTRL_CONNECTING )) {
Signature · 1 line
2.35.3
PATCH v2 13/13 nvmet-tcp: support secure channel concatenation
In reply to [email protected]From: Hannes Reinecke <[email protected]>
Evaluate the SC_C flag during DH-CHAP-HMAC negotiation and insert the generated PSK once negotiation has finished.
--- drivers/nvme/target/auth.c | 73 +++++++++++++++++++++++++- drivers/nvme/target/fabrics-cmd-auth.c | 46 ++++++++++++++-- drivers/nvme/target/fabrics-cmd.c | 30 ++++++++--- drivers/nvme/target/nvmet.h | 30 ++++++++--- drivers/nvme/target/tcp.c | 27 ++++++++++ 5 files changed, 186 insertions(+), 20 deletions(-)
drivers/nvme/target/auth.c +72 -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"
@@ -124,7 +125,7 @@ int nvmet_setup_dhgroup(struct nvmet_ctrl *ctrl, u8 dhgroup_id)return ret; } -int nvmet_setup_auth (struct nvmet_ctrl *ctrl) +int nvmet_setup_auth (struct nvmet_ctrl *ctrl,struct nvmet_req *req) {int ret =0 ;struct nvmet_host_link *p;
@@ -151,6 +152,11 @@ int nvmet_setup_auth(struct nvmet_ctrl *ctrl) goto out_unlock; } +if (nvmet_queue_is_tls (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" );
@@ -222,6 +228,9 @@ int 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);
@@ -250,6 +259,13 @@ 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_revoke (ctrl->tls_key); +key_put (ctrl->tls_key); + ctrl->tls_key =NULL ; + } +#endif }bool nvmet_check_auth_status (struct nvmet_req *req)
@@ -529,3 +545,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; +#ifdef CONFIG_NVME_TARGET_TCP_TLS +struct key *tls_key =NULL ; +#endif + + psk =nvme_auth_generate_psk (sq->ctrl->shash_id, + sq->dhchap_skey, + sq->dhchap_skey_len, + sq->dhchap_c1, sq->dhchap_c2, + hash_len, &psk_len); +if (IS_ERR (psk)) { +pr_warn ("%s: ctrl %d qid %d failed to generate PSK, error %ld\n" , + __func__, sq->ctrl->cntlid, sq->qid,PTR_ERR (psk)); +return ; + } + digest =nvme_auth_generate_digest (sq->ctrl->shash_id, psk, psk_len, + sq->ctrl->subsysnqn, + sq->ctrl->hostnqn); +if (IS_ERR (digest)) { +pr_warn ("%s: ctrl %d qid %d failed to generate digest, error %ld\n" , + __func__, sq->ctrl->cntlid, sq->qid,PTR_ERR (digest)); + goto out_free_psk; + } + tls_psk =nvme_auth_derive_tls_psk (sq->ctrl->shash_id, psk, psk_len, digest); +if (IS_ERR (tls_psk)) { +pr_warn ("%s: ctrl %d qid %d failed to derive TLS PSK, error %ld\n" , + __func__, sq->ctrl->cntlid, sq->qid,PTR_ERR (tls_psk)); + 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, true, 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_revoke (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 +41 -5
85 lines not on this page · raw message
drivers/nvme/target/fabrics-cmd.c +22 -8
@@ -198,10 +198,23 @@ 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); + +/* +* For secure concatenation the DH-HMAC-CHAP negotiation +* should only run on the admin queue; the I/O queues should +* be using TLS with the generated PSK. +*/ +if (ctrl->concat && req->sq->qid) + needs_auth = false; + +pr_debug ("%s: ctrl %d qid %d should %sauthenticate\n" , + __func__, ctrl->cntlid, req->sq->qid, + needs_auth ?"" :"not " );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)
@@ -255,7 +268,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)uuid_copy (&ctrl->hostid, &d->hostid); - ret =nvmet_setup_auth (ctrl); + ret =nvmet_setup_auth (ctrl, req);if (ret <0 ) {pr_err ("Failed to setup authentication, error %d\n" , ret);nvmet_ctrl_put (ctrl);
@@ -272,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_is_tls (req->sq) ?", TLS" :"" ); + req->cqe->result.u32 =cpu_to_le32 (nvmet_connect_result (ctrl, req)); out:kfree (d); complete:
@@ -336,7 +350,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 +24 -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 +key_serial_t tls_key;#endif struct completion free_done;struct completion confirm_done;
@@ -234,6 +237,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;
@@ -243,6 +247,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 {
@@ -705,13 +712,21 @@ static inline void nvmet_req_bio_put(struct nvmet_req *req, struct bio *bio)bio_put (bio); } +#ifdef NVME_TARGET_TCP_TLS +static inline bool nvmet_queue_is_tls(strucct nvmet_sq *sq) +{ +return sq->tls_key; +} +#else +static inline bool nvmet_queue_is_tls (struct nvmet_sq *sq) {return false; } +#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); -int nvmet_setup_auth (struct nvmet_ctrl *ctrl); +int 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);
@@ -721,16 +736,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_is_tls (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 int nvmet_setup_auth (struct nvmet_ctrl *ctrl) +static inline int nvmet_setup_auth (struct nvmet_ctrl *ctrl, +struct nvmet_req *req) {return 0 ; }
@@ -743,11 +760,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 +27 -0
@@ -1812,6 +1812,33 @@ 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 =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 if (test_bit (KEY_FLAG_REVOKED , &tls_key->flags) || +test_bit (KEY_FLAG_INVALIDATED , &tls_key->flags)) { +pr_warn ("%s: queue %d key %08x invalid\n" , + __func__, queue->idx, peerid); +key_put (tls_key); +spin_lock_bh (&queue->state_lock); + queue->state =NVMET_TCP_Q_FAILED ; +spin_unlock_bh (&queue->state_lock); + status = -EKEYREVOKED ; + }else { +pr_debug ("%s: queue %d using TLS PSK %x\n" , + __func__, queue->idx, peerid); + queue->nvme_sq.tls_key = peerid; +key_put (tls_key); + } + }if (status)nvmet_tcp_schedule_release_queue (queue);else
Signature · 1 line
2.35.3
Hannes Reinecke
maintainerQuoted 24 lines · On 1/27/24 17:30, [email protected] wrote:
From: Hannes Reinecke <[email protected]>
Hi all,
here's my attempt to implement secure concatenation for NVMe-of TCP as outlined in TP8018.
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 I have decided on resetting the connection from the nvme-tcp driver after the initial connection has been set up. Another way would have been to offload the connection reset to userspace, and let nvme-cli reset the connection. But that would be a modification to the userspace interface, and hence I didn't go that way. The drawback with this approach is that we'll create all I/O queues before resetting for TLS, even though these queues should never be used. But fixing that requires a larger rewrite of the TCP driver to unify the setup and reconnect paths. So keep it that way for now.
Quoted 6 lines
As usual, comments and reviews are welcome.
Changes to the original submission:
- Sanitize TLS key handling
- Fixup modconfig compilation
Ping?
Anyone interested in giving feedback?
Cheers,
Hannes
Signature · 5 lines
Dr. Hannes Reinecke Kernel Storage Architect [email protected] +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Ivo Totev, Andrew McDonald, Werner Knoblich