Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nvme: testcases for TLS support #158

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 34 additions & 2 deletions common/nvme
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ _nvme_connect_subsys() {
local no_wait=false
local hdr_digest=false
local data_digest=false
local tls=false
local concat=false
local port
local i
local -a ARGS
Expand Down Expand Up @@ -341,6 +343,14 @@ _nvme_connect_subsys() {
data_digest=true
shift 1
;;
--tls)
tls=true
shift 1
;;
--concat)
concat=true
shift 1
;;
*)
echo "WARNING: unknown argument: $1"
shift
Expand Down Expand Up @@ -398,6 +408,12 @@ _nvme_connect_subsys() {
if [[ ${data_digest} = true ]]; then
ARGS+=(--data-digest)
fi
if [[ ${tls} = true ]]; then
ARGS+=(--tls)
fi
if [[ ${concat} = true ]]; then
ARGS+=(--concat)
fi
ARGS+=(-o json)
connect=$(nvme connect "${ARGS[@]}" 2> /dev/null)

Expand Down Expand Up @@ -470,6 +486,7 @@ _fc_host_traddr() {
}

_create_nvmet_port() {
local tls="${1:-none}"
local trtype="${nvme_trtype}"
local traddr="${def_traddr}"
local adrfam="${def_adrfam}"
Expand Down Expand Up @@ -506,7 +523,13 @@ _create_nvmet_port() {
[[ "${adrfam}" != "loop" ]] ; then
echo "${trsvcid}" > "${portcfs}/addr_trsvcid"
fi

if [[ "${trtype}" == "tcp" ]] && \
[[ "${tls}" != "none" ]]; then
echo "tls1.3" > "${portcfs}/addr_tsas"
if [[ "${tls}" != "required" ]]; then
echo "not required" > "${portcfs}/addr_treq"
fi
fi
echo "${port}"
}

Expand Down Expand Up @@ -871,6 +894,7 @@ _nvmet_target_setup() {
local port p
local resv_enable=""
local num_ports=1
local tls="none"
local -a ARGS

while [[ $# -gt 0 ]]; do
Expand Down Expand Up @@ -903,6 +927,14 @@ _nvmet_target_setup() {
num_ports="$2"
shift 2
;;
--tls)
tls="not-required"
shift 1
;;
--force-tls)
tls="required"
shift 1
;;
*)
echo "WARNING: unknown argument: $1"
shift
Expand Down Expand Up @@ -949,7 +981,7 @@ _nvmet_target_setup() {

p=0
while (( p < num_ports )); do
port="$(_create_nvmet_port)"
port="$(_create_nvmet_port ${tls})"
_add_nvmet_subsys_to_port "${port}" "${subsysnqn}"
p=$(( p + 1 ))
done
Expand Down
93 changes: 93 additions & 0 deletions tests/nvme/059
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-3.0+
# Copyright (C) 2024 Hannes Reinecke, SUSE Labs
#
# Create TLS-encrypted connections

. tests/nvme/rc

DESCRIPTION="Create TLS-encrypted connections"
QUICK=1

requires() {
_nvme_requires
_have_loop
_have_kernel_option NVME_TCP_TLS
_have_kernel_option NVME_TARGET_TCP_TLS
_require_kernel_nvme_fabrics_feature tls
_require_nvme_trtype tcp
_require_nvme_cli_tls
}

set_conditions() {
_set_nvme_trtype "$@"
}

test() {
echo "Running ${TEST_NAME}"

_setup_nvmet

local hostkey
local ctrl

hostkey=$(nvme gen-tls-key -n "${def_hostnqn}" -c "${def_subsysnqn}" -m 1 -I 1 -i 2> /dev/null)
if [ -z "$hostkey" ] ; then
echo "nvme gen-tls-key failed"
return 1
fi

systemctl start tlshd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to check that it exists as a dependency

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also check the version of ktls-utils?
Or just explain in a comment if you have any expectations from it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point. Will check what we can do here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to "man systemctl" "EXIT STATUS" section, systemctl command returns exit status "4" for "no such unit". So it would work to check if "systemctl status tlshd" command's exist status is 4 or not.

I use Fedora, and needed to install "ktls-utils" package to run the test case. It would be the better to mention the word "ktls-utils" in the SKIP_REASONS message to help users to understand what is missing.


_nvmet_target_setup --blkdev file --tls

# Test unencrypted connection
echo "Test unencrypted connection w/ tls not required"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

umm, looks pretty useless...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think so. This is testing the 'not required' setting in nvmet, which should accept both TLS and non-TLS connections even if TLS is enabled on the target.

_nvme_connect_subsys

ctrl=$(_find_nvme_dev "${def_subsysnqn}")
if _nvme_ctrl_tls_key "$ctrl" > /dev/null; then
echo "WARNING: connection is encrypted"
fi

_nvme_disconnect_subsys

# Test encrypted connection
echo "Test encrypted connection w/ tls not required"
_nvme_connect_subsys --tls

ctrl=$(_find_nvme_dev "${def_subsysnqn}")
if ! _nvme_ctrl_tls_key "$ctrl" > /dev/null ; then
echo "WARNING: connection is not encrypted"
fi

_nvme_disconnect_subsys

# Reset target configuration
_nvmet_target_cleanup

_nvmet_target_setup --blkdev file --force-tls

# Test unencrypted connection
echo "Test unencrypted connection w/ tls required (should fail)"
_nvme_connect_subsys

_nvme_disconnect_subsys

# Test encrypted connection
echo "Test encrypted connection w/ tls required"
_nvme_connect_subsys --tls

ctrl=$(_find_nvme_dev "${def_subsysnqn}")
if ! _nvme_ctrl_tls_key "$ctrl" > /dev/null; then
echo "WARNING: connection is not encrypted"
fi

_nvme_disconnect_subsys
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there any room to test passing explicit keys and private keyrings to this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather not do that here. This is for testing the 'default' case, where PSKs are pre-populated in the keyring and the connection picks up the keys automatically. Explicit keys and keyrings are really just for testing.
But we should have a separate testcase for that, true.


systemctl stop tlshd

_nvmet_target_cleanup

echo "Test complete"
}
10 changes: 10 additions & 0 deletions tests/nvme/059.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
Running nvme/059
Test unencrypted connection w/ tls not required
disconnected 1 controller(s)
Test encrypted connection w/ tls not required
disconnected 1 controller(s)
Test unencrypted connection w/ tls required (should fail)
disconnected 0 controller(s)
Test encrypted connection w/ tls required
disconnected 1 controller(s)
Test complete
106 changes: 106 additions & 0 deletions tests/nvme/060
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-3.0+
# Copyright (C) 2022 Hannes Reinecke, SUSE Labs
#
# Create secure concatenation for TCP connections

. tests/nvme/rc

DESCRIPTION="Create authenticated TCP connections with secure concatenation"
QUICK=1

requires() {
_nvme_requires
_have_loop
_have_kernel_option NVME_AUTH
_have_kernel_option NVME_TCP_TLS
_have_kernel_option NVME_TARGET_AUTH
_have_kernel_option NVME_TARGET_TCP_TLS
_require_kernel_nvme_fabrics_feature dhchap_ctrl_secret
_require_kernel_nvme_fabrics_feature concat
_require_nvme_trtype_is_tcp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should be "_require_nvme_trtype tcp".

_require_nvme_cli_auth
}

set_conditions() {
_set_nvme_trtype "$@"
}

test() {
echo "Running ${TEST_NAME}"

_setup_nvmet

local hostkey

systemctl restart tlshd

hostkey="$(nvme gen-dhchap-key -m 1 -n ${def_hostnqn} 2> /dev/null)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

${def_hostnqn} should be surrounded with double quotations to avoid a shellcheck warning.

if [ -z "$hostkey" ] ; then
echo "nvme gen-dhchap-key failed"
return 1
fi

_nvmet_target_setup --blkdev file --hostkey "${hostkey}" --concat
_set_nvmet_hash "${def_hostnqn}" "hmac(sha256)"
_set_nvmet_dhgroup "${def_hostnqn}" "ffdhe2048"

echo "Test secure concatenation with SHA256"
_nvme_connect_subsys --dhchap-secret "${hostkey}" --concat

ctrl=$(_find_nvme_dev)
if [[ -z "$ctrl" ]]; then
echo "WARNING: connection failed"
exit 1
fi
tlskey=$(_nvme_ctrl_tls_key "$ctrl" || true)
if [[ -z "$tlskey" ]]; then
echo "WARNING: connection is not encrypted"
exit 1
fi

# Reset controller to force re-negotiation
echo "Reset controller"
if ! nvme reset "/dev/${ctrl}" ; then
echo "WARNING: failed to reset controller"
fi

new_tlskey=$(_nvme_ctrl_tls_key "$ctrl" || true)
if [[ -z "$new_tlskey" ]]; then
echo "WARNING: connection is not encrypted"
elif [[ "$new_tlskey" = "$tlskey" ]]; then
echo "WARNING: TLS key has not been renegotiated"
fi

_nvme_disconnect_subsys

hostkey="$(nvme gen-dhchap-key -m 2 -n ${def_hostnqn} 2> /dev/null)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, ${def_hostnqn} should be surrounded with double quotations to avoid a shellcheck warning.

if [ -z "$hostkey" ] ; then
echo "nvme gen-dhchap-key failed"
return 1
fi

_set_nvmet_hostkey "{def_hostnqn}" "${hostkey}"
_set_nvmet_hash "${def_hostnqn}" "hmac(sha384)"
_set_nvmet_dhgroup "${def_hostnqn}" "ffdhe3072"

echo "Test secure concatenation with SHA384"
_nvme_connect_subsys --dhchap-secret "${hostkey}" --concat

ctrl=$(_find_nvme_dev)
if [[ -z "$ctrl" ]]; then
echo "WARNING: connection failed"
exit 1
fi
tlskey=$(_nvme_ctrl_tls_key "$ctrl" || true)
if [[ -z "$tlskey" ]]; then
echo "WARNING: connection is not encrypted"
exit 1
fi

_nvme_disconnect_subsys

_nvmet_target_cleanup

echo "Test complete"
}
14 changes: 14 additions & 0 deletions tests/nvme/rc
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,14 @@ _require_nvme_cli_auth() {
return 0
}

_require_nvme_cli_tls() {
if ! nvme gen-tls-key --subsysnqn nvmf-test-subsys > /dev/null 2>&1; then
SKIP_REASON+=("nvme gen-tls-key command missing")
return 1
fi
return 0
}

_require_kernel_nvme_fabrics_feature() {
local feature="$1"

Expand Down Expand Up @@ -597,3 +605,9 @@ _nvme_reset_ctrl() {
_nvme_delete_ctrl() {
echo 1 > /sys/class/nvme/"$1"/delete_controller
}

_nvme_ctrl_tls_key() {
local ctrl="$1"

cat /sys/class/nvme/"$ctrl"/tls_key 2>/dev/null
}
Loading