nvme: In-band authentication support

Add helper functions to generaten Finite Field DH Ephemeral Parameters as specified in RFC 7919.

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

Reviewed-by Sagi Grimberg <[email protected]>

Reviewed-by Himanshu Madhani <[email protected]>

Add new definitions for NVMe In-band authentication as defined in the NVMe Base Specification v2.0.

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

Reviewed-by Sagi Grimberg <[email protected]>

Reviewed-by Himanshu Madhani <[email protected]>

---

+int nvme_auth_generate_key(struct nvme_ctrl *ctrl, u8 *secret, bool set_ctrl)

drivers/nvme/host/auth.c new file +19 -0

+{
+	u8 *key;
+	size_t key_len;
+	u8 key_hash;
+
+	if (!secret)
+		return 0;
+
+	if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
+		return -EINVAL;
+
+	/* Pass in the secret without the 'DHHC-1:XX:' prefix */
+	key = nvme_auth_extract_secret(secret + 10, key_hash,
+				       &key_len);
+	if (IS_ERR(key)) {
+		dev_dbg(ctrl->device, "failed to extract key, error %ld\n",
+			PTR_ERR(key));
+		return PTR_ERR(key);
+	}

Quoted 3 lines

+int nvme_auth_generate_key(struct nvme_ctrl *ctrl, u8 *secret, bool set_ctrl)

Maybe instead of set_ctrl introduct struct dhchap_key and pass a pointer into that?

Quoted 26 lines

drivers/nvme/host/auth.c new file +26 -0

+{
+    u8 *key;
+    size_t key_len;
+    u8 key_hash;
+
+    if (!secret)
+        return 0;
+
+    if (sscanf(secret, "DHHC-1:%hhd:%*s:", &key_hash) != 1)
+        return -EINVAL;
+
+    /* Pass in the secret without the 'DHHC-1:XX:' prefix */
+    key = nvme_auth_extract_secret(secret + 10, key_hash,
+                       &key_len);
+    if (IS_ERR(key)) {
+        dev_dbg(ctrl->device, "failed to extract key, error %ld\n",
+            PTR_ERR(key));
+        return PTR_ERR(key);
+    }
+
+    if (set_ctrl) {
+        ctrl->dhchap_ctrl_key = key;
+        ctrl->dhchap_ctrl_key_len = key_len;
+        ctrl->dhchap_ctrl_key_hash = key_hash;
+    } else {
+        ctrl->dhchap_key = key;

Quoted 3 lines

+        ctrl->dhchap_key_len = key_len;
+        ctrl->dhchap_key_hash = key_hash;
+    }

Then it becomes:
    dhchap_key->key = key;
    dhchap_key->len = key_len;
    dhchap_key->hash = key_hash;

Implement NVMe-oF In-Band authentication according to NVMe TPAR 8006. This patch adds two new fabric options 'dhchap_secret' to specify the pre-shared key (in ASCII respresentation according to NVMe 2.0 section 8.13.5.8 'Secret representation') and 'dhchap_ctrl_secret' to specify the pre-shared controller key for bi-directional authentication of both the host and the controller.
Re-authentication can be triggered by writing the PSK into the new controller sysfs attribute 'dhchap_secret' or 'dhchap_ctrl_secret'.

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

---
  drivers/nvme/host/Kconfig   |   11 +
  drivers/nvme/host/Makefile  |    1 +
  drivers/nvme/host/auth.c    | 1139 +++++++++++++++++++++++++++++++++++
  drivers/nvme/host/auth.h    |   26 +
  drivers/nvme/host/core.c    |  141 ++++-
  drivers/nvme/host/fabrics.c |   79 ++-
  drivers/nvme/host/fabrics.h |    7 +
  drivers/nvme/host/nvme.h    |   35 ++
  drivers/nvme/host/tcp.c     |    1 +
  drivers/nvme/host/trace.c   |   32 +
  10 files changed, 1465 insertions(+), 7 deletions(-)

create mode 100644 drivers/nvme/host/auth.c
create mode 100644 drivers/nvme/host/auth.h

drivers/nvme/host/Kconfig +11 -0

index dc0450ca23a3..49269c581ec4 100644
@@ -83,3 +83,14 @@ config NVME_TCP
  	  from https://github.com/linux-nvme/nvme-cli.
 
  	  If unsure, say N.
+
+config NVME_AUTH
+	bool "NVM Express over Fabrics In-Band Authentication"
+	depends on NVME_CORE
+	select CRYPTO_HMAC
+	select CRYPTO_SHA256
+	select CRYPTO_SHA512
+	help
+	  This provides support for NVMe over Fabrics In-Band Authentication.
+
+	  If unsure, say N.

drivers/nvme/host/Makefile +1 -0

index dfaacd472e5d..4bae2a4a8d8c 100644
@@ -15,6 +15,7 @@ nvme-core-$(CONFIG_NVME_MULTIPATH)	+= multipath.o
  nvme-core-$(CONFIG_BLK_DEV_ZONED)	+= zns.o
  nvme-core-$(CONFIG_FAULT_INJECTION_DEBUG_FS)	+= fault_inject.o
  nvme-core-$(CONFIG_NVME_HWMON)		+= hwmon.o
+nvme-core-$(CONFIG_NVME_AUTH)		+= auth.o
 
  nvme-y					+= pci.o
 

drivers/nvme/host/auth.c new file +108 -0

new file mode 100644
index 000000000000..f74ab4d8b990
@@ -0,0 +1,1139 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2020 Hannes Reinecke, SUSE Linux
+ */
+
+#include <linux/crc32.h>
+#include <linux/base64.h>
+#include <asm/unaligned.h>
+#include <crypto/hash.h>
+#include <crypto/dh.h>
+#include <crypto/ffdhe.h>
+#include "nvme.h"
+#include "fabrics.h"
+#include "auth.h"
+
+static atomic_t nvme_dhchap_seqnum = ATOMIC_INIT(0);
+
+struct nvme_dhchap_queue_context {
+	struct list_head entry;
+	struct work_struct auth_work;
+	struct nvme_ctrl *ctrl;
+	struct crypto_shash *shash_tfm;
+	void *buf;
+	size_t buf_size;
+	int qid;
+	int error;
+	u32 s1;
+	u32 s2;
+	u16 transaction;
+	u8 status;
+	u8 hash_id;
+	u8 hash_len;
+	u8 dhgroup_id;
+	u8 c1[64];
+	u8 c2[64];
+	u8 response[64];
+	u8 *host_response;
+};
+
+static struct nvme_auth_dhgroup_map {
+	int id;
+	const char name[16];
+	const char kpp[16];
+	int privkey_size;
+	int pubkey_size;
+} dhgroup_map[] = {
+	{ .id = NVME_AUTH_DHCHAP_DHGROUP_NULL,
+	  .name = "null", .kpp = "null",
+	  .privkey_size = 0, .pubkey_size = 0 },
+	{ .id = NVME_AUTH_DHCHAP_DHGROUP_2048,
+	  .name = "ffdhe2048", .kpp = "dh",
+	  .privkey_size = 256, .pubkey_size = 256 },
+	{ .id = NVME_AUTH_DHCHAP_DHGROUP_3072,
+	  .name = "ffdhe3072", .kpp = "dh",
+	  .privkey_size = 384, .pubkey_size = 384 },
+	{ .id = NVME_AUTH_DHCHAP_DHGROUP_4096,
+	  .name = "ffdhe4096", .kpp = "dh",
+	  .privkey_size = 512, .pubkey_size = 512 },
+	{ .id = NVME_AUTH_DHCHAP_DHGROUP_6144,
+	  .name = "ffdhe6144", .kpp = "dh",
+	  .privkey_size = 768, .pubkey_size = 768 },
+	{ .id = NVME_AUTH_DHCHAP_DHGROUP_8192,
+	  .name = "ffdhe8192", .kpp = "dh",
+	  .privkey_size = 1024, .pubkey_size = 1024 },
+};
+
+const char *nvme_auth_dhgroup_name(int dhgroup_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dhgroup_map); i++) {
+		if (dhgroup_map[i].id == dhgroup_id)
+			return dhgroup_map[i].name;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_name);
+
+int nvme_auth_dhgroup_pubkey_size(int dhgroup_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dhgroup_map); i++) {
+		if (dhgroup_map[i].id == dhgroup_id)
+			return dhgroup_map[i].pubkey_size;
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_pubkey_size);
+
+int nvme_auth_dhgroup_privkey_size(int dhgroup_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dhgroup_map); i++) {
+		if (dhgroup_map[i].id == dhgroup_id)
+			return dhgroup_map[i].privkey_size;
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_privkey_size);
+
+const char *nvme_auth_dhgroup_kpp(int dhgroup_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(dhgroup_map); i++) {
+		if (dhgroup_map[i].id == dhgroup_id)

drivers/nvme/host/auth.c new file +70 -0

+	for (i = 0; i < ARRAY_SIZE(dhgroup_map); i++) {
+		if (!strncmp(dhgroup_map[i].name, dhgroup_name,
+			     strlen(dhgroup_map[i].name)))
+			return dhgroup_map[i].id;
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_dhgroup_id);
+
+static struct nvme_dhchap_hash_map {
+	int id;
+	int len;
+	const char hmac[15];
+	const char digest[15];
+} hash_map[] = {
+	{.id = NVME_AUTH_DHCHAP_SHA256, .len = 32,
+	 .hmac = "hmac(sha256)", .digest = "sha256" },
+	{.id = NVME_AUTH_DHCHAP_SHA384, .len = 48,
+	 .hmac = "hmac(sha384)", .digest = "sha384" },
+	{.id = NVME_AUTH_DHCHAP_SHA512, .len = 64,
+	 .hmac = "hmac(sha512)", .digest = "sha512" },
+};
+
+const char *nvme_auth_hmac_name(int hmac_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hash_map); i++) {
+		if (hash_map[i].id == hmac_id)
+			return hash_map[i].hmac;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_hmac_name);
+
+const char *nvme_auth_digest_name(int hmac_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hash_map); i++) {
+		if (hash_map[i].id == hmac_id)
+			return hash_map[i].digest;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_digest_name);
+
+int nvme_auth_hmac_id(const char *hmac_name)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hash_map); i++) {
+		if (!strncmp(hash_map[i].hmac, hmac_name,
+			     strlen(hash_map[i].hmac)))
+			return hash_map[i].id;
+	}
+	return -1;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_hmac_id);
+
+int nvme_auth_hmac_hash_len(int hmac_id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(hash_map); i++) {
+		if (hash_map[i].id == hmac_id)
+			return hash_map[i].len;
+	}
+	return 0;
+}

drivers/nvme/host/auth.c new file +5 -0

+	/* Secret might be affixed with a ':' */
+	p = strrchr(secret, ':');
+	if (p)
+		allocated_len = p - secret;
+	key = kzalloc(allocated_len, GFP_KERNEL);

drivers/nvme/host/auth.c new file +125 -0

+	    key_len != 68) {
+		pr_err("Invalid DH-HMAC-CHAP key len %d\n",
+		       key_len);
+		kfree_sensitive(key);
+		return ERR_PTR(-EINVAL);
+	}
+	if (key_hash > 0 &&
+	    (key_len - 4) != nvme_auth_hmac_hash_len(key_hash)) {
+		pr_err("Invalid DH-HMAC-CHAP key len %d for %s\n", key_len,
+		       nvme_auth_hmac_name(key_hash));
+		kfree_sensitive(key);
+		return ERR_PTR(-EINVAL);
+	}
+
+	/* The last four bytes is the CRC in little-endian format */
+	key_len -= 4;
+	/*
+	 * The linux implementation doesn't do pre- and post-increments,
+	 * so we have to do it manually.
+	 */
+	crc = ~crc32(~0, key, key_len);
+
+	if (get_unaligned_le32(key + key_len) != crc) {
+		pr_err("DH-HMAC-CHAP key crc mismatch (key %08x, crc %08x)\n",
+		       get_unaligned_le32(key + key_len), crc);
+		kfree_sensitive(key);
+		return ERR_PTR(-EKEYREJECTED);
+	}
+	*out_len = key_len;
+	return key;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_extract_secret);
+
+u8 *nvme_auth_transform_key(u8 *key, size_t key_len, u8 key_hash, char *nqn)
+{
+	const char *hmac_name = nvme_auth_hmac_name(key_hash);
+	struct crypto_shash *key_tfm;
+	struct shash_desc *shash;
+	u8 *transformed_key;
+	int ret;
+
+	if (key_hash == 0) {
+		transformed_key = kmemdup(key, key_len, GFP_KERNEL);
+		return transformed_key ? transformed_key : ERR_PTR(-ENOMEM);
+	}
+
+	if (!key || !key_len) {
+		pr_warn("No key specified\n");
+		return ERR_PTR(-ENOKEY);
+	}
+	if (!hmac_name) {
+		pr_warn("Invalid key hash id %d\n", key_hash);
+		return ERR_PTR(-EINVAL);
+	}
+
+	key_tfm = crypto_alloc_shash(hmac_name, 0, 0);
+	if (IS_ERR(key_tfm))
+		return (u8 *)key_tfm;
+
+	shash = kmalloc(sizeof(struct shash_desc) +
+			crypto_shash_descsize(key_tfm),
+			GFP_KERNEL);
+	if (!shash) {
+		ret = -ENOMEM;
+		goto out_free_key;
+	}
+
+	transformed_key = kzalloc(crypto_shash_digestsize(key_tfm), GFP_KERNEL);
+	if (!transformed_key) {
+		ret = -ENOMEM;
+		goto out_free_shash;
+	}
+
+	shash->tfm = key_tfm;
+	ret = crypto_shash_setkey(key_tfm, key, key_len);
+	if (ret < 0)
+		goto out_free_shash;
+	ret = crypto_shash_init(shash);
+	if (ret < 0)
+		goto out_free_shash;
+	ret = crypto_shash_update(shash, nqn, strlen(nqn));
+	if (ret < 0)
+		goto out_free_shash;
+	ret = crypto_shash_update(shash, "NVMe-over-Fabrics", 17);
+	if (ret < 0)
+		goto out_free_shash;
+	ret = crypto_shash_final(shash, transformed_key);
+out_free_shash:
+	kfree(shash);
+out_free_key:
+	crypto_free_shash(key_tfm);
+	if (ret < 0) {
+		kfree_sensitive(transformed_key);
+		return ERR_PTR(ret);
+	}
+	return transformed_key;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_transform_key);
+
+static int nvme_auth_send(struct nvme_ctrl *ctrl, int qid,
+		void *data, size_t tl)
+{
+	struct nvme_command cmd = {};
+	blk_mq_req_flags_t flags = qid == NVME_QID_ANY ?
+		0 : BLK_MQ_REQ_NOWAIT | BLK_MQ_REQ_RESERVED;
+	struct request_queue *q = qid == NVME_QID_ANY ?
+		ctrl->fabrics_q : ctrl->connect_q;
+	int ret;
+
+	cmd.auth_send.opcode = nvme_fabrics_command;
+	cmd.auth_send.fctype = nvme_fabrics_type_auth_send;
+	cmd.auth_send.secp = NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER;
+	cmd.auth_send.spsp0 = 0x01;
+	cmd.auth_send.spsp1 = 0x01;
+	cmd.auth_send.tl = cpu_to_le32(tl);
+
+	ret = __nvme_submit_sync_cmd(q, &cmd, NULL, data, tl, 0, qid,
+				     0, flags);
+	if (ret > 0)
+		dev_warn(ctrl->device,
+			"qid %d auth_send failed with status %d\n", qid, ret);
+	else if (ret < 0)
+		dev_err(ctrl->device,
+			"qid %d auth_send failed with error %d\n", qid, ret);
+	return ret;

drivers/nvme/host/auth.c new file +23 -0

+	int ret;
+
+	cmd.auth_receive.opcode = nvme_fabrics_command;
+	cmd.auth_receive.fctype = nvme_fabrics_type_auth_receive;
+	cmd.auth_receive.secp = NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER;
+	cmd.auth_receive.spsp0 = 0x01;
+	cmd.auth_receive.spsp1 = 0x01;
+	cmd.auth_receive.al = cpu_to_le32(al);
+
+	ret = __nvme_submit_sync_cmd(q, &cmd, NULL, buf, al, 0, qid,
+				     0, flags);
+	if (ret > 0) {
+		dev_warn(ctrl->device,
+			 "qid %d auth_recv failed with status %x\n", qid, ret);
+		ret = -EIO;
+	} else if (ret < 0) {
+		dev_err(ctrl->device,
+			"qid %d auth_recv failed with error %d\n", qid, ret);
+	}
+
+	return ret;
+}
+

Fabrics commands might be sent to all queues, not just the admin one.

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

Reviewed-by Sagi Grimberg <[email protected]>

Reviewed-by Himanshu Madhani <[email protected]>

drivers/nvme/target/fabrics-cmd-auth.c new file +54 -0

+void nvmet_execute_auth_send(struct nvmet_req *req)
+{
+	struct nvmet_ctrl *ctrl = req->sq->ctrl;
+	struct nvmf_auth_dhchap_success2_data *data;
+	void *d;
+	u32 tl;
+	u16 status = 0;
+
+	if (req->cmd->auth_send.secp != NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
+		status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+		req->error_loc =
+			offsetof(struct nvmf_auth_send_command, secp);
+		goto done;
+	}
+	if (req->cmd->auth_send.spsp0 != 0x01) {
+		status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+		req->error_loc =
+			offsetof(struct nvmf_auth_send_command, spsp0);
+		goto done;
+	}
+	if (req->cmd->auth_send.spsp1 != 0x01) {
+		status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+		req->error_loc =
+			offsetof(struct nvmf_auth_send_command, spsp1);
+		goto done;
+	}
+	tl = le32_to_cpu(req->cmd->auth_send.tl);
+	if (!tl) {
+		status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+		req->error_loc =
+			offsetof(struct nvmf_auth_send_command, tl);
+		goto done;
+	}
+	if (!nvmet_check_transfer_len(req, tl)) {
+		pr_debug("%s: transfer length mismatch (%u)\n", __func__, tl);
+		return;
+	}
+
+	d = kmalloc(tl, GFP_KERNEL);
+	if (!d) {
+		status = NVME_SC_INTERNAL;
+		goto done;
+	}
+
+	status = nvmet_copy_from_sgl(req, 0, d, tl);
+	if (status) {
+		kfree(d);
+		goto done;
+	}
+
+	data = d;
+	pr_debug("%s: ctrl %d qid %d type %d id %d step %x\n", __func__,
+		 ctrl->cntlid, req->sq->qid, data->auth_type, data->auth_id,
+		 req->sq->dhchap_step);

Quoted 60 lines

drivers/nvme/target/fabrics-cmd-auth.c new file +9 -0

+void nvmet_execute_auth_send(struct nvmet_req *req)
+{
+    struct nvmet_ctrl *ctrl = req->sq->ctrl;
+    struct nvmf_auth_dhchap_success2_data *data;
+    void *d;
+    u32 tl;
+    u16 status = 0;
+
+    if (req->cmd->auth_send.secp !=

NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, secp); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp0 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp0); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp1 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp1); +        goto done;
+    }
+    tl = le32_to_cpu(req->cmd->auth_send.tl);
+    if (!tl) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, tl);
+        goto done;
+    }
+    if (!nvmet_check_transfer_len(req, tl)) {
+        pr_debug("%s: transfer length mismatch (%u)\n", __func__, tl); +        return;
+    }

drivers/nvme/target/fabrics-cmd-auth.c new file +21 -0

+
+    d = kmalloc(tl, GFP_KERNEL);
+    if (!d) {
+        status = NVME_SC_INTERNAL;
+        goto done;
+    }
+
+    status = nvmet_copy_from_sgl(req, 0, d, tl);
+    if (status) {
+        kfree(d);
+        goto done;
+    }
+
+    data = d;
+    pr_debug("%s: ctrl %d qid %d type %d id %d step %x\n", __func__,
+         ctrl->cntlid, req->sq->qid, data->auth_type, data->auth_id,
+         req->sq->dhchap_step);
+    if (data->auth_type != NVME_AUTH_COMMON_MESSAGES &&
+        data->auth_type != NVME_AUTH_DHCHAP_MESSAGES)
+        goto done_failure1;
+    if (data->auth_type == NVME_AUTH_COMMON_MESSAGES) {

Quoted 6 lines

+        if (data->auth_id == NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE) { +            /* Restart negotiation */
+            pr_debug("%s: ctrl %d qid %d reset negotiation\n", __func__, +                 ctrl->cntlid, req->sq->qid);
+            if (!req->sq->qid) {
+                status = nvmet_setup_auth(ctrl);

Aren't you leaking memory here?

Quoted 60 lines

drivers/nvme/target/fabrics-cmd-auth.c new file +9 -0

+void nvmet_execute_auth_send(struct nvmet_req *req)
+{
+    struct nvmet_ctrl *ctrl = req->sq->ctrl;
+    struct nvmf_auth_dhchap_success2_data *data;
+    void *d;
+    u32 tl;
+    u16 status = 0;
+
+    if (req->cmd->auth_send.secp !=

NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, secp); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp0 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp0); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp1 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp1); +        goto done;
+    }
+    tl = le32_to_cpu(req->cmd->auth_send.tl);
+    if (!tl) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, tl);
+        goto done;
+    }
+    if (!nvmet_check_transfer_len(req, tl)) {
+        pr_debug("%s: transfer length mismatch (%u)\n", __func__, tl); +        return;
+    }

drivers/nvme/target/fabrics-cmd-auth.c new file +21 -0

+
+    d = kmalloc(tl, GFP_KERNEL);
+    if (!d) {
+        status = NVME_SC_INTERNAL;
+        goto done;
+    }
+
+    status = nvmet_copy_from_sgl(req, 0, d, tl);
+    if (status) {
+        kfree(d);
+        goto done;
+    }
+
+    data = d;
+    pr_debug("%s: ctrl %d qid %d type %d id %d step %x\n", __func__,
+         ctrl->cntlid, req->sq->qid, data->auth_type, data->auth_id,
+         req->sq->dhchap_step);
+    if (data->auth_type != NVME_AUTH_COMMON_MESSAGES &&
+        data->auth_type != NVME_AUTH_DHCHAP_MESSAGES)
+        goto done_failure1;
+    if (data->auth_type == NVME_AUTH_COMMON_MESSAGES) {

Quoted 6 lines

+        if (data->auth_id == NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE) { +            /* Restart negotiation */
+            pr_debug("%s: ctrl %d qid %d reset negotiation\n", __func__, +                 ctrl->cntlid, req->sq->qid);
+            if (!req->sq->qid) {
+                status = nvmet_setup_auth(ctrl);

Aren't you leaking memory here?

Quoted 60 lines · Sagi Grimberg

Quoted 60 lines

drivers/nvme/target/fabrics-cmd-auth.c new file +9 -0

+void nvmet_execute_auth_send(struct nvmet_req *req)
+{
+    struct nvmet_ctrl *ctrl = req->sq->ctrl;
+    struct nvmf_auth_dhchap_success2_data *data;
+    void *d;
+    u32 tl;
+    u16 status = 0;
+
+    if (req->cmd->auth_send.secp !=

NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, secp); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp0 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp0); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp1 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp1); +        goto done;
+    }
+    tl = le32_to_cpu(req->cmd->auth_send.tl);
+    if (!tl) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, tl);
+        goto done;
+    }
+    if (!nvmet_check_transfer_len(req, tl)) {
+        pr_debug("%s: transfer length mismatch (%u)\n", __func__, tl); +        return;
+    }

drivers/nvme/target/fabrics-cmd-auth.c new file +21 -0

+
+    d = kmalloc(tl, GFP_KERNEL);
+    if (!d) {
+        status = NVME_SC_INTERNAL;
+        goto done;
+    }
+
+    status = nvmet_copy_from_sgl(req, 0, d, tl);
+    if (status) {
+        kfree(d);
+        goto done;
+    }
+
+    data = d;
+    pr_debug("%s: ctrl %d qid %d type %d id %d step %x\n", __func__,
+         ctrl->cntlid, req->sq->qid, data->auth_type, data->auth_id,
+         req->sq->dhchap_step);
+    if (data->auth_type != NVME_AUTH_COMMON_MESSAGES &&
+        data->auth_type != NVME_AUTH_DHCHAP_MESSAGES)
+        goto done_failure1;
+    if (data->auth_type == NVME_AUTH_COMMON_MESSAGES) {

Quoted 8 lines

Quoted 6 lines

+        if (data->auth_id == NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE) { +            /* Restart negotiation */
+            pr_debug("%s: ctrl %d qid %d reset negotiation\n", __func__, +                 ctrl->cntlid, req->sq->qid);
+            if (!req->sq->qid) {
+                status = nvmet_setup_auth(ctrl);

Aren't you leaking memory here?

I've checked, and I _think_ everything is in order.
Any particular concerns?
( 'd' is free at 'done_kfree', and we never exit without going through it AFAICS).
Have I missed something?

Quoted 71 lines · Hannes Reinecke

Quoted 61 lines · Sagi Grimberg

Quoted 61 lines

drivers/nvme/target/fabrics-cmd-auth.c new file +9 -0

+void nvmet_execute_auth_send(struct nvmet_req *req)
+{
+    struct nvmet_ctrl *ctrl = req->sq->ctrl;
+    struct nvmf_auth_dhchap_success2_data *data;
+    void *d;
+    u32 tl;
+    u16 status = 0;
+
+    if (req->cmd->auth_send.secp !=

NVME_AUTH_DHCHAP_PROTOCOL_IDENTIFIER) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, secp); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp0 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp0); +        goto done;
+    }
+    if (req->cmd->auth_send.spsp1 != 0x01) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, spsp1); +        goto done;
+    }
+    tl = le32_to_cpu(req->cmd->auth_send.tl);
+    if (!tl) {
+        status = NVME_SC_INVALID_FIELD | NVME_SC_DNR;
+        req->error_loc =
+            offsetof(struct nvmf_auth_send_command, tl);
+        goto done;
+    }
+    if (!nvmet_check_transfer_len(req, tl)) {
+        pr_debug("%s: transfer length mismatch (%u)\n", __func__, tl); +        return;
+    }

drivers/nvme/target/fabrics-cmd-auth.c new file +22 -0

+
+    d = kmalloc(tl, GFP_KERNEL);
+    if (!d) {
+        status = NVME_SC_INTERNAL;
+        goto done;
+    }
+
+    status = nvmet_copy_from_sgl(req, 0, d, tl);
+    if (status) {
+        kfree(d);
+        goto done;
+    }
+
+    data = d;
+    pr_debug("%s: ctrl %d qid %d type %d id %d step %x\n", __func__,
+         ctrl->cntlid, req->sq->qid, data->auth_type, data->auth_id,
+         req->sq->dhchap_step);
+    if (data->auth_type != NVME_AUTH_COMMON_MESSAGES &&
+        data->auth_type != NVME_AUTH_DHCHAP_MESSAGES)
+        goto done_failure1;
+    if (data->auth_type == NVME_AUTH_COMMON_MESSAGES) {
+        if (data->auth_id == NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE) {

Quoted 8 lines

Quoted 6 lines

+            /* Restart negotiation */
+            pr_debug("%s: ctrl %d qid %d reset negotiation\n", __func__,
+                 ctrl->cntlid, req->sq->qid);
+            if (!req->sq->qid) {
+                status = nvmet_setup_auth(ctrl);

Aren't you leaking memory here?

Quoted 5 lines

I've checked, and I _think_ everything is in order.
Any particular concerns?
( 'd' is free at 'done_kfree', and we never exit without going through it AFAICS).
Have I missed something?

You are calling nvmet_setup_auth for re-authentication, who is calling nvmet_destroy_auth to free the controller auth stuff?

Don't you need something like:

Signature · 6 lines

        if (!req->sq->qid) {
            nvmet_destroy_auth(ctrl);
            status = nvmet_setup_auth(ctrl);
            ...
        }
-- 

Quoted 18 lines

Quoted 10 lines

Quoted 10 lines

Quoted 9 lines

+    if (data->auth_type == NVME_AUTH_COMMON_MESSAGES) { +        if (data->auth_id == NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE) { +            /* Restart negotiation */
+            pr_debug("%s: ctrl %d qid %d reset negotiation\n", __func__,
+                 ctrl->cntlid, req->sq->qid);
+            if (!req->sq->qid) {
+                status = nvmet_setup_auth(ctrl);

Quoted 7 lines

Quoted 1 line

Aren't you leaking memory here?

I've checked, and I _think_ everything is in order.
Any particular concerns?
( 'd' is free at 'done_kfree', and we never exit without going through it AFAICS).
Have I missed something?

Quoted 11 lines

You are calling nvmet_setup_auth for re-authentication, who is calling nvmet_destroy_auth to free the controller auth stuff?

Don't you need something like:

Signature · 6 lines

         if (!req->sq->qid) {
             nvmet_destroy_auth(ctrl);
             status = nvmet_setup_auth(ctrl);
             ...
         }
-- 

nvme_setup_auth() should be re-entrant, ie it'll free old and reallocate new keys as required. Hence no need to call nvmet_destroy_auth(). At least that's the plan.

Always possible that I messed up things somewhere.

Hi all,

recent updates to the NVMe spec have added definitions for in-band authentication, and seeing that it provides some real benefit especially for NVMe-TCP here's an attempt to implement it.

Tricky bit here is that the specification orients itself on TLS 1.3, but supports only the FFDHE groups. Which of course the kernel doesn't support. I've been able to come up with a patch for this, but as this is my first attempt to fix anything in the crypto area I would invite people more familiar with these matters to have a look.

Also note that this is just for in-band authentication. Secure concatenation (ie starting TLS with the negotiated parameters) is not implemented; one would need to update the kernel TLS implementation for this, which at this time is beyond scope.

As usual, comments and reviews are welcome.

Changes to v5:
- Unify nvme_auth_generate_key()
- Unify nvme_auth_extract_key()

- Include reviews from Sagi

Quoted 17 lines · Hannes Reinecke

Hi all,

recent updates to the NVMe spec have added definitions for in-band authentication, and seeing that it provides some real benefit especially for NVMe-TCP here's an attempt to implement it.

Tricky bit here is that the specification orients itself on TLS 1.3, but supports only the FFDHE groups. Which of course the kernel doesn't support. I've been able to come up with a patch for this, but as this is my first attempt to fix anything in the crypto area I would invite people more familiar with these matters to have a look.

Also note that this is just for in-band authentication. Secure concatenation (ie starting TLS with the negotiated parameters) is not implemented; one would need to update the kernel TLS implementation for this, which at this time is beyond scope.

Quoted 5 lines

As usual, comments and reviews are welcome.

Changes to v5:
- Unify nvme_auth_generate_key()
- Unify nvme_auth_extract_key()

You mean nvme_auth_extract_secret() ?

Quoted 1 line

- Include reviews from Sagi

What about the bug fix folded in?

Quoted 18 lines

Quoted 18 lines

Hi all,

recent updates to the NVMe spec have added definitions for in-band authentication, and seeing that it provides some real benefit especially for NVMe-TCP here's an attempt to implement it.

Tricky bit here is that the specification orients itself on TLS 1.3, but supports only the FFDHE groups. Which of course the kernel doesn't support. I've been able to come up with a patch for this, but as this is my first attempt to fix anything in the crypto area I would invite people more familiar with these matters to have a look.

Also note that this is just for in-band authentication. Secure concatenation (ie starting TLS with the negotiated parameters) is not implemented; one would need to update the kernel TLS implementation for this, which at this time is beyond scope.

Quoted 8 lines

Quoted 5 lines

As usual, comments and reviews are welcome.

Changes to v5:
- Unify nvme_auth_generate_key()
- Unify nvme_auth_extract_key()

You mean nvme_auth_extract_secret() ?

Yes.

Quoted 3 lines

Quoted 1 line

- Include reviews from Sagi

What about the bug fix folded in?

Yeah, and that, to
Forgot to mention it.

Also note that I've already folded the nvme-cli patches into the git repository to ease testing; I gather that the interface won't change that much anymore, so I felt justified in doing so.

Quoted 28 lines

Quoted 18 lines

Quoted 18 lines

Hi all,

recent updates to the NVMe spec have added definitions for in-band authentication, and seeing that it provides some real benefit especially for NVMe-TCP here's an attempt to implement it.

Tricky bit here is that the specification orients itself on TLS 1.3, but supports only the FFDHE groups. Which of course the kernel doesn't support. I've been able to come up with a patch for this, but as this is my first attempt to fix anything in the crypto area I would invite people more familiar with these matters to have a look.

Also note that this is just for in-band authentication. Secure concatenation (ie starting TLS with the negotiated parameters) is not implemented; one would need to update the kernel TLS implementation for this, which at this time is beyond scope.

Quoted 8 lines

Quoted 5 lines

As usual, comments and reviews are welcome.

Changes to v5:
- Unify nvme_auth_generate_key()
- Unify nvme_auth_extract_key()

You mean nvme_auth_extract_secret() ?

Yes.

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

- Include reviews from Sagi

What about the bug fix folded in?

Yeah, and that, to
Forgot to mention it.

It is not the code that you shared in the other thread right?

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

What about the bug fix folded in?

Yeah, and that, to
Forgot to mention it.

It is not the code that you shared in the other thread right?

Yes, it is.
It has been folded into v6.

Quoted 6 lines

Quoted 6 lines

Quoted 5 lines

Quoted 2 lines

What about the bug fix folded in?

Yeah, and that, to
Forgot to mention it.

Quoted 4 lines

Quoted 2 lines

It is not the code that you shared in the other thread right?

Yes, it is.
It has been folded into v6.

I don't see it in patch 07/12

(quoted diff) +6 -0

+	ret = nvme_auth_process_dhchap_success1(ctrl, chap);
+	if (ret) {
+		/* Controller authentication failed */
+		goto fail2;
+	}
+

v5 had 'if (ret < 0) [' here.