[PATCHv2 blktests 0/9] Testsuite for nvme in-band authentication

[PATCHv2 blktests 0/9] Testsuite for nvme in-band authentication

[Hannes Reinecke]

Hi all,

some people (Hi Sagi!) have complained that nvme in-band authentication
provide far too many configuration options to test it with some easy
commands. So here's a test suite for testing (most of) the various
configuration options and features.

As usual, comments and reviews are welcome.

Changes to v1:
  - Move tests to the 'nvme' directory
  - Check for authentication failure on invalid keys

Hannes Reinecke (9):
  nvme/rc: do not print error message when no nvme device is found
  nvme/rc: clear allowed_hosts subdirectory
  nvme/rc: add functions for in-band authentication
  nvme/039: simple test for nvmeof-tcp connection
  nvme/040: create an authenticated nvmeof-tcp connection
  nvme/041: test different key types
  nvme/042: test hash and dhgroup variations
  nvme/043: test bi-directional authentication
  nvme/044: test re-authentication

 tests/nvme/039     |  56 ++++++++++++++++++++++
 tests/nvme/039.out |   6 +++
 tests/nvme/040     |  64 +++++++++++++++++++++++++
 tests/nvme/040.out |   6 +++
 tests/nvme/041     | 102 +++++++++++++++++++++++++++++++++++++++
 tests/nvme/041.out |  36 ++++++++++++++
 tests/nvme/042     |  88 ++++++++++++++++++++++++++++++++++
 tests/nvme/042.out |  37 +++++++++++++++
 tests/nvme/043     | 104 ++++++++++++++++++++++++++++++++++++++++
 tests/nvme/043.out |  14 ++++++
 tests/nvme/044     | 116 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/044.out |  22 +++++++++
 tests/nvme/rc      |  56 ++++++++++++++++++++++
 13 files changed, 707 insertions(+)
 create mode 100644 tests/nvme/039
 create mode 100644 tests/nvme/039.out
 create mode 100644 tests/nvme/040
 create mode 100644 tests/nvme/040.out
 create mode 100644 tests/nvme/041
 create mode 100644 tests/nvme/041.out
 create mode 100644 tests/nvme/042
 create mode 100644 tests/nvme/042.out
 create mode 100644 tests/nvme/043
 create mode 100644 tests/nvme/043.out
 create mode 100644 tests/nvme/044
 create mode 100644 tests/nvme/044.out

-- 
2.26.2

[PATCH 1/9] nvme/rc: do not print error message when no nvme device is found

[Hannes Reinecke]

_find_nvme_dev() will only print an nvme device node if one is found,
so no point in printing an error message.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/rc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 1c27cde..086c094 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -306,6 +306,7 @@ _find_nvme_dev() {
 	local subsysnqn
 	local dev
 	for dev in /sys/class/nvme/nvme*; do
+		[ -e "$dev" ] || continue
 		dev="$(basename "$dev")"
 		subsysnqn="$(cat "/sys/class/nvme/${dev}/subsysnqn")"
 		if [[ "$subsysnqn" == "$subsys" ]]; then
-- 
2.26.2

[PATCH 2/9] nvme/rc: clear allowed_hosts subdirectory

[Hannes Reinecke]

When removing a subsystem we need to clear out the allowed_hosts
subdirectory, otherwise removal will fail.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/rc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 086c094..aa223df 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -262,6 +262,7 @@ _remove_nvmet_subsystem() {
 	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
 
 	_remove_nvmet_ns "${nvmet_subsystem}" "1"
+	rm "${subsys_path}"/allowed_hosts/*
 	rmdir "${subsys_path}"
 }
 
-- 
2.26.2

[PATCH 3/9] nvme/rc: add functions for in-band authentication

[Hannes Reinecke]

Add functions to enable in-band authentication.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/rc | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index aa223df..7d05e4a 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -247,6 +247,17 @@ _create_nvmet_subsystem() {
 	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
 }
 
+_create_nvmet_host() {
+	local nvmet_subsystem="$1"
+	local nvmet_hostnqn="$2"
+	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+
+	mkdir "${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+	echo 0 > "${cfs_path}/attr_allow_any_host"
+	ln -s "${NVMET_CFS}/hosts/${nvmet_hostnqn}" \
+	   "${cfs_path}/allowed_hosts/${nvmet_hostnqn}"
+}
+
 _remove_nvmet_ns() {
 	local nvmet_subsystem="$1"
 	local nsid=$2
@@ -266,6 +277,13 @@ _remove_nvmet_subsystem() {
 	rmdir "${subsys_path}"
 }
 
+_remove_nvmet_host() {
+	local nvmet_host="$1"
+	local host_path="${NVMET_CFS}/hosts/${nvmet_host}"
+
+	rmdir "${host_path}"
+}
+
 _create_nvmet_passthru() {
 	local nvmet_subsystem="$1"
 	local subsys_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
@@ -302,6 +320,42 @@ _remove_nvmet_subsystem_from_port() {
 	rm "${NVMET_CFS}/ports/${port}/subsystems/${nvmet_subsystem}"
 }
 
+_set_nvmet_hostkey() {
+	local nvmet_hostnqn="$1"
+	local nvmet_hostkey="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_hostkey}" > \
+	     "${cfs_path}/dhchap_key"
+}
+
+_set_nvmet_ctrlkey() {
+	local nvmet_hostnqn="$1"
+	local nvmet_ctrlkey="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_ctrlkey}" > \
+	     "${cfs_path}/dhchap_ctrl_key"
+}
+
+_set_nvmet_hash() {
+	local nvmet_hostnqn="$1"
+	local nvmet_hash="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_hash}" > \
+	     "${cfs_path}/dhchap_hash"
+}
+
+_set_nvmet_dhgroup() {
+	local nvmet_hostnqn="$1"
+	local nvmet_dhgroup="$2"
+	local cfs_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	echo "${nvmet_dhgroup}" > \
+	     "${cfs_path}/dhchap_dhgroup"
+}
+
 _find_nvme_dev() {
 	local subsys=$1
 	local subsysnqn
-- 
2.26.2

[PATCH 4/9] nvme/039: simple test for nvmeof-tcp connection

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/039     | 56 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/039.out |  6 +++++
 2 files changed, 62 insertions(+)
 create mode 100644 tests/nvme/039
 create mode 100644 tests/nvme/039.out

diff --git a/tests/nvme/039 b/tests/nvme/039
new file mode 100644
index 0000000..eb015be
--- /dev/null
+++ b/tests/nvme/039
@@ -0,0 +1,56 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test nvme over tcp connection
+
+. tests/nvme/rc
+
+DESCRIPTION="Create single TCP connection via localhost"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+}
+
+
+test() {
+	local port
+	local genctr
+	local subsys="blktests-subsystem-"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local nvme_trtype="tcp"
+
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}1" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
+	_create_nvmet_host "${subsys}1" "${hostnqn}"
+
+	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+	     -a "${def_traddr}" -s "${def_trsvcid}" \
+	     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+	nvme list-subsys
+
+	nvme disconnect -n "${subsys}1"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
+	_remove_nvmet_subsystem "${subsys}1"
+
+	_remove_nvmet_port "${port}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/039.out b/tests/nvme/039.out
new file mode 100644
index 0000000..7970853
--- /dev/null
+++ b/tests/nvme/039.out
@@ -0,0 +1,6 @@
+Running nvme/039
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2

[PATCH 5/9] nvme/040: create an authenticated nvmeof-tcp connection

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/040     | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/040.out |  6 +++++
 2 files changed, 70 insertions(+)
 create mode 100644 tests/nvme/040
 create mode 100644 tests/nvme/040.out

diff --git a/tests/nvme/040 b/tests/nvme/040
new file mode 100644
index 0000000..9c2eb07
--- /dev/null
+++ b/tests/nvme/040
@@ -0,0 +1,64 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test nvme over tcp authentication
+
+. tests/nvme/rc
+
+DESCRIPTION="Create authenticated TCP connection via localhost"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+}
+
+
+test() {
+	local port
+	local genctr
+	local subsys="blktests-subsystem-"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local nvme_trtype="tcp"
+	local hostkey
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}1" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
+	_create_nvmet_host "${subsys}1" "${hostnqn}"
+	_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+
+	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+	     -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
+	     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+	nvme list-subsys
+
+	nvme disconnect -n "${subsys}1"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
+	_remove_nvmet_subsystem "${subsys}1"
+
+	_remove_nvmet_port "${port}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/040.out b/tests/nvme/040.out
new file mode 100644
index 0000000..b1c6825
--- /dev/null
+++ b/tests/nvme/040.out
@@ -0,0 +1,6 @@
+Running nvme/040
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2

[PATCH 6/9] nvme/041: test different key types

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/041     | 102 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/041.out |  36 ++++++++++++++++
 2 files changed, 138 insertions(+)
 create mode 100644 tests/nvme/041
 create mode 100644 tests/nvme/041.out

diff --git a/tests/nvme/041 b/tests/nvme/041
new file mode 100644
index 0000000..8fc4ab9
--- /dev/null
+++ b/tests/nvme/041
@@ -0,0 +1,102 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test nvme over tcp dhchap keys
+
+. tests/nvme/rc
+
+DESCRIPTION="Test dhchap keys for authenticated TCP connection via localhost"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+}
+
+
+test() {
+	local port
+	local genctr
+	local subsys="blktests-subsystem-"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local nvme_trtype="tcp"
+	local hostkey
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}1" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
+	_create_nvmet_host "${subsys}1" "${hostnqn}"
+	_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+
+	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+	     -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
+	     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+	nvme list-subsys
+
+	nvme disconnect -n "${subsys}1"
+
+	for hmac in 1 2 3; do
+		echo "Testing hmac ${hmac}"
+		hostkey="$(nvme gen-dhchap-key --hmac=${hmac} -n ${subsys}1 2> /dev/null)"
+		if [ $? -ne 0 ] ; then
+			echo "couldn't generate host key for hmac ${hmac}"
+			return 1
+		fi
+		_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+
+		nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+		     -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
+		     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+		nvme list-subsys
+
+		nvme disconnect -n "${subsys}1"
+	done
+
+	for key_len in 32 48 64; do
+		echo "Testing key length ${key_len}"
+		hostkey="$(nvme gen-dhchap-key --key-length=${key_len} -n ${subsys}1 2> /dev/null)"
+		if [ $? -ne 0 ] ; then
+			echo "couldn't generate host key for length ${key_len}"
+			return 1
+		fi
+		_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+
+		nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+		     -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
+		     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+		nvme list-subsys
+
+		nvme disconnect -n "${subsys}1"
+	done
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
+	_remove_nvmet_subsystem "${subsys}1"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/041.out b/tests/nvme/041.out
new file mode 100644
index 0000000..037c434
--- /dev/null
+++ b/tests/nvme/041.out
@@ -0,0 +1,36 @@
+Running nvme/041
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 1
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 2
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 3
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 32
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 48
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 64
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2

[PATCH 7/9] nvme/042: test hash and dhgroup variations

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/042     | 88 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/042.out | 37 +++++++++++++++++++
 2 files changed, 125 insertions(+)
 create mode 100644 tests/nvme/042
 create mode 100644 tests/nvme/042.out

diff --git a/tests/nvme/042 b/tests/nvme/042
new file mode 100644
index 0000000..722f6cc
--- /dev/null
+++ b/tests/nvme/042
@@ -0,0 +1,88 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test nvme over tcp hash and dh group selection
+
+. tests/nvme/rc
+
+DESCRIPTION="Test hash and DH group for authenticated TCP connection via localhost"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+}
+
+
+test() {
+	local port
+	local genctr
+	local subsys="blktests-subsystem-"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local nvme_trtype="tcp"
+	local hostkey
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}1" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
+	_create_nvmet_host "${subsys}1" "${hostnqn}"
+	_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+
+	for hash in "hmac(sha384)" "hmac(sha512)" ; do
+
+		echo "Testing hash ${hash}"
+
+		_set_nvmet_hash "${hostnqn}" "${hash}"
+
+		nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+		     -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
+		     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+		nvme list-subsys
+
+		nvme disconnect -n "${subsys}1"
+	done
+
+	for dhgroup in "ffdhe2048" "ffdhe3072" "ffdhe4096" "ffdhe6144" "ffdhe8192" ; do
+
+		echo "Testing DH group ${dhgroup}"
+
+		_set_nvmet_dhgroup "${hostnqn}" "${dhgroup}"
+
+		nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+		     -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
+		     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+		nvme list-subsys
+
+		nvme disconnect -n "${subsys}1"
+	done
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
+	_remove_nvmet_subsystem "${subsys}1"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/042.out b/tests/nvme/042.out
new file mode 100644
index 0000000..7ac606c
--- /dev/null
+++ b/tests/nvme/042.out
@@ -0,0 +1,37 @@
+Running nvme/042
+Testing hash hmac(sha384)
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hash hmac(sha512)
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe2048
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe3072
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe4096
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe6144
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe8192
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2

[PATCH 8/9] nvme/043: test bi-directional authentication

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/043     | 104 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/043.out |  14 ++++++
 2 files changed, 118 insertions(+)
 create mode 100644 tests/nvme/043
 create mode 100644 tests/nvme/043.out

diff --git a/tests/nvme/043 b/tests/nvme/043
new file mode 100644
index 0000000..0c00f95
--- /dev/null
+++ b/tests/nvme/043
@@ -0,0 +1,104 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test nvme over tcp bi-directional authentication
+
+. tests/nvme/rc
+
+DESCRIPTION="Test bi-directional authentication for TCP connection via localhost"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+}
+
+
+test() {
+	local port
+	local genctr
+	local subsys="blktests-subsystem-"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local nvme_trtype="tcp"
+	local hostkey
+	local ctrlkey
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	ctrlkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}1" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
+	_create_nvmet_host "${subsys}1" "${hostnqn}"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
+	_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+	_set_nvmet_ctrlkey "${hostnqn}" "${ctrlkey}"
+
+	# Step 1: Connect with just host authentication
+	echo "Test host authentication"
+	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+	     -a "${def_traddr}" -s "${def_trsvcid}" \
+	     -S "${hostkey}" \
+	     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+	nvme list-subsys
+
+	nvme disconnect -n "${subsys}1"
+
+	# Step 2: Connect with host authentication
+	# and invalid ctrl authentication
+	echo "Test host authentication and invalid ctrl authentication"
+	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+	     -a "${def_traddr}" -s "${def_trsvcid}" \
+	     -S "${hostkey}" -C "${hostkey}" \
+	     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+	nvmedev=$(_find_nvme_dev "${subsys}1")
+	if [ -n "${nvmedev}" ] ; then
+		nvme disconnect -d "${nvmedev}"
+	fi
+
+	# Step 3: Connect with host authentication
+	# and valid ctrl authentication
+	echo "Test host authentication and valid ctrl authentication"
+	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+	     -a "${def_traddr}" -s "${def_trsvcid}" \
+	     -S "${hostkey}" -C "${ctrlkey}" \
+	     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+	nvme list-subsys
+
+	nvme disconnect -n "${subsys}1"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
+	_remove_nvmet_subsystem "${subsys}1"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/043.out b/tests/nvme/043.out
new file mode 100644
index 0000000..9ad2d72
--- /dev/null
+++ b/tests/nvme/043.out
@@ -0,0 +1,14 @@
+Running nvme/043
+Test host authentication
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test host authentication and invalid ctrl authentication
+no controller found
+Test host authentication and valid ctrl authentication
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2

[PATCH 9/9] nvme/044: test re-authentication

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/044     | 116 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/044.out |  22 +++++++++
 2 files changed, 138 insertions(+)
 create mode 100644 tests/nvme/044
 create mode 100644 tests/nvme/044.out

diff --git a/tests/nvme/044 b/tests/nvme/044
new file mode 100644
index 0000000..e3a1e94
--- /dev/null
+++ b/tests/nvme/044
@@ -0,0 +1,116 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test nvme over tcp re-authentication
+
+. tests/nvme/rc
+
+DESCRIPTION="Test re-authentication for TCP connection via localhost"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+}
+
+
+test() {
+	local port
+	local genctr
+	local subsys="blktests-subsystem-"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local nvme_trtype="tcp"
+	local hostkey
+	local ctrlkey
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	ctrlkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}1" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
+	_create_nvmet_host "${subsys}1" "${hostnqn}"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
+	_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
+	_set_nvmet_ctrlkey "${hostnqn}" "${ctrlkey}"
+
+	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
+	     -a "${def_traddr}" -s "${def_trsvcid}" \
+	     -S "${hostkey}" -C "${ctrlkey}" \
+	     --hostnqn="${hostnqn}" --hostid="${hostid}"
+
+	nvme list-subsys
+
+	ctrl=$(_find_nvme_dev "${subsys}1")
+
+	echo "Re-authenticate with original host key"
+
+	echo "${hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
+
+	echo "Renew host key on the controller"
+
+	new_hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+
+	_set_nvmet_hostkey "${hostnqn}" "${new_hostkey}"
+
+	echo "Re-authenticate with new host key"
+
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
+
+	nvme list-subsys
+
+	echo "Renew ctrl key on the controller"
+
+	new_ctrlkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
+
+	_set_nvmet_ctrlkey "${hostnqn}" "${new_ctrlkey}"
+
+	echo "Re-authenticate with new ctrl key"
+
+	echo "${new_ctrlkey}" > /sys/class/nvme/${ctrl}/dhchap_ctrl_secret
+
+	nvme list-subsys
+
+	echo "Change DH group to ffdhe8192"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe8192"
+
+	echo "Re-authenticate with changed DH group"
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
+
+	nvme list-subsys
+
+	nvme disconnect -n "${subsys}1"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
+	_remove_nvmet_subsystem "${subsys}1"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/044.out b/tests/nvme/044.out
new file mode 100644
index 0000000..e1803f4
--- /dev/null
+++ b/tests/nvme/044.out
@@ -0,0 +1,22 @@
+Running nvme/044
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+Re-authenticate with original host key
+Renew host key on the controller
+Re-authenticate with new host key
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+Renew ctrl key on the controller
+Re-authenticate with new ctrl key
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+Change DH group to ffdhe8192
+Re-authenticate with changed DH group
+nvme-subsys0 - NQN=blktests-subsystem-1
+\
+ +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.26.2

Re: [PATCH 1/9] nvme/rc: do not print error message when no nvme device is found

[Sagi Grimberg]

> _find_nvme_dev() will only print an nvme device node if one is found,
> so no point in printing an error message.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/rc | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index 1c27cde..086c094 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -306,6 +306,7 @@ _find_nvme_dev() {
>   	local subsysnqn
>   	local dev
>   	for dev in /sys/class/nvme/nvme*; do
> +		[ -e "$dev" ] || continue
>   		dev="$(basename "$dev")"
>   		subsysnqn="$(cat "/sys/class/nvme/${dev}/subsysnqn")"
>   		if [[ "$subsysnqn" == "$subsys" ]]; then
> 

OK,

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 2/9] nvme/rc: clear allowed_hosts subdirectory

[Sagi Grimberg]

Reviewed-by: Sagi Grimberg <sagi@grimberg.me>

Re: [PATCH 3/9] nvme/rc: add functions for in-band authentication

[Sagi Grimberg]

On 11/22/21 9:55 AM, Hannes Reinecke wrote:
> Add functions to enable in-band authentication.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/rc | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 54 insertions(+)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index aa223df..7d05e4a 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -247,6 +247,17 @@ _create_nvmet_subsystem() {
>   	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
>   }
>   
> +_create_nvmet_host() {
> +	local nvmet_subsystem="$1"
> +	local nvmet_hostnqn="$2"
> +	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
> +
> +	mkdir "${NVMET_CFS}/hosts/${nvmet_hostnqn}"
> +	echo 0 > "${cfs_path}/attr_allow_any_host"
> +	ln -s "${NVMET_CFS}/hosts/${nvmet_hostnqn}" \
> +	   "${cfs_path}/allowed_hosts/${nvmet_hostnqn}"
> +}

I'd say that perhaps we can add optional parameters for
host key,hash,dhgrp so that simple tests can just add
a host with these params set?

Re: [PATCH 4/9] nvme/039: simple test for nvmeof-tcp connection

[Sagi Grimberg]

On 11/22/21 9:55 AM, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/039     | 56 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/039.out |  6 +++++
>   2 files changed, 62 insertions(+)
>   create mode 100644 tests/nvme/039
>   create mode 100644 tests/nvme/039.out
> 
> diff --git a/tests/nvme/039 b/tests/nvme/039
> new file mode 100644
> index 0000000..eb015be
> --- /dev/null
> +++ b/tests/nvme/039
> @@ -0,0 +1,56 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Test nvme over tcp connection
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Create single TCP connection via localhost"

Why is this test needed?

Re: [PATCH 5/9] nvme/040: create an authenticated nvmeof-tcp connection

[Sagi Grimberg]

On 11/22/21 9:55 AM, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/040     | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/040.out |  6 +++++
>   2 files changed, 70 insertions(+)
>   create mode 100644 tests/nvme/040
>   create mode 100644 tests/nvme/040.out
> 
> diff --git a/tests/nvme/040 b/tests/nvme/040
> new file mode 100644
> index 0000000..9c2eb07
> --- /dev/null
> +++ b/tests/nvme/040
> @@ -0,0 +1,64 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Test nvme over tcp authentication
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Create authenticated TCP connection via localhost"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_modules loop
> +	_require_nvme_trtype_is_fabrics
> +}
> +
> +
> +test() {
> +	local port
> +	local genctr
> +	local subsys="blktests-subsystem-"
> +	local hostid="$(uuidgen)"
> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
> +	local scratch="/tmp/blktest-ns1.img"
> +	local nvme_trtype="tcp"

Why is this tcp specific? what prevents this from passing with
loop/rdma?

AFAICT the rest of the tests are not tcp specific either, nor
is inband auth in general...

I think we should have the tests run with a user-defind nvme_trtype.

> +	local hostkey
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		echo "nvme gen-dhchap-key command missing"
> +		return 1
> +	fi

I think that this belongs in nvme/rc

> +
> +	_setup_nvmet
> +
> +	truncate -s 512M "${scratch}"
> +
> +	port="$(_create_nvmet_port "${nvme_trtype}")"
> +
> +	_create_nvmet_subsystem "${subsys}1" "${scratch}"
> +	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
> +	_create_nvmet_host "${subsys}1" "${hostnqn}"
> +	_set_nvmet_hostkey "${hostnqn}" "${hostkey}"

Can you comment that hash/dhgrp are used with their default values.

> +
> +	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
> +	     -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
> +	     --hostnqn="${hostnqn}" --hostid="${hostid}"
> +
> +	nvme list-subsys

Why not just look in the connect retcode? why do you need the 
list-subsys output?

> +
> +	nvme disconnect -n "${subsys}1"
> +
> +	_remove_nvmet_subsystem_from_port "${port}" "${subsys}1"
> +	_remove_nvmet_subsystem "${subsys}1"
> +
> +	_remove_nvmet_port "${port}"
> +
> +	rm ${scratch}
> +
> +	echo "Test complete"
> +}
> diff --git a/tests/nvme/040.out b/tests/nvme/040.out
> new file mode 100644
> index 0000000..b1c6825
> --- /dev/null
> +++ b/tests/nvme/040.out
> @@ -0,0 +1,6 @@
> +Running nvme/040
> +nvme-subsys0 - NQN=blktests-subsystem-1
> +\
> + +- nvme0 tcp traddr=127.0.0.1,trsvcid=4420 live
> +NQN:blktests-subsystem-1 disconnected 1 controller(s)
> +Test complete
> 

Re: [PATCH 6/9] nvme/041: test different key types

[Sagi Grimberg]

On 11/22/21 9:55 AM, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/041     | 102 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/041.out |  36 ++++++++++++++++
>   2 files changed, 138 insertions(+)
>   create mode 100644 tests/nvme/041
>   create mode 100644 tests/nvme/041.out
> 
> diff --git a/tests/nvme/041 b/tests/nvme/041
> new file mode 100644
> index 0000000..8fc4ab9
> --- /dev/null
> +++ b/tests/nvme/041
> @@ -0,0 +1,102 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Test nvme over tcp dhchap keys
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Test dhchap keys for authenticated TCP connection via localhost"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_modules loop
> +	_require_nvme_trtype_is_fabrics
> +}
> +
> +
> +test() {
> +	local port
> +	local genctr
> +	local subsys="blktests-subsystem-"
> +	local hostid="$(uuidgen)"
> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
> +	local scratch="/tmp/blktest-ns1.img"
> +	local nvme_trtype="tcp"
> +	local hostkey
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		echo "nvme gen-dhchap-key command missing"
> +		return 1
> +	fi

Don't we want to test different secret formats?

Re: [PATCH 9/9] nvme/044: test re-authentication

[Sagi Grimberg]

On 11/22/21 9:55 AM, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/044     | 116 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/044.out |  22 +++++++++
>   2 files changed, 138 insertions(+)
>   create mode 100644 tests/nvme/044
>   create mode 100644 tests/nvme/044.out
> 
> diff --git a/tests/nvme/044 b/tests/nvme/044
> new file mode 100644
> index 0000000..e3a1e94
> --- /dev/null
> +++ b/tests/nvme/044
> @@ -0,0 +1,116 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Test nvme over tcp re-authentication
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Test re-authentication for TCP connection via localhost"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_modules loop
> +	_require_nvme_trtype_is_fabrics
> +}
> +
> +
> +test() {
> +	local port
> +	local genctr
> +	local subsys="blktests-subsystem-"
> +	local hostid="$(uuidgen)"
> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
> +	local scratch="/tmp/blktest-ns1.img"
> +	local nvme_trtype="tcp"
> +	local hostkey
> +	local ctrlkey
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		echo "nvme gen-dhchap-key command missing"
> +		return 1
> +	fi
> +
> +	ctrlkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		echo "nvme gen-dhchap-key command missing"
> +		return 1
> +	fi
> +
> +	_setup_nvmet
> +
> +	truncate -s 512M "${scratch}"
> +
> +	port="$(_create_nvmet_port "${nvme_trtype}")"
> +
> +	_create_nvmet_subsystem "${subsys}1" "${scratch}"
> +	_add_nvmet_subsys_to_port "${port}" "${subsys}1"
> +	_create_nvmet_host "${subsys}1" "${hostnqn}"
> +
> +	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
> +	_set_nvmet_hostkey "${hostnqn}" "${hostkey}"
> +	_set_nvmet_ctrlkey "${hostnqn}" "${ctrlkey}"
> +
> +	nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
> +	     -a "${def_traddr}" -s "${def_trsvcid}" \
> +	     -S "${hostkey}" -C "${ctrlkey}" \
> +	     --hostnqn="${hostnqn}" --hostid="${hostid}"
> +
> +	nvme list-subsys
> +
> +	ctrl=$(_find_nvme_dev "${subsys}1")
> +
> +	echo "Re-authenticate with original host key"
> +
> +	echo "${hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
> +
> +	echo "Renew host key on the controller"
> +
> +	new_hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
> +
> +	_set_nvmet_hostkey "${hostnqn}" "${new_hostkey}"
> +
> +	echo "Re-authenticate with new host key"
> +
> +	echo "${new_hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
> +
> +	nvme list-subsys
> +
> +	echo "Renew ctrl key on the controller"
> +
> +	new_ctrlkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
> +
> +	_set_nvmet_ctrlkey "${hostnqn}" "${new_ctrlkey}"
> +
> +	echo "Re-authenticate with new ctrl key"
> +
> +	echo "${new_ctrlkey}" > /sys/class/nvme/${ctrl}/dhchap_ctrl_secret
> +
> +	nvme list-subsys
> +
> +	echo "Change DH group to ffdhe8192"
> +
> +	_set_nvmet_dhgroup "${hostnqn}" "ffdhe8192"
> +
> +	echo "Re-authenticate with changed DH group"
> +	echo "${new_hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
> +
> +	nvme list-subsys

Here you may want to compare the lists, but why not to a local
variable?

If you insist of printing, how about introducing a
generic filter helper to filter-out the transport from list-subsys?

Re: [PATCH 4/9] nvme/039: simple test for nvmeof-tcp connection

[Hannes Reinecke]

On 11/22/21 9:52 AM, Sagi Grimberg wrote:
> 
> 
> On 11/22/21 9:55 AM, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/039     | 56 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/039.out |  6 +++++
>>   2 files changed, 62 insertions(+)
>>   create mode 100644 tests/nvme/039
>>   create mode 100644 tests/nvme/039.out
>>
>> diff --git a/tests/nvme/039 b/tests/nvme/039
>> new file mode 100644
>> index 0000000..eb015be
>> --- /dev/null
>> +++ b/tests/nvme/039
>> @@ -0,0 +1,56 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Test nvme over tcp connection
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Create single TCP connection via localhost"
> 
> Why is this test needed?

To establish a baseline that I didn't mess up _normal_ connections when
adding the authentication stuff.
But yeah, I can fold it in with the next test.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)

Re: [PATCH 5/9] nvme/040: create an authenticated nvmeof-tcp connection

[Hannes Reinecke]

On 11/22/21 9:57 AM, Sagi Grimberg wrote:
> 
> 
> On 11/22/21 9:55 AM, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/040     | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/040.out |  6 +++++
>>   2 files changed, 70 insertions(+)
>>   create mode 100644 tests/nvme/040
>>   create mode 100644 tests/nvme/040.out
>>
>> diff --git a/tests/nvme/040 b/tests/nvme/040
>> new file mode 100644
>> index 0000000..9c2eb07
>> --- /dev/null
>> +++ b/tests/nvme/040
>> @@ -0,0 +1,64 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Test nvme over tcp authentication
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Create authenticated TCP connection via localhost"
>> +QUICK=1
>> +
>> +requires() {
>> +    _nvme_requires
>> +    _have_modules loop
>> +    _require_nvme_trtype_is_fabrics
>> +}
>> +
>> +
>> +test() {
>> +    local port
>> +    local genctr
>> +    local subsys="blktests-subsystem-"
>> +    local hostid="$(uuidgen)"
>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>> +    local scratch="/tmp/blktest-ns1.img"
>> +    local nvme_trtype="tcp"
> 
> Why is this tcp specific? what prevents this from passing with
> loop/rdma?
> 

Nothing in principle.

> AFAICT the rest of the tests are not tcp specific either, nor
> is inband auth in general...
> 
> I think we should have the tests run with a user-defind nvme_trtype.
> 

If you can tell me how to do that I'll happily make the change.

>> +    local hostkey
>> +
>> +    echo "Running ${TEST_NAME}"
>> +
>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>> +    if [ $? -ne 0 ] ; then
>> +        echo "nvme gen-dhchap-key command missing"
>> +        return 1
>> +    fi
> 
> I think that this belongs in nvme/rc
> 
>> +
>> +    _setup_nvmet
>> +
>> +    truncate -s 512M "${scratch}"
>> +
>> +    port="$(_create_nvmet_port "${nvme_trtype}")"
>> +
>> +    _create_nvmet_subsystem "${subsys}1" "${scratch}"
>> +    _add_nvmet_subsys_to_port "${port}" "${subsys}1"
>> +    _create_nvmet_host "${subsys}1" "${hostnqn}"
>> +    _set_nvmet_hostkey "${hostnqn}" "${hostkey}"
> 
> Can you comment that hash/dhgrp are used with their default values.
> 
Sure.

>> +
>> +    nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
>> +         -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
>> +         --hostnqn="${hostnqn}" --hostid="${hostid}"
>> +
>> +    nvme list-subsys
> 
> Why not just look in the connect retcode? why do you need the
> list-subsys output?
> 
Hmm. Possibly I could; just found it more obvious the check if the
namespace shows up in the system as expected.
Reasoning being that it doesn't help if nvme returns no error, but
the namespace doesn't show up.
But I'm open to suggestions.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)

Re: [PATCH 3/9] nvme/rc: add functions for in-band authentication

[Hannes Reinecke]

On 11/22/21 9:52 AM, Sagi Grimberg wrote:
> 
> 
> On 11/22/21 9:55 AM, Hannes Reinecke wrote:
>> Add functions to enable in-band authentication.
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/rc | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 54 insertions(+)
>>
>> diff --git a/tests/nvme/rc b/tests/nvme/rc
>> index aa223df..7d05e4a 100644
>> --- a/tests/nvme/rc
>> +++ b/tests/nvme/rc
>> @@ -247,6 +247,17 @@ _create_nvmet_subsystem() {
>>       _create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
>>   }
>>   +_create_nvmet_host() {
>> +    local nvmet_subsystem="$1"
>> +    local nvmet_hostnqn="$2"
>> +    local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
>> +
>> +    mkdir "${NVMET_CFS}/hosts/${nvmet_hostnqn}"
>> +    echo 0 > "${cfs_path}/attr_allow_any_host"
>> +    ln -s "${NVMET_CFS}/hosts/${nvmet_hostnqn}" \
>> +       "${cfs_path}/allowed_hosts/${nvmet_hostnqn}"
>> +}
> 
> I'd say that perhaps we can add optional parameters for
> host key,hash,dhgrp so that simple tests can just add
> a host with these params set?

That's what I had initially, but then moved away from it seeing that
I'll have to have these functions anyway.

But sure, can do that ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)

Re: [PATCH 6/9] nvme/041: test different key types

[Hannes Reinecke]

On 11/22/21 9:58 AM, Sagi Grimberg wrote:
> 
> 
> On 11/22/21 9:55 AM, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/041     | 102 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/041.out |  36 ++++++++++++++++
>>   2 files changed, 138 insertions(+)
>>   create mode 100644 tests/nvme/041
>>   create mode 100644 tests/nvme/041.out
>>
>> diff --git a/tests/nvme/041 b/tests/nvme/041
>> new file mode 100644
>> index 0000000..8fc4ab9
>> --- /dev/null
>> +++ b/tests/nvme/041
>> @@ -0,0 +1,102 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Test nvme over tcp dhchap keys
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Test dhchap keys for authenticated TCP connection via
>> localhost"
>> +QUICK=1
>> +
>> +requires() {
>> +    _nvme_requires
>> +    _have_modules loop
>> +    _require_nvme_trtype_is_fabrics
>> +}
>> +
>> +
>> +test() {
>> +    local port
>> +    local genctr
>> +    local subsys="blktests-subsystem-"
>> +    local hostid="$(uuidgen)"
>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>> +    local scratch="/tmp/blktest-ns1.img"
>> +    local nvme_trtype="tcp"
>> +    local hostkey
>> +
>> +    echo "Running ${TEST_NAME}"
>> +
>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>> +    if [ $? -ne 0 ] ; then
>> +        echo "nvme gen-dhchap-key command missing"
>> +        return 1
>> +    fi
> 
> Don't we want to test different secret formats?

We do.
But this is just a check if the nvme cli contains functionality for
generating keys in the first place.

Maybe we should make this a requirement (like
_requre_nvme_trtype_is_fabrics).

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)

Re: [PATCH 4/9] nvme/039: simple test for nvmeof-tcp connection

[Sagi Grimberg]

>>> +DESCRIPTION="Create single TCP connection via localhost"
>>
>> Why is this test needed?
> 
> To establish a baseline that I didn't mess up _normal_ connections when
> adding the authentication stuff.
> But yeah, I can fold it in with the next test.

This is something all other tests already do though...

Re: [PATCH 9/9] nvme/044: test re-authentication

[Hannes Reinecke]

On 11/22/21 10:02 AM, Sagi Grimberg wrote:
> 
> 
> On 11/22/21 9:55 AM, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/044     | 116 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/044.out |  22 +++++++++
>>   2 files changed, 138 insertions(+)
>>   create mode 100644 tests/nvme/044
>>   create mode 100644 tests/nvme/044.out
>>
>> diff --git a/tests/nvme/044 b/tests/nvme/044
>> new file mode 100644
>> index 0000000..e3a1e94
>> --- /dev/null
>> +++ b/tests/nvme/044
>> @@ -0,0 +1,116 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Test nvme over tcp re-authentication
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Test re-authentication for TCP connection via localhost"
>> +QUICK=1
>> +
>> +requires() {
>> +    _nvme_requires
>> +    _have_modules loop
>> +    _require_nvme_trtype_is_fabrics
>> +}
>> +
>> +
>> +test() {
>> +    local port
>> +    local genctr
>> +    local subsys="blktests-subsystem-"
>> +    local hostid="$(uuidgen)"
>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>> +    local scratch="/tmp/blktest-ns1.img"
>> +    local nvme_trtype="tcp"
>> +    local hostkey
>> +    local ctrlkey
>> +
>> +    echo "Running ${TEST_NAME}"
>> +
>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>> +    if [ $? -ne 0 ] ; then
>> +        echo "nvme gen-dhchap-key command missing"
>> +        return 1
>> +    fi
>> +
>> +    ctrlkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>> +    if [ $? -ne 0 ] ; then
>> +        echo "nvme gen-dhchap-key command missing"
>> +        return 1
>> +    fi
>> +
>> +    _setup_nvmet
>> +
>> +    truncate -s 512M "${scratch}"
>> +
>> +    port="$(_create_nvmet_port "${nvme_trtype}")"
>> +
>> +    _create_nvmet_subsystem "${subsys}1" "${scratch}"
>> +    _add_nvmet_subsys_to_port "${port}" "${subsys}1"
>> +    _create_nvmet_host "${subsys}1" "${hostnqn}"
>> +
>> +    _set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
>> +    _set_nvmet_hostkey "${hostnqn}" "${hostkey}"
>> +    _set_nvmet_ctrlkey "${hostnqn}" "${ctrlkey}"
>> +
>> +    nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
>> +         -a "${def_traddr}" -s "${def_trsvcid}" \
>> +         -S "${hostkey}" -C "${ctrlkey}" \
>> +         --hostnqn="${hostnqn}" --hostid="${hostid}"
>> +
>> +    nvme list-subsys
>> +
>> +    ctrl=$(_find_nvme_dev "${subsys}1")
>> +
>> +    echo "Re-authenticate with original host key"
>> +
>> +    echo "${hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
>> +
>> +    echo "Renew host key on the controller"
>> +
>> +    new_hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>> +
>> +    _set_nvmet_hostkey "${hostnqn}" "${new_hostkey}"
>> +
>> +    echo "Re-authenticate with new host key"
>> +
>> +    echo "${new_hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
>> +
>> +    nvme list-subsys
>> +
>> +    echo "Renew ctrl key on the controller"
>> +
>> +    new_ctrlkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>> +
>> +    _set_nvmet_ctrlkey "${hostnqn}" "${new_ctrlkey}"
>> +
>> +    echo "Re-authenticate with new ctrl key"
>> +
>> +    echo "${new_ctrlkey}" > /sys/class/nvme/${ctrl}/dhchap_ctrl_secret
>> +
>> +    nvme list-subsys
>> +
>> +    echo "Change DH group to ffdhe8192"
>> +
>> +    _set_nvmet_dhgroup "${hostnqn}" "ffdhe8192"
>> +
>> +    echo "Re-authenticate with changed DH group"
>> +    echo "${new_hostkey}" > /sys/class/nvme/${ctrl}/dhchap_secret
>> +
>> +    nvme list-subsys
> 
> Here you may want to compare the lists, but why not to a local
> variable?
> 
Actually, I don't need as the list is stored in the output file; if the
lists are different we'll have a diff to the output file and the test
will fail.

But yeah, I see your point.

> If you insist of printing, how about introducing a
> generic filter helper to filter-out the transport from list-subsys?

See my comment to the previous tests.
If it's sufficient to check the return code from nvme connect we can go
with that.
Or checking for the namespace device node like I did in 43.

Lemme see.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)

Re: [PATCH 4/9] nvme/039: simple test for nvmeof-tcp connection

[Hannes Reinecke]

On 11/22/21 10:40 AM, Sagi Grimberg wrote:
> 
>>>> +DESCRIPTION="Create single TCP connection via localhost"
>>>
>>> Why is this test needed?
>>
>> To establish a baseline that I didn't mess up _normal_ connections when
>> adding the authentication stuff.
>> But yeah, I can fold it in with the next test.
> 
> This is something all other tests already do though...

Hmm. Yeah, you are right.
I'll be dropping it.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)

Re: [PATCH 5/9] nvme/040: create an authenticated nvmeof-tcp connection

[Sagi Grimberg]

On 11/22/21 11:22 AM, Hannes Reinecke wrote:
> On 11/22/21 9:57 AM, Sagi Grimberg wrote:
>>
>>
>> On 11/22/21 9:55 AM, Hannes Reinecke wrote:
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>> ---
>>>    tests/nvme/040     | 64 ++++++++++++++++++++++++++++++++++++++++++++++
>>>    tests/nvme/040.out |  6 +++++
>>>    2 files changed, 70 insertions(+)
>>>    create mode 100644 tests/nvme/040
>>>    create mode 100644 tests/nvme/040.out
>>>
>>> diff --git a/tests/nvme/040 b/tests/nvme/040
>>> new file mode 100644
>>> index 0000000..9c2eb07
>>> --- /dev/null
>>> +++ b/tests/nvme/040
>>> @@ -0,0 +1,64 @@
>>> +#!/bin/bash
>>> +# SPDX-License-Identifier: GPL-3.0+
>>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>>> +#
>>> +# Test nvme over tcp authentication
>>> +
>>> +. tests/nvme/rc
>>> +
>>> +DESCRIPTION="Create authenticated TCP connection via localhost"
>>> +QUICK=1
>>> +
>>> +requires() {
>>> +    _nvme_requires
>>> +    _have_modules loop
>>> +    _require_nvme_trtype_is_fabrics
>>> +}
>>> +
>>> +
>>> +test() {
>>> +    local port
>>> +    local genctr
>>> +    local subsys="blktests-subsystem-"
>>> +    local hostid="$(uuidgen)"
>>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>>> +    local scratch="/tmp/blktest-ns1.img"
>>> +    local nvme_trtype="tcp"
>>
>> Why is this tcp specific? what prevents this from passing with
>> loop/rdma?
>>
> 
> Nothing in principle.
> 
>> AFAICT the rest of the tests are not tcp specific either, nor
>> is inband auth in general...
>>
>> I think we should have the tests run with a user-defind nvme_trtype.
>>
> 
> If you can tell me how to do that I'll happily make the change.

Remove the above setting for nvme_trtype=tcp and verify the test
is fine using:

$ nvme_trtype=[tcp|loop|rdma] ./check tests/nvme/040

> 
>>> +    local hostkey
>>> +
>>> +    echo "Running ${TEST_NAME}"
>>> +
>>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>>> +    if [ $? -ne 0 ] ; then
>>> +        echo "nvme gen-dhchap-key command missing"
>>> +        return 1
>>> +    fi
>>
>> I think that this belongs in nvme/rc
>>
>>> +
>>> +    _setup_nvmet
>>> +
>>> +    truncate -s 512M "${scratch}"
>>> +
>>> +    port="$(_create_nvmet_port "${nvme_trtype}")"
>>> +
>>> +    _create_nvmet_subsystem "${subsys}1" "${scratch}"
>>> +    _add_nvmet_subsys_to_port "${port}" "${subsys}1"
>>> +    _create_nvmet_host "${subsys}1" "${hostnqn}"
>>> +    _set_nvmet_hostkey "${hostnqn}" "${hostkey}"
>>
>> Can you comment that hash/dhgrp are used with their default values.
>>
> Sure.
> 
>>> +
>>> +    nvme connect -t "${nvme_trtype}" -n "${subsys}1" \
>>> +         -a "${def_traddr}" -s "${def_trsvcid}" -S "${hostkey}" \
>>> +         --hostnqn="${hostnqn}" --hostid="${hostid}"
>>> +
>>> +    nvme list-subsys
>>
>> Why not just look in the connect retcode? why do you need the
>> list-subsys output?
>>
> Hmm. Possibly I could; just found it more obvious the check if the
> namespace shows up in the system as expected.
> Reasoning being that it doesn't help if nvme returns no error, but
> the namespace doesn't show up.
> But I'm open to suggestions.

What does the namespace has to do with inband auth?
But if you really want to, you can still try to access
the subsystem and retrieve the hostkey and/or validate
the ns device node exist or something like that...

Re: [PATCH 6/9] nvme/041: test different key types

[Sagi Grimberg]

>>> +test() {
>>> +    local port
>>> +    local genctr
>>> +    local subsys="blktests-subsystem-"
>>> +    local hostid="$(uuidgen)"
>>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>>> +    local scratch="/tmp/blktest-ns1.img"
>>> +    local nvme_trtype="tcp"
>>> +    local hostkey
>>> +
>>> +    echo "Running ${TEST_NAME}"
>>> +
>>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys}1 2> /dev/null)"
>>> +    if [ $? -ne 0 ] ; then
>>> +        echo "nvme gen-dhchap-key command missing"
>>> +        return 1
>>> +    fi
>>
>> Don't we want to test different secret formats?
> 
> We do.
> But this is just a check if the nvme cli contains functionality for
> generating keys in the first place.
> 
> Maybe we should make this a requirement (like
> _requre_nvme_trtype_is_fabrics).

Absolutely.