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 <matt@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Paul Dale <ppzgs1@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/28461)

diff --git a/providers/common/include/prov/provider_util.h b/providers/common/include/prov/provider_util.h
index e29a1d4ce2bc60b127f98e9a2afb68b21cc49094..17f7652cb6c1396983746839704c1ecb33b9693e 100644 (file)
--- 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 ee4b4cd38463c570720e1a096ebcbdfe3f54523e..9c1f79ca62dac6a5ca69e2532d911530697ad877 100644 (file)
--- 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 b8e9b7a76e104254829305c4df9b5e8840a51d90..a5661eaa0b55ad6d1145a1f817d2c5c9d4a172d2 100644 (file)
--- 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 ac31d359c912aba22f7819bf82e7e696b67615f7..19295f6e587d1be0c2d5bbcb6eadb6ae1665a85b 100644 (file)
--- 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 d637e06d538599fe8b03634bba4e20fdd9d9a6e7..59587fedbe9b6fbea15beae80082fadde351c716 100644 (file)
--- 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