[openssl] master update

Richard Levitte levitte at openssl.org
Tue Sep 3 08:36:54 UTC 2019


The branch master has been updated
       via  3ca9d210c94b9b88b89b224797aa403dfe97ccce (commit)
      from  7964e3709af59675795ab1f4f69a935980379a66 (commit)


- Log -----------------------------------------------------------------
commit 3ca9d210c94b9b88b89b224797aa403dfe97ccce
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Aug 23 14:03:28 2019 +0200

    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 at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/9678)

-----------------------------------------------------------------------

Summary of changes:
 crypto/err/openssl.txt                  |  1 +
 crypto/evp/digest.c                     |  6 +--
 crypto/evp/evp_enc.c                    |  7 ++--
 crypto/evp/evp_err.c                    |  2 +
 crypto/evp/evp_fetch.c                  | 21 ++++++----
 crypto/evp/evp_locl.h                   |  8 +++-
 crypto/evp/exchange.c                   | 74 ++++++++++++++++++++-------------
 crypto/evp/keymgmt_meth.c               |  4 +-
 crypto/evp/mac_meth.c                   |  6 +--
 doc/internal/man3/evp_generic_fetch.pod |  9 +++-
 include/openssl/evperr.h                |  5 +--
 11 files changed, 87 insertions(+), 56 deletions(-)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index 58f6c4894f..9b682d5084 100644
--- 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 dc7f922a11..bb6d31bf4f 100644
--- 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 96a15ef897..142ffecfed 100644
--- 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 749f189be3..63174f98f6 100644
--- 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 fa85178a7e..662195e4de 100644
--- 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 3fd73212a4..a7b36dbc0e 100644
--- 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 20503b5f67..e69e4fd0b2 100644
--- 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 67c33eb78b..72ef1bdb0c 100644
--- 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 1c0d6425f7..a317127e15 100644
--- 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 0688ac0170..b77391e386 100644
--- 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 34966f84cd..714f170bd9 100644
--- 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


More information about the openssl-commits mailing list