Skip to contentSkip to conversation

nvme: implement secure concatenation

PATCH Superseded Hannes Reinecke 4 participants 14 messages Jul 18, 2024 – Jul 22, 2024

Subsystems (5)
Crypto API NVMe NVMe target fscrypt: file system level encryption support +1 more subsystems

v6 of 15 Superseded Latest is v15 →

lore
Share & tools
lore
author
#1 lore raw

PATCH v6 1/8 crypto,fs: Separate out hkdf_extract() and hkdf_expand()

In reply to Hannes Reinecke

Separate out the HKDF functions into a separate file to make them available to other callers.

Cc Eric Biggers <[email protected]>
Cc [email protected]
Signed-off-by Hannes Reinecke <[email protected]>
---
 crypto/Makefile       |   1 +
 crypto/hkdf.c         | 112 ++++++++++++++++++++++++++++++++++++++++++
 fs/crypto/hkdf.c      |  68 ++++---------------------
 include/crypto/hkdf.h |  18 +++++++
 4 files changed, 140 insertions(+), 59 deletions(-)
 create mode 100644 crypto/hkdf.c
 create mode 100644 include/crypto/hkdf.h
crypto/Makefile +1 -0
index edbbaa3ffef5..b77fc360f0ff 100644
@@ -29,6 +29,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 +112 -0

112 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
new file mode 100644
index 000000000000..bf06c080d7ed
@@ -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
author
#4 lore raw

PATCH v6 2/8 nvme: add nvme_auth_generate_psk()

In reply to Hannes Reinecke

Add a function to generate a NVMe PSK from the shared credentials negotiated by DH-HMAC-CHAP.

Signed-off-by Hannes Reinecke <[email protected]>
Reviewed-by Sagi Grimberg <[email protected]>
---
 drivers/nvme/common/auth.c | 87 ++++++++++++++++++++++++++++++++++++++
 include/linux/nvme-auth.h  |  3 ++
 2 files changed, 90 insertions(+)
drivers/nvme/common/auth.c +87 -0
index a3455f1d67fa..f6d21960b140 100644
@@ -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,91 @@ 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
+ *
+ * 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)).
+ */
+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
index c1d0bc5d9624..b13884b04dfd 100644
@@ -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
author
#5 lore raw

PATCH v6 3/8 nvme: add nvme_auth_generate_digest()

In reply to Hannes Reinecke

Add a function to calculate the PSK digest as specified in TP8018.

Signed-off-by Hannes Reinecke <[email protected]>
Reviewed-by Sagi Grimberg <[email protected]>
---
 drivers/nvme/common/auth.c | 126 +++++++++++++++++++++++++++++++++++++
 include/linux/nvme-auth.h  |   2 +
 2 files changed, 128 insertions(+)
drivers/nvme/common/auth.c +126 -0
index f6d21960b140..7e40f205d3e4 100644
@@ -558,5 +558,131 @@ int nvme_auth_generate_psk(u8 hmac_id, u8 *skey, size_t skey_len,
 }
 EXPORT_SYMBOL_GPL(nvme_auth_generate_psk);
 
+/*
+ * 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.
+ */
+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
index b13884b04dfd..998f06bf10fd 100644
@@ -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
author
#6 lore raw

PATCH v6 4/8 nvme: add nvme_auth_derive_tls_psk()

In reply to Hannes Reinecke

Add a function to derive the TLS PSK as specified TP8018.

Signed-off-by Hannes Reinecke <[email protected]>
Reviewed-by Sagi Grimberg <[email protected]>
---
 drivers/nvme/common/auth.c | 90 ++++++++++++++++++++++++++++++++++++++
 include/linux/nvme-auth.h  |  2 +
 2 files changed, 92 insertions(+)
drivers/nvme/common/auth.c +90 -0
index 7e40f205d3e4..0b000a562c0f 100644
@@ -684,5 +684,95 @@ int nvme_auth_generate_digest(u8 hmac_id, u8 *psk, size_t psk_len,
 }
 EXPORT_SYMBOL_GPL(nvme_auth_generate_digest);
 
+/*
+ * Derive a TLS PSK as specified in TP8018 Section 3.6.1.3:
+ *   TLS PSK and PSK identity Derivation
+ *
+ * The TLS PSK shall be derived as follows from an input PSK
+ * (i.e., either a retained PSK or a generated PSK) and a PSK
+ * identity using the HKDF-Extract and HKDF-Expand-Label operations
+ * (refer to RFC 5869 and RFC 8446) where the hash function is the
+ * one specified by the hash specifier of the PSK identity:
+ * 1. PRK = HKDF-Extract(0, Input PSK); and
+ * 2. TLS PSK = HKDF-Expand-Label(PRK, "nvme-tls-psk", PskIdentityContext, L),
+ * where PskIdentityContext is the hash identifier indicated in
+ * the PSK identity concatenated to a space character and to the
+ * Base64 PSK digest (i.e., "<hash> <PSK digest>") and L is the
+ * output size in bytes of the hash function (i.e., 32 for SHA-256
+ * and 48 for SHA-384).
+ */
+int nvme_auth_derive_tls_psk(int hmac_id, u8 *psk, size_t psk_len,
+		u8 *psk_digest, u8 **ret_psk)
+{
+	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 -EINVAL;
+	}
+	if (hmac_id == NVME_AUTH_HASH_SHA512) {
+		pr_warn("%s: unsupported hash algorithm %s\n",
+			__func__, hmac_name);
+		return -EINVAL;
+	}
+
+	hmac_tfm = crypto_alloc_shash(hmac_name, 0, 0);
+	if (IS_ERR(hmac_tfm))
+		return PTR_ERR(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) {
+		kfree(tls_key);
+		goto out_free_info;
+	}
+	*ret_psk = tls_key;
+
+out_free_info:
+	kfree(info);
+out_free_prk:
+	kfree(prk);
+out_free_shash:
+	crypto_free_shash(hmac_tfm);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_derive_tls_psk);
+
 MODULE_DESCRIPTION("NVMe Authentication framework");
 MODULE_LICENSE("GPL v2");
include/linux/nvme-auth.h +2 -0
index 998f06bf10fd..60e069a6757f 100644
@@ -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
author
#7 lore raw

PATCH v6 5/8 nvme-keyring: add nvme_tls_psk_refresh()

In reply to Hannes Reinecke

Add a function to refresh a generated PSK in the specified keyring.

Signed-off-by Hannes Reinecke <[email protected]>
Reviewed-by Sagi Grimberg <[email protected]>
---
 drivers/nvme/common/keyring.c | 50 +++++++++++++++++++++++++++++++++++
 include/linux/nvme-keyring.h  |  7 +++++
 2 files changed, 57 insertions(+)
drivers/nvme/common/keyring.c +50 -0
index 5b886fd6c3e7..9e847c3ee820 100644
@@ -123,6 +123,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
index e092e3e6ffb2..65400c89bde5 100644
@@ -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);
 struct key *nvme_tls_key_lookup(key_serial_t key_id);
 #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 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

Hannes Reinecke

author
#0 lore raw

v6 cover nvme: implement secure concatenation

Hi all,

here's my attempt to implement secure concatenation for NVMe-of TCP as outlined in TP8018.
The original (v5) patchset had been split in two, and this is the second part based on top of the patchset 'nvme: fixes for secure concatenation' sent earlier to the mailinglist.

Secure concatenation means that a TLS PSK is generated from the key material negotiated by the DH-HMAC-CHAP protocol, and the TLS PSK is then used for a subsequent TLS connection.
The difference between the original definition of secure concatenation and the method outlined in TP8018 is that with TP8018 the connection is reset after DH-HMAC-CHAP negotiation, and a new connection is setup with the generated TLS PSK.

To implement that Sagi came up with the idea to directly reset the admin queue once the DH-CHAP negotiation has completed; that way it will be transparent to the upper layers and we don't have to worry about exposing queues which should not be used.

As usual, comments and reviews are welcome.

Patchset can be found at git.kernel.org:/pub/scm/linux/kernel/git/hare/nvme.git branch secure-concat.v6

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 (8):
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

 crypto/Makefile                        |   1 +
 crypto/hkdf.c                          | 112 +++++++++
 drivers/nvme/common/auth.c             | 303 +++++++++++++++++++++++++
 drivers/nvme/common/keyring.c          |  50 ++++
 drivers/nvme/host/auth.c               | 100 +++++++-
 drivers/nvme/host/fabrics.c            |  34 ++-
 drivers/nvme/host/fabrics.h            |   3 +
 drivers/nvme/host/tcp.c                |  65 +++++-
 drivers/nvme/target/auth.c             |  72 +++++-
 drivers/nvme/target/fabrics-cmd-auth.c |  49 +++-
 drivers/nvme/target/fabrics-cmd.c      |  26 ++-
 drivers/nvme/target/nvmet.h            |  38 +++-
 drivers/nvme/target/tcp.c              |  23 +-
 fs/crypto/hkdf.c                       |  68 +-----
 include/crypto/hkdf.h                  |  18 ++
 include/linux/nvme-auth.h              |   7 +
 include/linux/nvme-keyring.h           |   7 +
 include/linux/nvme.h                   |   7 +
 18 files changed, 889 insertions(+), 94 deletions(-)
 create mode 100644 crypto/hkdf.c
 create mode 100644 include/crypto/hkdf.h
Signature · 1 line
2.35.3
author
#12 lore raw

PATCH v6 7/8 nvme-fabrics: reset admin connection for secure concatenation

When 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.

Signed-off-by Hannes Reinecke <[email protected]>
Reviewed-by Sagi Grimberg <[email protected]>
---
 drivers/nvme/host/tcp.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/nvme/host/tcp.c +9 -0
index b748e4033df2..08ac764ebfb0 100644
@@ -2254,6 +2254,15 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
 	if (ret)
 		return ret;
 
+	if (new && ctrl->opts && ctrl->opts->concat && !ctrl->tls_pskid) {
+		dev_dbg(ctrl->device, "restart admin queue for secure concatenation\n");
+		nvme_stop_keep_alive(ctrl);
+		nvme_tcp_teardown_admin_queue(ctrl, false);
+		ret = nvme_tcp_configure_admin_queue(ctrl, false);
+		if (ret)
+			return ret;
+	}
+
 	if (ctrl->icdoff) {
 		ret = -EOPNOTSUPP;
 		dev_err(ctrl->device, "icdoff is not supported!\n");
Signature · 1 line
2.35.3
author
#8 lore raw

PATCH v6 6/8 nvme-tcp: request secure channel concatenation

In reply to Hannes Reinecke

Add a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset.

Signed-off-by Hannes Reinecke <[email protected]>
---
 drivers/nvme/host/auth.c    | 100 ++++++++++++++++++++++++++++++++++--
 drivers/nvme/host/fabrics.c |  34 ++++++++++--
 drivers/nvme/host/fabrics.h |   3 ++
 drivers/nvme/host/tcp.c     |  56 +++++++++++++++++---
 include/linux/nvme.h        |   7 +++
 5 files changed, 188 insertions(+), 12 deletions(-)
drivers/nvme/host/auth.c +97 -3
index 371e14f0a203..5f71aa7b5e27 100644
@@ -12,6 +12,7 @@
 #include "nvme.h"
 #include "fabrics.h"
 #include <linux/nvme-auth.h>
+#include <linux/nvme-keyring.h>
 
 #define CHAP_BUF_SIZE 4096
 static 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,76 @@ 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;
+	}
+
+	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,
+				       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 =

@@ -833,6 +913,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 +1000,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
index 432efcbf9e2f..93e9041b9657 100644
@@ -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
index 21d75dc4a3a0..9cf5b020adba 100644
@@ -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/tcp.c +50 -6

98 lines not on this page · raw message

include/linux/nvme.h +7 -0
index 5c2290f0d5af..5626894d7700 100644
@@ -1676,6 +1676,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
author
#13 lore raw

PATCH v6 8/8 nvmet-tcp: support secure channel concatenation

In reply to Hannes Reinecke

Evaluate the SC_C flag during DH-CHAP-HMAC negotiation and insert the generated PSK once negotiation has finished.

Signed-off-by Hannes Reinecke <[email protected]>
---
 drivers/nvme/target/auth.c             | 72 +++++++++++++++++++++++++-
 drivers/nvme/target/fabrics-cmd-auth.c | 49 +++++++++++++++---
 drivers/nvme/target/fabrics-cmd.c      | 26 +++++++---
 drivers/nvme/target/nvmet.h            | 38 +++++++++++---
 drivers/nvme/target/tcp.c              | 23 +++++++-
 5 files changed, 185 insertions(+), 23 deletions(-)
drivers/nvme/target/auth.c +71 -1
index 7897d02c681d..560321df5bf6 100644
@@ -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, 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_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 +18 -8
index 344ab75d8864..1aabf55ef0a8 100644
@@ -199,10 +199,19 @@ 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);
+
+	/* Do not authenticate I/O queues for secure concatenation */
+	if (ctrl->concat && req->sq->qid)
+		needs_auth = false;
+
+	pr_debug("%s: ctrl %d qid %d should %sauthenticate, tls psk %08x\n",
+		 __func__, ctrl->cntlid, req->sq->qid,
+		 needs_auth ? "" : "not ", nvmet_queue_tls_keyid(req->sq));
 	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)

@@ -254,7 +263,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);

@@ -272,12 +281,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:

@@ -336,7 +346,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
index 190f55e6d753..c2e17201c757 100644
@@ -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
index 5bff0d5464d1..49849f028966 100644
@@ -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
maintainer
#2 lore raw

Re: [PATCH 1/8] crypto,fs: Separate out hkdf_extract() and hkdf_expand()

In reply to Hannes Reinecke
Quoted 6 lines · Hannes Reinecke

Separate out the HKDF functions into a separate file to make them available to other callers.

Cc Eric Biggers <[email protected]>
Cc [email protected]
Signed-off-by Hannes Reinecke <[email protected]>
Quoted 8 lines
---
 crypto/Makefile       |   1 +
 crypto/hkdf.c         | 112 ++++++++++++++++++++++++++++++++++++++++++
 fs/crypto/hkdf.c      |  68 ++++---------------------
 include/crypto/hkdf.h |  18 +++++++
 4 files changed, 140 insertions(+), 59 deletions(-)
 create mode 100644 crypto/hkdf.c
 create mode 100644 include/crypto/hkdf.h

I was only sent patch 1, so this is unreviewable as there is no context.

- Eric

maintainer
#3 lore raw

Re: [PATCH 1/8] crypto,fs: Separate out hkdf_extract() and hkdf_expand()

Quoted 16 lines · Eric Biggers
Quoted 6 lines · Hannes Reinecke

Separate out the HKDF functions into a separate file to make them available to other callers.

Cc Eric Biggers <[email protected]>
Cc [email protected]
Signed-off-by Hannes Reinecke <[email protected]>
Quoted 8 lines
---
  crypto/Makefile       |   1 +
  crypto/hkdf.c         | 112 ++++++++++++++++++++++++++++++++++++++++++
  fs/crypto/hkdf.c      |  68 ++++---------------------
  include/crypto/hkdf.h |  18 +++++++
  4 files changed, 140 insertions(+), 59 deletions(-)

create mode 100644 crypto/hkdf.c
create mode 100644 include/crypto/hkdf.h

Quoted 2 lines

I was only sent patch 1, so this is unreviewable as there is no context.

Sorry. Will include you with the full patchset for the next round. The remainder of the patch series is just calling into the just exported functions to derive TLS PSK keys, wasn't sure if you'd be interested in that.

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
maintainer
#9 lore raw

Re: [PATCH 6/8] nvme-tcp: request secure channel concatenation

In reply to Hannes Reinecke
Quoted 118 lines · Hannes Reinecke

Add a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset.

Signed-off-by Hannes Reinecke <[email protected]>
---
  drivers/nvme/host/auth.c    | 100 ++++++++++++++++++++++++++++++++++--
  drivers/nvme/host/fabrics.c |  34 ++++++++++--
  drivers/nvme/host/fabrics.h |   3 ++
  drivers/nvme/host/tcp.c     |  56 +++++++++++++++++---
  include/linux/nvme.h        |   7 +++
  5 files changed, 188 insertions(+), 12 deletions(-)
drivers/nvme/host/auth.c +65 -3
index 371e14f0a203..5f71aa7b5e27 100644
@@ -12,6 +12,7 @@
  #include "nvme.h"
  #include "fabrics.h"
  #include <linux/nvme-auth.h>
+#include <linux/nvme-keyring.h>
 
  #define CHAP_BUF_SIZE 4096
  static 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,76 @@ 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;
+	}
+
+	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,
+				       true, tls_psk, psk_len, digest);
+	if (IS_ERR(tls_key)) {
+		ret = PTR_ERR(tls_key);

drivers/nvme/host/auth.c static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap) · line 751

Quoted 10 lines
drivers/nvme/host/auth.c +10 -0
+		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);
+	}

Any reason why error in nvme_tls_psk_refres() does not goto the normal error handling? Is it acceptable to fail here? Maybe I'm missing something...

Quoted 100 lines
drivers/nvme/host/auth.c +22 -0
+	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 +913,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 +1000,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 +15 -3
index 432efcbf9e2f..93e9041b9657 100644
@@ -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) {

drivers/nvme/host/fabrics.c static int nvmf_parse_options(struct nvmf_ctrl_options *opts, · line 1103

Quoted 10 lines
drivers/nvme/host/fabrics.c +10 -0
+		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;
+		}

I think you mean it is unsupported?

Quoted 71 lines
drivers/nvme/host/fabrics.c +6 -0
+		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
index 21d75dc4a3a0..9cf5b020adba 100644
@@ -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/tcp.c +7 -5
index 5885aa452aa1..b748e4033df2 100644
@@ -227,7 +227,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)

@@ -1942,7 +1942,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);

@@ -1972,9 +1972,25 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl)
  {
  	int i, ret;
 
-	if (nvme_tcp_tls_configured(ctrl) && !ctrl->tls_pskid) {
-		dev_err(ctrl->device, "no PSK negotiated\n");
-		return -ENOKEY;
+	if (nvme_tcp_tls_configured(ctrl)) {
+		if (ctrl->opts->concat) {
+			/*
+			 * The generated PSK is stored in the
+			 * fabric options

drivers/nvme/host/tcp.c static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) · line 1989

Quoted 10 lines
drivers/nvme/host/tcp.c +10 -0
+			 */
+			if (!ctrl->opts->tls_key) {
+				dev_err(ctrl->device, "no PSK generated\n");
+				return -ENOKEY;
+			}
+			if (ctrl->tls_pskid &&
+			    ctrl->tls_pskid != key_serial(ctrl->opts->tls_key)) {
+				dev_err(ctrl->device, "Stale PSK id %08x\n", ctrl->tls_pskid);
+				ctrl->tls_pskid = 0;
+			}

What is the user supposed to do with a stale psk? meaning why print an error and continue as usual?

Quoted 24 lines
drivers/nvme/host/tcp.c +17 -0
+		} else if (!ctrl->tls_pskid) {
+			dev_err(ctrl->device, "no PSK negotiated\n");
+			return -ENOKEY;
+		}
  	}
 
  	for (i = 1; i < ctrl->queue_count; i++) {

@@ -2205,6 +2221,30 @@ static void nvme_tcp_reconnect_or_remove(struct nvme_ctrl *ctrl,
  	}
  }
 
+/*
+ * The TLS key needs to be revoked when:
+ * - concatenation is enabled and
+ *   -> This is a generated key and only valid for this session
+ * - the generated key is present in ctrl->tls_key and
+ *   -> authentication has completed and the key has been generated
+ * - tls has been enabled
+ *   -> otherwise we are about to reset the admin queue after authentication
+ *      to enable TLS with the generated key
+ */
+static bool nvme_tcp_key_revoke_needed(struct nvme_ctrl *ctrl)
+{
+	return ctrl->opts->concat && ctrl->opts->tls_key && ctrl->tls_pskid;

drivers/nvme/host/tcp.c static void nvme_tcp_reconnect_or_remove(struct nvme_ctrl *ctrl, · line 2246

Quoted 10 lines
drivers/nvme/host/tcp.c +10 -0
+}
+
+static void nvme_tcp_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;
+}

I think this belongs in a common place no?

drivers/nvme/host/tcp.c static void nvme_tcp_error_recovery_work(struct work_struct *work) · line 2352

Quoted 10 lines
drivers/nvme/host/tcp.c +3 -0
+
  static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
  {
  	struct nvmf_ctrl_options *opts = ctrl->opts;

@@ -2308,6 +2348,8 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work)
  				struct nvme_tcp_ctrl, err_work);
  	struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl;
 
+	if (nvme_tcp_key_revoke_needed(ctrl))
+		nvme_tcp_revoke_tls_key(ctrl);

Should this maybe move to nvme_tcp_teardown_admin_qeueue ?

drivers/nvme/host/tcp.c static void nvme_reset_ctrl_work(struct work_struct *work) · line 2394

Quoted 9 lines
drivers/nvme/host/tcp.c +2 -0
  	nvme_stop_keep_alive(ctrl);
  	flush_work(&ctrl->async_event_work);
  	nvme_tcp_teardown_io_queues(ctrl, false);

@@ -2348,6 +2390,8 @@ static void nvme_reset_ctrl_work(struct work_struct *work)
  		container_of(work, struct nvme_ctrl, reset_work);
  	int ret;
 
+	if (nvme_tcp_key_revoke_needed(ctrl))
+		nvme_tcp_revoke_tls_key(ctrl);

Would eliminate this call...

maintainer
#10 lore raw

Re: [PATCH 6/8] nvme-tcp: request secure channel concatenation

Quoted 130 lines · Sagi Grimberg
Quoted 126 lines · Hannes Reinecke

Add a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset.

Signed-off-by Hannes Reinecke <[email protected]>

---
  drivers/nvme/host/auth.c    | 100 ++++++++++++++++++++++++++++++++++-- drivers/nvme/host/fabrics.c |  34 ++++++++++-- drivers/nvme/host/fabrics.h |   3 ++ drivers/nvme/host/tcp.c     |  56 +++++++++++++++++--- include/linux/nvme.h        |   7 +++

  5 files changed, 188 insertions(+), 12 deletions(-)
drivers/nvme/host/auth.c +7 -1
index 371e14f0a203..5f71aa7b5e27 100644
@@ -12,6 +12,7 @@
   #include "nvme.h"
   #include "fabrics.h"
   #include <linux/nvme-auth.h>
+#include <linux/nvme-keyring.h>
   #define CHAP_BUF_SIZE 4096
   static 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;

drivers/nvme/host/auth.c +2 -1
@@ -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,

drivers/nvme/host/auth.c +4 -1
@@ -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",

drivers/nvme/host/auth.c +56 -0
@@ -677,6 +687,76 @@ 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;
+    }
+
+    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,
+                       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;
Quoted 8 lines

drivers/nvme/host/auth.c static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap) · line 751

Quoted 6 lines
drivers/nvme/host/auth.c +6 -0
+        kfree_sensitive(tls_psk);
+    }
+    if (ctrl->opts->tls_key) {
+        key_revoke(ctrl->opts->tls_key);
+        key_put(ctrl->opts->tls_key);
+    }

Any reason why error in nvme_tls_psk_refres() does not goto the normal error handling? Is it acceptable to fail here? Maybe I'm missing something...

Hmm? But we do; we are setting 'ret' to a non-zero value, and then trigger the normal error handling ...
Or did you mean something else?

Quoted 110 lines
Quoted 110 lines
drivers/nvme/host/auth.c +8 -0
+    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 =

drivers/nvme/host/auth.c +1 -0
@@ -833,6 +913,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;
      }

drivers/nvme/host/auth.c +6 -0
@@ -912,6 +1000,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 +4 -3
index 432efcbf9e2f..93e9041b9657 100644
@@ -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;
          }

drivers/nvme/host/fabrics.c +10 -0
@@ -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);

drivers/nvme/host/fabrics.c +6 -0
@@ -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;
+        }
Quoted 9 lines

drivers/nvme/host/fabrics.c static int nvmf_parse_options(struct nvmf_ctrl_options *opts, · line 1103

Quoted 6 lines
drivers/nvme/host/fabrics.c +5 -0
+        if (opts->tls_key) {
+            pr_err("Cannot specify a TLS key for secure 
 concatenation\n");
+            ret = -EINVAL;
+            goto out;
+        }

I think you mean it is unsupported?

Of course.

Quoted 74 lines
Quoted 74 lines
drivers/nvme/host/fabrics.c +6 -0
+        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
index 21d75dc4a3a0..9cf5b020adba 100644
@@ -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/tcp.c +2 -2
index 5885aa452aa1..b748e4033df2 100644
@@ -227,7 +227,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)

@@ -1942,7 +1942,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);

drivers/nvme/host/tcp.c +10 -3
@@ -1972,9 +1972,25 @@ static int __nvme_tcp_alloc_io_queues(struct 
 nvme_ctrl *ctrl)
   {
       int i, ret;
-    if (nvme_tcp_tls_configured(ctrl) && !ctrl->tls_pskid) {
-        dev_err(ctrl->device, "no PSK negotiated\n");
-        return -ENOKEY;
+    if (nvme_tcp_tls_configured(ctrl)) {
+        if (ctrl->opts->concat) {
+            /*
+             * The generated PSK is stored in the
+             * fabric options
+             */
+            if (!ctrl->opts->tls_key) {
+                dev_err(ctrl->device, "no PSK generated\n");
+                return -ENOKEY;
+            }
Quoted 9 lines

drivers/nvme/host/tcp.c static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl) · line 1989

Quoted 6 lines
drivers/nvme/host/tcp.c +5 -0
+            if (ctrl->tls_pskid &&
+                ctrl->tls_pskid != key_serial(ctrl->opts->tls_key)) {
+                dev_err(ctrl->device, "Stale PSK id %08x\n", 
 ctrl->tls_pskid);
+                ctrl->tls_pskid = 0;
+            }

What is the user supposed to do with a stale psk? meaning why print an error and continue as usual?

Technically TLS handshake is independent from secure concatenation.
While we do generate a new TLS PSK from secure concatenation, there is
no guarantee that this TLS PSK is then selected from the TLS handshake.
So theoretically it's possible that the TLS handshake arrives at a different TLS PSK than the generated one. But that is not actually an error, as both steps (authentication and
TLS handshake) have been successful. However, it's definitely an odd
situation, and we should tell the user about it.

Quoted 28 lines
Quoted 28 lines
drivers/nvme/host/tcp.c +12 -0
+        } else if (!ctrl->tls_pskid) {
+            dev_err(ctrl->device, "no PSK negotiated\n");
+            return -ENOKEY;
+        }
       }
       for (i = 1; i < ctrl->queue_count; i++) {

@@ -2205,6 +2221,30 @@ static void nvme_tcp_reconnect_or_remove(struct 
 nvme_ctrl *ctrl,
       }
   }
+/*
+ * The TLS key needs to be revoked when:
+ * - concatenation is enabled and
+ *   -> This is a generated key and only valid for this session
+ * - the generated key is present in ctrl->tls_key and
+ *   -> authentication has completed and the key has been generated
+ * - tls has been enabled
+ *   -> otherwise we are about to reset the admin queue after

authentication
+ *      to enable TLS with the generated key
+ */

drivers/nvme/host/tcp.c +7 -0
+static bool nvme_tcp_key_revoke_needed(struct nvme_ctrl *ctrl)
+{
+    return ctrl->opts->concat && ctrl->opts->tls_key && ctrl->tls_pskid;
+}
+
+static void nvme_tcp_revoke_tls_key(struct nvme_ctrl *ctrl)
+{
Quoted 9 lines
Quoted 6 lines

+    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;
+}

I think this belongs in a common place no?

?
Where?
'ctrl-opts' is pretty much initiator specific, so I am not sure where it should go ...

Quoted 4 lines
Quoted 4 lines

+
  static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new) {
      struct nvmf_ctrl_options *opts = ctrl->opts;

Quoted 9 lines

drivers/nvme/host/tcp.c static void nvme_tcp_error_recovery_work(struct · line 2352

Quoted 6 lines
drivers/nvme/host/tcp.c +2 -0
@@ -2308,6 +2348,8 @@ static void nvme_tcp_error_recovery_work(struct 
 work_struct *work)
                   struct nvme_tcp_ctrl, err_work);
       struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl;
+    if (nvme_tcp_key_revoke_needed(ctrl))
+        nvme_tcp_revoke_tls_key(ctrl);

Should this maybe move to nvme_tcp_teardown_admin_qeueue ?

Good point, will do.

Quoted 11 lines

drivers/nvme/host/tcp.c static void nvme_reset_ctrl_work(struct · line 2394

Quoted 9 lines
drivers/nvme/host/tcp.c +2 -0
       nvme_stop_keep_alive(ctrl);
       flush_work(&ctrl->async_event_work);
       nvme_tcp_teardown_io_queues(ctrl, false);

@@ -2348,6 +2390,8 @@ static void nvme_reset_ctrl_work(struct 
 work_struct *work)
           container_of(work, struct nvme_ctrl, reset_work);
       int ret;
+    if (nvme_tcp_key_revoke_needed(ctrl))
+        nvme_tcp_revoke_tls_key(ctrl);

Would eliminate this call...

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
maintainer
#11 lore raw

Re: [PATCH 6/8] nvme-tcp: request secure channel concatenation

Quoted 18 lines · Hannes Reinecke
Quoted 17 lines · Sagi Grimberg
Quoted 13 lines · Hannes Reinecke

Add a fabrics option 'concat' to request secure channel concatenation. When secure channel concatenation is enabled a 'generated PSK' is inserted into the keyring such that it's available after reset.

Signed-off-by Hannes Reinecke <[email protected]>

---
  drivers/nvme/host/auth.c    | 100 ++++++++++++++++++++++++++++++++++-- drivers/nvme/host/fabrics.c |  34 ++++++++++-- drivers/nvme/host/fabrics.h |   3 ++ drivers/nvme/host/tcp.c     |  56 +++++++++++++++++--- include/linux/nvme.h        |   7 +++

  5 files changed, 188 insertions(+), 12 deletions(-)

[ .. ]

Quoted 14 lines
Quoted 4 lines
Quoted 4 lines

+
  static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new) {
      struct nvmf_ctrl_options *opts = ctrl->opts;

Quoted 9 lines

drivers/nvme/host/tcp.c static void nvme_tcp_error_recovery_work(struct · line 2352

Quoted 6 lines
drivers/nvme/host/tcp.c +2 -0
@@ -2308,6 +2348,8 @@ static void nvme_tcp_error_recovery_work(struct 
 work_struct *work)
                   struct nvme_tcp_ctrl, err_work);
       struct nvme_ctrl *ctrl = &tcp_ctrl->ctrl;
+    if (nvme_tcp_key_revoke_needed(ctrl))
+        nvme_tcp_revoke_tls_key(ctrl);

Should this maybe move to nvme_tcp_teardown_admin_qeueue ?

Quoted 2 lines

Good point, will do.

Actually, no.

That's precisely where we _cannot_ do it.
(Cf patch to reset the admin queue for secure concatenation). We have three stages during secure concatenation:
1. authentication:
ctrl->opts->concat = true, ctrl->opts->tls_key = NULL queue->tls_enabled = false
2. reset after authentication:
ctrl->opts->concat = true, ctrl->opts->tls_key = <set> queue->tls_enabled = false
3. TLS enabled on the queue:
ctrl->opts->concat = true, ctrl->opts->tls_key = <set> queue->tls_enabled = true

As we are calling 'nvme_teardown_admin_queue' in step 2) we cannot clear the tls_key in that function.

Cheers,

Hannes

Quoted 15 lines
Quoted 11 lines

drivers/nvme/host/tcp.c static void nvme_reset_ctrl_work(struct · line 2394

Quoted 9 lines
drivers/nvme/host/tcp.c +2 -0
       nvme_stop_keep_alive(ctrl);
       flush_work(&ctrl->async_event_work);
       nvme_tcp_teardown_io_queues(ctrl, false);

@@ -2348,6 +2390,8 @@ static void nvme_reset_ctrl_work(struct 
 work_struct *work)
           container_of(work, struct nvme_ctrl, reset_work);
       int ret;
+    if (nvme_tcp_key_revoke_needed(ctrl))
+        nvme_tcp_revoke_tls_key(ctrl);

Would eliminate this call...

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