Fix data races in EVP_CIPHER_fetch and EVP_MD_fetch
authorMatt Caswell <matt@openssl.org>
Wed, 14 Aug 2019 17:09:28 +0000 (18:09 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 29 Aug 2019 09:50:47 +0000 (10:50 +0100)
Don't modify the cipher/md we just fetched - it could be shared by multiple
threads.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9590)

crypto/evp/digest.c
crypto/evp/evp_enc.c

index b829b814dde0fb50033d45ae8cfda8e76b8dab2b..dc7f922a1196642ec94de76da7b467860fcf586f 100644 (file)
@@ -630,6 +630,17 @@ static void *evp_md_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
         return NULL;
     }
 
+#ifndef FIPS_MODE
+    /*
+     * FIPS module note: since internal fetches will be entirely
+     * provider based, we know that none of its code depends on legacy
+     * NIDs or any functionality that use them.
+     *
+     * TODO(3.x) get rid of the need for legacy NIDs
+     */
+    md->type = OBJ_sn2nid(name);
+#endif
+
     for (; fns->function_id != 0; fns++) {
         switch (fns->function_id) {
         case OSSL_FUNC_DIGEST_NEWCTX:
@@ -736,18 +747,6 @@ EVP_MD *EVP_MD_fetch(OPENSSL_CTX *ctx, const char *algorithm,
                           evp_md_from_dispatch, evp_md_up_ref,
                           evp_md_free);
 
-#ifndef FIPS_MODE
-    /* TODO(3.x) get rid of the need for legacy NIDs */
-    if (md != NULL) {
-        /*
-         * FIPS module note: since internal fetches will be entirely
-         * provider based, we know that none of its code depends on legacy
-         * NIDs or any functionality that use them.
-         */
-        md->type = OBJ_sn2nid(algorithm);
-    }
-#endif
-
     return md;
 }
 
index 5723fe888e95e1f2ce1c85a37db3688d898f771f..96a15ef897e10c225ca3859453779ce40d9630c2 100644 (file)
@@ -1251,10 +1251,6 @@ static void *evp_cipher_from_dispatch(const char *name,
     EVP_CIPHER *cipher = NULL;
     int fnciphcnt = 0, fnctxcnt = 0;
 
-    /*
-     * The legacy NID is set by EVP_CIPHER_fetch() if the name exists in
-     * the object database.
-     */
     if ((cipher = EVP_CIPHER_meth_new(0, 0, 0)) == NULL
         || (cipher->name = OPENSSL_strdup(name)) == NULL) {
         EVP_CIPHER_meth_free(cipher);
@@ -1262,6 +1258,17 @@ static void *evp_cipher_from_dispatch(const char *name,
         return NULL;
     }
 
+#ifndef FIPS_MODE
+    /*
+     * FIPS module note: since internal fetches will be entirely
+     * provider based, we know that none of its code depends on legacy
+     * NIDs or any functionality that use them.
+     *
+     * TODO(3.x) get rid of the need for legacy NIDs
+     */
+    cipher->nid = OBJ_sn2nid(name);
+#endif
+
     for (; fns->function_id != 0; fns++) {
         switch (fns->function_id) {
         case OSSL_FUNC_CIPHER_NEWCTX:
@@ -1382,18 +1389,6 @@ EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm,
                           evp_cipher_from_dispatch, evp_cipher_up_ref,
                           evp_cipher_free);
 
-#ifndef FIPS_MODE
-    /* TODO(3.x) get rid of the need for legacy NIDs */
-    if (cipher != NULL) {
-        /*
-         * FIPS module note: since internal fetches will be entirely
-         * provider based, we know that none of its code depends on legacy
-         * NIDs or any functionality that use them.
-         */
-        cipher->nid = OBJ_sn2nid(algorithm);
-    }
-#endif
-
     return cipher;
 }