[PATCHv3 blktests 00/10] Testsuite for nvme in-band authentication

[PATCHv3 blktests 00/10] 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 v2:
   - Merge first two tests
   - Add optional arguments to _nvme_connect_subsys()
   - Convert to use _nvme_connect_subsys()

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

Hannes Reinecke (10):
  nvme/rc: do not print error message when no nvme device is found
  nvme/rc: clear allowed_hosts subdirectory
  nvme/rc: clear hosts directory in _cleanup_nvmet()
  nvme/rc: add functions for in-band authentication
  nvme/rc: add more arguments to _nvme_connect_subsys()
  nvme/039: create authenticated connections
  nvme/040: test dhchap key types for authenticated connections
  nvme/041: test hash and dh group variations for authenticated
    connections
  nvme/042: test bi-directional authentication
  nvme/043: test re-authentication

 tests/nvme/039     |  83 +++++++++++++++++++++++++++
 tests/nvme/039.out |   7 +++
 tests/nvme/040     |  95 +++++++++++++++++++++++++++++++
 tests/nvme/040.out |  16 ++++++
 tests/nvme/041     |  89 +++++++++++++++++++++++++++++
 tests/nvme/041.out |  18 ++++++
 tests/nvme/042     | 107 +++++++++++++++++++++++++++++++++++
 tests/nvme/042.out |   8 +++
 tests/nvme/043     | 136 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/043.out |  12 ++++
 tests/nvme/rc      |  97 ++++++++++++++++++++++++++++++++
 11 files changed, 668 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

-- 
2.29.2

[PATCH 01/10] 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>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 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.29.2

[PATCH 02/10] 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>
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
---
 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.29.2

[PATCH 03/10] nvme/rc: clear hosts directory in _cleanup_nvmet()

[Hannes Reinecke]

From: Hannes Reinecke <hare@suse.com>

When clearing out the configuration we need to clear the 'hosts'
directory, too, as scripts might have added host configurations.

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

diff --git a/tests/nvme/rc b/tests/nvme/rc
index aa223df..c284602 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -122,6 +122,12 @@ _cleanup_nvmet() {
 		rmdir "${subsys}"
 	done
 
+	for host in "${NVMET_CFS}"/hosts/*; do
+		name=$(basename "${host}")
+		echo "WARNING: Test did not clean up host: ${name}"
+		rmdir "${host}"
+	done
+
 	shopt -u nullglob
 	trap SIGINT
 
-- 
2.29.2

[PATCH 04/10] 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 | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/tests/nvme/rc b/tests/nvme/rc
index c284602..6e08b41 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -73,6 +73,17 @@ _require_nvme_trtype_is_fabrics() {
 	return 0
 }
 
+_require_nvme_cli_auth() {
+	local hostkey
+
+	hostkey="$(nvme gen-dhchap-key -n nvmf-test-subsys 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		SKIP_REASON="nvme gen-dhchap-key command missing"
+		return 1
+	fi
+	return 0
+}
+
 _test_dev_nvme_ctrl() {
 	echo "/dev/char/$(cat "${TEST_DEV_SYSFS}/device/dev")"
 }
@@ -253,6 +264,25 @@ _create_nvmet_subsystem() {
 	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
 }
 
+_create_nvmet_host() {
+	local nvmet_subsystem="$1"
+	local nvmet_hostnqn="$2"
+	local nvmet_hostkey="$3"
+	local nvmet_ctrlkey="$4"
+	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
+	local host_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
+
+	mkdir "${host_path}"
+	echo 0 > "${cfs_path}/attr_allow_any_host"
+	ln -s "${host_path}" "${cfs_path}/allowed_hosts/${nvmet_hostnqn}"
+	if [[ "${nvmet_hostkey}" ]] ; then
+		echo "${nvmet_hostkey}" > "${host_path}/dhchap_key"
+	fi
+	if [[ "${nvmet_ctrlkey}" ]] ; then
+		echo "${nvmet_ctrlkey}" > "${host_path}/dhchap_ctrl_key"
+	fi
+}
+
 _remove_nvmet_ns() {
 	local nvmet_subsystem="$1"
 	local nsid=$2
@@ -272,6 +302,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}"
@@ -308,6 +345,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.29.2

[PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys()

[Hannes Reinecke]

From: Hannes Reinecke <hare@suse.com>

Add optional arguments for setting 'hostnqn', 'hostid', 'hostkey',
and 'ctrlkey' to _nvme_connect_subsys() to make it usable for
testing nvme in-band authentication.

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

diff --git a/tests/nvme/rc b/tests/nvme/rc
index 6e08b41..663d597 100644
--- a/tests/nvme/rc
+++ b/tests/nvme/rc
@@ -188,11 +188,27 @@ _nvme_connect_subsys() {
 	local subsysnqn="$2"
 	local traddr="${3:-$def_traddr}"
 	local trsvcid="${4:-$def_trsvcid}"
+	local hostnqn="${5:-$def_hostnqn}"
+	local hostid="${6:-$def_hostid}"
+	local hostkey="${7}"
+	local ctrlkey="${8}"
 
 	ARGS=(-t "${trtype}" -n "${subsysnqn}")
 	if [[ "${trtype}" != "loop" ]]; then
 		ARGS+=(-a "${traddr}" -s "${trsvcid}")
 	fi
+	if [[ "${hostnqn}" != "$def_hostnqn" ]]; then
+		ARGS+=(--hostnqn="${hostnqn}")
+	fi
+	if [[ "${hostid}" != "$def_hostid" ]]; then
+		ARGS+=(--hostid="${hostid}")
+	fi
+	if [[ -n "${hostkey}" ]]; then
+		ARGS+=(--dhchap-secret="${hostkey}")
+	fi
+	if [[ -n "${ctrlkey}" ]]; then
+		ARGS+=(--dhchap-ctrl-secret="${ctrlkey}")
+	fi
 	nvme connect "${ARGS[@]}"
 }
 
-- 
2.29.2

[PATCH 06/10] nvme/039: create authenticated connections

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/039     | 83 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/039.out |  7 ++++
 2 files changed, 90 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..4fbe7de
--- /dev/null
+++ b/tests/nvme/039
@@ -0,0 +1,83 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Create authenticated connections
+
+. tests/nvme/rc
+
+DESCRIPTION="Create authenticated connections"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local hostkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}"
+	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}"
+
+	# Test unauthenticated connection (should fail)
+	echo "Test unauthenticated connection"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trscvid}" \
+			     "${hostnqn}" "${hostid}"
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -n "$ctrldev" ] ; then
+		echo "nvme controller found"
+	fi
+
+	_nvme_disconnect_subsys "${subsys}"
+
+	# Test authenticated connection
+	echo "Test authenticated connection"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trscvid}" \
+			     "${hostnqn}" "${hostid}" "${hostkey}"
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	_nvme_disconnect_subsys "${subsys}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
+	_remove_nvmet_subsystem "${subsys}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/039.out b/tests/nvme/039.out
new file mode 100644
index 0000000..e42d2f1
--- /dev/null
+++ b/tests/nvme/039.out
@@ -0,0 +1,7 @@
+Running nvme/039
+Test unauthenticated connection
+no controller found
+NQN:blktests-subsystem-1 disconnected 0 controller(s)
+Test authenticated connection
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.29.2

[PATCH 07/10] nvme/040: test dhchap key types for authenticated connections

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/040     | 95 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/040.out | 16 ++++++++
 2 files changed, 111 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..cebc58b
--- /dev/null
+++ b/tests/nvme/040
@@ -0,0 +1,95 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test dhchap key types for authenticated connections
+
+. tests/nvme/rc
+
+DESCRIPTION="Test dhchap key types for authenticated connections"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local hostkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}"
+	_create_nvmet_host "${subsys}" "${hostnqn}"
+
+	for hmac in 0 1 2 3; do
+		echo "Testing hmac ${hmac}"
+		hostkey="$(nvme gen-dhchap-key --hmac=${hmac} -n ${subsys} 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_subsys "${nvme_trtype}" "${subsys}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+
+		ctrldev=$(_find_nvme_dev "${subsys}")
+		if [ -z "$ctrldev" ] ; then
+			echo "nvme controller not found"
+		fi
+
+		_nvme_disconnect_subsys "${subsys}"
+	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} 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_subsys "${nvme_trtype}" "${subsys}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+
+		ctrldev=$(_find_nvme_dev "${subsys}")
+		if [ -z "$ctrldev" ] ; then
+			echo "nvme controller not found"
+		fi
+
+		_nvme_disconnect_subsys "${subsys}"
+	done
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
+	_remove_nvmet_subsystem "${subsys}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/040.out b/tests/nvme/040.out
new file mode 100644
index 0000000..3643da4
--- /dev/null
+++ b/tests/nvme/040.out
@@ -0,0 +1,16 @@
+Running nvme/040
+Testing hmac 0
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 1
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 2
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hmac 3
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 32
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 48
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing key length 64
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.29.2

[PATCH 08/10] nvme/041: test hash and dh group variations for authenticated connections

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/041     | 89 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/041.out | 18 ++++++++++
 2 files changed, 107 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..034c715
--- /dev/null
+++ b/tests/nvme/041
@@ -0,0 +1,89 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test hash and dh group variations for authenticated connections
+
+. tests/nvme/rc
+
+DESCRIPTION="Test hash and DH group variations for authenticated connections"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local hostkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	_setup_nvmet
+
+	truncate -s 512M "${scratch}"
+
+	port="$(_create_nvmet_port "${nvme_trtype}")"
+
+	_create_nvmet_subsystem "${subsys}" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}"
+	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}"
+
+	for hash in "hmac(sha256)" "hmac(sha384)" "hmac(sha512)" ; do
+
+		echo "Testing hash ${hash}"
+
+		_set_nvmet_hash "${hostnqn}" "${hash}"
+
+		_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+
+		ctrldev=$(_find_nvme_dev "${subsys}")
+		if [ -z "$ctrldev" ] ; then
+			echo "nvme controller not found"
+		fi
+
+		_nvme_disconnect_subsys "${subsys}"
+	done
+
+	for dhgroup in "ffdhe2048" "ffdhe3072" "ffdhe4096" "ffdhe6144" "ffdhe8192" ; do
+
+		echo "Testing DH group ${dhgroup}"
+
+		_set_nvmet_dhgroup "${hostnqn}" "${dhgroup}"
+
+		_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+				     "${def_traddr}" "${def_trsvcid}" \
+				     "${hostnqn}" "${hostid}" \
+				     "${hostkey}"
+
+		ctrldev=$(_find_nvme_dev "${subsys}")
+		if [ -z "$ctrldev" ] ; then
+			echo "nvme controller not found"
+		fi
+
+		_nvme_disconnect_subsys "${subsys}"
+	done
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
+	_remove_nvmet_subsystem "${subsys}"
+
+	_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..e8558d3
--- /dev/null
+++ b/tests/nvme/041.out
@@ -0,0 +1,18 @@
+Running nvme/041
+Testing hash hmac(sha256)
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hash hmac(sha384)
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing hash hmac(sha512)
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe2048
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe3072
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe4096
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe6144
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Testing DH group ffdhe8192
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.29.2

[PATCH 09/10] nvme/042: test bi-directional authentication

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/042     | 107 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/042.out |   8 ++++
 2 files changed, 115 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..f3954b4
--- /dev/null
+++ b/tests/nvme/042
@@ -0,0 +1,107 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test bi-directional authentication
+
+. tests/nvme/rc
+
+DESCRIPTION="Test bi-directional authentication"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+}
+
+
+test() {
+	local port
+	local subsys="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local hostkey
+	local ctrlkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	ctrlkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}"
+	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}" "${ctrlkey}"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
+
+	# Step 1: Connect with host authentication only
+	echo "Test host authentication"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}"
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	_nvme_disconnect_subsys "${subsys}"
+
+	# Step 2: Connect with host authentication
+	# and invalid ctrl authentication
+	echo "Test host authentication and invalid ctrl authentication"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}" "${hostkey}"
+	ctrldev=$(_find_nvme_dev "${subsys}1")
+	if [ -n "${ctrldev}" ] ; then
+		echo "nvme controller found!"
+		_nvme_disconnect_subsys "${ctrldev}"
+	fi
+
+	# Step 3: Connect with host authentication
+	# and valid ctrl authentication
+	echo "Test host authentication and valid ctrl authentication"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}" "${ctrlkey}"
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	_nvme_disconnect_subsys "${subsys}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
+	_remove_nvmet_subsystem "${subsys}"
+
+	_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..17a461b
--- /dev/null
+++ b/tests/nvme/042.out
@@ -0,0 +1,8 @@
+Running nvme/042
+Test host authentication
+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
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.29.2

[PATCH 10/10] nvme/043: test re-authentication

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/043     | 136 +++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/043.out |  12 ++++
 2 files changed, 148 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..c217ade
--- /dev/null
+++ b/tests/nvme/043
@@ -0,0 +1,136 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Test re-authentication
+
+. tests/nvme/rc
+
+DESCRIPTION="Test re-authentication"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local hostkey
+	local ctrlkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
+	if [ $? -ne 0 ] ; then
+		echo "nvme gen-dhchap-key command missing"
+		return 1
+	fi
+
+	ctrlkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}"
+	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}" "${ctrlkey}"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
+
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trsvcid}" \
+			     "${hostnqn}" "${hostid}" \
+			     "${hostkey}" "${ctrlkey}"
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	echo "Re-authenticate with original host key"
+
+	echo "${hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	echo "Renew host key on the controller"
+
+	new_hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
+
+	_set_nvmet_hostkey "${hostnqn}" "${new_hostkey}"
+
+	echo "Re-authenticate with new host key"
+
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	echo "Renew ctrl key on the controller"
+
+	new_ctrlkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
+
+	_set_nvmet_ctrlkey "${hostnqn}" "${new_ctrlkey}"
+
+	echo "Re-authenticate with new ctrl key"
+
+	echo "${new_ctrlkey}" > /sys/class/nvme/${ctrldev}/dhchap_ctrl_secret
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	echo "Change DH group to ffdhe8192"
+
+	_set_nvmet_dhgroup "${hostnqn}" "ffdhe8192"
+
+	echo "Re-authenticate with changed DH group"
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	echo "Change hash to hmac(sha512)"
+
+	_set_nvmet_hash "${hostnqn}" "hmac(sha512)"
+
+	echo "Re-authenticate with changed hash"
+	echo "${new_hostkey}" > /sys/class/nvme/${ctrldev}/dhchap_secret
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	_nvme_disconnect_subsys "${subsys}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
+	_remove_nvmet_subsystem "${subsys}"
+
+	_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..a0c5022
--- /dev/null
+++ b/tests/nvme/043.out
@@ -0,0 +1,12 @@
+Running nvme/043
+Re-authenticate with original host key
+Renew host key on the controller
+Re-authenticate with new host key
+Renew ctrl key on the controller
+Re-authenticate with new ctrl key
+Change DH group to ffdhe8192
+Re-authenticate with changed DH group
+Change hash to hmac(sha512)
+Re-authenticate with changed hash
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.29.2

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Sagi Grimberg]

On 11/23/21 9:49 AM, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/042     | 107 +++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/042.out |   8 ++++
>   2 files changed, 115 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..f3954b4
> --- /dev/null
> +++ b/tests/nvme/042
> @@ -0,0 +1,107 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Test bi-directional authentication
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Test bi-directional authentication"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_modules loop
> +	_require_nvme_trtype_is_fabrics
> +}
> +
> +
> +test() {
> +	local port
> +	local subsys="blktests-subsystem-1"
> +	local hostid="$(uuidgen)"
> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
> +	local scratch="/tmp/blktest-ns1.img"
> +	local hostkey
> +	local ctrlkey
> +	local ctrldev
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		echo "nvme gen-dhchap-key command missing"
> +		return 1
> +	fi
> +
> +	ctrlkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
> +	_add_nvmet_subsys_to_port "${port}" "${subsys}"
> +	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}" "${ctrlkey}"
> +
> +	_set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
> +
> +	# Step 1: Connect with host authentication only
> +	echo "Test host authentication"
> +	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
> +			     "${def_traddr}" "${def_trsvcid}" \
> +			     "${hostnqn}" "${hostid}" \
> +			     "${hostkey}"
> +
> +	ctrldev=$(_find_nvme_dev "${subsys}")
> +	if [ -z "$ctrldev" ] ; then
> +		echo "nvme controller not found"
> +	fi
> +
> +	_nvme_disconnect_subsys "${subsys}"
> +
> +	# Step 2: Connect with host authentication
> +	# and invalid ctrl authentication
> +	echo "Test host authentication and invalid ctrl authentication"
> +	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
> +			     "${def_traddr}" "${def_trsvcid}" \
> +			     "${hostnqn}" "${hostid}" \
> +			     "${hostkey}" "${hostkey}"
> +	ctrldev=$(_find_nvme_dev "${subsys}1")
> +	if [ -n "${ctrldev}" ] ; then
> +		echo "nvme controller found!"
> +		_nvme_disconnect_subsys "${ctrldev}"
> +	fi

This test case has a weird behavior. Why isn't _nvme_connect_subsys
failing?

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Hannes Reinecke]

On 11/28/21 2:36 PM, Sagi Grimberg wrote:
> 
> 
> On 11/23/21 9:49 AM, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>   tests/nvme/042     | 107 +++++++++++++++++++++++++++++++++++++++++++++
>>   tests/nvme/042.out |   8 ++++
>>   2 files changed, 115 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..f3954b4
>> --- /dev/null
>> +++ b/tests/nvme/042
>> @@ -0,0 +1,107 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Test bi-directional authentication
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Test bi-directional authentication"
>> +QUICK=1
>> +
>> +requires() {
>> +    _nvme_requires
>> +    _have_modules loop
>> +    _require_nvme_trtype_is_fabrics
>> +}
>> +
>> +
>> +test() {
>> +    local port
>> +    local subsys="blktests-subsystem-1"
>> +    local hostid="$(uuidgen)"
>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>> +    local scratch="/tmp/blktest-ns1.img"
>> +    local hostkey
>> +    local ctrlkey
>> +    local ctrldev
>> +
>> +    echo "Running ${TEST_NAME}"
>> +
>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
>> +    if [ $? -ne 0 ] ; then
>> +        echo "nvme gen-dhchap-key command missing"
>> +        return 1
>> +    fi
>> +
>> +    ctrlkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
>> +    _add_nvmet_subsys_to_port "${port}" "${subsys}"
>> +    _create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}" 
>> "${ctrlkey}"
>> +
>> +    _set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
>> +
>> +    # Step 1: Connect with host authentication only
>> +    echo "Test host authentication"
>> +    _nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
>> +                 "${def_traddr}" "${def_trsvcid}" \
>> +                 "${hostnqn}" "${hostid}" \
>> +                 "${hostkey}"
>> +
>> +    ctrldev=$(_find_nvme_dev "${subsys}")
>> +    if [ -z "$ctrldev" ] ; then
>> +        echo "nvme controller not found"
>> +    fi
>> +
>> +    _nvme_disconnect_subsys "${subsys}"
>> +
>> +    # Step 2: Connect with host authentication
>> +    # and invalid ctrl authentication
>> +    echo "Test host authentication and invalid ctrl authentication"
>> +    _nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
>> +                 "${def_traddr}" "${def_trsvcid}" \
>> +                 "${hostnqn}" "${hostid}" \
>> +                 "${hostkey}" "${hostkey}"
>> +    ctrldev=$(_find_nvme_dev "${subsys}1")
>> +    if [ -n "${ctrldev}" ] ; then
>> +        echo "nvme controller found!"
>> +        _nvme_disconnect_subsys "${ctrldev}"
>> +    fi
> 
> This test case has a weird behavior. Why isn't _nvme_connect_subsys
> failing?

Oh, but it is.
The line 'nvme controller found' is not part of the .out file, hence the 
test will fail.

Call it an obscure feature of blktests :-)

But yeah, I can make it more obvious that this is the failure case.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Sagi Grimberg]

On 11/28/21 7:34 PM, Hannes Reinecke wrote:
> On 11/28/21 2:36 PM, Sagi Grimberg wrote:
>>
>>
>> On 11/23/21 9:49 AM, Hannes Reinecke wrote:
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>> ---
>>>   tests/nvme/042     | 107 +++++++++++++++++++++++++++++++++++++++++++++
>>>   tests/nvme/042.out |   8 ++++
>>>   2 files changed, 115 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..f3954b4
>>> --- /dev/null
>>> +++ b/tests/nvme/042
>>> @@ -0,0 +1,107 @@
>>> +#!/bin/bash
>>> +# SPDX-License-Identifier: GPL-3.0+
>>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>>> +#
>>> +# Test bi-directional authentication
>>> +
>>> +. tests/nvme/rc
>>> +
>>> +DESCRIPTION="Test bi-directional authentication"
>>> +QUICK=1
>>> +
>>> +requires() {
>>> +    _nvme_requires
>>> +    _have_modules loop
>>> +    _require_nvme_trtype_is_fabrics
>>> +}
>>> +
>>> +
>>> +test() {
>>> +    local port
>>> +    local subsys="blktests-subsystem-1"
>>> +    local hostid="$(uuidgen)"
>>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>>> +    local scratch="/tmp/blktest-ns1.img"
>>> +    local hostkey
>>> +    local ctrlkey
>>> +    local ctrldev
>>> +
>>> +    echo "Running ${TEST_NAME}"
>>> +
>>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
>>> +    if [ $? -ne 0 ] ; then
>>> +        echo "nvme gen-dhchap-key command missing"
>>> +        return 1
>>> +    fi
>>> +
>>> +    ctrlkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
>>> +    _add_nvmet_subsys_to_port "${port}" "${subsys}"
>>> +    _create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}" 
>>> "${ctrlkey}"
>>> +
>>> +    _set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
>>> +
>>> +    # Step 1: Connect with host authentication only
>>> +    echo "Test host authentication"
>>> +    _nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
>>> +                 "${def_traddr}" "${def_trsvcid}" \
>>> +                 "${hostnqn}" "${hostid}" \
>>> +                 "${hostkey}"
>>> +
>>> +    ctrldev=$(_find_nvme_dev "${subsys}")
>>> +    if [ -z "$ctrldev" ] ; then
>>> +        echo "nvme controller not found"
>>> +    fi
>>> +
>>> +    _nvme_disconnect_subsys "${subsys}"
>>> +
>>> +    # Step 2: Connect with host authentication
>>> +    # and invalid ctrl authentication
>>> +    echo "Test host authentication and invalid ctrl authentication"
>>> +    _nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
>>> +                 "${def_traddr}" "${def_trsvcid}" \
>>> +                 "${hostnqn}" "${hostid}" \
>>> +                 "${hostkey}" "${hostkey}"
>>> +    ctrldev=$(_find_nvme_dev "${subsys}1")
>>> +    if [ -n "${ctrldev}" ] ; then
>>> +        echo "nvme controller found!"
>>> +        _nvme_disconnect_subsys "${ctrldev}"
>>> +    fi
>>
>> This test case has a weird behavior. Why isn't _nvme_connect_subsys
>> failing?
> 
> Oh, but it is.
> The line 'nvme controller found' is not part of the .out file, hence the 
> test will fail.
> 
> Call it an obscure feature of blktests :-)
> 
> But yeah, I can make it more obvious that this is the failure case.

Hannes, my question is why isn't nvme connect not failing (i.e.
returning a non-zero status code)?

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Hannes Reinecke]

On 12/8/21 2:09 PM, Sagi Grimberg wrote:
> 
> 
> On 11/28/21 7:34 PM, Hannes Reinecke wrote:
>> On 11/28/21 2:36 PM, Sagi Grimberg wrote:
>>>
>>>
>>> On 11/23/21 9:49 AM, Hannes Reinecke wrote:
>>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>>> ---
>>>>   tests/nvme/042     | 107
>>>> +++++++++++++++++++++++++++++++++++++++++++++
>>>>   tests/nvme/042.out |   8 ++++
>>>>   2 files changed, 115 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..f3954b4
>>>> --- /dev/null
>>>> +++ b/tests/nvme/042
>>>> @@ -0,0 +1,107 @@
>>>> +#!/bin/bash
>>>> +# SPDX-License-Identifier: GPL-3.0+
>>>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>>>> +#
>>>> +# Test bi-directional authentication
>>>> +
>>>> +. tests/nvme/rc
>>>> +
>>>> +DESCRIPTION="Test bi-directional authentication"
>>>> +QUICK=1
>>>> +
>>>> +requires() {
>>>> +    _nvme_requires
>>>> +    _have_modules loop
>>>> +    _require_nvme_trtype_is_fabrics
>>>> +}
>>>> +
>>>> +
>>>> +test() {
>>>> +    local port
>>>> +    local subsys="blktests-subsystem-1"
>>>> +    local hostid="$(uuidgen)"
>>>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>>>> +    local scratch="/tmp/blktest-ns1.img"
>>>> +    local hostkey
>>>> +    local ctrlkey
>>>> +    local ctrldev
>>>> +
>>>> +    echo "Running ${TEST_NAME}"
>>>> +
>>>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
>>>> +    if [ $? -ne 0 ] ; then
>>>> +        echo "nvme gen-dhchap-key command missing"
>>>> +        return 1
>>>> +    fi
>>>> +
>>>> +    ctrlkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
>>>> +    _add_nvmet_subsys_to_port "${port}" "${subsys}"
>>>> +    _create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}"
>>>> "${ctrlkey}"
>>>> +
>>>> +    _set_nvmet_dhgroup "${hostnqn}" "ffdhe2048"
>>>> +
>>>> +    # Step 1: Connect with host authentication only
>>>> +    echo "Test host authentication"
>>>> +    _nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
>>>> +                 "${def_traddr}" "${def_trsvcid}" \
>>>> +                 "${hostnqn}" "${hostid}" \
>>>> +                 "${hostkey}"
>>>> +
>>>> +    ctrldev=$(_find_nvme_dev "${subsys}")
>>>> +    if [ -z "$ctrldev" ] ; then
>>>> +        echo "nvme controller not found"
>>>> +    fi
>>>> +
>>>> +    _nvme_disconnect_subsys "${subsys}"
>>>> +
>>>> +    # Step 2: Connect with host authentication
>>>> +    # and invalid ctrl authentication
>>>> +    echo "Test host authentication and invalid ctrl authentication"
>>>> +    _nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
>>>> +                 "${def_traddr}" "${def_trsvcid}" \
>>>> +                 "${hostnqn}" "${hostid}" \
>>>> +                 "${hostkey}" "${hostkey}"
>>>> +    ctrldev=$(_find_nvme_dev "${subsys}1")
>>>> +    if [ -n "${ctrldev}" ] ; then
>>>> +        echo "nvme controller found!"
>>>> +        _nvme_disconnect_subsys "${ctrldev}"
>>>> +    fi
>>>
>>> This test case has a weird behavior. Why isn't _nvme_connect_subsys
>>> failing?
>>
>> Oh, but it is.
>> The line 'nvme controller found' is not part of the .out file, hence
>> the test will fail.
>>
>> Call it an obscure feature of blktests :-)
>>
>> But yeah, I can make it more obvious that this is the failure case.
> 
> Hannes, my question is why isn't nvme connect not failing (i.e.
> returning a non-zero status code)?
> 
I guess it does, but we're never actually checking the return value.
Let me check.

Cheers,

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

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Sagi Grimberg]

>>>> This test case has a weird behavior. Why isn't _nvme_connect_subsys
>>>> failing?
>>>
>>> Oh, but it is.
>>> The line 'nvme controller found' is not part of the .out file, hence
>>> the test will fail.
>>>
>>> Call it an obscure feature of blktests :-)
>>>
>>> But yeah, I can make it more obvious that this is the failure case.
>>
>> Hannes, my question is why isn't nvme connect not failing (i.e.
>> returning a non-zero status code)?
>>
> I guess it does, but we're never actually checking the return value.
> Let me check.

I tested it manually, and the connect does not fail, but rather succeeds
and the controller is removed - which is strange.

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Hannes Reinecke]

On 12/12/21 10:53 AM, Sagi Grimberg wrote:
> 
>>>>> This test case has a weird behavior. Why isn't _nvme_connect_subsys
>>>>> failing?
>>>>
>>>> Oh, but it is.
>>>> The line 'nvme controller found' is not part of the .out file, hence
>>>> the test will fail.
>>>>
>>>> Call it an obscure feature of blktests :-)
>>>>
>>>> But yeah, I can make it more obvious that this is the failure case.
>>>
>>> Hannes, my question is why isn't nvme connect not failing (i.e.
>>> returning a non-zero status code)?
>>>
>> I guess it does, but we're never actually checking the return value.
>> Let me check.
> 
> I tested it manually, and the connect does not fail, but rather succeeds
> and the controller is removed - which is strange.

Curious. "That shouldn't have happened (tm)".

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

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

[Chaitanya Kulkarni]

On 11/22/21 11:49 PM, Hannes Reinecke wrote:
> _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>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Re: [PATCH 02/10] nvme/rc: clear allowed_hosts subdirectory

[Chaitanya Kulkarni]

On 11/22/21 11:49 PM, Hannes Reinecke wrote:
> 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>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Re: [PATCH 03/10] nvme/rc: clear hosts directory in _cleanup_nvmet()

[Chaitanya Kulkarni]

On 11/22/21 11:49 PM, Hannes Reinecke wrote:
> From: Hannes Reinecke <hare@suse.com>
> 
> When clearing out the configuration we need to clear the 'hosts'
> directory, too, as scripts might have added host configurations.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Re: [PATCH 04/10] nvme/rc: add functions for in-band authentication

[Chaitanya Kulkarni]

On 11/22/21 11:49 PM, Hannes Reinecke wrote:
> Add functions to enable in-band authentication.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

> ---
>   tests/nvme/rc | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 73 insertions(+)
> 
> diff --git a/tests/nvme/rc b/tests/nvme/rc
> index c284602..6e08b41 100644
> --- a/tests/nvme/rc
> +++ b/tests/nvme/rc
> @@ -73,6 +73,17 @@ _require_nvme_trtype_is_fabrics() {
>   	return 0
>   }
>   
> +_require_nvme_cli_auth() {
> +	local hostkey
> +
> +	hostkey="$(nvme gen-dhchap-key -n nvmf-test-subsys 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		SKIP_REASON="nvme gen-dhchap-key command missing"
> +		return 1
> +	fi
> +	return 0
> +}
> +
>   _test_dev_nvme_ctrl() {
>   	echo "/dev/char/$(cat "${TEST_DEV_SYSFS}/device/dev")"
>   }
> @@ -253,6 +264,25 @@ _create_nvmet_subsystem() {
>   	_create_nvmet_ns "${nvmet_subsystem}" "1" "${blkdev}" "${uuid}"
>   }
>   
> +_create_nvmet_host() {
> +	local nvmet_subsystem="$1"
> +	local nvmet_hostnqn="$2"
> +	local nvmet_hostkey="$3"
> +	local nvmet_ctrlkey="$4"
> +	local cfs_path="${NVMET_CFS}/subsystems/${nvmet_subsystem}"
> +	local host_path="${NVMET_CFS}/hosts/${nvmet_hostnqn}"
> +
> +	mkdir "${host_path}"

perhaps check return value of above mkdir and abort early ?

> +	echo 0 > "${cfs_path}/attr_allow_any_host"
> +	ln -s "${host_path}" "${cfs_path}/allowed_hosts/${nvmet_hostnqn}"
> +	if [[ "${nvmet_hostkey}" ]] ; then
> +		echo "${nvmet_hostkey}" > "${host_path}/dhchap_key"
> +	fi
> +	if [[ "${nvmet_ctrlkey}" ]] ; then
> +		echo "${nvmet_ctrlkey}" > "${host_path}/dhchap_ctrl_key"
> +	fi
> +}
> +
>   _remove_nvmet_ns() {
>   	local nvmet_subsystem="$1"
>   	local nsid=$2
> @@ -272,6 +302,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}"
> @@ -308,6 +345,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
> 

Re: [PATCH 05/10] nvme/rc: add more arguments to _nvme_connect_subsys()

[Chaitanya Kulkarni]

On 11/22/21 11:49 PM, Hannes Reinecke wrote:
> From: Hannes Reinecke <hare@suse.com>
> 
> Add optional arguments for setting 'hostnqn', 'hostid', 'hostkey',
> and 'ctrlkey' to _nvme_connect_subsys() to make it usable for
> testing nvme in-band authentication.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---

Looks good.

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

Re: [PATCH 06/10] nvme/039: create authenticated connections

[Chaitanya Kulkarni]

On 11/22/21 11:49 PM, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/039     | 83 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/039.out |  7 ++++
>   2 files changed, 90 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..4fbe7de
> --- /dev/null
> +++ b/tests/nvme/039
> @@ -0,0 +1,83 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Create authenticated connections
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Create authenticated connections"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_modules loop
> +	_require_nvme_trtype_is_fabrics
> +	_require_nvme_cli_auth
> +}
> +
> +
> +test() {
> +	local port
> +	local subsys="blktests-subsystem-1"
> +	local hostid="$(uuidgen)"
> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
> +	local scratch="/tmp/blktest-ns1.img"
> +	local hostkey
> +	local ctrldev
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		echo "nvme gen-dhchap-key command missing"
> +		return 1
> +	fi
> +

We add following in the patch 4 that is called in requires of this
testcase, by any chance above check for gen-dhcap-key is duplicate ? :-

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

Re: [PATCH 06/10] nvme/039: create authenticated connections

[Chaitanya Kulkarni]

On 11/22/21 11:49 PM, Hannes Reinecke wrote:
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
>   tests/nvme/039     | 83 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/nvme/039.out |  7 ++++
>   2 files changed, 90 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..4fbe7de
> --- /dev/null
> +++ b/tests/nvme/039
> @@ -0,0 +1,83 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-3.0+
> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
> +#
> +# Create authenticated connections
> +
> +. tests/nvme/rc
> +
> +DESCRIPTION="Create authenticated connections"
> +QUICK=1
> +
> +requires() {
> +	_nvme_requires
> +	_have_modules loop
> +	_require_nvme_trtype_is_fabrics
> +	_require_nvme_cli_auth
> +}
> +
> +
> +test() {
> +	local port
> +	local subsys="blktests-subsystem-1"
> +	local hostid="$(uuidgen)"
> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
> +	local scratch="/tmp/blktest-ns1.img"
> +	local hostkey
> +	local ctrldev
> +
> +	echo "Running ${TEST_NAME}"
> +
> +	hostkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
> +	_add_nvmet_subsys_to_port "${port}" "${subsys}"
> +	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}"
> +

Starting from _setup_nvmet() to _create_nvmet_host() seems to be 
duplicated in most of the cases (39/40/41/42/43).

Let's avoid the code duplication by creating a helper in nvme/rc ?

Re: [PATCH 06/10] nvme/039: create authenticated connections

[Chaitanya Kulkarni]

> +
> +	_nvme_disconnect_subsys "${subsys}"
> +
> +	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
> +	_remove_nvmet_subsystem "${subsys}"
> +
> +	_remove_nvmet_port "${port}"
> +
> +	_remove_nvmet_host "${hostnqn}"
> +
> +	rm ${scratch}
> +

The above sequence is also worth moving to helper in nvme/rc
to avoid the code duplication in all the existing and future
testcases.

Re: [PATCH 06/10] nvme/039: create authenticated connections

[Hannes Reinecke]

On 12/13/21 11:55 PM, Chaitanya Kulkarni wrote:
> On 11/22/21 11:49 PM, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>    tests/nvme/039     | 83 ++++++++++++++++++++++++++++++++++++++++++++++
>>    tests/nvme/039.out |  7 ++++
>>    2 files changed, 90 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..4fbe7de
>> --- /dev/null
>> +++ b/tests/nvme/039
>> @@ -0,0 +1,83 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Create authenticated connections
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Create authenticated connections"
>> +QUICK=1
>> +
>> +requires() {
>> +	_nvme_requires
>> +	_have_modules loop
>> +	_require_nvme_trtype_is_fabrics
>> +	_require_nvme_cli_auth
>> +}
>> +
>> +
>> +test() {
>> +	local port
>> +	local subsys="blktests-subsystem-1"
>> +	local hostid="$(uuidgen)"
>> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>> +	local scratch="/tmp/blktest-ns1.img"
>> +	local hostkey
>> +	local ctrldev
>> +
>> +	echo "Running ${TEST_NAME}"
>> +
>> +	hostkey="$(nvme gen-dhchap-key -n ${subsys} 2> /dev/null)"
>> +	if [ $? -ne 0 ] ; then
>> +		echo "nvme gen-dhchap-key command missing"
>> +		return 1
>> +	fi
>> +
> 
> We add following in the patch 4 that is called in requires of this
> testcase, by any chance above check for gen-dhcap-key is duplicate ? :-
> 
> +_require_nvme_cli_auth() {
> +	local hostkey
> +
> +	hostkey="$(nvme gen-dhchap-key -n nvmf-test-subsys 2> /dev/null)"
> +	if [ $? -ne 0 ] ; then
> +		SKIP_REASON="nvme gen-dhchap-key command missing"
> +		return 1
> +	fi
> +	return 0
> +}
> +
> 
You are right, this can be removed.

Will be resubmitting.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 06/10] nvme/039: create authenticated connections

[Hannes Reinecke]

On 12/14/21 12:01 AM, Chaitanya Kulkarni wrote:
> On 11/22/21 11:49 PM, Hannes Reinecke wrote:
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> ---
>>    tests/nvme/039     | 83 ++++++++++++++++++++++++++++++++++++++++++++++
>>    tests/nvme/039.out |  7 ++++
>>    2 files changed, 90 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..4fbe7de
>> --- /dev/null
>> +++ b/tests/nvme/039
>> @@ -0,0 +1,83 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-3.0+
>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>> +#
>> +# Create authenticated connections
>> +
>> +. tests/nvme/rc
>> +
>> +DESCRIPTION="Create authenticated connections"
>> +QUICK=1
>> +
>> +requires() {
>> +	_nvme_requires
>> +	_have_modules loop
>> +	_require_nvme_trtype_is_fabrics
>> +	_require_nvme_cli_auth
>> +}
>> +
>> +
>> +test() {
>> +	local port
>> +	local subsys="blktests-subsystem-1"
>> +	local hostid="$(uuidgen)"
>> +	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>> +	local scratch="/tmp/blktest-ns1.img"
>> +	local hostkey
>> +	local ctrldev
>> +
>> +	echo "Running ${TEST_NAME}"
>> +
>> +	hostkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
>> +	_add_nvmet_subsys_to_port "${port}" "${subsys}"
>> +	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}"
>> +
> 
> Starting from _setup_nvmet() to _create_nvmet_host() seems to be
> duplicated in most of the cases (39/40/41/42/43).
> 
> Let's avoid the code duplication by creating a helper in nvme/rc ?
> 
I deliberately did not create a single helper here, as I'm not a fan of 
functions with thousands of arguments (we would need at least 5 
arguments here).
And also it allows you to expand that by creating two ports to the same 
subsystem etc.

So I'd rather keep it that way.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer

Re: [PATCH 06/10] nvme/039: create authenticated connections

[Chaitanya Kulkarni]

On 12/13/21 11:16 PM, Hannes Reinecke wrote:
> External email: Use caution opening links or attachments
> 
> 
> On 12/14/21 12:01 AM, Chaitanya Kulkarni wrote:
>> On 11/22/21 11:49 PM, Hannes Reinecke wrote:
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>> ---
>>>    tests/nvme/039     | 83 
>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>    tests/nvme/039.out |  7 ++++
>>>    2 files changed, 90 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..4fbe7de
>>> --- /dev/null
>>> +++ b/tests/nvme/039
>>> @@ -0,0 +1,83 @@
>>> +#!/bin/bash
>>> +# SPDX-License-Identifier: GPL-3.0+
>>> +# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
>>> +#
>>> +# Create authenticated connections
>>> +
>>> +. tests/nvme/rc
>>> +
>>> +DESCRIPTION="Create authenticated connections"
>>> +QUICK=1
>>> +
>>> +requires() {
>>> +    _nvme_requires
>>> +    _have_modules loop
>>> +    _require_nvme_trtype_is_fabrics
>>> +    _require_nvme_cli_auth
>>> +}
>>> +
>>> +
>>> +test() {
>>> +    local port
>>> +    local subsys="blktests-subsystem-1"
>>> +    local hostid="$(uuidgen)"
>>> +    local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
>>> +    local scratch="/tmp/blktest-ns1.img"
>>> +    local hostkey
>>> +    local ctrldev
>>> +
>>> +    echo "Running ${TEST_NAME}"
>>> +
>>> +    hostkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
>>> +    _add_nvmet_subsys_to_port "${port}" "${subsys}"
>>> +    _create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}"
>>> +
>>
>> Starting from _setup_nvmet() to _create_nvmet_host() seems to be
>> duplicated in most of the cases (39/40/41/42/43).
>>
>> Let's avoid the code duplication by creating a helper in nvme/rc ?
>>
> I deliberately did not create a single helper here, as I'm not a fan of
> functions with thousands of arguments (we would need at least 5
> arguments here).
> And also it allows you to expand that by creating two ports to the same
> subsystem etc.
> 
> So I'd rather keep it that way.
> 

Right now there are 5 testcases, I'm sure we will endup adding
more testcases since in-band auth is a big chunk of code (thanks to
you :)), it will get duplicated, why maintain duplicated code ?

Regarding number of argument subsys, scratch variables have static 
values that are used in all the testcases (39,40,41,42,43).
Can we just move those to nvme/rc ? with that we'll only have to
pass 5-2=3 parameters to the helper which is pretty reasonable.

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Sagi Grimberg]

>>>>>> This test case has a weird behavior. Why isn't _nvme_connect_subsys
>>>>>> failing?
>>>>>
>>>>> Oh, but it is.
>>>>> The line 'nvme controller found' is not part of the .out file, hence
>>>>> the test will fail.
>>>>>
>>>>> Call it an obscure feature of blktests :-)
>>>>>
>>>>> But yeah, I can make it more obvious that this is the failure case.
>>>>
>>>> Hannes, my question is why isn't nvme connect not failing (i.e.
>>>> returning a non-zero status code)?
>>>>
>>> I guess it does, but we're never actually checking the return value.
>>> Let me check.
>>
>> I tested it manually, and the connect does not fail, but rather succeeds
>> and the controller is removed - which is strange.
> 
> Curious. "That shouldn't have happened (tm)".

Hannes, before I take v10 to a test drive, did you fix this particular
issue?

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Hannes Reinecke]

On 3/28/22 13:20, Sagi Grimberg wrote:
> 
>>>>>>> This test case has a weird behavior. Why isn't _nvme_connect_subsys
>>>>>>> failing?
>>>>>>
>>>>>> Oh, but it is.
>>>>>> The line 'nvme controller found' is not part of the .out file, hence
>>>>>> the test will fail.
>>>>>>
>>>>>> Call it an obscure feature of blktests :-)
>>>>>>
>>>>>> But yeah, I can make it more obvious that this is the failure case.
>>>>>
>>>>> Hannes, my question is why isn't nvme connect not failing (i.e.
>>>>> returning a non-zero status code)?
>>>>>
>>>> I guess it does, but we're never actually checking the return value.
>>>> Let me check.
>>>
>>> I tested it manually, and the connect does not fail, but rather succeeds
>>> and the controller is removed - which is strange.
>>
>> Curious. "That shouldn't have happened (tm)".
> 
> Hannes, before I take v10 to a test drive, did you fix this particular
> issue?

Now that you mention it: Not sure. Will be checking.

Cheers,

Hannes

Re: [PATCH 09/10] nvme/042: test bi-directional authentication

[Hannes Reinecke]

On 3/28/22 14:02, Hannes Reinecke wrote:
> On 3/28/22 13:20, Sagi Grimberg wrote:
>>
>>>>>>>> This test case has a weird behavior. Why isn't _nvme_connect_subsys
>>>>>>>> failing?
>>>>>>>
>>>>>>> Oh, but it is.
>>>>>>> The line 'nvme controller found' is not part of the .out file, hence
>>>>>>> the test will fail.
>>>>>>>
>>>>>>> Call it an obscure feature of blktests :-)
>>>>>>>
>>>>>>> But yeah, I can make it more obvious that this is the failure case.
>>>>>>
>>>>>> Hannes, my question is why isn't nvme connect not failing (i.e.
>>>>>> returning a non-zero status code)?
>>>>>>
>>>>> I guess it does, but we're never actually checking the return value.
>>>>> Let me check.
>>>>
>>>> I tested it manually, and the connect does not fail, but rather 
>>>> succeeds
>>>> and the controller is removed - which is strange.
>>>
>>> Curious. "That shouldn't have happened (tm)".
>>
>> Hannes, before I take v10 to a test drive, did you fix this particular
>> issue?
> 
> Now that you mention it: Not sure. Will be checking.
> 
Found it. One shouldn't overwrite the error code with 'success' after 
sending failure2 ...

Cheers,

Hannes

Re: [PATCHv3 blktests 00/10] Testsuite for nvme in-band authentication

[Chaitanya Kulkarni]

On 11/22/21 23:49, Hannes Reinecke wrote:
> 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.
> 

In-Band authentication blktests are failing on your current
version :-

blktests (master) # nvme_trtype=loop ./check nvme/039
nvme/039 (Create authenticated connections)                  [failed]
     runtime    ...  1.646s
     --- tests/nvme/039.out	2022-06-01 12:30:34.556807396 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/039.out.bad	2022-06-05 
16:39:19.335636058 -0700
     @@ -1,6 +1,7 @@
      Running nvme/039
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Test unauthenticated connection
     -no controller found
     +no controller found: failed to write to nvme-fabrics device
      NQN:blktests-subsystem-1 disconnected 0 controller(s)
      Test authenticated connection
     ...
     (Run 'diff -u tests/nvme/039.out 
/mnt/data/blktests/results/nodev/nvme/039.out.bad' to see the entire diff)
blktests (master) # nvme_trtype=loop ./check nvme/040
nvme/040 (Test dhchap key types for authenticated connections)
nvme/040 (Test dhchap key types for authenticated connections) [failed]
     runtime    ...  11.194s
     --- tests/nvme/040.out	2022-06-01 12:30:36.681858248 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/040.out.bad	2022-06-05 
16:39:49.307314951 -0700
     @@ -1,4 +1,5 @@
      Running nvme/040
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Testing hmac 0
      NQN:blktests-subsystem-1 disconnected 1 controller(s)
      Testing hmac 1
blktests (master) #
blktests (master) # nvme_trtype=loop ./check nvme/041
nvme/041 (Test hash and DH group variations for authenticated 
connections) [failed]
     runtime    ...  14.782s
     --- tests/nvme/041.out	2022-06-01 12:30:39.065915298 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/041.out.bad	2022-06-05 
16:40:11.733822941 -0700
     @@ -1,4 +1,5 @@
      Running nvme/041
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Testing hash hmac(sha256)
      NQN:blktests-subsystem-1 disconnected 1 controller(s)
      Testing hash hmac(sha384)
blktests (master) # nvme_trtype=loop ./check nvme/043
nvme/043 (Test re-authentication)                            [failed]
     runtime    ...  6.042s
     --- tests/nvme/043.out	2022-06-01 12:30:45.089059428 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/043.out.bad	2022-06-05 
16:40:24.409110049 -0700
     @@ -1,4 +1,5 @@
      Running nvme/043
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Re-authenticate with original host key
      Renew host key on the controller
      Re-authenticate with new host key


-ck

Re: [PATCHv3 blktests 00/10] Testsuite for nvme in-band authentication

[Chaitanya Kulkarni]

On 6/5/22 16:58, Chaitanya Kulkarni wrote:
> On 11/22/21 23:49, Hannes Reinecke wrote:
>> 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.
>>
> 
> In-Band authentication blktests are failing on your current
> version :-
> 

Testcases are also failing for TCP :-

# nvme_trtype=tcp ./check nvme/043
nvme/043 (Test re-authentication)                            [failed]
     runtime  6.042s  ...  5.760s
     --- tests/nvme/043.out	2022-06-01 12:30:45.089059428 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/043.out.bad	2022-06-05 
17:32:21.177871159 -0700
     @@ -1,4 +1,5 @@
      Running nvme/043
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Re-authenticate with original host key
      Renew host key on the controller
      Re-authenticate with new host key
blktests (master) # for i in 39 40 41 42 43; do nvme_trtype=tcp ./check 
nvme/0${i}; done
nvme/039 (Create authenticated connections)                  [failed]
     runtime  1.646s  ...  1.350s
     --- tests/nvme/039.out	2022-06-01 12:30:34.556807396 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/039.out.bad	2022-06-05 
17:32:51.343541607 -0700
     @@ -1,6 +1,7 @@
      Running nvme/039
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Test unauthenticated connection
     -no controller found
     +no controller found: failed to write to nvme-fabrics device
      NQN:blktests-subsystem-1 disconnected 0 controller(s)
      Test authenticated connection
     ...
     (Run 'diff -u tests/nvme/039.out 
/mnt/data/blktests/results/nodev/nvme/039.out.bad' to see the entire diff)
nvme/040 (Test dhchap key types for authenticated connections) [failed]
     runtime  11.194s  ...  9.091s
     --- tests/nvme/040.out	2022-06-01 12:30:36.681858248 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/040.out.bad	2022-06-05 
17:33:00.570746684 -0700
     @@ -1,4 +1,5 @@
      Running nvme/040
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Testing hmac 0
      NQN:blktests-subsystem-1 disconnected 1 controller(s)
      Testing hmac 1
nvme/041 (Test hash and DH group variations for authenticated 
connections) [failed]
     runtime  14.782s  ...  8.541s
     --- tests/nvme/041.out	2022-06-01 12:30:39.065915298 -0700
     +++ /mnt/data/blktests/results/nodev/nvme/041.out.bad	2022-06-05 
17:33:09.241939406 -0700
     @@ -1,4 +1,5 @@
      Running nvme/041
     +tests/nvme/rc: line 269: printf: write error: Invalid argument
      Testing hash hmac(sha256)
      NQN:blktests-subsystem-1 disconnected 1 controller(s)
      Testing hash hmac(sha384)
nvme/042 (Test bi-directional authentication)



with trtype_tcp OOPs is observed :-

RIP: 0010:blk_mq_alloc_request_hctx+0x52/0x180
Code: 24 10 48 89 6c 24 08 f3 48 ab 89 54 24 10 89 74 24 18 c7 44 24 20 
01 00 00 00 f6 c2 03 0f 84 f8 00 00 00 48 c7 c0 fb ff ff ff <39> 5d 58 
77 1b 48 8b 54 24 40 65 48 2b 14 25 28 00 00 00 0f 85 0b
RSP: 0018:ffffc900026b3d20 EFLAGS: 00010206
RAX: fffffffffffffffb RBX: 0000000000000000 RCX: 0000000000000000
RDX: 0000000000000003 RSI: 0000000000000023 RDI: ffffc900026b3d60
RBP: 0000000000000000 R08: 0000000000000003 R09: 0000000000000000
R10: ffffc900026b3de0 R11: 0000000000000018 R12: 0000000000000000
R13: ffff88811e04a000 R14: 0000000000000000 R15: 0000000000000048
  __nvme_submit_sync_cmd+0x6b/0x1a0 [nvme_core]
  nvme_auth_submit+0x90/0xd0 [nvme_core]
  __nvme_auth_work+0xae/0xd58 [nvme_core]
  ? try_to_wake_up+0x94/0x560
  process_one_work+0x1af/0x380
  worker_thread+0x50/0x3a0
  ? rescuer_thread+0x390/0x390
  kthread+0xe7/0x110
  ? kthread_complete_and_exit+0x20/0x20
  ret_from_fork+0x22/0x30
  </TASK>
[40]kdb>


If you have not tested the code on RDMA please test it as I'm still
figuring out the setup.

-ck

[PATCH 06/10] nvme/039: create authenticated connections

[Hannes Reinecke]

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 tests/nvme/039     | 83 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/nvme/039.out |  7 ++++
 2 files changed, 90 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..4fbe7de
--- /dev/null
+++ b/tests/nvme/039
@@ -0,0 +1,83 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2021 Hannes Reinecke, SUSE Labs
+#
+# Create authenticated connections
+
+. tests/nvme/rc
+
+DESCRIPTION="Create authenticated connections"
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_modules loop
+	_require_nvme_trtype_is_fabrics
+	_require_nvme_cli_auth
+}
+
+
+test() {
+	local port
+	local subsys="blktests-subsystem-1"
+	local hostid="$(uuidgen)"
+	local hostnqn="nqn.2014-08.org.nvmexpress:uuid:${hostid}"
+	local scratch="/tmp/blktest-ns1.img"
+	local hostkey
+	local ctrldev
+
+	echo "Running ${TEST_NAME}"
+
+	hostkey="$(nvme gen-dhchap-key -n ${subsys} 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}" "${scratch}"
+	_add_nvmet_subsys_to_port "${port}" "${subsys}"
+	_create_nvmet_host "${subsys}" "${hostnqn}" "${hostkey}"
+
+	# Test unauthenticated connection (should fail)
+	echo "Test unauthenticated connection"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trscvid}" \
+			     "${hostnqn}" "${hostid}"
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -n "$ctrldev" ] ; then
+		echo "nvme controller found"
+	fi
+
+	_nvme_disconnect_subsys "${subsys}"
+
+	# Test authenticated connection
+	echo "Test authenticated connection"
+	_nvme_connect_subsys "${nvme_trtype}" "${subsys}" \
+			     "${def_traddr}" "${def_trscvid}" \
+			     "${hostnqn}" "${hostid}" "${hostkey}"
+
+	ctrldev=$(_find_nvme_dev "${subsys}")
+	if [ -z "$ctrldev" ] ; then
+		echo "nvme controller not found"
+	fi
+
+	_nvme_disconnect_subsys "${subsys}"
+
+	_remove_nvmet_subsystem_from_port "${port}" "${subsys}"
+	_remove_nvmet_subsystem "${subsys}"
+
+	_remove_nvmet_port "${port}"
+
+	_remove_nvmet_host "${hostnqn}"
+
+	rm ${scratch}
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/039.out b/tests/nvme/039.out
new file mode 100644
index 0000000..e42d2f1
--- /dev/null
+++ b/tests/nvme/039.out
@@ -0,0 +1,7 @@
+Running nvme/039
+Test unauthenticated connection
+no controller found
+NQN:blktests-subsystem-1 disconnected 0 controller(s)
+Test authenticated connection
+NQN:blktests-subsystem-1 disconnected 1 controller(s)
+Test complete
-- 
2.29.2