linux: use EVP_PKEY_CTX_add1_hkdf_info only once in compat function

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())

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;
 	}
@@ -1275,7 +1299,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;
 	}

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 <[email protected]>
+ */
+
+#include <openssl/core_names.h>
+#include <openssl/evp.h>
+#include <openssl/hmac.h>
+#include <openssl/kdf.h>
+#include <openssl/params.h>
+#include <stdio.h>
+#include <string.h>
+
+#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;
+}

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