Refactor how KEYMGMT methods get associated with other methods

KEYMGMT methods were attached to other methods after those were fully
created and registered, thereby creating a potential data race, if two
threads tried to create the exact same method at the same time.

Instead of this, we change the method creating function to take an
extra data parameter, passed all the way from the public fetching
function.  In the case of EVP_KEYEXCH, we pass all the necessary data
that evp_keyexch_from_dispatch() needs to be able to fetch the
appropriate KEYMGMT method on the fly.

Fixes #9592

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9678)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 58f6c4894fea088eb45b3f185b6fa905c67c1669..9b682d5084d10262f6a0b5ade490adefd6a2ee00 100644 (file)
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -2484,6 +2484,7 @@ EVP_R_NOT_XOF_OR_INVALID_LENGTH:178:not XOF or invalid length
 EVP_R_NO_CIPHER_SET:131:no cipher set
 EVP_R_NO_DEFAULT_DIGEST:158:no default digest
 EVP_R_NO_DIGEST_SET:139:no digest set
+EVP_R_NO_KEYMGMT_AVAILABLE:199:no keymgmt available
 EVP_R_NO_KEYMGMT_PRESENT:196:no keymgmt present
 EVP_R_NO_KEY_SET:154:no key set
 EVP_R_NO_OPERATION_SET:149:no operation set

diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c
index dc7f922a1196642ec94de76da7b467860fcf586f..bb6d31bf4ff0c7baa42d4c0b1c3ca40db8225dbb 100644 (file)
--- a/crypto/evp/digest.c
+++ b/crypto/evp/digest.c
@@ -617,7 +617,7 @@ int EVP_MD_CTX_ctrl(EVP_MD_CTX *ctx, int cmd, int p1, void *p2)
 }
 
 static void *evp_md_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
-                                  OSSL_PROVIDER *prov)
+                                  OSSL_PROVIDER *prov, void *unused)
 {
     EVP_MD *md = NULL;
     int fncnt = 0;
@@ -744,7 +744,7 @@ EVP_MD *EVP_MD_fetch(OPENSSL_CTX *ctx, const char *algorithm,
 {
     EVP_MD *md =
         evp_generic_fetch(ctx, OSSL_OP_DIGEST, algorithm, properties,
-                          evp_md_from_dispatch, evp_md_up_ref,
+                          evp_md_from_dispatch, NULL, evp_md_up_ref,
                           evp_md_free);
 
     return md;
@@ -756,5 +756,5 @@ void EVP_MD_do_all_ex(OPENSSL_CTX *libctx,
 {
     evp_generic_do_all(libctx, OSSL_OP_DIGEST,
                        (void (*)(void *, void *))fn, arg,
-                       evp_md_from_dispatch, evp_md_free);
+                       evp_md_from_dispatch, NULL, evp_md_free);
 }

diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c
index 96a15ef897e10c225ca3859453779ce40d9630c2..142ffecfed7ad141f08948071dc4bb3b88423a1f 100644 (file)
--- a/crypto/evp/evp_enc.c
+++ b/crypto/evp/evp_enc.c
@@ -1246,7 +1246,8 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in)
 
 static void *evp_cipher_from_dispatch(const char *name,
                                       const OSSL_DISPATCH *fns,
-                                      OSSL_PROVIDER *prov)
+                                      OSSL_PROVIDER *prov,
+                                      void *unused)
 {
     EVP_CIPHER *cipher = NULL;
     int fnciphcnt = 0, fnctxcnt = 0;
@@ -1386,7 +1387,7 @@ EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm,
 {
     EVP_CIPHER *cipher =
         evp_generic_fetch(ctx, OSSL_OP_CIPHER, algorithm, properties,
-                          evp_cipher_from_dispatch, evp_cipher_up_ref,
+                          evp_cipher_from_dispatch, NULL, evp_cipher_up_ref,
                           evp_cipher_free);
 
     return cipher;
@@ -1398,5 +1399,5 @@ void EVP_CIPHER_do_all_ex(OPENSSL_CTX *libctx,
 {
     evp_generic_do_all(libctx, OSSL_OP_CIPHER,
                        (void (*)(void *, void *))fn, arg,
-                       evp_cipher_from_dispatch, evp_cipher_free);
+                       evp_cipher_from_dispatch, NULL, evp_cipher_free);
 }

diff --git a/crypto/evp/evp_err.c b/crypto/evp/evp_err.c
index 749f189be3db375edfc4ffba576ae94c5f5a1bdb..63174f98f62c8c86972262d2cc18cad412b2fd83 100644 (file)
--- a/crypto/evp/evp_err.c
+++ b/crypto/evp/evp_err.c
@@ -99,6 +99,8 @@ static const ERR_STRING_DATA EVP_str_reasons[] = {
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_CIPHER_SET), "no cipher set"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_DEFAULT_DIGEST), "no default digest"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_DIGEST_SET), "no digest set"},
+    {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_KEYMGMT_AVAILABLE),
+    "no keymgmt available"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_KEYMGMT_PRESENT), "no keymgmt present"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_KEY_SET), "no key set"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_OPERATION_SET), "no operation set"},

diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c
index fa85178a7e06698dddb13cd3a9675795edd00a90..662195e4de08275c1d38f12b7800c2980e84e6e4 100644 (file)
--- a/crypto/evp/evp_fetch.c
+++ b/crypto/evp/evp_fetch.c
@@ -41,7 +41,8 @@ struct method_data_st {
     const char *name;
     OSSL_METHOD_CONSTRUCT_METHOD *mcm;
     void *(*method_from_dispatch)(const char *, const OSSL_DISPATCH *,
-                                  OSSL_PROVIDER *);
+                                  OSSL_PROVIDER *, void *);
+    void *method_data;
     int (*refcnt_up_method)(void *method);
     void (*destruct_method)(void *method);
 };
@@ -142,7 +143,8 @@ static void *construct_method(const char *name, const OSSL_DISPATCH *fns,
 {
     struct method_data_st *methdata = data;
 
-    return methdata->method_from_dispatch(name, fns, prov);
+    return methdata->method_from_dispatch(name, fns, prov,
+                                          methdata->method_data);
 }
 
 static void destruct_method(void *method, void *data)
@@ -156,7 +158,9 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
                         const char *name, const char *properties,
                         void *(*new_method)(const char *name,
                                             const OSSL_DISPATCH *fns,
-                                            OSSL_PROVIDER *prov),
+                                            OSSL_PROVIDER *prov,
+                                            void *method_data),
+                        void *method_data,
                         int (*up_ref_method)(void *),
                         void (*free_method)(void *))
 {
@@ -205,6 +209,7 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
         mcmdata.destruct_method = free_method;
         mcmdata.refcnt_up_method = up_ref_method;
         mcmdata.destruct_method = free_method;
+        mcmdata.method_data = method_data;
         if ((method = ossl_method_construct(libctx, operation_id, name,
                                             properties, 0 /* !force_cache */,
                                             &mcm, &mcmdata)) != NULL) {
@@ -239,7 +244,7 @@ struct do_all_data_st {
     void (*user_fn)(void *method, void *arg);
     void *user_arg;
     void *(*new_method)(const char *name, const OSSL_DISPATCH *fns,
-                        OSSL_PROVIDER *prov);
+                        OSSL_PROVIDER *prov, void *method_data);
     void (*free_method)(void *);
 };
 
@@ -248,7 +253,7 @@ static void do_one(OSSL_PROVIDER *provider, const OSSL_ALGORITHM *algo,
 {
     struct do_all_data_st *data = vdata;
     void *method = data->new_method(algo->algorithm_name,
-                                    algo->implementation, provider);
+                                    algo->implementation, provider, NULL);
 
     if (method != NULL) {
         data->user_fn(method, data->user_arg);
@@ -261,7 +266,9 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
                         void *user_arg,
                         void *(*new_method)(const char *name,
                                             const OSSL_DISPATCH *fns,
-                                            OSSL_PROVIDER *prov),
+                                            OSSL_PROVIDER *prov,
+                                            void *method_data),
+                        void *method_data,
                         void (*free_method)(void *))
 {
     struct do_all_data_st data;
@@ -270,5 +277,5 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
     data.free_method = free_method;
     data.user_fn = user_fn;
     data.user_arg = user_arg;
-    ossl_algorithm_do_all(libctx, operation_id, NULL, do_one, &data);
+    ossl_algorithm_do_all(libctx, operation_id, method_data, do_one, &data);
 }

diff --git a/crypto/evp/evp_locl.h b/crypto/evp/evp_locl.h
index 3fd73212a4b346f57340a966b93aed085452e03a..a7b36dbc0e2ca6c2c113170b703dd1b88c30ae73 100644 (file)
--- a/crypto/evp/evp_locl.h
+++ b/crypto/evp/evp_locl.h
@@ -141,7 +141,9 @@ void *evp_generic_fetch(OPENSSL_CTX *ctx, int operation_id,
                         const char *algorithm, const char *properties,
                         void *(*new_method)(const char *name,
                                             const OSSL_DISPATCH *fns,
-                                            OSSL_PROVIDER *prov),
+                                            OSSL_PROVIDER *prov,
+                                            void *method_data),
+                        void *method_data,
                         int (*up_ref_method)(void *),
                         void (*free_method)(void *));
 void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
@@ -149,7 +151,9 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
                         void *user_arg,
                         void *(*new_method)(const char *name,
                                             const OSSL_DISPATCH *fns,
-                                            OSSL_PROVIDER *prov),
+                                            OSSL_PROVIDER *prov,
+                                            void *method_data),
+                        void *method_data,
                         void (*free_method)(void *));
 
 /* Helper functions to avoid duplicating code */

diff --git a/crypto/evp/exchange.c b/crypto/evp/exchange.c
index 20503b5f6724fe1d3f66f327be8acfd40e7e8ffd..e69e4fd0b2832145714a7372b0211e168bfbf9c7 100644 (file)
--- a/crypto/evp/exchange.c
+++ b/crypto/evp/exchange.c
@@ -32,20 +32,45 @@ static EVP_KEYEXCH *evp_keyexch_new(OSSL_PROVIDER *prov)
     return exchange;
 }
 
+struct keymgmt_data_st {
+    OPENSSL_CTX *ctx;
+    const char *properties;
+};
+
 static void *evp_keyexch_from_dispatch(const char *name,
                                        const OSSL_DISPATCH *fns,
-                                       OSSL_PROVIDER *prov)
+                                       OSSL_PROVIDER *prov,
+                                       void *vkeymgmt_data)
 {
+    /*
+     * Key exchange cannot work without a key, and key management
+     * from the same provider to manage its keys.  We therefore fetch
+     * a key management method using the same algorithm and properties
+     * and pass that down to evp_generic_fetch to be passed on to our
+     * evp_keyexch_from_dispatch, which will attach the key management
+     * method to the newly created key exchange method as long as the
+     * provider matches.
+     */
+    struct keymgmt_data_st *keymgmt_data = vkeymgmt_data;
+    EVP_KEYMGMT *keymgmt = EVP_KEYMGMT_fetch(keymgmt_data->ctx, name,
+                                             keymgmt_data->properties);
     EVP_KEYEXCH *exchange = NULL;
     int fncnt = 0;
 
+    if (keymgmt == NULL || EVP_KEYMGMT_provider(keymgmt) != prov) {
+        ERR_raise(ERR_LIB_EVP, EVP_R_NO_KEYMGMT_AVAILABLE);
+        goto err;
+    }
+
     if ((exchange = evp_keyexch_new(prov)) == NULL
         || (exchange->name = OPENSSL_strdup(name)) == NULL) {
-        EVP_KEYEXCH_free(exchange);
-        EVPerr(0, ERR_R_MALLOC_FAILURE);
-        return NULL;
+        ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
+        goto err;
     }
 
+    exchange->keymgmt = keymgmt;
+    keymgmt = NULL;              /* avoid double free on failure below */
+
     for (; fns->function_id != 0; fns++) {
         switch (fns->function_id) {
         case OSSL_FUNC_KEYEXCH_NEWCTX:
@@ -96,13 +121,17 @@ static void *evp_keyexch_from_dispatch(const char *name,
          * and freectx. The dupctx, set_peer and set_params functions are
          * optional.
          */
-        EVP_KEYEXCH_free(exchange);
         EVPerr(EVP_F_EVP_KEYEXCH_FROM_DISPATCH,
                EVP_R_INVALID_PROVIDER_FUNCTIONS);
-        return NULL;
+        goto err;
     }
 
     return exchange;
+
+ err:
+    EVP_KEYEXCH_free(exchange);
+    EVP_KEYMGMT_free(keymgmt);
+    return NULL;
 }
 
 void EVP_KEYEXCH_free(EVP_KEYEXCH *exchange)
@@ -137,31 +166,16 @@ OSSL_PROVIDER *EVP_KEYEXCH_provider(const EVP_KEYEXCH *exchange)
 EVP_KEYEXCH *EVP_KEYEXCH_fetch(OPENSSL_CTX *ctx, const char *algorithm,
                                const char *properties)
 {
-    /*
-     * Key exchange cannot work without a key, and we key management
-     * from the same provider to manage its keys.
-     */
-    EVP_KEYEXCH *keyexch =
-        evp_generic_fetch(ctx, OSSL_OP_KEYEXCH, algorithm, properties,
-                          evp_keyexch_from_dispatch,
-                          (int (*)(void *))EVP_KEYEXCH_up_ref,
-                          (void (*)(void *))EVP_KEYEXCH_free);
-
-    /* If the method is newly created, there's no keymgmt attached */
-    if (keyexch->keymgmt == NULL) {
-        EVP_KEYMGMT *keymgmt = EVP_KEYMGMT_fetch(ctx, algorithm, properties);
-
-        if (keymgmt == NULL
-            || (EVP_KEYEXCH_provider(keyexch)
-                != EVP_KEYMGMT_provider(keymgmt))) {
-            EVP_KEYEXCH_free(keyexch);
-            EVP_KEYMGMT_free(keymgmt);
-            EVPerr(EVP_F_EVP_KEYEXCH_FETCH, EVP_R_NO_KEYMGMT_PRESENT);
-            return NULL;
-        }
+    EVP_KEYEXCH *keyexch = NULL;
+    struct keymgmt_data_st keymgmt_data;
+
+    keymgmt_data.ctx = ctx;
+    keymgmt_data.properties = properties;
+    keyexch = evp_generic_fetch(ctx, OSSL_OP_KEYEXCH, algorithm, properties,
+                                evp_keyexch_from_dispatch, &keymgmt_data,
+                                (int (*)(void *))EVP_KEYEXCH_up_ref,
+                                (void (*)(void *))EVP_KEYEXCH_free);
 
-        keyexch->keymgmt = keymgmt;
-    }
     return keyexch;
 }
 

diff --git a/crypto/evp/keymgmt_meth.c b/crypto/evp/keymgmt_meth.c
index 67c33eb78b9c9c0da6cdba600807ef75f784fac2..72ef1bdb0c222d61f2d144c3c33650b61ddfdc15 100644 (file)
--- a/crypto/evp/keymgmt_meth.c
+++ b/crypto/evp/keymgmt_meth.c
@@ -34,7 +34,7 @@ static void *keymgmt_new(void)
 }
 
 static void *keymgmt_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
-                                   OSSL_PROVIDER *prov)
+                                   OSSL_PROVIDER *prov, void *unused)
 {
     EVP_KEYMGMT *keymgmt = NULL;
 
@@ -156,7 +156,7 @@ EVP_KEYMGMT *EVP_KEYMGMT_fetch(OPENSSL_CTX *ctx, const char *algorithm,
 {
     EVP_KEYMGMT *keymgmt =
         evp_generic_fetch(ctx, OSSL_OP_KEYMGMT, algorithm, properties,
-                          keymgmt_from_dispatch,
+                          keymgmt_from_dispatch, NULL,
                           (int (*)(void *))EVP_KEYMGMT_up_ref,
                           (void (*)(void *))EVP_KEYMGMT_free);
 

diff --git a/crypto/evp/mac_meth.c b/crypto/evp/mac_meth.c
index 1c0d6425f7b39ed2d23f1015e7d6aea70140a8d0..a317127e157867e887bf494afc87af2d7a78aca4 100644 (file)
--- a/crypto/evp/mac_meth.c
+++ b/crypto/evp/mac_meth.c
@@ -48,7 +48,7 @@ static void *evp_mac_new(void)
 }
 
 static void *evp_mac_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
-                                   OSSL_PROVIDER *prov)
+                                   OSSL_PROVIDER *prov, void *unused)
 {
     EVP_MAC *mac = NULL;
     int fnmaccnt = 0, fnctxcnt = 0;
@@ -154,7 +154,7 @@ EVP_MAC *EVP_MAC_fetch(OPENSSL_CTX *libctx, const char *algorithm,
                        const char *properties)
 {
     return evp_generic_fetch(libctx, OSSL_OP_MAC, algorithm, properties,
-                             evp_mac_from_dispatch, evp_mac_up_ref,
+                             evp_mac_from_dispatch, NULL, evp_mac_up_ref,
                              evp_mac_free);
 }
 
@@ -205,5 +205,5 @@ void EVP_MAC_do_all_ex(OPENSSL_CTX *libctx,
 {
     evp_generic_do_all(libctx, OSSL_OP_MAC,
                        (void (*)(void *, void *))fn, arg,
-                       evp_mac_from_dispatch, evp_mac_free);
+                       evp_mac_from_dispatch, NULL, evp_mac_free);
 }

diff --git a/doc/internal/man3/evp_generic_fetch.pod b/doc/internal/man3/evp_generic_fetch.pod
index 0688ac01707411673b21e4048bd7b650fa0fc282..b77391e38668a75fed868ac42f56b2dccb874978 100644 (file)
--- a/doc/internal/man3/evp_generic_fetch.pod
+++ b/doc/internal/man3/evp_generic_fetch.pod
@@ -11,8 +11,11 @@ evp_generic_fetch - generic algorithm fetcher and method creator for EVP
 
  void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
                          const char *name, const char *properties,
-                         void *(*new_method)(const OSSL_DISPATCH *fns,
-                                             OSSL_PROVIDER *prov),
+                         void *(*new_method)(const char *name,
+                                             const OSSL_DISPATCH *fns,
+                                             OSSL_PROVIDER *prov,
+                                             void *method_data),
+                         void *method_data,
                          int (*up_ref_method)(void *),
                          void (*free_method)(void *));
 
@@ -31,6 +34,8 @@ The three functions are supposed to:
 
 creates an internal method from function pointers found in the
 dispatch table C<fns>.
+The algorithm I<name>, provider I<prov>, and I<method_data> are
+also passed to be used as new_method() sees fit.
 
 =item up_ref_method()
 

diff --git a/include/openssl/evperr.h b/include/openssl/evperr.h
index 34966f84cd5f6c16abcddf545b4f86b02e12e00d..714f170bd9d4ad8214558d3c590b6ea6220564cd 100644 (file)
--- a/include/openssl/evperr.h
+++ b/include/openssl/evperr.h
@@ -41,10 +41,6 @@ int ERR_load_EVP_strings(void);
 #  define EVP_F_ARIA_GCM_INIT_KEY                          0
 #  define EVP_F_ARIA_INIT_KEY                              0
 #  define EVP_F_B64_NEW                                    0
-#  define EVP_F_BLAKE2B_MAC_CTRL                           0
-#  define EVP_F_BLAKE2B_MAC_INIT                           0
-#  define EVP_F_BLAKE2S_MAC_CTRL                           0
-#  define EVP_F_BLAKE2S_MAC_INIT                           0
 #  define EVP_F_CAMELLIA_INIT_KEY                          0
 #  define EVP_F_CHACHA20_POLY1305_CTRL                     0
 #  define EVP_F_CMLL_T4_INIT_KEY                           0
@@ -218,6 +214,7 @@ int ERR_load_EVP_strings(void);
 # define EVP_R_NO_CIPHER_SET                              131
 # define EVP_R_NO_DEFAULT_DIGEST                          158
 # define EVP_R_NO_DIGEST_SET                              139
+# define EVP_R_NO_KEYMGMT_AVAILABLE                       199
 # define EVP_R_NO_KEYMGMT_PRESENT                         196
 # define EVP_R_NO_KEY_SET                                 154
 # define EVP_R_NO_OPERATION_SET                           149