libnvme: clarify dhchap key usage

There are two dhchap keys, a 'host' and a 'ctrl' key. Additionally,
the libnvme interface prefixes function calls with 'nvme_ctrl_XX'
and 'nvme_host_XX', depending on whether the first argument is a
controller or a host.
But that means that the function 'nvme_ctrl_dhchap_key' does _not_
refer to the dhchap ctrl key, but in fact to the dhchap host key.
So to avoid all confusion make the naming consistent, and use the
'_host_key' suffix for all functions referring to the dhchap host
key, and '_ctrl_key' to all functions referring to the dhchap ctrl key.

Which, incidentally, uncovered quite a few mixups.

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

Author: hreinecke

libnvme/libnvme/nvme.i

@@ -369,7 +369,7 @@ struct nvme_host {
 	char *hostid;
 	char *hostsymname;
 	%extend {
-		char *dhchap_key;
+		char *dhchap_host_key;
 	}
 };
 
@@ -457,19 +457,18 @@ struct nvme_ctrl {
 		char *tls_key_identity;
 		char *tls_key;
 
+		char *dhchap_host_key;
+		char *dhchap_ctrl_key;
+
 		/**
 		 * We are remapping the following members of the C code's
 		 * nvme_ctrl_t to different names in Python. Here's the mapping:
 		 *
 		 * C code                 Python (SWIG)
 		 * =====================  =====================
 		 * ctrl->s                ctrl->subsystem
-		 * ctrl->dhchap_key       ctrl->dhchap_host_key
-		 * ctrl->dhchap_ctrl_key  ctrl->dhchap_key
 		 */
 		struct nvme_subsystem *subsystem; // Maps to "s" in the C code
-		char *dhchap_host_key;            // Maps to "dhchap_key" in the C code
-		char *dhchap_key;                 // Maps to "dhchap_ctrl_key" in the C code
 	}
 };
 
@@ -562,7 +561,7 @@ struct nvme_ns {
 		if (hostsymname)
 			nvme_host_set_hostsymname(h, hostsymname);
 		if (hostkey)
-			nvme_host_set_dhchap_key(h, hostkey);
+			nvme_host_set_dhchap_host_key(h, hostkey);
 		return h;
 	}
 	~nvme_host() {
@@ -593,11 +592,11 @@ struct nvme_ns {
 }
 
 %{
-	const char *nvme_host_dhchap_key_get(struct nvme_host *h) {
-		return nvme_host_get_dhchap_key(h);
+	const char *nvme_host_dhchap_host_key_get(struct nvme_host *h) {
+		return nvme_host_get_dhchap_host_key(h);
 	}
-	void nvme_host_dhchap_key_set(struct nvme_host *h, char *key) {
-		nvme_host_set_dhchap_key(h, key);
+	void nvme_host_dhchap_host_key_set(struct nvme_host *h, char *key) {
+		nvme_host_set_dhchap_host_key(h, key);
 	}
 %};
 
@@ -880,11 +879,11 @@ struct nvme_ns {
 	const char *nvme_ctrl_state_get(struct nvme_ctrl *c) {
 		return nvme_ctrl_get_state(c);
 	}
-	const char *nvme_ctrl_dhchap_key_get(struct nvme_ctrl *c) {
-		return nvme_ctrl_get_dhchap_key(c);
+	const char *nvme_ctrl_dhchap_ctrl_key_get(struct nvme_ctrl *c) {
+		return nvme_ctrl_get_dhchap_ctrl_key(c);
 	}
-	void nvme_ctrl_dhchap_key_set(struct nvme_ctrl *c, const char *key) {
-		nvme_ctrl_set_dhchap_key(c, key);
+	void nvme_ctrl_dhchap_ctrl_key_set(struct nvme_ctrl *c, const char *key) {
+		nvme_ctrl_set_dhchap_ctrl_key(c, key);
 	}
 	const char *nvme_ctrl_dhchap_host_key_get(struct nvme_ctrl *c) {
 		return nvme_ctrl_get_dhchap_host_key(c);

libnvme/src/accessors.ld

@@ -58,8 +58,8 @@ LIBNVME_ACCESSORS_3 {
 		nvme_ctrl_set_traddr;
 		nvme_ctrl_get_trsvcid;
 		nvme_ctrl_set_trsvcid;
-		nvme_ctrl_get_dhchap_key;
-		nvme_ctrl_set_dhchap_key;
+		nvme_ctrl_get_dhchap_host_key;
+		nvme_ctrl_set_dhchap_host_key;
 		nvme_ctrl_get_dhchap_ctrl_key;
 		nvme_ctrl_set_dhchap_ctrl_key;
 		nvme_ctrl_get_keyring;
@@ -108,8 +108,8 @@ LIBNVME_ACCESSORS_3 {
 		nvme_host_set_hostnqn;
 		nvme_host_get_hostid;
 		nvme_host_set_hostid;
-		nvme_host_get_dhchap_key;
-		nvme_host_set_dhchap_key;
+		nvme_host_get_dhchap_host_key;
+		nvme_host_set_dhchap_host_key;
 		nvme_host_get_hostsymname;
 		nvme_host_set_hostsymname;
 		nvme_host_get_pdc_enabled_valid;

libnvme/src/libnvme.ld

@@ -11,7 +11,6 @@ LIBNVME_3 {
 		nvme_ctrl_first_path;
 		nvme_ctrl_get_address;
 		nvme_ctrl_get_config;
-		nvme_ctrl_get_dhchap_host_key;
 		nvme_ctrl_get_phy_slot;
 		nvme_ctrl_get_src_addr;
 		nvme_ctrl_get_state;
@@ -26,7 +25,6 @@ LIBNVME_3 {
 		nvme_ctrl_next_path;
 		nvme_ctrl_release_transport_handle;
 		nvme_reset_ctrl;
-		nvme_ctrl_set_dhchap_host_key;
 		nvme_filter_ctrls;
 		nvme_describe_key_serial;
 		nvme_disconnect_ctrl;

libnvme/src/nvme/accessors.c

@@ -292,15 +292,17 @@ const char *nvme_ctrl_get_trsvcid(const struct nvme_ctrl *p)
 	return p->trsvcid;
 }
 
-void nvme_ctrl_set_dhchap_key(struct nvme_ctrl *p, const char *dhchap_key)
+void nvme_ctrl_set_dhchap_host_key(
+		struct nvme_ctrl *p,
+		const char *dhchap_host_key)
 {
-	free(p->dhchap_key);
-	p->dhchap_key = dhchap_key ? strdup(dhchap_key) : NULL;
+	free(p->dhchap_host_key);
+	p->dhchap_host_key = dhchap_host_key ? strdup(dhchap_host_key) : NULL;
 }
 
-const char *nvme_ctrl_get_dhchap_key(const struct nvme_ctrl *p)
+const char *nvme_ctrl_get_dhchap_host_key(const struct nvme_ctrl *p)
 {
-	return p->dhchap_key;
+	return p->dhchap_host_key;
 }
 
 void nvme_ctrl_set_dhchap_ctrl_key(
@@ -586,15 +588,17 @@ const char *nvme_host_get_hostid(const struct nvme_host *p)
 	return p->hostid;
 }
 
-void nvme_host_set_dhchap_key(struct nvme_host *p, const char *dhchap_key)
+void nvme_host_set_dhchap_host_key(
+		struct nvme_host *p,
+		const char *dhchap_host_key)
 {
-	free(p->dhchap_key);
-	p->dhchap_key = dhchap_key ? strdup(dhchap_key) : NULL;
+	free(p->dhchap_host_key);
+	p->dhchap_host_key = dhchap_host_key ? strdup(dhchap_host_key) : NULL;
 }
 
-const char *nvme_host_get_dhchap_key(const struct nvme_host *p)
+const char *nvme_host_get_dhchap_host_key(const struct nvme_host *p)
 {
-	return p->dhchap_key;
+	return p->dhchap_host_key;
 }
 
 void nvme_host_set_hostsymname(struct nvme_host *p, const char *hostsymname)

libnvme/src/nvme/accessors.h

@@ -407,19 +407,21 @@ void nvme_ctrl_set_trsvcid(struct nvme_ctrl *p, const char *trsvcid);
 const char *nvme_ctrl_get_trsvcid(const struct nvme_ctrl *p);
 
 /**
- * nvme_ctrl_set_dhchap_key() - Set dhchap_key.
+ * nvme_ctrl_set_dhchap_host_key() - Set dhchap_host_key.
  * @p: The &struct nvme_ctrl instance to update.
- * @dhchap_key: New string; a copy is stored. Pass NULL to clear.
+ * @dhchap_host_key: New string; a copy is stored. Pass NULL to clear.
  */
-void nvme_ctrl_set_dhchap_key(struct nvme_ctrl *p, const char *dhchap_key);
+void nvme_ctrl_set_dhchap_host_key(
+		struct nvme_ctrl *p,
+		const char *dhchap_host_key);
 
 /**
- * nvme_ctrl_get_dhchap_key() - Get dhchap_key.
+ * nvme_ctrl_get_dhchap_host_key() - Get dhchap_host_key.
  * @p: The &struct nvme_ctrl instance to query.
  *
- * Return: The value of the dhchap_key field, or NULL if not set.
+ * Return: The value of the dhchap_host_key field, or NULL if not set.
  */
-const char *nvme_ctrl_get_dhchap_key(const struct nvme_ctrl *p);
+const char *nvme_ctrl_get_dhchap_host_key(const struct nvme_ctrl *p);
 
 /**
  * nvme_ctrl_set_dhchap_ctrl_key() - Set dhchap_ctrl_key.
@@ -808,19 +810,21 @@ void nvme_host_set_hostid(struct nvme_host *p, const char *hostid);
 const char *nvme_host_get_hostid(const struct nvme_host *p);
 
 /**
- * nvme_host_set_dhchap_key() - Set dhchap_key.
+ * nvme_host_set_dhchap_host_key() - Set dhchap_host_key.
  * @p: The &struct nvme_host instance to update.
- * @dhchap_key: New string; a copy is stored. Pass NULL to clear.
+ * @dhchap_host_key: New string; a copy is stored. Pass NULL to clear.
  */
-void nvme_host_set_dhchap_key(struct nvme_host *p, const char *dhchap_key);
+void nvme_host_set_dhchap_host_key(
+		struct nvme_host *p,
+		const char *dhchap_host_key);
 
 /**
- * nvme_host_get_dhchap_key() - Get dhchap_key.
+ * nvme_host_get_dhchap_host_key() - Get dhchap_host_key.
  * @p: The &struct nvme_host instance to query.
  *
- * Return: The value of the dhchap_key field, or NULL if not set.
+ * Return: The value of the dhchap_host_key field, or NULL if not set.
  */
-const char *nvme_host_get_dhchap_key(const struct nvme_host *p);
+const char *nvme_host_get_dhchap_host_key(const struct nvme_host *p);
 
 /**
  * nvme_host_set_hostsymname() - Set hostsymname.

libnvme/src/nvme/fabrics.c

@@ -672,7 +672,7 @@ static int build_options(nvme_host_t h, nvme_ctrl_t c, char **argstr)
 {
 	struct nvme_fabrics_config *cfg = nvme_ctrl_get_config(c);
 	const char *transport = nvme_ctrl_get_transport(c);
-	const char *hostnqn, *hostid, *hostkey, *ctrlkey;
+	const char *hostnqn, *hostid, *hostkey, *ctrlkey = NULL;
 	bool discover = false, discovery_nqn = false;
 	struct nvme_global_ctx *ctx = h->ctx;
 	long keyring_id = 0;
@@ -708,11 +708,12 @@ static int build_options(nvme_host_t h, nvme_ctrl_t c, char **argstr)
 
 	hostnqn = nvme_host_get_hostnqn(h);
 	hostid = nvme_host_get_hostid(h);
-	hostkey = nvme_host_get_dhchap_key(h);
+	hostkey = nvme_host_get_dhchap_host_key(h);
 	if (!hostkey)
 		hostkey = nvme_ctrl_get_dhchap_host_key(c);
 
-	ctrlkey = nvme_ctrl_get_dhchap_key(c);
+	if (hostkey)
+		ctrlkey = nvme_ctrl_get_dhchap_ctrl_key(c);
 
 	if (cfg->tls && cfg->concat) {
 		nvme_msg(h->ctx, LOG_ERR, "cannot specify --tls and --concat together\n");
@@ -1020,9 +1021,9 @@ int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
 			key = nvme_ctrl_get_dhchap_host_key(fc);
 			if (key)
 				nvme_ctrl_set_dhchap_host_key(c, key);
-			key = nvme_ctrl_get_dhchap_key(fc);
+			key = nvme_ctrl_get_dhchap_ctrl_key(fc);
 			if (key)
-				nvme_ctrl_set_dhchap_key(c, key);
+				nvme_ctrl_set_dhchap_ctrl_key(c, key);
 			key = nvme_ctrl_get_keyring(fc);
 			if (key)
 				nvme_ctrl_set_keyring(c, key);
@@ -1854,7 +1855,7 @@ static int setup_connection(struct nvmf_context *fctx, struct nvme_host *h,
 		bool discovery)
 {
 	if (fctx->hostkey)
-		nvme_host_set_dhchap_key(h, fctx->hostkey);
+		nvme_host_set_dhchap_host_key(h, fctx->hostkey);
 
 	if (!fctx->trsvcid)
 		fctx->trsvcid = nvmf_get_default_trsvcid(fctx->transport,
@@ -2437,7 +2438,7 @@ int nvmf_config_modify(struct nvme_global_ctx *ctx,
 	}
 
 	if (fctx->hostkey)
-		nvme_host_set_dhchap_key(h, fctx->hostkey);
+		nvme_host_set_dhchap_host_key(h, fctx->hostkey);
 
 	s = nvme_lookup_subsystem(h, NULL, fctx->subsysnqn);
 	if (!s) {
@@ -2455,7 +2456,7 @@ int nvmf_config_modify(struct nvme_global_ctx *ctx,
 	}
 
 	if (fctx->ctrlkey)
-		nvme_ctrl_set_dhchap_key(c, fctx->ctrlkey);
+		nvme_ctrl_set_dhchap_ctrl_key(c, fctx->ctrlkey);
 
 	nvme_parse_tls_args(fctx->keyring, fctx->tls_key,
 			    fctx->tls_key_identity, fctx->cfg, c);
@@ -3081,8 +3082,11 @@ int nvmf_connect(struct nvme_global_ctx *ctx, struct nvmf_context *fctx)
 	if (err)
 		return err;
 
-	if (fctx->ctrlkey)
-		nvme_ctrl_set_dhchap_key(c, fctx->ctrlkey);
+	if (fctx->hostkey) {
+		nvme_ctrl_set_dhchap_host_key(c, fctx->hostkey);
+		if (fctx->ctrlkey)
+			nvme_ctrl_set_dhchap_ctrl_key(c, fctx->ctrlkey);
+	}
 
 	nvme_parse_tls_args(fctx->keyring, fctx->tls_key,
 		fctx->tls_key_identity, fctx->cfg, c);

libnvme/src/nvme/json.c

@@ -112,7 +112,7 @@ static void json_parse_port(nvme_subsystem_t s, struct json_object *port_obj)
 		nvme_ctrl_set_dhchap_host_key(c, json_object_get_string(attr_obj));
 	attr_obj = json_object_object_get(port_obj, "dhchap_ctrl_key");
 	if (attr_obj)
-		nvme_ctrl_set_dhchap_key(c, json_object_get_string(attr_obj));
+		nvme_ctrl_set_dhchap_ctrl_key(c, json_object_get_string(attr_obj));
 	attr_obj = json_object_object_get(port_obj, "keyring");
 	if (attr_obj)
 		nvme_ctrl_set_keyring(c, json_object_get_string(attr_obj));
@@ -175,7 +175,7 @@ static void json_parse_host(struct nvme_global_ctx *ctx, struct json_object *hos
 	h = nvme_lookup_host(ctx, hostnqn, hostid);
 	attr_obj = json_object_object_get(host_obj, "dhchap_key");
 	if (attr_obj)
-		nvme_host_set_dhchap_key(h, json_object_get_string(attr_obj));
+		nvme_host_set_dhchap_host_key(h, json_object_get_string(attr_obj));
 	attr_obj = json_object_object_get(host_obj, "hostsymname");
 	if (attr_obj)
 		nvme_host_set_hostsymname(h, json_object_get_string(attr_obj));
@@ -308,7 +308,7 @@ static void json_update_port(struct json_object *ctrl_array, nvme_ctrl_t c)
 	if (value)
 		json_object_object_add(port_obj, "dhchap_key",
 				       json_object_new_string(value));
-	value = nvme_ctrl_get_dhchap_key(c);
+	value = nvme_ctrl_get_dhchap_ctrl_key(c);
 	if (value)
 		json_object_object_add(port_obj, "dhchap_ctrl_key",
 				       json_object_new_string(value));
@@ -405,7 +405,7 @@ int json_update_config(struct nvme_global_ctx *ctx, int fd)
 		if (hostid)
 			json_object_object_add(host_obj, "hostid",
 					       json_object_new_string(hostid));
-		dhchap_key = nvme_host_get_dhchap_key(h);
+		dhchap_key = nvme_host_get_dhchap_host_key(h);
 		if (dhchap_key)
 			json_object_object_add(host_obj, "dhchap_key",
 					       json_object_new_string(dhchap_key));
@@ -475,7 +475,7 @@ static void json_dump_ctrl(struct json_object *ctrl_array, nvme_ctrl_t c)
 	if (value)
 		json_object_object_add(ctrl_obj, "dhchap_key",
 				       json_object_new_string(value));
-	value = nvme_ctrl_get_dhchap_key(c);
+	value = nvme_ctrl_get_dhchap_ctrl_key(c);
 	if (value)
 		json_object_object_add(ctrl_obj, "dhchap_ctrl_key",
 				       json_object_new_string(value));
@@ -635,7 +635,7 @@ int json_dump_tree(struct nvme_global_ctx *ctx)
 		if (hostid)
 			json_object_object_add(host_obj, "hostid",
 					       json_object_new_string(hostid));
-		dhchap_key = nvme_host_get_dhchap_key(h);
+		dhchap_key = nvme_host_get_dhchap_host_key(h);
 		if (dhchap_key)
 			json_object_object_add(host_obj, "dhchap_key",
 					       json_object_new_string(dhchap_key));

libnvme/src/nvme/private.h

@@ -198,7 +198,7 @@ struct nvme_ctrl {
 	char *subsysnqn;
 	char *traddr;
 	char *trsvcid;
-	char *dhchap_key;
+	char *dhchap_host_key;
 	char *dhchap_ctrl_key;
 	char *keyring;
 	char *tls_key_identity;
@@ -240,7 +240,7 @@ struct nvme_host {
 
 	char *hostnqn;
 	char *hostid;
-	char *dhchap_key;
+	char *dhchap_host_key;
 	char *hostsymname;
 	bool pdc_enabled;
 	bool pdc_enabled_valid; /* set if pdc_enabled doesn't have an undefined