Skip to content

Commit eff0ffe

Browse files
cleechigaw
authored andcommitted
linux: fix HKDF TLS key derivation back to OpenSSL 3.0.8
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 removes the hkdf_info_printf helper, which ended up being more of a hinderence with this change, in favor of building the info vector with calls like snprintf(p, 257, "%c%s", strlen(s), s) where HkdfLabel.label and HkdfLabel.context have a maxiumum length of 256 each. Signed-off-by: Chris Leech <cleech@redhat.com>
1 parent 86ceb42 commit eff0ffe

File tree

1 file changed

+74
-51
lines changed

1 file changed

+74
-51
lines changed

src/nvme/linux.c

Lines changed: 74 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -686,45 +686,10 @@ static DEFINE_CLEANUP_FUNC(
686686
cleanup_evp_pkey_ctx, EVP_PKEY_CTX *, EVP_PKEY_CTX_free)
687687
#define _cleanup_evp_pkey_ctx_ __cleanup__(cleanup_evp_pkey_ctx)
688688

689-
/*
690-
* hkdf_info_printf()
691-
*
692-
* Helper function to append variable length label and context to an HkdfLabel
693-
*
694-
* RFC 8446 (TLS 1.3) Section 7.1 defines the HKDF-Expand-Label function as a
695-
* specialization of the HKDF-Expand function (RFC 5869), where the info
696-
* parameter is structured as an HkdfLabel.
697-
*
698-
* An HkdfLabel structure includes two variable length vectors (label and
699-
* context) which must be preceded by their content length as per RFC 8446
700-
* Section 3.4 (and not NUL terminated as per Section 7.1). Additionally,
701-
* HkdfLabel.label must begin with "tls13 "
702-
*
703-
* Returns the number of bytes appended to the HKDF info buffer, or -1 on an
704-
* error.
705-
*/
706-
__attribute__((format(printf, 2, 3)))
707-
static int hkdf_info_printf(EVP_PKEY_CTX *ctx, char *fmt, ...)
708-
{
709-
_cleanup_free_ char *str;
710-
uint8_t len;
711-
int ret;
712-
va_list myargs;
713-
714-
va_start(myargs, fmt);
715-
ret = vasprintf(&str, fmt, myargs);
716-
va_end(myargs);
717-
if (ret < 0)
718-
return ret;
719-
if (ret > 255)
720-
return -1;
721-
len = ret;
722-
if (EVP_PKEY_CTX_add1_hkdf_info(ctx, (unsigned char *)&len, 1) <= 0)
723-
return -1;
724-
if (EVP_PKEY_CTX_add1_hkdf_info(ctx, (unsigned char *)str, len) <= 0)
725-
return -1;
726-
return (ret + 1);
727-
}
689+
/* NVMe is using the TLS 1.3 HkdfLabel structure */
690+
#define HKDF_INFO_MAX_LEN 514
691+
#define HKDF_INFO_LABEL_MAX 256
692+
#define HKDF_INFO_CONTEXT_MAX 256
728693

729694
/*
730695
* derive_retained_key()
@@ -760,9 +725,19 @@ static int derive_retained_key(int hmac, const char *hostnqn,
760725
size_t key_len)
761726
{
762727
_cleanup_evp_pkey_ctx_ EVP_PKEY_CTX *ctx = NULL;
763-
uint16_t length = htons(key_len & 0xFFFF);
728+
_cleanup_free_ uint8_t *hkdf_info = NULL;
729+
char *hkdf_label;
764730
const EVP_MD *md;
765731
size_t hmac_len;
732+
char *pos;
733+
int ret;
734+
735+
/* +1 byte so that the snprintf terminating null can not overflow */
736+
hkdf_info = malloc(HKDF_INFO_MAX_LEN + 1);
737+
if (!hkdf_info) {
738+
errno = ENOMEM;
739+
return -1;
740+
}
766741

767742
if (hmac == NVME_HMAC_ALG_NONE) {
768743
memcpy(retained, configured, key_len);
@@ -793,16 +768,34 @@ static int derive_retained_key(int hmac, const char *hostnqn,
793768
errno = ENOKEY;
794769
return -1;
795770
}
796-
if (EVP_PKEY_CTX_add1_hkdf_info(ctx,
797-
(const unsigned char *)&length, 2) <= 0) {
771+
772+
if (key_len > USHRT_MAX) {
773+
errno = EINVAL;
774+
return -1;
775+
}
776+
pos = (char *)hkdf_info;
777+
*(uint16_t *)pos = htons(key_len & 0xFFFF);
778+
pos += sizeof(uint16_t);
779+
780+
hkdf_label = "tls13 HostNQN";
781+
ret = snprintf(pos, HKDF_INFO_LABEL_MAX + 1, "%c%s",
782+
(int)strlen(hkdf_label), hkdf_label);
783+
if (ret <= 0 || ret > HKDF_INFO_LABEL_MAX) {
798784
errno = ENOKEY;
799785
return -1;
800786
}
801-
if (hkdf_info_printf(ctx, "tls13 HostNQN") <= 0) {
787+
pos += ret;
788+
789+
ret = snprintf(pos, HKDF_INFO_CONTEXT_MAX + 1, "%c%s",
790+
(int)strlen(hostnqn), hostnqn);
791+
if (ret <= 0 || ret > HKDF_INFO_CONTEXT_MAX) {
802792
errno = ENOKEY;
803793
return -1;
804794
}
805-
if (hkdf_info_printf(ctx, "%s", hostnqn) <= 0) {
795+
pos += ret;
796+
797+
if (EVP_PKEY_CTX_add1_hkdf_info(ctx, hkdf_info,
798+
(pos - (char *)hkdf_info)) <= 0) {
806799
errno = ENOKEY;
807800
return -1;
808801
}
@@ -911,9 +904,19 @@ static int derive_tls_key(int version, unsigned char cipher,
911904
unsigned char *psk, size_t key_len)
912905
{
913906
_cleanup_evp_pkey_ctx_ EVP_PKEY_CTX *ctx = NULL;
914-
uint16_t length = htons(key_len & 0xFFFF);
907+
_cleanup_free_ uint8_t *hkdf_info = NULL;
908+
char *hkdf_label;
915909
const EVP_MD *md;
916910
size_t hmac_len;
911+
char *pos;
912+
int ret;
913+
914+
/* +1 byte so that the snprintf terminating null can not overflow */
915+
hkdf_info = malloc(HKDF_INFO_MAX_LEN + 1);
916+
if (!hkdf_info) {
917+
errno = ENOMEM;
918+
return -1;
919+
}
917920

918921
md = select_hmac(cipher, &hmac_len);
919922
if (!md || !hmac_len) {
@@ -939,33 +942,53 @@ static int derive_tls_key(int version, unsigned char cipher,
939942
errno = ENOKEY;
940943
return -1;
941944
}
942-
if (EVP_PKEY_CTX_add1_hkdf_info(ctx,
943-
(const unsigned char *)&length, 2) <= 0) {
944-
errno = ENOKEY;
945+
946+
if (key_len > USHRT_MAX) {
947+
errno = EINVAL;
945948
return -1;
946949
}
947-
if (hkdf_info_printf(ctx, "tls13 nvme-tls-psk") <= 0) {
950+
pos = (char *)hkdf_info;
951+
*(uint16_t *)pos = htons(key_len & 0xFFFF);
952+
pos += sizeof(uint16_t);
953+
954+
hkdf_label = "tls13 nvme-tls-psk";
955+
ret = snprintf(pos, HKDF_INFO_LABEL_MAX + 1, "%c%s",
956+
(int)strlen(hkdf_label), hkdf_label);
957+
if (ret <= 0 || ret > HKDF_INFO_LABEL_MAX) {
948958
errno = ENOKEY;
949959
return -1;
950960
}
961+
pos += ret;
962+
951963
switch (version) {
952964
case 0:
953-
if (hkdf_info_printf(ctx, "%s", context) <= 0) {
965+
ret = snprintf(pos, HKDF_INFO_CONTEXT_MAX + 1, "%c%s",
966+
(int)strlen(context), context);
967+
if (ret <= 0 || ret > HKDF_INFO_CONTEXT_MAX) {
954968
errno = ENOKEY;
955969
return -1;
956970
}
971+
pos += ret;
957972
break;
958973
case 1:
959-
if (hkdf_info_printf(ctx, "%02d %s", cipher, context) <= 0) {
974+
ret = snprintf(pos, HKDF_INFO_CONTEXT_MAX + 1, "%c%02d %s",
975+
(int)strlen(context) + 3, cipher, context);
976+
if (ret <= 0 || ret > HKDF_INFO_CONTEXT_MAX) {
960977
errno = ENOKEY;
961978
return -1;
962979
}
980+
pos += ret;
963981
break;
964982
default:
965983
errno = ENOKEY;
966984
return -1;
967985
}
968986

987+
if (EVP_PKEY_CTX_add1_hkdf_info(ctx, hkdf_info,
988+
(pos - (char *)hkdf_info)) <= 0) {
989+
errno = ENOKEY;
990+
return -1;
991+
}
969992
if (EVP_PKEY_derive(ctx, psk, &key_len) <= 0) {
970993
errno = ENOKEY;
971994
return -1;

0 commit comments

Comments
 (0)