nvme: implement secure concatenaion

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

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

---
  drivers/nvme/host/auth.c    | 108 +++++++++++++++++++++++++++++++++++-
  drivers/nvme/host/fabrics.c |  34 +++++++++++-
  drivers/nvme/host/fabrics.h |   3 +
  drivers/nvme/host/nvme.h    |   2 +
  drivers/nvme/host/sysfs.c   |   4 +-
  drivers/nvme/host/tcp.c     |  47 ++++++++++++++--
  include/linux/nvme.h        |   7 +++
  7 files changed, 191 insertions(+), 14 deletions(-)

drivers/nvme/host/auth.c +91 -3

index 371e14f0a203..902c8ba59562 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,84 @@ static void nvme_auth_free_dhchap(struct nvme_dhchap_queue_context *chap)
  		crypto_free_kpp(chap->dh_tfm);
  }
 
+void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl)
+{
+	dev_dbg(ctrl->device, "Wipe generated TLS PSK %08x\n",
+		key_serial(ctrl->opts->tls_key));
+	key_revoke(ctrl->opts->tls_key);
+	key_put(ctrl->opts->tls_key);
+	ctrl->opts->tls_key = NULL;
+}
+EXPORT_SYMBOL_GPL(nvme_auth_revoke_tls_key);
+
+static int nvme_auth_secure_concat(struct nvme_ctrl *ctrl,
+				   struct nvme_dhchap_queue_context *chap)
+{
+	u8 *psk, *digest, *tls_psk;
+	struct key *tls_key;
+	size_t psk_len;
+	int ret = 0;
+
+	if (!chap->sess_key) {
+		dev_warn(ctrl->device,
+			 "%s: qid %d no session key negotiated\n",
+			 __func__, chap->qid);
+		return -ENOKEY;
+	}
+
+	ret = nvme_auth_generate_psk(chap->hash_id, chap->sess_key,
+				     chap->sess_key_len,
+				     chap->c1, chap->c2,
+				     chap->hash_len, &psk, &psk_len);
+	if (ret) {
+		dev_warn(ctrl->device,
+			 "%s: qid %d failed to generate PSK, error %d\n",
+			 __func__, chap->qid, ret);
+		return ret;
+	}
+	dev_dbg(ctrl->device,
+		  "%s: generated psk %*ph\n", __func__, (int)psk_len, psk);
+
+	ret = nvme_auth_generate_digest(chap->hash_id, psk, psk_len,
+					ctrl->opts->subsysnqn,
+					ctrl->opts->host->nqn, &digest);
+	if (ret) {
+		dev_warn(ctrl->device,
+			 "%s: qid %d failed to generate digest, error %d\n",
+			 __func__, chap->qid, ret);
+		goto out_free_psk;
+	};
+	dev_dbg(ctrl->device, "%s: generated digest %s\n",
+		 __func__, digest);
+	ret = nvme_auth_derive_tls_psk(chap->hash_id, psk, psk_len, digest, &tls_psk);
+	if (ret) {
+		dev_warn(ctrl->device,
+			 "%s: qid %d failed to derive TLS psk, error %d\n",
+			 __func__, chap->qid, ret);
+		goto out_free_digest;
+	};
+
+	tls_key = nvme_tls_psk_refresh(ctrl->opts->keyring, ctrl->opts->host->nqn,
+				       ctrl->opts->subsysnqn, chap->hash_id,
+				       tls_psk, psk_len, digest);
+	if (IS_ERR(tls_key)) {
+		ret = PTR_ERR(tls_key);
+		dev_warn(ctrl->device,
+			 "%s: qid %d failed to insert generated key, error %d\n",
+			 __func__, chap->qid, ret);
+		tls_key = NULL;
+	}
+	kfree_sensitive(tls_psk);
+	if (ctrl->opts->tls_key)
+		nvme_auth_revoke_tls_key(ctrl);
+	ctrl->opts->tls_key = tls_key;
+out_free_digest:
+	kfree_sensitive(digest);
+out_free_psk:
+	kfree_sensitive(psk);
+	return ret;
+}
+
  static void nvme_queue_auth_work(struct work_struct *work)
  {
  	struct nvme_dhchap_queue_context *chap =

drivers/nvme/host/auth.c +13 -0

+		if (!chap->qid && ctrl->opts->concat &&
+		    (ret = nvme_auth_secure_concat(ctrl, chap))) {
+			dev_warn(ctrl->device,
+				 "%s: qid %d failed to enable secure concatenation\n",
+				 __func__, chap->qid);
+			chap->error = ret;
+		}
  		return;
  	}
 

@@ -912,6 +1008,12 @@ static void nvme_ctrl_auth_work(struct work_struct *work)
  			 "qid 0: authentication failed\n");
  		return;
  	}
+	/*
+	 * Only run authentication on the admin queue for
+	 * secure concatenation
+	 */
+	if (ctrl->opts->concat)
+		return;
 
  	for (q = 1; q < ctrl->queue_count; q++) {
  		ret = nvme_auth_negotiate(ctrl, q);

drivers/nvme/host/fabrics.c +31 -3

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/nvme.h +2 -0

index 313a4f978a2c..4c735b88b434 100644
@@ -1132,6 +1132,7 @@ void nvme_auth_stop(struct nvme_ctrl *ctrl);
  int nvme_auth_negotiate(struct nvme_ctrl *ctrl, int qid);
  int nvme_auth_wait(struct nvme_ctrl *ctrl, int qid);
  void nvme_auth_free(struct nvme_ctrl *ctrl);
+void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl);
  #else
  static inline int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
  {

@@ -1154,6 +1155,7 @@ static inline int nvme_auth_wait(struct nvme_ctrl *ctrl, int qid)
  	return -EPROTONOSUPPORT;
  }
  static inline void nvme_auth_free(struct nvme_ctrl *ctrl) {};
+static void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl) {};
  #endif
 
  u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,

drivers/nvme/host/sysfs.c +2 -2

index b68a9e5f1ea3..efb35eef1915 100644
@@ -780,10 +780,10 @@ static umode_t nvme_tls_attrs_are_visible(struct kobject *kobj,
  		return 0;
 
  	if (a == &dev_attr_tls_key.attr &&
-	    !ctrl->opts->tls)
+	    !ctrl->opts->tls && !ctrl->opts->concat)
  		return 0;
  	if (a == &dev_attr_tls_configured_key.attr &&
-	    !ctrl->opts->tls_key)
+	    (!ctrl->opts->tls_key || ctrl->opts->concat))
  		return 0;
  	if (a == &dev_attr_tls_keyring.attr &&
  	    !ctrl->opts->keyring)

drivers/nvme/host/tcp.c +36 -5

index 3e416af2659f..b8a3461b617c 100644
@@ -233,7 +233,7 @@ static inline bool nvme_tcp_tls_configured(struct nvme_ctrl *ctrl)
  	if (!IS_ENABLED(CONFIG_NVME_TCP_TLS))
  		return 0;
 
-	return ctrl->opts->tls;
+	return ctrl->opts->tls || ctrl->opts->concat;
  }
 
  static inline struct blk_mq_tags *nvme_tcp_tagset(struct nvme_tcp_queue *queue)

@@ -1948,7 +1948,7 @@ static int nvme_tcp_alloc_admin_queue(struct nvme_ctrl *ctrl)
  	if (nvme_tcp_tls_configured(ctrl)) {
  		if (ctrl->opts->tls_key)
  			pskid = key_serial(ctrl->opts->tls_key);
-		else {
+		else if (ctrl->opts->tls) {
  			pskid = nvme_tls_psk_default(ctrl->opts->keyring,
  						      ctrl->opts->host->nqn,
  						      ctrl->opts->subsysnqn);

@@ -1978,9 +1978,25 @@ static int __nvme_tcp_alloc_io_queues(struct nvme_ctrl *ctrl)
  {
  	int i, ret;
 
-	if (nvme_tcp_tls_configured(ctrl) && !ctrl->tls_pskid) {
-		dev_err(ctrl->device, "no PSK negotiated\n");
-		return -ENOKEY;
+	if (nvme_tcp_tls_configured(ctrl)) {
+		if (ctrl->opts->concat) {
+			/*
+			 * The generated PSK is stored in the
+			 * fabric options
+			 */
+			if (!ctrl->opts->tls_key) {
+				dev_err(ctrl->device, "no PSK generated\n");
+				return -ENOKEY;
+			}
+			if (ctrl->tls_pskid &&
+			    ctrl->tls_pskid != key_serial(ctrl->opts->tls_key)) {
+				dev_err(ctrl->device, "Stale PSK id %08x\n", ctrl->tls_pskid);
+				ctrl->tls_pskid = 0;
+			}
+		} else if (!ctrl->tls_pskid) {
+			dev_err(ctrl->device, "no PSK negotiated\n");
+			return -ENOKEY;
+		}
  	}
 
  	for (i = 1; i < ctrl->queue_count; i++) {

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

Quoted 19 lines · Hannes Reinecke

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

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

---
  drivers/nvme/host/auth.c    | 108 +++++++++++++++++++++++++++++++++++- drivers/nvme/host/fabrics.c |  34 +++++++++++- drivers/nvme/host/fabrics.h |   3 + drivers/nvme/host/nvme.h    |   2 + drivers/nvme/host/sysfs.c   |   4 +- drivers/nvme/host/tcp.c     |  47 ++++++++++++++-- include/linux/nvme.h        |   7 +++

  7 files changed, 191 insertions(+), 14 deletions(-)

drivers/nvme/host/auth.c +0 -0

index 371e14f0a203..902c8ba59562 100644

Quoted 7 lines

drivers/nvme/host/auth.c +1 -0

@@ -833,6 +921,14 @@ static void nvme_queue_auth_work(struct 
 work_struct *work)
       }
       if (!ret) {
           chap->error = 0;
+        /* Secure concatenation can only be enabled on the admin 

queue */

I'd add a warning if that is not the case here.

Quoted 245 lines

+        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 +1008,12 @@ static void nvme_ctrl_auth_work(struct 
 work_struct *work)
                "qid 0: authentication failed\n");
           return;
       }
+    /*
+     * Only run authentication on the admin queue for
+     * secure concatenation
+     */
+    if (ctrl->opts->concat)
+        return;
       for (q = 1; q < ctrl->queue_count; q++) {
           ret = nvme_auth_negotiate(ctrl, q);

drivers/nvme/host/fabrics.c +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 +17 -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;
+        }
+        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/nvme.h +2 -0

index 313a4f978a2c..4c735b88b434 100644
@@ -1132,6 +1132,7 @@ void nvme_auth_stop(struct nvme_ctrl *ctrl);
   int nvme_auth_negotiate(struct nvme_ctrl *ctrl, int qid);
   int nvme_auth_wait(struct nvme_ctrl *ctrl, int qid);
   void nvme_auth_free(struct nvme_ctrl *ctrl);
+void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl);
   #else
   static inline int nvme_auth_init_ctrl(struct nvme_ctrl *ctrl)
   {

@@ -1154,6 +1155,7 @@ static inline int nvme_auth_wait(struct 
 nvme_ctrl *ctrl, int qid)
       return -EPROTONOSUPPORT;
   }
   static inline void nvme_auth_free(struct nvme_ctrl *ctrl) {};
+static void nvme_auth_revoke_tls_key(struct nvme_ctrl *ctrl) {};
   #endif
   u32 nvme_command_effects(struct nvme_ctrl *ctrl, struct nvme_ns *ns,

drivers/nvme/host/sysfs.c +0 -0

index b68a9e5f1ea3..efb35eef1915 100644
@@ -780,10 +780,10 @@ static umode_t nvme_tls_attrs_are_visible(struct 
 kobject *kobj,
           return 0;

      if (a == &dev_attr_tls_key.attr &&
-        !ctrl->opts->tls)
+        !ctrl->opts->tls && !ctrl->opts->concat) return 0;
      if (a == &dev_attr_tls_configured_key.attr && -        !ctrl->opts->tls_key)
+        (!ctrl->opts->tls_key || ctrl->opts->concat)) return 0;
      if (a == &dev_attr_tls_keyring.attr && !ctrl->opts->keyring)

drivers/nvme/host/tcp.c +2 -2

index 3e416af2659f..b8a3461b617c 100644
@@ -233,7 +233,7 @@ static inline bool nvme_tcp_tls_configured(struct 
 nvme_ctrl *ctrl)
       if (!IS_ENABLED(CONFIG_NVME_TCP_TLS))
           return 0;
-    return ctrl->opts->tls;
+    return ctrl->opts->tls || ctrl->opts->concat;
   }
   static inline struct blk_mq_tags *nvme_tcp_tagset(struct nvme_tcp_queue *queue)

@@ -1948,7 +1948,7 @@ static int nvme_tcp_alloc_admin_queue(struct 
 nvme_ctrl *ctrl)
       if (nvme_tcp_tls_configured(ctrl)) {
           if (ctrl->opts->tls_key)
               pskid = key_serial(ctrl->opts->tls_key);
-        else {
+        else if (ctrl->opts->tls) {
               pskid = nvme_tls_psk_default(ctrl->opts->keyring,
                                 ctrl->opts->host->nqn,

                                ctrl->opts->subsysnqn);

drivers/nvme/host/tcp.c +27 -3

@@ -1978,9 +1978,25 @@ static int __nvme_tcp_alloc_io_queues(struct 
 nvme_ctrl *ctrl)
   {
       int i, ret;
-    if (nvme_tcp_tls_configured(ctrl) && !ctrl->tls_pskid) {
-        dev_err(ctrl->device, "no PSK negotiated\n");
-        return -ENOKEY;
+    if (nvme_tcp_tls_configured(ctrl)) {
+        if (ctrl->opts->concat) {
+            /*
+             * The generated PSK is stored in the
+             * fabric options
+             */
+            if (!ctrl->opts->tls_key) {
+                dev_err(ctrl->device, "no PSK generated\n");
+                return -ENOKEY;
+            }
+            if (ctrl->tls_pskid &&
+                ctrl->tls_pskid != key_serial(ctrl->opts->tls_key)) {
+                dev_err(ctrl->device, "Stale PSK id %08x\n", ctrl- 
 >tls_pskid);
+                ctrl->tls_pskid = 0;
+            }
+        } else if (!ctrl->tls_pskid) {
+            dev_err(ctrl->device, "no PSK negotiated\n");
+            return -ENOKEY;
+        }
       }
       for (i = 1; i < ctrl->queue_count; i++) {

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

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

drivers/nvme/host/tcp.c +5 -0

+static bool nvme_tcp_key_revoke_needed(struct nvme_ctrl *ctrl)
+{
+    return ctrl->opts->concat && ctrl->opts->tls_key && ctrl->tls_pskid;
+}
+

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

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

Quoted 5 lines

drivers/nvme/host/tcp.c +2 -0

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

Having this sprayed in various places in the code is really confusing.

Can you please add a small comment on each call-site? just for our future selves reading this code?

Quoted 15 lines · Hannes Reinecke

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

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

---
  drivers/nvme/host/auth.c    | 108 +++++++++++++++++++++++++++++++++++- drivers/nvme/host/fabrics.c |  34 +++++++++++- drivers/nvme/host/fabrics.h |   3 + drivers/nvme/host/nvme.h    |   2 + drivers/nvme/host/sysfs.c   |   4 +- drivers/nvme/host/tcp.c     |  47 ++++++++++++++-- include/linux/nvme.h        |   7 +++

  7 files changed, 191 insertions(+), 14 deletions(-)

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

Quoted 6 lines

drivers/nvme/host/tcp.c +2 -0

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

Having this sprayed in various places in the code is really confusing.

Can you please add a small comment on each call-site? just for our future selves reading this code?

Outside of that, patch looks good.

Quoted 20 lines · Sagi Grimberg

Quoted 16 lines · Hannes Reinecke

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

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

---
  drivers/nvme/host/auth.c    | 108
+++++++++++++++++++++++++++++++++++-
  drivers/nvme/host/fabrics.c |  34 +++++++++++- drivers/nvme/host/fabrics.h |   3 + drivers/nvme/host/nvme.h    |   2 + drivers/nvme/host/sysfs.c   |   4 +- drivers/nvme/host/tcp.c     |  47 ++++++++++++++-- include/linux/nvme.h        |   7 +++

  7 files changed, 191 insertions(+), 14 deletions(-)

[ .. ]

Quoted 9 lines

drivers/nvme/host/tcp.c static void · line 2349

Quoted 6 lines

drivers/nvme/host/tcp.c +2 -0

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

Having this sprayed in various places in the code is really confusing.

Quoted 5 lines

Can you please add a small comment on each call-site? just for our future selves reading this code?

Outside of that, patch looks good.

Weelll ...
We need to reset the negotiated PSK exactly in three places:
- reset
- error recovery
- teardown
Much like we need to do for every other queue-related resource.

And in one of your previous reviews you stated that you do _not_ want to have 'nvme_auth_revoke_tls_key()' checking if the key needs to be revoked, but rather have a check function.

Otherwise I could just move the check into nvme_auth_revoke_tls_key()' and drop the 'revoke needed' call.

Furthermore we don't need to check if the key needs to be revoked during teardown (answer will always be 'yes').

So I'm quite unsure what to do now ... document that we need to release the key when doing a reset or error recovery? Move the check back into nvme_auth_tls_revoke_key()? Hmm?

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      | 33 +++++++++---
  drivers/nvme/target/nvmet.h            | 38 +++++++++++---
  drivers/nvme/target/tcp.c              | 23 +++++++-
  5 files changed, 192 insertions(+), 23 deletions(-)

drivers/nvme/target/auth.c +71 -1

index 7897d02c681d..7470ac020db6 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, 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 +1 -2

index 3f2857c17d95..cf4b38c0e7bd 100644
@@ -43,8 +43,26 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
  		 data->auth_protocol[0].dhchap.halen,
  		 data->auth_protocol[0].dhchap.dhlen);
  	req->sq->dhchap_tid = le16_to_cpu(data->t_id);
-	if (data->sc_c)
-		return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH;
+	if (data->sc_c != NVME_AUTH_SECP_NOSC) {

drivers/nvme/target/fabrics-cmd-auth.c +32 -4

+		case NVME_AUTH_SECP_REPLACETLSPSK:
+			if (!nvmet_queue_tls_keyid(req->sq))
+				return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH;
+			break;
+		default:
+			return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH;
+		}
+		ctrl->concat = true;
+	}
 
  	if (data->napd != 1)
  		return NVME_AUTH_DHCHAP_FAILURE_HASH_UNUSABLE;

@@ -103,6 +121,13 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
  			 nvme_auth_dhgroup_name(fallback_dhgid));
  		ctrl->dh_gid = fallback_dhgid;
  	}
+	if (ctrl->dh_gid == NVME_AUTH_DHGROUP_NULL &&
+	    ctrl->concat) {
+		pr_debug("%s: ctrl %d qid %d: NULL DH group invalid "
+			 "for secure channel concatenation\n", __func__,
+			 ctrl->cntlid, req->sq->qid);
+		return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH;
+	}
  	pr_debug("%s: ctrl %d qid %d: selected DH group %s (%d)\n",
  		 __func__, ctrl->cntlid, req->sq->qid,
  		 nvme_auth_dhgroup_name(ctrl->dh_gid), ctrl->dh_gid);

@@ -154,6 +179,12 @@ static u8 nvmet_auth_reply(struct nvmet_req *req, void *d)
  	kfree(response);
  	pr_debug("%s: ctrl %d qid %d host authenticated\n",
  		 __func__, ctrl->cntlid, req->sq->qid);
+	if (!data->cvalid && ctrl->concat) {
+		pr_debug("%s: ctrl %d qid %d invalid challenge\n",
+			 __func__, ctrl->cntlid, req->sq->qid);
+		return NVME_AUTH_DHCHAP_FAILURE_FAILED;
+	}
+	req->sq->dhchap_s2 = le32_to_cpu(data->seqnum);
  	if (data->cvalid) {
  		req->sq->dhchap_c2 = kmemdup(data->rval + data->hl, data->hl,
  					     GFP_KERNEL);

@@ -163,11 +194,15 @@ static u8 nvmet_auth_reply(struct nvmet_req *req, void *d)
  		pr_debug("%s: ctrl %d qid %d challenge %*ph\n",
  			 __func__, ctrl->cntlid, req->sq->qid, data->hl,
  			 req->sq->dhchap_c2);
-	} else {
+	}
+	if (req->sq->dhchap_s2 == 0) {
+		if (ctrl->concat)
+			nvmet_auth_insert_psk(req->sq);
  		req->sq->authenticated = true;
+		kfree(req->sq->dhchap_c2);
  		req->sq->dhchap_c2 = NULL;
-	}
-	req->sq->dhchap_s2 = le32_to_cpu(data->seqnum);
+	} else if (!data->cvalid)
+		req->sq->authenticated = true;
 
  	return 0;
  }

@@ -241,7 +276,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
  			pr_debug("%s: ctrl %d qid %d reset negotiation\n",
  				 __func__, ctrl->cntlid, req->sq->qid);
  			if (!req->sq->qid) {
-				dhchap_status = nvmet_setup_auth(ctrl);
+				dhchap_status = nvmet_setup_auth(ctrl, req);
  				if (dhchap_status) {
  					pr_err("ctrl %d qid 0 failed to setup re-authentication\n",
  					       ctrl->cntlid);

@@ -298,6 +333,8 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
  		}
  		goto done_kfree;
  	case NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2:
+		if (ctrl->concat)
+			nvmet_auth_insert_psk(req->sq);
  		req->sq->authenticated = true;
  		pr_debug("%s: ctrl %d qid %d ctrl authenticated\n",
  			 __func__, ctrl->cntlid, req->sq->qid);

drivers/nvme/target/fabrics-cmd.c +25 -8

index c4b2eddd5666..9a1256deee51 100644
@@ -199,10 +199,26 @@ static u16 nvmet_install_queue(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
  	return ret;
  }
 
-static u32 nvmet_connect_result(struct nvmet_ctrl *ctrl)
+static u32 nvmet_connect_result(struct nvmet_ctrl *ctrl, struct nvmet_req *req)
  {
+	bool needs_auth = nvmet_has_auth(ctrl, req);
+	key_serial_t keyid = nvmet_queue_tls_keyid(req->sq);
+
+	/* Do not authenticate I/O queues for secure concatenation */
+	if (ctrl->concat && req->sq->qid)
+		needs_auth = false;
+
+	if (keyid)
+		pr_debug("%s: ctrl %d qid %d should %sauthenticate, tls psk %08x\n",
+			 __func__, ctrl->cntlid, req->sq->qid,
+			 needs_auth ? "" : "not ", keyid);
+	else
+		pr_debug("%s: ctrl %d qid %d should %sauthenticate%s\n",
+			 __func__, ctrl->cntlid, req->sq->qid,
+			 needs_auth ? "" : "not ",
+			 ctrl->concat ? ", secure concatenation" : "");
  	return (u32)ctrl->cntlid |
-		(nvmet_has_auth(ctrl) ? NVME_CONNECT_AUTHREQ_ATR : 0);
+		(needs_auth ? NVME_CONNECT_AUTHREQ_ATR : 0);
  }
 
  static void nvmet_execute_admin_connect(struct nvmet_req *req)

@@ -251,7 +267,7 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
 
  	uuid_copy(&ctrl->hostid, &d->hostid);
 
-	dhchap_status = nvmet_setup_auth(ctrl);
+	dhchap_status = nvmet_setup_auth(ctrl, req);
  	if (dhchap_status) {
  		pr_err("Failed to setup authentication, dhchap status %u\n",
  		       dhchap_status);

@@ -269,12 +285,13 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
  		goto out;
  	}
 
-	pr_info("creating %s controller %d for subsystem %s for NQN %s%s%s.\n",
+	pr_info("creating %s controller %d for subsystem %s for NQN %s%s%s%s.\n",
  		nvmet_is_disc_subsys(ctrl->subsys) ? "discovery" : "nvm",
  		ctrl->cntlid, ctrl->subsys->subsysnqn, ctrl->hostnqn,
-		ctrl->pi_support ? " T10-PI is enabled" : "",
-		nvmet_has_auth(ctrl) ? " with DH-HMAC-CHAP" : "");
-	req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl));
+		ctrl->pi_support ? ", T10-PI" : "",
+		nvmet_has_auth(ctrl, req) ? ", DH-HMAC-CHAP" : "",
+		nvmet_queue_tls_keyid(req->sq) ? ", TLS" : "");
+	req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl, req));
  out:
  	kfree(d);
  complete:

@@ -330,7 +347,7 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
  		goto out_ctrl_put;
 
  	pr_debug("adding queue %d to ctrl %d.\n", qid, ctrl->cntlid);
-	req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl));
+	req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl, req));
  out:
  	kfree(d);
  complete:

drivers/nvme/target/nvmet.h +32 -6

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 +4 -2

index 7c51c2a8c109..671600b5c64b 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);

Quoted 143 lines · 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      | 33 +++++++++--- drivers/nvme/target/nvmet.h            | 38 +++++++++++--- drivers/nvme/target/tcp.c              | 23 +++++++-

  5 files changed, 192 insertions(+), 23 deletions(-)

drivers/nvme/target/auth.c +49 -1

index 7897d02c681d..7470ac020db6 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-

Quoted 1 line

subsysnqn,

+                       sq->ctrl->shash_id, tls_psk, psk_len, digest); +    if (IS_ERR(tls_key)) {
+        pr_warn("%s: ctrl %d qid %d failed to refresh key, error %ld\n", +            __func__, sq->ctrl->cntlid, sq->qid, PTR_ERR(tls_key)); +        tls_key = NULL;
+        kfree_sensitive(tls_psk);
+    }
+    if (sq->ctrl->tls_key)
+        key_put(sq->ctrl->tls_key);
+    sq->ctrl->tls_key = tls_key;

drivers/nvme/target/auth.c +7 -0

+#endif
+
+out_free_digest:
+    kfree_sensitive(digest);
+out_free_psk:
+    kfree_sensitive(psk);
+}

drivers/nvme/target/fabrics-cmd-auth.c +4 -2

index 3f2857c17d95..cf4b38c0e7bd 100644
@@ -43,8 +43,26 @@ static u8 nvmet_auth_negotiate(struct nvmet_req 
 *req, void *d)
            data->auth_protocol[0].dhchap.halen,
            data->auth_protocol[0].dhchap.dhlen);
       req->sq->dhchap_tid = le16_to_cpu(data->t_id);
-    if (data->sc_c)
-        return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH;
+    if (data->sc_c != NVME_AUTH_SECP_NOSC) {
+        if (!IS_ENABLED(CONFIG_NVME_TARGET_TCP_TLS))
+            return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH;
+        /* Secure concatenation can only be enabled on the admin 

queue */
+        if (req->sq->qid)

Quoted 6 lines

+            return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; +        switch (data->sc_c) {
+        case NVME_AUTH_SECP_NEWTLSPSK:
+            if (nvmet_queue_tls_keyid(req->sq))
+                return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; +            break;

fallthru instead?

Quoted 280 lines

+        case NVME_AUTH_SECP_REPLACETLSPSK:
+            if (!nvmet_queue_tls_keyid(req->sq))
+                return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; +            break;
+        default:
+            return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH; +        }
+        ctrl->concat = true;
+    }
      if (data->napd != 1)
          return NVME_AUTH_DHCHAP_FAILURE_HASH_UNUSABLE;

drivers/nvme/target/fabrics-cmd-auth.c +7 -0

@@ -103,6 +121,13 @@ static u8 nvmet_auth_negotiate(struct nvmet_req 
 *req, void *d)
                nvme_auth_dhgroup_name(fallback_dhgid));
           ctrl->dh_gid = fallback_dhgid;
       }
+    if (ctrl->dh_gid == NVME_AUTH_DHGROUP_NULL &&
+        ctrl->concat) {
+        pr_debug("%s: ctrl %d qid %d: NULL DH group invalid "
+             "for secure channel concatenation\n", __func__,
+             ctrl->cntlid, req->sq->qid);
+        return NVME_AUTH_DHCHAP_FAILURE_CONCAT_MISMATCH;
+    }
       pr_debug("%s: ctrl %d qid %d: selected DH group %s (%d)\n",
            __func__, ctrl->cntlid, req->sq->qid,

           nvme_auth_dhgroup_name(ctrl->dh_gid), ctrl->dh_gid);

drivers/nvme/target/fabrics-cmd-auth.c +6 -0

@@ -154,6 +179,12 @@ static u8 nvmet_auth_reply(struct nvmet_req *req, 
 void *d)
       kfree(response);
       pr_debug("%s: ctrl %d qid %d host authenticated\n",
            __func__, ctrl->cntlid, req->sq->qid);
+    if (!data->cvalid && ctrl->concat) {
+        pr_debug("%s: ctrl %d qid %d invalid challenge\n",
+             __func__, ctrl->cntlid, req->sq->qid);
+        return NVME_AUTH_DHCHAP_FAILURE_FAILED;
+    }
+    req->sq->dhchap_s2 = le32_to_cpu(data->seqnum);
       if (data->cvalid) {
           req->sq->dhchap_c2 = kmemdup(data->rval + data->hl, data->hl,

                           GFP_KERNEL);

drivers/nvme/target/fabrics-cmd-auth.c +8 -4

@@ -163,11 +194,15 @@ static u8 nvmet_auth_reply(struct nvmet_req 
 *req, void *d)
           pr_debug("%s: ctrl %d qid %d challenge %*ph\n",
                __func__, ctrl->cntlid, req->sq->qid, data->hl,
                req->sq->dhchap_c2);
-    } else {
+    }
+    if (req->sq->dhchap_s2 == 0) {
+        if (ctrl->concat)
+            nvmet_auth_insert_psk(req->sq);
           req->sq->authenticated = true;
+        kfree(req->sq->dhchap_c2);
           req->sq->dhchap_c2 = NULL;
-    }
-    req->sq->dhchap_s2 = le32_to_cpu(data->seqnum);
+    } else if (!data->cvalid)
+        req->sq->authenticated = true;
       return 0;
   }

@@ -241,7 +276,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
               pr_debug("%s: ctrl %d qid %d reset negotiation\n",
                    __func__, ctrl->cntlid, req->sq->qid);
               if (!req->sq->qid) {
-                dhchap_status = nvmet_setup_auth(ctrl);
+                dhchap_status = nvmet_setup_auth(ctrl, req);
                   if (dhchap_status) {
                       pr_err("ctrl %d qid 0 failed to setup re- authentication\n",
                              ctrl->cntlid);

@@ -298,6 +333,8 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
           }
           goto done_kfree;

      case NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2:
+        if (ctrl->concat)
+            nvmet_auth_insert_psk(req->sq); req->sq->authenticated = true; pr_debug("%s: ctrl %d qid %d ctrl authenticated\n", __func__, ctrl->cntlid, req->sq->qid);

drivers/nvme/target/fabrics-cmd.c +17 -1

index c4b2eddd5666..9a1256deee51 100644
@@ -199,10 +199,26 @@ static u16 nvmet_install_queue(struct nvmet_ctrl 
 *ctrl, struct nvmet_req *req)
       return ret;
   }
-static u32 nvmet_connect_result(struct nvmet_ctrl *ctrl)
+static u32 nvmet_connect_result(struct nvmet_ctrl *ctrl, struct 
 nvmet_req *req)
   {
+    bool needs_auth = nvmet_has_auth(ctrl, req);
+    key_serial_t keyid = nvmet_queue_tls_keyid(req->sq);
+
+    /* Do not authenticate I/O queues for secure concatenation */
+    if (ctrl->concat && req->sq->qid)
+        needs_auth = false;
+
+    if (keyid)
+        pr_debug("%s: ctrl %d qid %d should %sauthenticate, tls psk 
 %08x\n",
+             __func__, ctrl->cntlid, req->sq->qid,
+             needs_auth ? "" : "not ", keyid);
+    else
+        pr_debug("%s: ctrl %d qid %d should %sauthenticate%s\n",
+             __func__, ctrl->cntlid, req->sq->qid,
+             needs_auth ? "" : "not ",
+             ctrl->concat ? ", secure concatenation" : "");

      return (u32)ctrl->cntlid |
-        (nvmet_has_auth(ctrl) ? NVME_CONNECT_AUTHREQ_ATR : 0); +        (needs_auth ? NVME_CONNECT_AUTHREQ_ATR : 0); }
  static void nvmet_execute_admin_connect(struct nvmet_req *req)

drivers/nvme/target/fabrics-cmd.c +6 -5

@@ -251,7 +267,7 @@ static void nvmet_execute_admin_connect(struct 
 nvmet_req *req)
       uuid_copy(&ctrl->hostid, &d->hostid);
-    dhchap_status = nvmet_setup_auth(ctrl);
+    dhchap_status = nvmet_setup_auth(ctrl, req);
       if (dhchap_status) {
           pr_err("Failed to setup authentication, dhchap status %u\n",
                  dhchap_status);

@@ -269,12 +285,13 @@ static void nvmet_execute_admin_connect(struct 
 nvmet_req *req)
           goto out;
       }
-    pr_info("creating %s controller %d for subsystem %s for NQN 
 %s%s%s.\n",
+    pr_info("creating %s controller %d for subsystem %s for NQN 
 %s%s%s%s.\n",
           nvmet_is_disc_subsys(ctrl->subsys) ? "discovery" : "nvm",
           ctrl->cntlid, ctrl->subsys->subsysnqn, ctrl->hostnqn,
-        ctrl->pi_support ? " T10-PI is enabled" : "",
-        nvmet_has_auth(ctrl) ? " with DH-HMAC-CHAP" : "");
-    req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl));
+        ctrl->pi_support ? ", T10-PI" : "",
+        nvmet_has_auth(ctrl, req) ? ", DH-HMAC-CHAP" : "",
+        nvmet_queue_tls_keyid(req->sq) ? ", TLS" : "");
+    req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl, req));

  out:
      kfree(d);
  complete:

drivers/nvme/target/fabrics-cmd.c +1 -1

@@ -330,7 +347,7 @@ static void nvmet_execute_io_connect(struct 
 nvmet_req *req)
           goto out_ctrl_put;
       pr_debug("adding queue %d to ctrl %d.\n", qid, ctrl->cntlid);
-    req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl));
+    req->cqe->result.u32 = cpu_to_le32(nvmet_connect_result(ctrl, req));

  out:
      kfree(d);
  complete:

drivers/nvme/target/nvmet.h +24 -1

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);

drivers/nvme/target/nvmet.h +5 -3

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

drivers/nvme/target/nvmet.h +3 -2

@@ -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 +4 -2

index 7c51c2a8c109..671600b5c64b 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);

drivers/nvme/target/tcp.c static void nvmet_tcp_tls_handshake_done(void *data, int status, · line 1816

Quoted 6 lines

drivers/nvme/target/tcp.c +5 -0

       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)) {

It is not clear to me how this can happen. Can you explain?

Other than that, patch looks good.

Quoted 30 lines

Quoted 30 lines

drivers/nvme/target/tcp.c +4 -2

index 7c51c2a8c109..671600b5c64b 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);

Quoted 9 lines

drivers/nvme/target/tcp.c static void nvmet_tcp_tls_handshake_done(void *data, int status, · line 1816

Quoted 6 lines

drivers/nvme/target/tcp.c +5 -0

 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)) {

It is not clear to me how this can happen. Can you explain?

Passing key information between kernel and handshake daemon is done purely by key IDs (not the keys itself).
So we will be getting a key ID back from the handshaked daemon, and we need to validate that; the user (or admin software) could have invalidated the key while the handshake was running, or before we had a chance to process the reply from the handshake daemon.