From acd30ba1215f97b5db1dacb6aad2a896660f389b Mon Sep 17 00:00:00 2001 From: Hannes Reinecke Date: Fri, 5 Sep 2025 08:55:35 +0200 Subject: [PATCH 1/3] nvme/linux: check for empty digest in gen_tls_identity() Newer GCC complain about 'digest' being an empty argument, which is technically true, but in practice can only happen if version == 0, which we already check. So add a warning to keep GCC happy. Signed-off-by: Hannes Reinecke --- src/nvme/linux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nvme/linux.c b/src/nvme/linux.c index 1ab2b015f..3a3d3ee39 100644 --- a/src/nvme/linux.c +++ b/src/nvme/linux.c @@ -1275,7 +1275,7 @@ static int gen_tls_identity(const char *hostnqn, const char *subsysnqn, version, cipher, hostnqn, subsysnqn); return strlen(identity); } - if (version > 1) { + if (version > 1 || !digest) { errno = EINVAL; return -1; } From 718f5c98891a23342d71d1152318624f9c59b301 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Tue, 16 Sep 2025 19:24:12 +0200 Subject: [PATCH 2/3] linux: use EVP_PKEY_CTX_add1_hkdf_info only once in compat function OpenSSL prior to 3.3.1 had an issue with EVP_PKEY_CTX_add1_hkdf_info() where it acted like a 'set1' function instead of an 'add1' as documented. Work around that by building the entire info vector outside of the OpenSSL API and only calling this function once. This is the same workaround used in commit eff0ffef0273 ("linux: fix HKDF TLS key derivation back to OpenSSL 3.0.8"). Signed-off-by: Daniel Wagner --- src/nvme/linux.c | 90 ++++++++++++++++++++++++++++++------------------ 1 file changed, 57 insertions(+), 33 deletions(-) diff --git a/src/nvme/linux.c b/src/nvme/linux.c index 3a3d3ee39..a6a1f4d2d 100644 --- a/src/nvme/linux.c +++ b/src/nvme/linux.c @@ -814,9 +814,11 @@ static int derive_retained_key_compat(int hmac, const char *hostnqn, size_t key_len) { _cleanup_evp_pkey_ctx_ EVP_PKEY_CTX *ctx = NULL; - uint16_t length = key_len & 0xFFFF; + _cleanup_free_ uint8_t *hkdf_info = NULL; const EVP_MD *md; size_t hmac_len; + char *pos; + int ret; if (hmac == NVME_HMAC_ALG_NONE) { memcpy(retained, configured, key_len); @@ -847,23 +849,28 @@ static int derive_retained_key_compat(int hmac, const char *hostnqn, errno = ENOKEY; return -1; } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)&length, 2) <= 0) { - errno = ENOKEY; - return -1; - } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)"tls13 ", 6) <= 0) { - errno = ENOKEY; + + /* +1 byte so that the snprintf terminating null can not overflow */ + hkdf_info = malloc(HKDF_INFO_MAX_LEN + 1); + if (!hkdf_info) { + errno = ENOMEM; return -1; } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)"HostNQN", 7) <= 0) { + + pos = (char *)hkdf_info; + *(uint16_t *)pos = cpu_to_le16(key_len); + pos += sizeof(uint16_t); + + ret = snprintf(pos, HKDF_INFO_LABEL_MAX + 1, + "tls13 HostNQN%s", hostnqn); + if (ret <= 0 || ret > HKDF_INFO_LABEL_MAX) { errno = ENOKEY; return -1; } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)hostnqn, strlen(hostnqn)) <= 0) { + pos += ret; + + if (EVP_PKEY_CTX_add1_hkdf_info(ctx, hkdf_info, + (pos - (char *)hkdf_info)) <= 0) { errno = ENOKEY; return -1; } @@ -1002,9 +1009,11 @@ static int derive_tls_key_compat(int version, unsigned char cipher, unsigned char *psk, size_t key_len) { _cleanup_evp_pkey_ctx_ EVP_PKEY_CTX *ctx = NULL; - uint16_t length = key_len & 0xFFFF; + _cleanup_free_ uint8_t *hkdf_info = NULL; const EVP_MD *md; size_t hmac_len; + char *pos; + int ret; md = select_hmac(cipher, &hmac_len); if (!md || !hmac_len) { @@ -1030,35 +1039,50 @@ static int derive_tls_key_compat(int version, unsigned char cipher, errno = ENOKEY; return -1; } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)&length, 2) <= 0) { - errno = ENOKEY; - return -1; - } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)"tls13 ", 6) <= 0) { - errno = ENOKEY; + + /* +1 byte so that the snprintf terminating null can not overflow */ + hkdf_info = malloc(HKDF_INFO_MAX_LEN + 1); + if (!hkdf_info) { + errno = ENOMEM; return -1; } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)"nvme-tls-psk", 12) <= 0) { + + pos = (char *)hkdf_info; + *(uint16_t *)pos = cpu_to_le16(key_len); + pos += sizeof(uint16_t); + + ret = snprintf(pos, HKDF_INFO_LABEL_MAX + 1, "tls13 nvme-tls-psk"); + if (ret <= 0 || ret > HKDF_INFO_LABEL_MAX) { errno = ENOKEY; return -1; } - if (version == 1) { - char hash_str[5]; + pos += ret; - sprintf(hash_str, "%02d ", cipher); - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)hash_str, - strlen(hash_str)) <= 0) { + switch (version) { + case 0: + ret = snprintf(pos, HKDF_INFO_CONTEXT_MAX + 1, "%s", context); + if (ret <= 0 || ret > HKDF_INFO_CONTEXT_MAX) { errno = ENOKEY; return -1; } + pos += ret; + break; + case 1: + ret = snprintf(pos, HKDF_INFO_CONTEXT_MAX + 1, "%02d %s", + cipher, context); + if (ret <= 0 || ret > HKDF_INFO_CONTEXT_MAX) { + errno = ENOKEY; + return -1; + } + pos += ret; + break; + default: + errno = ENOKEY; + return -1; } - if (EVP_PKEY_CTX_add1_hkdf_info(ctx, - (const unsigned char *)context, - strlen(context)) <= 0) { + + if (EVP_PKEY_CTX_add1_hkdf_info(ctx, hkdf_info, + (pos - (char *)hkdf_info)) <= 0) { errno = ENOKEY; return -1; } From dc17fef7e38bdeacb572390e27f525f3de461693 Mon Sep 17 00:00:00 2001 From: Daniel Wagner Date: Wed, 17 Sep 2025 10:12:10 +0200 Subject: [PATCH 3/3] test: add hkdf_add1 test The EVP_PKEY_CTX_add1_hkdf_info implementation had a bug in the past which made it behave like a set instead of add function. When linking against external builds warn about it. The libnvme implementation works around this problem, but it's better to have this logged during the configure step, so there is chance to debug this. Signed-off-by: Daniel Wagner --- meson.build | 16 +++++++++ test/hkdf_add1.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++ test/meson.build | 8 +++++ 3 files changed, 115 insertions(+) create mode 100644 test/hkdf_add1.c diff --git a/meson.build b/meson.build index c48904da1..ec60ea680 100644 --- a/meson.build +++ b/meson.build @@ -252,6 +252,22 @@ else conf.set('fallthrough', 'do {} while (0) /* fallthrough */') endif +if openssl_dep.found() + if openssl_dep.type_name() != 'internal' + # Check for a bug in the EVP_PKEY_CTX_add1_hkdf_info implementation + res = cc.run( + files('test/hkdf_add1.c'), + dependencies: [openssl_dep], + name: 'check hkdf_add1' + ) + if res.returncode() == 1 + warning('EVP_PKEY_CTX_add1_hkdf_info bahaves incorrectly') + else + message('EVP_PKEY_CTX_add1_hkdf_info behaves sanely') + endif + endif +endif + ################################################################################ substs = configuration_data() substs.set('NAME', meson.project_name()) diff --git a/test/hkdf_add1.c b/test/hkdf_add1.c new file mode 100644 index 000000000..ceb0ac9df --- /dev/null +++ b/test/hkdf_add1.c @@ -0,0 +1,91 @@ +// SPDX-License-Identifier: LGPL-2.1-or-later +/** + * This file is part of libnvme. + * Copyright (c) 2025 SUSE LLC. + * + * Authors: Daniel Wagner + */ + +#include +#include +#include +#include +#include +#include +#include + +#define SHA256_LEN 32 + +static EVP_PKEY_CTX *setup_ctx(void) +{ + EVP_PKEY_CTX *ctx = NULL; + const char *salt = "salt"; + const char *key = "key"; + + ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL); + if (!ctx) + return NULL; + if (EVP_PKEY_derive_init(ctx) <= 0) + goto free_ctx; + if (EVP_PKEY_CTX_set_hkdf_md(ctx, EVP_sha256()) <= 0) + goto free_ctx; + if (EVP_PKEY_CTX_set1_hkdf_salt(ctx, + (unsigned char *)salt, strlen(salt)) <= 0) + goto free_ctx; + if (EVP_PKEY_CTX_set1_hkdf_key(ctx, + (unsigned char *)key, strlen(key)) <= 0) + goto free_ctx; + + return ctx; + +free_ctx: + EVP_PKEY_CTX_free(ctx); + return NULL; +} + +int main(void) +{ + unsigned char out[SHA256_LEN], out2[SHA256_LEN]; + size_t len = sizeof(out); + const char *a = "a"; + const char *b = "b"; + EVP_PKEY_CTX *ctx; + + /* out = A + B */ + ctx = setup_ctx(); + if (!ctx) + return 1; + if (EVP_PKEY_CTX_add1_hkdf_info(ctx, + (unsigned char *)a, strlen(a)) <= 0) + goto free_ctx; + if (EVP_PKEY_CTX_add1_hkdf_info(ctx, + (unsigned char *)b, strlen(b)) <= 0) + goto free_ctx; + if (EVP_PKEY_derive(ctx, out, &len) <= 0) + goto free_ctx; + EVP_PKEY_CTX_free(ctx); + + /* out = B */ + ctx = setup_ctx(); + if (!ctx) + return 1; + if (EVP_PKEY_CTX_add1_hkdf_info(ctx, + (unsigned char *)b, strlen(b)) <= 0) + goto free_ctx; + if (EVP_PKEY_derive(ctx, out2, &len) <= 0) + goto free_ctx; + EVP_PKEY_CTX_free(ctx); + + printf("EVP_PKEY_CTX_add1_hkdf_info behavior: "); + if (!memcmp(out, out2, len)) { + printf("set\n"); + return 1; + } + + printf("add\n"); + return 0; + +free_ctx: + EVP_PKEY_CTX_free(ctx); + return 1; +} diff --git a/test/meson.build b/test/meson.build index d3d64562f..464d1ba10 100644 --- a/test/meson.build +++ b/test/meson.build @@ -131,3 +131,11 @@ if json_c_dep.found() subdir('sysfs') subdir('config') endif + +if openssl_dep.found() + hkdf_add1 = executable( + 'hkdf_add1', + ['hkdf_add1.c'], + dependencies: openssl_dep, + ) +endif