From 4e1fed1bbb25c382efa732e4aabbc4d47ff25557 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Tue, 8 Oct 2024 13:39:53 +0200 Subject: [PATCH 01/10] test: add test case for importing/exporting PSKs Test the pre-shared key interchange format import/export function. Signed-off-by: Daniel Wagner --- test/meson.build | 9 ++++ test/psk.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 test/psk.c diff --git a/test/meson.build b/test/meson.build index ca2a7927..2ab7e312 100644 --- a/test/meson.build +++ b/test/meson.build @@ -104,6 +104,15 @@ if conf.get('HAVE_NETDB') test('util', test_util) endif +psk = executable( + 'test-psk', + ['psk.c'], + dependencies: libnvme_dep, + include_directories: [incdir, internal_incdir] +) + +test('psk', psk) + subdir('ioctl') subdir('nbft') diff --git a/test/psk.c b/test/psk.c new file mode 100644 index 00000000..f76e2826 --- /dev/null +++ b/test/psk.c @@ -0,0 +1,115 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/** + * This file is part of libnvme. + * Copyright (c) 2024 Daniel Wagner, SUSE Software Solutions + */ + +#include "nvme/linux.h" +#include +#include +#include + +#include + +#include + +static int test_rc; + +struct test_data { + const unsigned char configured_psk[48]; + size_t psk_length; + unsigned char version; + unsigned char hmac; + const char *exported_psk; +}; + +static struct test_data test_data[] = { + { { 0x55, 0x12, 0xDB, 0xB6, + 0x73, 0x7D, 0x01, 0x06, + 0xF6, 0x59, 0x75, 0xB7, + 0x73, 0xDF, 0xB0, 0x11, + 0xFF, 0xC3, 0x44, 0xBC, + 0xF4, 0x42, 0xE2, 0xDD, + 0x6D, 0x8B, 0xC4, 0x87, + 0x0B, 0x5D, 0x5B, 0x03}, + 32, 1, NVME_HMAC_ALG_SHA2_256, + "NVMeTLSkey-1:01:VRLbtnN9AQb2WXW3c9+wEf/DRLz0QuLdbYvEhwtdWwNf9LrZ:" }, +}; + +static void check_str(const char *exp, const char *res) +{ + if (!strcmp(res, exp)) + return; + + printf("ERROR: got '%s', expected '%s'\n", res, exp); + + test_rc = 1; +} + +static void export_test(struct test_data *test) +{ + char *psk; + + if (test->version != 1) + return; + + printf("test nvme_export_tls_key %s\n", test->exported_psk); + + psk = nvme_export_tls_key(test->configured_psk, test->psk_length); + if (!psk) { + test_rc = 1; + printf("ERROR: nvme_export_tls_key() failed with %d\n", errno); + return; + } + check_str(test->exported_psk, psk); + free(psk); +} + +static void import_test(struct test_data *test) +{ + unsigned char *psk; + int psk_length; + unsigned int hmac; + + if (test->version != 1) + return; + + printf("test nvme_import_tls_key %s\n", test->exported_psk); + + psk = nvme_import_tls_key(test->exported_psk, &psk_length, &hmac); + if (!psk) { + test_rc = 1; + printf("ERROR: nvme_import_tls_key() failed with %d\n", errno); + return; + } + + if (test->hmac != hmac) { + test_rc = 1; + printf("ERROR: hmac parsing failed\n"); + goto out; + } + + if (test->psk_length != psk_length) { + test_rc = 1; + printf("ERROR: length parsing failed\n"); + goto out; + } + if (memcmp(test->configured_psk, psk, psk_length)) { + test_rc = 1; + printf("ERROR: parsing psk failed\n"); + } + +out: + free(psk); +} + +int main(void) +{ + for (int i = 0; i < ARRAY_SIZE(test_data); i++) + export_test(&test_data[i]); + + for (int i = 0; i < ARRAY_SIZE(test_data); i++) + import_test(&test_data[i]); + + return test_rc ? EXIT_FAILURE : EXIT_SUCCESS; +} From 2a8629eb47010f143bf22b4e9e51455f79c53f6c Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Tue, 8 Oct 2024 14:32:44 +0200 Subject: [PATCH 02/10] linux: add import/export function for TLS pre-shared keys The existing import/export function do not handle different version of the interchange format nor do the handle the HMAC independent of the version. Thus allow the caller to select version and HMAC independently when exporting resp. importing. This makes this interface also future proof when new HMAC or key lengths are added to the spec. Signed-off-by: Daniel Wagner --- src/libnvme.map | 7 ++++ src/nvme/linux.c | 95 ++++++++++++++++++++++++++++++++++++++++-------- src/nvme/linux.h | 37 +++++++++++++++++++ 3 files changed, 123 insertions(+), 16 deletions(-) diff --git a/src/libnvme.map b/src/libnvme.map index fbdc6c79..7374a0d7 100644 --- a/src/libnvme.map +++ b/src/libnvme.map @@ -1,4 +1,11 @@ # SPDX-License-Identifier: LGPL-2.1-or-later +LIBNVME_1.11 { + global: + nvme_export_tls_key_versioned; + nvme_import_tls_key_versioned; +}; + + LIBNVME_1.10 { global: nvme_free_uri; diff --git a/src/nvme/linux.c b/src/nvme/linux.c index f934e855..8701c59b 100644 --- a/src/nvme/linux.c +++ b/src/nvme/linux.c @@ -1489,20 +1489,40 @@ long nvme_insert_tls_key(const char *keyring, const char *key_type, configured_key, key_len); } -char *nvme_export_tls_key(const unsigned char *key_data, int key_len) +/* + * PSK Interchange Format + * NVMeTLSkey-::: + * + * x: version as one ASCII char + * yy: hmac encoded as two ASCII chars + * 00: no transform ('configured PSK') + * 01: SHA-256 + * 02: SHA-384 + * s: 32 or 48 bytes binary followed by a CRC-32 of the configured PSK + * (4 bytes) encoded as base64 + */ +char *nvme_export_tls_key_versioned(unsigned char version, unsigned char hmac, + const unsigned char *key_data, + size_t key_len) { unsigned char raw_secret[52]; char *encoded_key; unsigned int raw_len, encoded_len, len; unsigned long crc = crc32(0L, NULL, 0); - if (key_len == 32) { + switch (hmac) { + case NVME_HMAC_ALG_SHA2_256: + if (key_len != 32) + goto err_inval; raw_len = 32; - } else if (key_len == 48) { + break; + case NVME_HMAC_ALG_SHA2_384: + if (key_len != 48) + goto err_inval; raw_len = 48; - } else { - errno = EINVAL; - return NULL; + break; + default: + goto err_inval; } memcpy(raw_secret, key_data, raw_len); @@ -1519,35 +1539,61 @@ char *nvme_export_tls_key(const unsigned char *key_data, int key_len) return NULL; } memset(encoded_key, 0, encoded_len); - len = sprintf(encoded_key, "NVMeTLSkey-1:%02x:", - key_len == 32 ? 1 : 2); + len = sprintf(encoded_key, "NVMeTLSkey-%x:%02x:", version, hmac); len += base64_encode(raw_secret, raw_len, encoded_key + len); encoded_key[len++] = ':'; encoded_key[len++] = '\0'; return encoded_key; + +err_inval: + errno = EINVAL; + return NULL; + } -unsigned char *nvme_import_tls_key(const char *encoded_key, int *key_len, - unsigned int *hmac) +char *nvme_export_tls_key(const unsigned char *key_data, int key_len) +{ + unsigned char hmac; + + if (key_len == 32) + hmac = NVME_HMAC_ALG_SHA2_256; + else + hmac = NVME_HMAC_ALG_SHA2_384; + + return nvme_export_tls_key_versioned(1, hmac, key_data, key_len); +} + +unsigned char *nvme_import_tls_key_versioned(const char *encoded_key, + unsigned char *version, + unsigned char *hmac, + size_t *key_len) { unsigned char decoded_key[128], *key_data; unsigned int crc = crc32(0L, NULL, 0); unsigned int key_crc; - int err, decoded_len; + int err, _version, _hmac, decoded_len; + + if (sscanf(encoded_key, "NVMeTLSkey-%d:%02x:*s", + &_version, &_hmac) != 2) { + errno = EINVAL; + return NULL; + } - if (sscanf(encoded_key, "NVMeTLSkey-1:%02x:*s", &err) != 1) { + if (_version != 1) { errno = EINVAL; return NULL; } - switch (err) { - case 1: + *version = _version; + + switch (_hmac) { + case NVME_HMAC_ALG_SHA2_256: if (strlen(encoded_key) != 65) { errno = EINVAL; return NULL; } break; - case 2: + case NVME_HMAC_ALG_SHA2_384: if (strlen(encoded_key) != 89) { errno = EINVAL; return NULL; @@ -1557,8 +1603,8 @@ unsigned char *nvme_import_tls_key(const char *encoded_key, int *key_len, errno = EINVAL; return NULL; } + *hmac = _hmac; - *hmac = err; err = base64_decode(encoded_key + 16, strlen(encoded_key) - 17, decoded_key); if (err < 0) { @@ -1593,3 +1639,20 @@ unsigned char *nvme_import_tls_key(const char *encoded_key, int *key_len, *key_len = decoded_len; return key_data; } + +unsigned char *nvme_import_tls_key(const char *encoded_key, int *key_len, + unsigned int *hmac) +{ + unsigned char version, _hmac; + unsigned char *psk; + size_t len; + + psk = nvme_import_tls_key_versioned(encoded_key, &version, + &_hmac, &len); + if (!psk) + return NULL; + + *hmac = _hmac; + *key_len = len; + return psk; +} diff --git a/src/nvme/linux.h b/src/nvme/linux.h index 8e5e8adb..5dbc0926 100644 --- a/src/nvme/linux.h +++ b/src/nvme/linux.h @@ -436,6 +436,25 @@ long nvme_revoke_tls_key(const char *keyring, const char *key_type, */ char *nvme_export_tls_key(const unsigned char *key_data, int key_len); +/** + * nvme_export_tls_key_versioned() - Export a TLS pre-shared key + * @version: Indicated the representation of the TLS PSK + * @hmac: HMAC algorithm used to transfor the configured PSK + * in a retained PSK + * @key_data: Raw data of the key + * @key_len: Length of @key_data + * + * Returns @key_data in the PSK Interchange format as defined in section + * 3.6.1.5 of the NVMe TCP Transport specification. + * + * Return: The string containing the TLS identity or NULL with errno set + * on error. It is the responsibility of the caller to free the returned + * string. + */ +char *nvme_export_tls_key_versioned(unsigned char version, unsigned char hmac, + const unsigned char *key_data, + size_t key_len); + /** * nvme_import_tls_key() - Import a TLS key * @encoded_key: TLS key in PSK interchange format @@ -451,6 +470,24 @@ char *nvme_export_tls_key(const unsigned char *key_data, int key_len); unsigned char *nvme_import_tls_key(const char *encoded_key, int *key_len, unsigned int *hmac); +/** + * nvme_import_tls_key_versioned() - Import a TLS key + * @encoded_key: TLS key in PSK interchange format + * @version: Indicated the representation of the TLS PSK + * @hmac: HMAC algorithm used to transfor the configured + * PSK in a retained PSK + * @key_len: Length of the resulting key data + * + * Imports @key_data in the PSK Interchange format as defined in section + * 3.6.1.5 of the NVMe TCP Transport specification. + * + * Return: The raw data of the PSK or NULL with errno set on error. It is + * the responsibility of the caller to free the returned string. + */ +unsigned char *nvme_import_tls_key_versioned(const char *encoded_key, + unsigned char *version, + unsigned char *hmac, + size_t *key_len); /** * nvme_submit_passthru - Low level ioctl wrapper for passthru commands * @fd: File descriptor of the nvme device From 95ab7ae009cf0d38c0b45474ad50d4de10cc5045 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Tue, 8 Oct 2024 14:39:32 +0200 Subject: [PATCH 03/10] test: extend psk to test new 'versioned' API Also test for nvme_{import|export}_tls_key_versioned API. Signed-off-by: Daniel Wagner --- test/psk.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/test/psk.c b/test/psk.c index f76e2826..f5276e7a 100644 --- a/test/psk.c +++ b/test/psk.c @@ -71,10 +71,13 @@ static void import_test(struct test_data *test) int psk_length; unsigned int hmac; - if (test->version != 1) + if (test->version != 1 || + !(test->hmac == NVME_HMAC_ALG_SHA2_256 || + test->hmac == NVME_HMAC_ALG_SHA2_384)) return; - printf("test nvme_import_tls_key %s\n", test->exported_psk); + printf("test nvme_import_tls_key hmac %d %s\n", + test->hmac, test->exported_psk); psk = nvme_import_tls_key(test->exported_psk, &psk_length, &hmac); if (!psk) { @@ -103,6 +106,80 @@ static void import_test(struct test_data *test) free(psk); } +static void export_versioned_test(struct test_data *test) +{ + char *psk; + + if (test->version != 1) + return; + + printf("test nvme_export_tls_key_versioned hmac %d %s\n", + test->hmac, test->exported_psk); + + psk = nvme_export_tls_key_versioned(test->version, test->hmac, + test->configured_psk, + test->psk_length); + if (!psk) { + test_rc = 1; + printf("ERROR: nvme_export_tls_key_versioned() failed with %d\n", + errno); + return; + } + + check_str(test->exported_psk, psk); + + free(psk); +} + +static void import_versioned_test(struct test_data *test) +{ + unsigned char *psk; + unsigned char version; + unsigned char hmac; + size_t psk_length; + + if (test->version != 1) + return; + + printf("test nvme_import_tls_key_versioned hmac %d %s\n", + test->hmac, test->exported_psk); + + psk = nvme_import_tls_key_versioned(test->exported_psk, &version, + &hmac, &psk_length); + if (!psk) { + test_rc = 1; + printf("ERROR: nvme_import_tls_key_versioned() failed with %d\n", + errno); + return; + } + + if (test->version != version) { + test_rc = 1; + printf("ERROR: version parsing failed\n"); + goto out; + } + + if (test->hmac != hmac) { + test_rc = 1; + printf("ERROR: hmac parsing failed\n"); + goto out; + } + + if (test->psk_length != psk_length) { + test_rc = 1; + printf("ERROR: length parsing failed\n"); + goto out; + } + + if (memcmp(test->configured_psk, psk, psk_length)) { + test_rc = 1; + printf("ERROR: parsing psk failed\n"); + } + +out: + free(psk); +} + int main(void) { for (int i = 0; i < ARRAY_SIZE(test_data); i++) @@ -111,5 +188,11 @@ int main(void) for (int i = 0; i < ARRAY_SIZE(test_data); i++) import_test(&test_data[i]); + for (int i = 0; i < ARRAY_SIZE(test_data); i++) + export_versioned_test(&test_data[i]); + + for (int i = 0; i < ARRAY_SIZE(test_data); i++) + import_versioned_test(&test_data[i]); + return test_rc ? EXIT_FAILURE : EXIT_SUCCESS; } From 6a7fefc1f00086e7a11971cb04ae804f0bebbec7 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Tue, 8 Oct 2024 15:20:12 +0200 Subject: [PATCH 04/10] linux: support PSK interchange format HMAC none The pre-shared key interchange format also has 'no transform' option when the configured key should be used as retained key. Update the export/imports to support this case. Signed-off-by: Daniel Wagner --- src/nvme/linux.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/nvme/linux.c b/src/nvme/linux.c index 8701c59b..8ecd9c24 100644 --- a/src/nvme/linux.c +++ b/src/nvme/linux.c @@ -1511,19 +1511,22 @@ char *nvme_export_tls_key_versioned(unsigned char version, unsigned char hmac, unsigned long crc = crc32(0L, NULL, 0); switch (hmac) { + case NVME_HMAC_ALG_NONE: + if (key_len != 32 && key_len != 48) + goto err_inval; + break; case NVME_HMAC_ALG_SHA2_256: if (key_len != 32) goto err_inval; - raw_len = 32; break; case NVME_HMAC_ALG_SHA2_384: if (key_len != 48) goto err_inval; - raw_len = 48; break; default: goto err_inval; } + raw_len = key_len; memcpy(raw_secret, key_data, raw_len); crc = crc32(crc, raw_secret, raw_len); @@ -1573,6 +1576,7 @@ unsigned char *nvme_import_tls_key_versioned(const char *encoded_key, unsigned int crc = crc32(0L, NULL, 0); unsigned int key_crc; int err, _version, _hmac, decoded_len; + size_t len; if (sscanf(encoded_key, "NVMeTLSkey-%d:%02x:*s", &_version, &_hmac) != 2) { @@ -1586,18 +1590,19 @@ unsigned char *nvme_import_tls_key_versioned(const char *encoded_key, } *version = _version; + len = strlen(encoded_key); switch (_hmac) { + case NVME_HMAC_ALG_NONE: + if (len != 65 && len != 89) + goto err_inval; + break; case NVME_HMAC_ALG_SHA2_256: - if (strlen(encoded_key) != 65) { - errno = EINVAL; - return NULL; - } + if (len != 65) + goto err_inval; break; case NVME_HMAC_ALG_SHA2_384: - if (strlen(encoded_key) != 89) { - errno = EINVAL; - return NULL; - } + if (len != 89) + goto err_inval; break; default: errno = EINVAL; @@ -1605,8 +1610,7 @@ unsigned char *nvme_import_tls_key_versioned(const char *encoded_key, } *hmac = _hmac; - err = base64_decode(encoded_key + 16, strlen(encoded_key) - 17, - decoded_key); + err = base64_decode(encoded_key + 16, len - 17, decoded_key); if (err < 0) { errno = ENOKEY; return NULL; @@ -1638,6 +1642,10 @@ unsigned char *nvme_import_tls_key_versioned(const char *encoded_key, *key_len = decoded_len; return key_data; + +err_inval: + errno = EINVAL; + return NULL; } unsigned char *nvme_import_tls_key(const char *encoded_key, int *key_len, From 545f19df752b0e2b718e6d27475ca38b2b7f28d4 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Tue, 8 Oct 2024 15:23:13 +0200 Subject: [PATCH 05/10] test/psk: test all available HMACs Extend the test case also to check for the NONE and SHA2-384 algorithm Signed-off-by: Daniel Wagner --- test/psk.c | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/test/psk.c b/test/psk.c index f5276e7a..02cb6d81 100644 --- a/test/psk.c +++ b/test/psk.c @@ -24,6 +24,16 @@ struct test_data { }; static struct test_data test_data[] = { + { { 0x55, 0x12, 0xDB, 0xB6, + 0x73, 0x7D, 0x01, 0x06, + 0xF6, 0x59, 0x75, 0xB7, + 0x73, 0xDF, 0xB0, 0x11, + 0xFF, 0xC3, 0x44, 0xBC, + 0xF4, 0x42, 0xE2, 0xDD, + 0x6D, 0x8B, 0xC4, 0x87, + 0x0B, 0x5D, 0x5B, 0x03}, + 32, 1, NVME_HMAC_ALG_NONE, + "NVMeTLSkey-1:00:VRLbtnN9AQb2WXW3c9+wEf/DRLz0QuLdbYvEhwtdWwNf9LrZ:" }, { { 0x55, 0x12, 0xDB, 0xB6, 0x73, 0x7D, 0x01, 0x06, 0xF6, 0x59, 0x75, 0xB7, @@ -34,6 +44,20 @@ static struct test_data test_data[] = { 0x0B, 0x5D, 0x5B, 0x03}, 32, 1, NVME_HMAC_ALG_SHA2_256, "NVMeTLSkey-1:01:VRLbtnN9AQb2WXW3c9+wEf/DRLz0QuLdbYvEhwtdWwNf9LrZ:" }, + { { 0x55, 0x12, 0xDB, 0xB6, + 0x73, 0x7D, 0x01, 0x06, + 0xF6, 0x59, 0x75, 0xB7, + 0x73, 0xDF, 0xB0, 0x11, + 0xFF, 0xC3, 0x44, 0xBC, + 0xF4, 0x42, 0xE2, 0xDD, + 0x6D, 0x8B, 0xC4, 0x87, + 0x0B, 0x5D, 0x5B, 0x03, + 0xFF, 0xC3, 0x44, 0xBC, + 0xF4, 0x42, 0xE2, 0xDD, + 0x6D, 0x8B, 0xC4, 0x87, + 0x0B, 0x5D, 0x5B, 0x03}, + 48, 1, NVME_HMAC_ALG_SHA2_384, + "NVMeTLSkey-1:02:VRLbtnN9AQb2WXW3c9+wEf/DRLz0QuLdbYvEhwtdWwP/w0S89ELi3W2LxIcLXVsDn8kXZQ==:" }, }; static void check_str(const char *exp, const char *res) @@ -50,10 +74,13 @@ static void export_test(struct test_data *test) { char *psk; - if (test->version != 1) + if (test->version != 1 || + !(test->hmac == NVME_HMAC_ALG_SHA2_256 || + test->hmac == NVME_HMAC_ALG_SHA2_384)) return; - printf("test nvme_export_tls_key %s\n", test->exported_psk); + printf("test nvme_export_tls_key hmac %d %s\n", + test->hmac, test->exported_psk); psk = nvme_export_tls_key(test->configured_psk, test->psk_length); if (!psk) { From 581bc62de80d0bf47e16c48b18a04364380aa110 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Wed, 9 Oct 2024 13:33:20 +0200 Subject: [PATCH 06/10] test: make config-diff more flexible to use The config-diff script is expecting a sysfs tar file besides an input and a output file. Let's make the sysfs tar file optional so we can use this config diff script more flexible. Signed-off-by: Daniel Wagner --- test/config/config-diff.sh | 52 ++++++++++++++++++++++++++++---------- test/config/meson.build | 20 +++++++++------ 2 files changed, 50 insertions(+), 22 deletions(-) mode change 100644 => 100755 test/config/config-diff.sh diff --git a/test/config/config-diff.sh b/test/config/config-diff.sh old mode 100644 new mode 100755 index c15b42c4..abec20e1 --- a/test/config/config-diff.sh +++ b/test/config/config-diff.sh @@ -1,24 +1,48 @@ #!/bin/bash -e # SPDX-License-Identifier: LGPL-2.1-or-later -BUILD_DIR=$1 -CONFIG_DUMP=$2 -SYSDIR_INPUT=$3 -CONFIG_JSON=$4 -EXPECTED_OUTPUT=$5 +positional_args=() +sysfs_tar="" +config_json="" -ACTUAL_OUTPUT="${BUILD_DIR}"/$(basename "${EXPECTED_OUTPUT}") +while [[ $# -gt 0 ]]; do + case $1 in + --sysfs-tar) + sysfs_tar=$2 + shift 1 + ;; + --config-json) + config_json=$2 + shift 1 + ;; + *) + positional_args+=("$1") + shift + ;; + esac +done -TEST_NAME="$(basename -s .tar.xz $SYSDIR_INPUT)" -TEST_DIR="$BUILD_DIR/$TEST_NAME" +set -- "${positional_args[@]}" -rm -rf "${TEST_DIR}" -mkdir "${TEST_DIR}" -tar -x -f "${SYSDIR_INPUT}" -C "${TEST_DIR}" +test_binary="$1" +build_dir="$2" +expected_output="$3" -LIBNVME_SYSFS_PATH="$TEST_DIR" \ +sysfs_path="" +if [[ -n "${sysfs_tar}" ]]; then + test_name="$(basename -s .tar.xz ${sysfs_tar})" + sysfs_path="${build_dir}/${test_name}" + + rm -rf "${sysfs_path}" + mkdir "${sysfs_path}" + tar -x -f "${sysfs_tar}" -C "${sysfs_path}" +fi + +output="${build_dir}"/$(basename "${expected_output}") + +LIBNVME_SYSFS_PATH="${sysfs_path}" \ LIBNVME_HOSTNQN=nqn.2014-08.org.nvmexpress:uuid:ce4fee3e-c02c-11ee-8442-830d068a36c6 \ LIBNVME_HOSTID=ce4fee3e-c02c-11ee-8442-830d068a36c6 \ -"${CONFIG_DUMP}" "${CONFIG_JSON}" > "${ACTUAL_OUTPUT}" || echo "test failed" +"${test_binary}" "${config_json}" > "${output}" || echo "test failed" -diff -u "${EXPECTED_OUTPUT}" "${ACTUAL_OUTPUT}" +diff -u "${expected_output}" "${output}" diff --git a/test/config/meson.build b/test/config/meson.build index c1ee7ca7..f976c69b 100644 --- a/test/config/meson.build +++ b/test/config/meson.build @@ -7,6 +7,10 @@ diff = find_program('diff', required : false) if diff.found() + + srcdir = meson.current_source_dir() + builddir = meson.current_build_dir() + config_dump = executable( 'test-config-dump', ['config-dump.c'], @@ -26,11 +30,11 @@ if diff.found() t_file, config_diff, args : [ - meson.current_build_dir(), config_dump.full_path(), - files('data'/t_file + '.tar.xz'), - files('data'/t_file + '.json'), - files('data'/t_file + '.out'), + builddir, + srcdir + '/data/' + t_file + '.out', + '--sysfs-tar', srcdir + '/data/' + t_file + '.tar.xz', + '--config-json', srcdir + '/data/' + t_file + '.json', ], depends : config_dump, ) @@ -47,11 +51,11 @@ if diff.found() 'hostnqn-order', config_diff, args : [ - meson.current_build_dir(), test_hostnqn_order.full_path(), - files('data/hostnqn-order.tar.xz'), - files('data/hostnqn-order.json'), - files('data/hostnqn-order.out'), + builddir, + srcdir + '/data/hostnqn-order.out', + '--sysfs-tar', srcdir + '/data/hostnqn-order.tar.xz', + '--config-json', srcdir + '/data/hostnqn-order.json', ], depends : test_hostnqn_order, ) From 558965c8fb175c3dfda5d1e62dbfe75f08be91e1 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Wed, 9 Oct 2024 18:58:26 +0200 Subject: [PATCH 07/10] linux: reorder variable declarations Use the inverse x-mas tree pattern for variable declarations. Signed-off-by: Daniel Wagner --- src/nvme/linux.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/nvme/linux.c b/src/nvme/linux.c index 8ecd9c24..35b5812a 100644 --- a/src/nvme/linux.c +++ b/src/nvme/linux.c @@ -600,9 +600,9 @@ static int derive_retained_key(int hmac, const char *hostnqn, unsigned char *retained, size_t key_len) { - const EVP_MD *md; _cleanup_evp_pkey_ctx_ EVP_PKEY_CTX *ctx = NULL; uint16_t length = key_len & 0xFFFF; + const EVP_MD *md; size_t hmac_len; md = select_hmac(hmac, &hmac_len); @@ -662,10 +662,10 @@ static int derive_tls_key(int version, int hmac, const char *context, unsigned char *retained, unsigned char *psk, size_t key_len) { - const EVP_MD *md; _cleanup_evp_pkey_ctx_ EVP_PKEY_CTX *ctx = NULL; - size_t hmac_len; uint16_t length = key_len & 0xFFFF; + const EVP_MD *md; + size_t hmac_len; md = select_hmac(hmac, &hmac_len); if (!md || hmac_len > key_len) { @@ -805,10 +805,10 @@ static int derive_psk_digest(const char *hostnqn, const char *subsysnqn, char *digest, size_t digest_len) { static const char hmac_seed[] = "NVMe-over-Fabrics"; - size_t hmac_len; const EVP_MD *md = select_hmac(hmac, &hmac_len); _cleanup_hmac_ctx_ HMAC_CTX *hmac_ctx = NULL; _cleanup_free_ unsigned char *psk_ctx = NULL; + size_t hmac_len; size_t len; hmac_ctx = HMAC_CTX_new(); @@ -886,10 +886,10 @@ int nvme_gen_dhchap_key(char *hostnqn, enum nvme_hmac_alg hmac, unsigned char *key) { const char hmac_seed[] = "NVMe-over-Fabrics"; - OSSL_PARAM params[2], *p = params; _cleanup_ossl_lib_ctx_ OSSL_LIB_CTX *lib_ctx = NULL; _cleanup_evp_mac_ctx_ EVP_MAC_CTX *mac_ctx = NULL; _cleanup_evp_mac_ EVP_MAC *mac = NULL; + OSSL_PARAM params[2], *p = params; char *progq = NULL; char *digest; size_t len; @@ -970,14 +970,14 @@ static int derive_psk_digest(const char *hostnqn, const char *subsysnqn, char *digest, size_t digest_len) { static const char hmac_seed[] = "NVMe-over-Fabrics"; - size_t hmac_len; - OSSL_PARAM params[2], *p = params; _cleanup_ossl_lib_ctx_ OSSL_LIB_CTX *lib_ctx = NULL; _cleanup_evp_mac_ctx_ EVP_MAC_CTX *mac_ctx = NULL; + _cleanup_free_ unsigned char *psk_ctx = NULL; _cleanup_evp_mac_ EVP_MAC *mac = NULL; + OSSL_PARAM params[2], *p = params; + size_t hmac_len; char *progq = NULL; char *dig = NULL; - _cleanup_free_ unsigned char *psk_ctx = NULL; size_t len; lib_ctx = OSSL_LIB_CTX_new(); @@ -1160,9 +1160,9 @@ char *nvme_generate_tls_key_identity(const char *hostnqn, const char *subsysnqn, int version, int hmac, unsigned char *configured_key, int key_len) { + _cleanup_free_ unsigned char *psk = NULL; char *identity; size_t identity_len; - _cleanup_free_ unsigned char *psk = NULL; int ret = -1; identity_len = nvme_identity_len(hmac, version, hostnqn, subsysnqn); @@ -1342,10 +1342,10 @@ long nvme_insert_tls_key_versioned(const char *keyring, const char *key_type, int version, int hmac, unsigned char *configured_key, int key_len) { - key_serial_t keyring_id, key; + _cleanup_free_ unsigned char *psk = NULL; _cleanup_free_ char *identity = NULL; + key_serial_t keyring_id, key; size_t identity_len; - _cleanup_free_ unsigned char *psk = NULL; int ret; keyring_id = nvme_lookup_keyring(keyring); @@ -1505,10 +1505,10 @@ char *nvme_export_tls_key_versioned(unsigned char version, unsigned char hmac, const unsigned char *key_data, size_t key_len) { - unsigned char raw_secret[52]; - char *encoded_key; unsigned int raw_len, encoded_len, len; unsigned long crc = crc32(0L, NULL, 0); + unsigned char raw_secret[52]; + char *encoded_key; switch (hmac) { case NVME_HMAC_ALG_NONE: @@ -1552,7 +1552,6 @@ char *nvme_export_tls_key_versioned(unsigned char version, unsigned char hmac, err_inval: errno = EINVAL; return NULL; - } char *nvme_export_tls_key(const unsigned char *key_data, int key_len) @@ -1574,8 +1573,8 @@ unsigned char *nvme_import_tls_key_versioned(const char *encoded_key, { unsigned char decoded_key[128], *key_data; unsigned int crc = crc32(0L, NULL, 0); - unsigned int key_crc; int err, _version, _hmac, decoded_len; + unsigned int key_crc; size_t len; if (sscanf(encoded_key, "NVMeTLSkey-%d:%02x:*s", From 2d12b9668b4434f585d09d8f5b6b10eb86479577 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Thu, 10 Oct 2024 10:36:50 +0200 Subject: [PATCH 08/10] tree: add getter/setters for tls_key and keyring Extend the ctrl API to allow the users to set the tls_key/keyring on the ctrl object directly. Signed-off-by: Daniel Wagner --- src/libnvme.map | 4 ++++ src/nvme/private.h | 2 ++ src/nvme/tree.c | 30 ++++++++++++++++++++++++++++++ src/nvme/tree.h | 31 +++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+) diff --git a/src/libnvme.map b/src/libnvme.map index 7374a0d7..47c053db 100644 --- a/src/libnvme.map +++ b/src/libnvme.map @@ -1,6 +1,10 @@ # SPDX-License-Identifier: LGPL-2.1-or-later LIBNVME_1.11 { global: + nvme_ctrl_get_keyring; + nvme_ctrl_get_tls_key; + nvme_ctrl_set_keyring; + nvme_ctrl_set_tls_key; nvme_export_tls_key_versioned; nvme_import_tls_key_versioned; }; diff --git a/src/nvme/private.h b/src/nvme/private.h index 3fa5aca0..efeaae83 100644 --- a/src/nvme/private.h +++ b/src/nvme/private.h @@ -85,6 +85,8 @@ struct nvme_ctrl { char *trsvcid; char *dhchap_key; char *dhchap_ctrl_key; + char *keyring; + char *tls_key; char *cntrltype; char *cntlid; char *dctype; diff --git a/src/nvme/tree.c b/src/nvme/tree.c index bdb527d2..4f073bbd 100644 --- a/src/nvme/tree.c +++ b/src/nvme/tree.c @@ -1151,6 +1151,36 @@ void nvme_ctrl_set_dhchap_key(nvme_ctrl_t c, const char *key) c->dhchap_ctrl_key = strdup(key); } +const char *nvme_ctrl_get_keyring(nvme_ctrl_t c) +{ + return c->keyring; +} + +void nvme_ctrl_set_keyring(nvme_ctrl_t c, const char *keyring) +{ + if (c->keyring) { + free(c->keyring); + c->keyring = NULL; + } + if (keyring) + c->keyring = strdup(keyring); +} + +const char *nvme_ctrl_get_tls_key(nvme_ctrl_t c) +{ + return c->tls_key; +} + +void nvme_ctrl_set_tls_key(nvme_ctrl_t c, const char *key) +{ + if (c->tls_key) { + free(c->tls_key); + c->tls_key = NULL; + } + if (key) + c->tls_key = strdup(key); +} + void nvme_ctrl_set_discovered(nvme_ctrl_t c, bool discovered) { c->discovered = discovered; diff --git a/src/nvme/tree.h b/src/nvme/tree.h index 1b583cda..6615a77b 100644 --- a/src/nvme/tree.h +++ b/src/nvme/tree.h @@ -1098,6 +1098,37 @@ const char *nvme_ctrl_get_dhchap_key(nvme_ctrl_t c); */ void nvme_ctrl_set_dhchap_key(nvme_ctrl_t c, const char *key); +/** + * nvme_ctrl_get_keyring() - Return keyring + * @c: Controller to be used for the lookup + * + * Return: Keyring or NULL if not set + */ +const char *nvme_ctrl_get_keyring(nvme_ctrl_t c); + +/** + * nvme_ctrl_set_keyring() - Set keyring + * @c: Controller for which the keyring should be set + * @keyring: Keyring name + */ +void nvme_ctrl_set_keyring(nvme_ctrl_t c, const char *keyring); + +/** + * nvme_ctrl_get_tls_key() - Return TLS key + * @c: Controller to be used for the lookup + * + * Return: PSK in interchange format or NULL if not set + */ +const char *nvme_ctrl_get_tls_key(nvme_ctrl_t c); + +/** + * nvme_ctrl_set_tls_key() - Set TLS key + * @c: Controller for which the key should be set + * @key: Key in interchange format to set or NULL to + * clear existing key + */ +void nvme_ctrl_set_tls_key(nvme_ctrl_t c, const char *key); + /** * nvme_ctrl_get_config() - Fabrics configuration of a controller * @c: Controller instance From b28fb19f19785f3ddd46d5b85fb000eff1eddbd1 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Thu, 10 Oct 2024 10:43:28 +0200 Subject: [PATCH 09/10] fabrics: move key store operation to connect When the JSON parser detects a TLS key it inserts it into the keystore. Keystore operations on the default '.nvme' keyring are privileged operations (root) thus the parser will fail. This will fail nvme-cli commands which are run as normal user. Let's move the key store operations to the connect call path where we need the right permission. A nice side benefit is that we also are able to pass in a configured key. Signed-off-by: Daniel Wagner --- src/nvme/fabrics.c | 71 +++++++++++++++++++++++++- src/nvme/json.c | 122 ++++++++++----------------------------------- src/nvme/linux.c | 43 ++++++++++------ src/nvme/private.h | 14 ++++++ src/nvme/tree.c | 19 ++++--- 5 files changed, 148 insertions(+), 121 deletions(-) diff --git a/src/nvme/fabrics.c b/src/nvme/fabrics.c index acf12bc3..06e879f7 100644 --- a/src/nvme/fabrics.c +++ b/src/nvme/fabrics.c @@ -545,13 +545,58 @@ static bool traddr_is_hostname(nvme_root_t r, nvme_ctrl_t c) return true; } +static long insert_tls_key(nvme_host_t h, nvme_ctrl_t c, + long keyring_id, const char *encoded_key) +{ + const char *hostnqn = nvme_host_get_hostnqn(h); + const char *subsysnqn = nvme_ctrl_get_subsysnqn(c); + _cleanup_free_ unsigned char *key_data = NULL; + unsigned char version; + unsigned char hmac; + size_t key_len; + long key_id; + + if (!hostnqn || !subsysnqn) { + nvme_msg(h->r, LOG_ERR, "Invalid NQNs (%s, %s)\n", + hostnqn, subsysnqn); + return -1; + } + + if (nvme_set_keyring(keyring_id) < 0) { + nvme_msg(h->r, LOG_ERR, "Failed to set keyring\n"); + return -1; + } + + key_data = nvme_import_tls_key_versioned(encoded_key, &version, + &hmac, &key_len); + if (!key_data) { + nvme_msg(h->r, LOG_ERR, "Failed to decode TLS Key '%s'\n", + encoded_key); + return -1; + } + + key_id = __nvme_insert_tls_key_versioned(keyring_id, "psk", + hostnqn, subsysnqn, + version, hmac, key_data, key_len); + if (key_id <= 0) { + nvme_msg(h->r, LOG_ERR, "Failed to insert TLS KEY, error %d\n", + errno); + return -1; + } + + return key_id; +} + static int build_options(nvme_host_t h, nvme_ctrl_t c, char **argstr) { struct nvme_fabrics_config *cfg = nvme_ctrl_get_config(c); const char *transport = nvme_ctrl_get_transport(c); const char *hostnqn, *hostid, *hostkey, *ctrlkey; + const char *tls_key, *keyring; bool discover = false, discovery_nqn = false; nvme_root_t r = h->r; + long keyring_id = 0; + long key_id = 0; if (!transport) { nvme_msg(h->r, LOG_ERR, "need a transport (-t) argument\n"); @@ -573,19 +618,41 @@ static int build_options(nvme_host_t h, nvme_ctrl_t c, char **argstr) errno = ENOMEM; return -1; } + if (!strcmp(nvme_ctrl_get_subsysnqn(c), NVME_DISC_SUBSYS_NAME)) { nvme_ctrl_set_discovery_ctrl(c, true); nvme_ctrl_set_unique_discovery_ctrl(c, false); discovery_nqn = true; } + if (nvme_ctrl_is_discovery_ctrl(c)) discover = true; + hostnqn = nvme_host_get_hostnqn(h); hostid = nvme_host_get_hostid(h); hostkey = nvme_host_get_dhchap_key(h); if (!hostkey) hostkey = nvme_ctrl_get_dhchap_host_key(c); + ctrlkey = nvme_ctrl_get_dhchap_key(c); + + keyring = nvme_ctrl_get_keyring(c); + if (keyring) + keyring_id = nvme_lookup_keyring(keyring); + else + keyring_id = cfg->keyring; + + tls_key = nvme_ctrl_get_tls_key(c); + if (tls_key) { + key_id = insert_tls_key(h, c, keyring_id, tls_key); + if (key_id < 0) { + errno = ENVME_CONNECT_INVAL; + return -1; + } + } else { + key_id = cfg->tls_key; + } + if (add_argument(r, argstr, transport, transport) || add_argument(r, argstr, traddr, nvme_ctrl_get_traddr(c)) || @@ -627,9 +694,9 @@ static int build_options(nvme_host_t h, nvme_ctrl_t c, char **argstr) cfg->fast_io_fail_tmo, false)) || (strcmp(transport, "loop") && add_int_argument(r, argstr, tos, cfg->tos, true)) || - add_int_argument(r, argstr, keyring, cfg->keyring, false) || + add_int_argument(r, argstr, keyring, keyring_id, false) || (!strcmp(transport, "tcp") && - add_int_argument(r, argstr, tls_key, cfg->tls_key, false)) || + add_int_argument(r, argstr, tls_key, key_id, false)) || add_bool_argument(r, argstr, duplicate_connect, cfg->duplicate_connect) || add_bool_argument(r, argstr, disable_sqflow, diff --git a/src/nvme/json.c b/src/nvme/json.c index 2c769f2b..d29475cb 100644 --- a/src/nvme/json.c +++ b/src/nvme/json.c @@ -25,62 +25,10 @@ #define JSON_UPDATE_BOOL_OPTION(c, k, a, o) \ if (!strcmp(# a, k ) && !c->a) c->a = json_object_get_boolean(o); -static void json_import_nvme_tls_key(nvme_ctrl_t c, const char *keyring_str, - const char *encoded_key) -{ - struct nvme_fabrics_config *cfg = nvme_ctrl_get_config(c); - const char *hostnqn = nvme_host_get_hostnqn(c->s->h); - const char *subsysnqn = nvme_ctrl_get_subsysnqn(c); - int key_len; - unsigned int hmac; - long key_id; - _cleanup_free_ unsigned char *key_data = NULL; - - if (!hostnqn || !subsysnqn) { - nvme_msg(NULL, LOG_ERR, "Invalid NQNs (%s, %s)\n", - hostnqn, subsysnqn); - return; - } - key_data = nvme_import_tls_key(encoded_key, &key_len, &hmac); - if (!key_data) { - nvme_msg(NULL, LOG_ERR, "Failed to decode TLS Key '%s'\n", - encoded_key); - return; - } - key_id = nvme_insert_tls_key_versioned(keyring_str, "psk", - hostnqn, subsysnqn, - 0, hmac, key_data, key_len); - if (key_id <= 0) - nvme_msg(NULL, LOG_ERR, "Failed to insert TLS KEY, error %d\n", - errno); - else { - cfg->tls_key = key_id; - cfg->tls = true; - } -} - -static void json_export_nvme_tls_key(long keyring_id, long tls_key, - struct json_object *obj) -{ - int key_len; - _cleanup_free_ unsigned char *key_data = NULL; - - key_data = nvme_read_key(keyring_id, tls_key, &key_len); - if (key_data) { - _cleanup_free_ char *tls_str = NULL; - - tls_str = nvme_export_tls_key(key_data, key_len); - if (tls_str) - json_object_object_add(obj, "tls_key", - json_object_new_string(tls_str)); - } -} - static void json_update_attributes(nvme_ctrl_t c, struct json_object *ctrl_obj) { struct nvme_fabrics_config *cfg = nvme_ctrl_get_config(c); - const char *keyring_str = NULL, *encoded_key = NULL; json_object_object_foreach(ctrl_obj, key_str, val_obj) { JSON_UPDATE_INT_OPTION(cfg, key_str, @@ -120,31 +68,13 @@ static void json_update_attributes(nvme_ctrl_t c, if (!strcmp("discovery", key_str) && !nvme_ctrl_is_discovery_ctrl(c)) nvme_ctrl_set_discovery_ctrl(c, true); - /* - * The JSON configuration holds the keyring description - * which needs to be converted into the keyring serial number. - */ - if (!strcmp("keyring", key_str) && cfg->keyring == 0) { - long keyring; - - keyring_str = json_object_get_string(val_obj); - keyring = nvme_lookup_keyring(keyring_str); - if (keyring) { - cfg->keyring = keyring; - nvme_set_keyring(cfg->keyring); - } - } - if (!strcmp("tls_key", key_str) && cfg->tls_key == 0) - encoded_key = json_object_get_string(val_obj); + if (!strcmp("keyring", key_str)) + nvme_ctrl_set_keyring(c, + json_object_get_string(val_obj)); + if (!strcmp("tls_key", key_str)) + nvme_ctrl_set_tls_key(c, + json_object_get_string(val_obj)); } - - /* - * We might need the keyring information from the above loop, - * so we can only import the TLS key once all entries are - * processed. - */ - if (encoded_key) - json_import_nvme_tls_key(c, keyring_str, encoded_key); } static void json_parse_port(nvme_subsystem_t s, struct json_object *port_obj) @@ -181,6 +111,12 @@ static void json_parse_port(nvme_subsystem_t s, struct json_object *port_obj) attr_obj = json_object_object_get(port_obj, "dhchap_ctrl_key"); if (attr_obj) nvme_ctrl_set_dhchap_key(c, json_object_get_string(attr_obj)); + attr_obj = json_object_object_get(port_obj, "keyring"); + if (attr_obj) + nvme_ctrl_set_keyring(c, json_object_get_string(attr_obj)); + attr_obj = json_object_object_get(port_obj, "tls_key"); + if (attr_obj) + nvme_ctrl_set_tls_key(c, json_object_get_string(attr_obj)); } static void json_parse_subsys(nvme_host_t h, struct json_object *subsys_obj) @@ -368,6 +304,15 @@ static void json_update_port(struct json_object *ctrl_array, nvme_ctrl_t c) if (value) json_object_object_add(port_obj, "dhchap_ctrl_key", json_object_new_string(value)); + JSON_BOOL_OPTION(cfg, port_obj, tls); + value = nvme_ctrl_get_keyring(c); + if (value) + json_object_object_add(port_obj, "keyring", + json_object_new_string(value)); + value = nvme_ctrl_get_tls_key(c); + if (value) + json_object_object_add(port_obj, "tls_key", + json_object_new_string(value)); JSON_INT_OPTION(cfg, port_obj, nr_io_queues, 0); JSON_INT_OPTION(cfg, port_obj, nr_write_queues, 0); JSON_INT_OPTION(cfg, port_obj, nr_poll_queues, 0); @@ -384,7 +329,6 @@ static void json_update_port(struct json_object *ctrl_array, nvme_ctrl_t c) JSON_BOOL_OPTION(cfg, port_obj, disable_sqflow); JSON_BOOL_OPTION(cfg, port_obj, hdr_digest); JSON_BOOL_OPTION(cfg, port_obj, data_digest); - JSON_BOOL_OPTION(cfg, port_obj, tls); JSON_BOOL_OPTION(cfg, port_obj, concat); if (nvme_ctrl_is_persistent(c)) json_object_object_add(port_obj, "persistent", @@ -392,23 +336,6 @@ static void json_update_port(struct json_object *ctrl_array, nvme_ctrl_t c) if (nvme_ctrl_is_discovery_ctrl(c)) json_object_object_add(port_obj, "discovery", json_object_new_boolean(true)); - /* - * Store the keyring description in the JSON config file. - */ - if (cfg->keyring) { - _cleanup_free_ char *desc = - nvme_describe_key_serial(cfg->keyring); - - if (desc) { - json_object_object_add(port_obj, "keyring", - json_object_new_string(desc)); - } - } - /* - * Store the TLS key in PSK interchange format - */ - if (cfg->tls_key) - json_export_nvme_tls_key(cfg->keyring, cfg->tls_key, port_obj); json_object_array_add(ctrl_array, port_obj); } @@ -564,9 +491,10 @@ static void json_dump_ctrl(struct json_object *ctrl_array, nvme_ctrl_t c) if (!strcmp(transport, "tcp")) { JSON_BOOL_OPTION(cfg, ctrl_obj, tls); - if (cfg->tls_key) - json_export_nvme_tls_key(cfg->keyring, cfg->tls_key, - ctrl_obj); + value = nvme_ctrl_get_tls_key(c); + if (value) + json_object_object_add(ctrl_obj, "tls_key", + json_object_new_string(value)); } JSON_BOOL_OPTION(cfg, ctrl_obj, concat); if (nvme_ctrl_is_persistent(c)) diff --git a/src/nvme/linux.c b/src/nvme/linux.c index 35b5812a..30a45bd0 100644 --- a/src/nvme/linux.c +++ b/src/nvme/linux.c @@ -1337,27 +1337,17 @@ int nvme_scan_tls_keys(const char *keyring, nvme_scan_tls_keys_cb_t cb, return ret; } -long nvme_insert_tls_key_versioned(const char *keyring, const char *key_type, - const char *hostnqn, const char *subsysnqn, - int version, int hmac, - unsigned char *configured_key, int key_len) +long __nvme_insert_tls_key_versioned(key_serial_t keyring_id, const char *key_type, + const char *hostnqn, const char *subsysnqn, + int version, int hmac, + unsigned char *configured_key, int key_len) { _cleanup_free_ unsigned char *psk = NULL; _cleanup_free_ char *identity = NULL; - key_serial_t keyring_id, key; size_t identity_len; + key_serial_t key; int ret; - keyring_id = nvme_lookup_keyring(keyring); - if (keyring_id == 0) { - errno = ENOKEY; - return 0; - } - - ret = nvme_set_keyring(keyring_id); - if (ret < 0) - return 0; - identity_len = nvme_identity_len(hmac, version, hostnqn, subsysnqn); if (identity_len < 0) return 0; @@ -1387,6 +1377,29 @@ long nvme_insert_tls_key_versioned(const char *keyring, const char *key_type, return key; } +long nvme_insert_tls_key_versioned(const char *keyring, const char *key_type, + const char *hostnqn, const char *subsysnqn, + int version, int hmac, + unsigned char *configured_key, int key_len) +{ + key_serial_t keyring_id; + int ret; + + keyring_id = nvme_lookup_keyring(keyring); + if (keyring_id == 0) { + errno = ENOKEY; + return 0; + } + + ret = nvme_set_keyring(keyring_id); + if (ret < 0) + return 0; + return __nvme_insert_tls_key_versioned(keyring_id, key_type, + hostnqn, subsysnqn, + version, hmac, + configured_key, key_len); +} + long nvme_revoke_tls_key(const char *keyring, const char *key_type, const char *identity) { diff --git a/src/nvme/private.h b/src/nvme/private.h index efeaae83..b1834b9c 100644 --- a/src/nvme/private.h +++ b/src/nvme/private.h @@ -13,6 +13,10 @@ #include #include +#ifdef CONFIG_KEYUTILS +#include +#endif + #include "fabrics.h" #include "mi.h" @@ -299,4 +303,14 @@ void __nvme_mi_mctp_set_ops(const struct __mi_mctp_socket_ops *newops); #define SECTOR_SIZE 512 #define SECTOR_SHIFT 9 +#ifdef CONFIG_KEYUTILS +long __nvme_insert_tls_key_versioned(key_serial_t keyring_id, const char *key_type, + const char *hostnqn, const char *subsysnqn, + int version, int hmac, + unsigned char *configured_key, int key_len); +#else +#define __nvme_insert_tls_key_versioned(keyring_id, key_type, hostnqn, subsysnqn, \ + version, hmac, configured_key, key_len) 0 +#endif + #endif /* _LIBNVME_PRIVATE_H */ diff --git a/src/nvme/tree.c b/src/nvme/tree.c index 4f073bbd..be38f9ad 100644 --- a/src/nvme/tree.c +++ b/src/nvme/tree.c @@ -1916,10 +1916,9 @@ static char *nvme_ctrl_lookup_phy_slot(nvme_root_t r, const char *address) static int nvme_configure_ctrl(nvme_root_t r, nvme_ctrl_t c, const char *path, const char *name) { - DIR *d; + _cleanup_free_ char *tls_key = NULL; char *host_key, *ctrl_key; - - _cleanup_free_ char *tls_psk = NULL; + DIR *d; d = opendir(path); if (!d) { @@ -1963,14 +1962,20 @@ static int nvme_configure_ctrl(nvme_root_t r, nvme_ctrl_t c, const char *path, c->dhchap_ctrl_key = ctrl_key; } - tls_psk = nvme_get_ctrl_attr(c, "tls_key"); - if (tls_psk) { + tls_key = nvme_get_ctrl_attr(c, "tls_key"); + if (tls_key) { char *endptr; - long key_id = strtol(tls_psk, &endptr, 16); + long key_id = strtol(tls_key, &endptr, 16); - if (endptr != tls_psk) { + if (endptr != tls_key) { c->cfg.tls_key = key_id; c->cfg.tls = true; + + if (!nvme_ctrl_get_tls_key(c)) { + tls_key = nvme_describe_key_serial(c->cfg.tls_key); + if (tls_key) + nvme_ctrl_set_tls_key(c, tls_key); + } } } From b9e1addd82b57896db07c8a4123c2609b2ba45c1 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Thu, 10 Oct 2024 10:50:46 +0200 Subject: [PATCH 10/10] test: add pre-shared key json tests Add a test case for the PSK API to ensure that the generated JSON is correct. Signed-off-by: Daniel Wagner --- test/config/data/psk.json | 21 ++++++++++ test/config/data/psk.out | 21 ++++++++++ test/config/meson.build | 18 ++++++++ test/config/psk-json.c | 86 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 146 insertions(+) create mode 100644 test/config/data/psk.json create mode 100644 test/config/data/psk.out create mode 100644 test/config/psk-json.c diff --git a/test/config/data/psk.json b/test/config/data/psk.json new file mode 100644 index 00000000..5bba948d --- /dev/null +++ b/test/config/data/psk.json @@ -0,0 +1,21 @@ +[ + { + "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:befdec4c-2234-11b2-a85c-ca77c773af36", + "hostid":"2cd2c43b-a90a-45c1-a8cd-86b33ab273b6", + "subsystems":[ + { + "nqn":"nqn.io-1", + "ports":[ + { + "transport":"tcp", + "traddr":"192.168.154.148", + "trsvcid":"4420", + "dhchap_key":"none", + "tls":true, + "tls_key":"NVMeTLSkey-1:01:Hhc5sFjwSZ6w5hPY19tqprajYtuYci3tN+Z2wGViDk3rpSR+:" + } + ] + } + ] + } +] diff --git a/test/config/data/psk.out b/test/config/data/psk.out new file mode 100644 index 00000000..5bba948d --- /dev/null +++ b/test/config/data/psk.out @@ -0,0 +1,21 @@ +[ + { + "hostnqn":"nqn.2014-08.org.nvmexpress:uuid:befdec4c-2234-11b2-a85c-ca77c773af36", + "hostid":"2cd2c43b-a90a-45c1-a8cd-86b33ab273b6", + "subsystems":[ + { + "nqn":"nqn.io-1", + "ports":[ + { + "transport":"tcp", + "traddr":"192.168.154.148", + "trsvcid":"4420", + "dhchap_key":"none", + "tls":true, + "tls_key":"NVMeTLSkey-1:01:Hhc5sFjwSZ6w5hPY19tqprajYtuYci3tN+Z2wGViDk3rpSR+:" + } + ] + } + ] + } +] diff --git a/test/config/meson.build b/test/config/meson.build index f976c69b..302dca96 100644 --- a/test/config/meson.build +++ b/test/config/meson.build @@ -60,4 +60,22 @@ if diff.found() depends : test_hostnqn_order, ) + test_psk_json = executable( + 'test-psk-json', + ['psk-json.c'], + dependencies: libnvme_dep, + include_directories: [incdir], + ) + test( + 'psk-json', + config_diff, + args : [ + test_psk_json.full_path(), + builddir, + srcdir + '/data/psk.out', + '--config-json', srcdir + '/data/psk.json', + ], + depends : test_psk_json, + ) + endif diff --git a/test/config/psk-json.c b/test/config/psk-json.c new file mode 100644 index 00000000..93d6e9f8 --- /dev/null +++ b/test/config/psk-json.c @@ -0,0 +1,86 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/** + * This file is part of libnvme. + * Copyright (c) 2024 Daniel Wagner, SUSE LLC + */ + +#include "nvme/linux.h" +#include "nvme/tree.h" +#include +#include +#include +#include + +#include + +static bool import_export_key(nvme_ctrl_t c) +{ + unsigned char version, hmac, *key; + char *encoded_key; + size_t len; + + key = nvme_import_tls_key_versioned(nvme_ctrl_get_tls_key(c), + &version, &hmac, &len); + if (!key) { + printf("ERROR: nvme_import_tls_key_versioned failed with %d\n", + errno); + return false; + + } + + encoded_key = nvme_export_tls_key_versioned(version, hmac, + key, len); + if (!encoded_key) { + printf("ERROR: nvme_export_tls_key_versioned failed with %d\n", + errno); + return false; + } + + nvme_ctrl_set_tls_key(c, encoded_key); + return true; +} + +static bool psk_json_test(char *file) +{ + bool pass = false; + nvme_root_t r; + nvme_host_t h; + nvme_subsystem_t s; + nvme_ctrl_t c; + int err; + + r = nvme_create_root(stderr, LOG_ERR); + if (!r) + return false; + + err = nvme_read_config(r, file); + if (err) + goto out; + + + nvme_for_each_host(r, h) + nvme_for_each_subsystem(h, s) + nvme_subsystem_for_each_ctrl(s, c) + if (!import_export_key(c)) + goto out; + + err = nvme_dump_config(r); + if (err) + goto out; + + pass = true; + +out: + nvme_free_tree(r); + return pass; +} + +int main(int argc, char *argv[]) +{ + bool pass; + + pass = psk_json_test(argv[1]); + fflush(stdout); + + exit(pass ? EXIT_SUCCESS : EXIT_FAILURE); +}