From b74d5a175c2de9f56a43169043e0db047b73e9ef Mon Sep 17 00:00:00 2001 From: Neil Horman Date: Fri, 12 Sep 2025 13:11:46 -0400 Subject: [PATCH] Fix ossl_prov_set_macctx This function fails to construct a param list that includes the passed in property query string in the param lists when allocating subordonate algorithms. Make sure we allow callers to pass a param list (so that providers for subordonate algorithms can be selected), and merge those into the param list that this function builds on its own. Reviewed-by: Matt Caswell Reviewed-by: Shane Lontis Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/28461) --- providers/common/include/prov/provider_util.h | 3 +- providers/common/provider_util.c | 32 ++++++++++++++++--- .../signature/mac_legacy_sig.c | 4 +-- ssl/record/methods/tls1_meth.c | 5 +-- ssl/statem/extensions.c | 9 ++---- 5 files changed, 36 insertions(+), 17 deletions(-) diff --git a/providers/common/include/prov/provider_util.h b/providers/common/include/prov/provider_util.h index e29a1d4ce2b..17f7652cb6c 100644 --- a/providers/common/include/prov/provider_util.h +++ b/providers/common/include/prov/provider_util.h @@ -105,7 +105,8 @@ int ossl_prov_set_macctx(EVP_MAC_CTX *macctx, const char *ciphername, const char *mdname, const char *engine, - const char *properties); + const char *properties, + const OSSL_PARAM param[]); /* MAC functions */ /* diff --git a/providers/common/provider_util.c b/providers/common/provider_util.c index ee4b4cd3846..9c1f79ca62d 100644 --- a/providers/common/provider_util.c +++ b/providers/common/provider_util.c @@ -221,9 +221,12 @@ int ossl_prov_set_macctx(EVP_MAC_CTX *macctx, const char *ciphername, const char *mdname, const char *engine, - const char *properties) + const char *properties, + const OSSL_PARAM param[]) { - OSSL_PARAM mac_params[5], *mp = mac_params; + OSSL_PARAM mac_params[5], *mp = mac_params, *mergep; + int free_merge = 0; + int ret; if (mdname != NULL) *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, @@ -243,8 +246,29 @@ int ossl_prov_set_macctx(EVP_MAC_CTX *macctx, *mp = OSSL_PARAM_construct_end(); - return EVP_MAC_CTX_set_params(macctx, mac_params); + /* + * OSSL_PARAM_merge returns NULL and sets an error if either + * list passed to it is NULL, and we aren't guaranteed that the + * passed in value of param is not NULL here. + * Given that we just want the union of the two lists, even if one + * is empty, we have to check for that case, and if param is NULL, + * just use the mac_params list. In turn we only free the merge + * result if we actually did the merge + */ + if (param == NULL) { + mergep = mac_params; + } else { + free_merge = 1; + mergep = OSSL_PARAM_merge(mac_params, param); + if (mergep == NULL) + return 0; + } + + ret = EVP_MAC_CTX_set_params(macctx, mergep); + if (free_merge == 1) + OSSL_PARAM_free(mergep); + return ret; } int ossl_prov_macctx_load(EVP_MAC_CTX **macctx, @@ -291,7 +315,7 @@ int ossl_prov_macctx_load(EVP_MAC_CTX **macctx, if (pengine != NULL && !OSSL_PARAM_get_utf8_string_ptr(pengine, &engine)) return 0; - if (ossl_prov_set_macctx(*macctx, ciphername, mdname, engine, properties)) + if (ossl_prov_set_macctx(*macctx, ciphername, mdname, engine, properties, NULL)) return 1; EVP_MAC_CTX_free(*macctx); diff --git a/providers/implementations/signature/mac_legacy_sig.c b/providers/implementations/signature/mac_legacy_sig.c index b8e9b7a76e1..a5661eaa0b5 100644 --- a/providers/implementations/signature/mac_legacy_sig.c +++ b/providers/implementations/signature/mac_legacy_sig.c @@ -126,11 +126,11 @@ static int mac_digest_sign_init(void *vpmacctx, const char *mdname, void *vkey, (char *)ciphername, (char *)mdname, (char *)engine, - pmacctx->key->properties)) + pmacctx->key->properties, params)) return 0; if (!EVP_MAC_init(pmacctx->macctx, pmacctx->key->priv_key, - pmacctx->key->priv_key_len, params)) + pmacctx->key->priv_key_len, NULL)) return 0; return 1; diff --git a/ssl/record/methods/tls1_meth.c b/ssl/record/methods/tls1_meth.c index ac31d359c91..19295f6e587 100644 --- a/ssl/record/methods/tls1_meth.c +++ b/ssl/record/methods/tls1_meth.c @@ -28,7 +28,7 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, { EVP_CIPHER_CTX *ciph_ctx; EVP_PKEY *mac_key; - OSSL_PARAM params[3], *p = params; + OSSL_PARAM params[2], *p = params; int enc = (rl->direction == OSSL_RECORD_DIRECTION_WRITE) ? 1 : 0; if (level != OSSL_RECORD_PROTECTION_LEVEL_APPLICATION) @@ -75,9 +75,6 @@ static int tls1_set_crypto_state(OSSL_RECORD_LAYER *rl, int level, (int)mackeylen); } - *p++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, - (char *)EVP_MD_get0_name(md), 0); - /* * We want the underlying mac to use our passed property query when allocating * its internal digest as well diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c index d637e06d538..59587fedbe9 100644 --- a/ssl/statem/extensions.c +++ b/ssl/statem/extensions.c @@ -1537,7 +1537,7 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md, int ret = -1; int usepskfored = 0; SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s); - OSSL_PARAM params[3] = { OSSL_PARAM_END, OSSL_PARAM_END, OSSL_PARAM_END }; + OSSL_PARAM params[2] = { OSSL_PARAM_END, OSSL_PARAM_END }; /* Ensure cast to size_t is safe */ if (!ossl_assert(hashsizei > 0)) { @@ -1668,12 +1668,9 @@ int tls_psk_do_binder(SSL_CONNECTION *s, const EVP_MD *md, if (!sign) binderout = tmpbinder; - if (sctx->propq != NULL) { - params[0] = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, - (char *)EVP_MD_get0_name(md), 0); - params[1] = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_PROPERTIES, + if (sctx->propq != NULL) + params[0] = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_PROPERTIES, (char *)sctx->propq, 0); - } bindersize = hashsize; if (EVP_DigestSignInit_ex(mctx, NULL, EVP_MD_get0_name(md), sctx->libctx, sctx->propq, mackey, params) <= 0 -- 2.47.3