[openssl] master update

Dr. Paul Dale pauli at openssl.org
Fri Aug 27 00:15:30 UTC 2021


The branch master has been updated
       via  194fcc9ae09ea7cbe0b3b60c67061e51bb24de79 (commit)
       via  f38af1258561eb0213b344c607037a528136099f (commit)
       via  6f25d3c47995c6e4948212950566dfbe541904df (commit)
      from  4fdb0d2535323373650bd68e7a659f9320828857 (commit)


- Log -----------------------------------------------------------------
commit 194fcc9ae09ea7cbe0b3b60c67061e51bb24de79
Author: Matt Caswell <matt at openssl.org>
Date:   Wed Aug 25 14:39:29 2021 +0100

    Add a test for running the config twice
    
    Make sure there are no leaks from running the config file twice.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16425)

commit f38af1258561eb0213b344c607037a528136099f
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Aug 24 17:41:39 2021 +0100

    Add locking for the provider_conf.c
    
    Avoid races where 2 threads attempt to configure activation of providers
    at the same time. E.g. via an explicit and an implict load of the config
    file at the same time.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16425)

commit 6f25d3c47995c6e4948212950566dfbe541904df
Author: Matt Caswell <matt at openssl.org>
Date:   Tue Aug 17 10:32:49 2021 +0100

    When activating providers via config check we've not already activated them
    
    We skip the activation if we already configured them.
    
    Fixes #16250
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16425)

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

Summary of changes:
 crypto/provider_conf.c             | 106 +++++++++++++++++++++++++------------
 test/build.info                    |   6 ++-
 test/prov_config_test.c            |  61 +++++++++++++++++++++
 test/recipes/30-test_prov_config.t |  32 +++++++++++
 4 files changed, 170 insertions(+), 35 deletions(-)
 create mode 100644 test/prov_config_test.c
 create mode 100644 test/recipes/30-test_prov_config.t

diff --git a/crypto/provider_conf.c b/crypto/provider_conf.c
index fe66e1158e..da3796d914 100644
--- a/crypto/provider_conf.c
+++ b/crypto/provider_conf.c
@@ -12,6 +12,7 @@
 #include <openssl/err.h>
 #include <openssl/conf.h>
 #include <openssl/safestack.h>
+#include <openssl/provider.h>
 #include "internal/provider.h"
 #include "internal/cryptlib.h"
 #include "provider_local.h"
@@ -21,6 +22,7 @@ DEFINE_STACK_OF(OSSL_PROVIDER)
 /* PROVIDER config module */
 
 typedef struct {
+    CRYPTO_RWLOCK *lock;
     STACK_OF(OSSL_PROVIDER) *activated_providers;
 } PROVIDER_CONF_GLOBAL;
 
@@ -31,6 +33,12 @@ static void *prov_conf_ossl_ctx_new(OSSL_LIB_CTX *libctx)
     if (pcgbl == NULL)
         return NULL;
 
+    pcgbl->lock = CRYPTO_THREAD_lock_new();
+    if (pcgbl->lock == NULL) {
+        OPENSSL_free(pcgbl);
+        return NULL;
+    }
+
     return pcgbl;
 }
 
@@ -42,6 +50,7 @@ static void prov_conf_ossl_ctx_free(void *vpcgbl)
                               ossl_provider_free);
 
     OSSL_TRACE(CONF, "Cleaned up providers\n");
+    CRYPTO_THREAD_lock_free(pcgbl->lock);
     OPENSSL_free(pcgbl);
 }
 
@@ -107,6 +116,26 @@ static int provider_conf_params(OSSL_PROVIDER *prov,
     return ok;
 }
 
+static int prov_already_activated(const char *name,
+                                  STACK_OF(OSSL_PROVIDER) *activated)
+{
+    int i, max;
+
+    if (activated == NULL)
+        return 0;
+
+    max = sk_OSSL_PROVIDER_num(activated);
+    for (i = 0; i < max; i++) {
+        OSSL_PROVIDER *tstprov = sk_OSSL_PROVIDER_value(activated, i);
+
+        if (strcmp(OSSL_PROVIDER_get0_name(tstprov), name) == 0) {
+            return 1;
+        }
+    }
+
+    return 0;
+}
+
 static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
                               const char *value, const CONF *cnf)
 {
@@ -156,46 +185,55 @@ static int provider_conf_load(OSSL_LIB_CTX *libctx, const char *name,
     }
 
     if (activate) {
-        /*
-        * There is an attempt to activate a provider, so we should disable
-        * loading of fallbacks. Otherwise a misconfiguration could mean the
-        * intended provider does not get loaded. Subsequent fetches could then
-        * fallback to the default provider - which may be the wrong thing.
-        */
-        if (!ossl_provider_disable_fallback_loading(libctx)) {
+        if (!CRYPTO_THREAD_write_lock(pcgbl->lock)) {
             ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
             return 0;
         }
-        prov = ossl_provider_find(libctx, name, 1);
-        if (prov == NULL)
-            prov = ossl_provider_new(libctx, name, NULL, 1);
-        if (prov == NULL) {
-            if (soft)
-                ERR_clear_error();
-            return 0;
-        }
-
-        if (path != NULL)
-            ossl_provider_set_module_path(prov, path);
-
-        ok = provider_conf_params(prov, NULL, NULL, value, cnf);
+        if (!prov_already_activated(name, pcgbl->activated_providers)) {
+            /*
+            * There is an attempt to activate a provider, so we should disable
+            * loading of fallbacks. Otherwise a misconfiguration could mean the
+            * intended provider does not get loaded. Subsequent fetches could
+            * then fallback to the default provider - which may be the wrong
+            * thing.
+            */
+            if (!ossl_provider_disable_fallback_loading(libctx)) {
+                CRYPTO_THREAD_unlock(pcgbl->lock);
+                ERR_raise(ERR_LIB_CRYPTO, ERR_R_INTERNAL_ERROR);
+                return 0;
+            }
+            prov = ossl_provider_find(libctx, name, 1);
+            if (prov == NULL)
+                prov = ossl_provider_new(libctx, name, NULL, 1);
+            if (prov == NULL) {
+                CRYPTO_THREAD_unlock(pcgbl->lock);
+                if (soft)
+                    ERR_clear_error();
+                return 0;
+            }
 
-        if (ok) {
-            if (!ossl_provider_activate(prov, 1, 0)) {
-                ok = 0;
-            } else if (!ossl_provider_add_to_store(prov, &actual, 0)) {
-                ossl_provider_deactivate(prov);
-                ok = 0;
-            } else {
-                if (pcgbl->activated_providers == NULL)
-                    pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null();
-                sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual);
-                ok = 1;
+            if (path != NULL)
+                ossl_provider_set_module_path(prov, path);
+
+            ok = provider_conf_params(prov, NULL, NULL, value, cnf);
+
+            if (ok) {
+                if (!ossl_provider_activate(prov, 1, 0)) {
+                    ok = 0;
+                } else if (!ossl_provider_add_to_store(prov, &actual, 0)) {
+                    ossl_provider_deactivate(prov);
+                    ok = 0;
+                } else {
+                    if (pcgbl->activated_providers == NULL)
+                        pcgbl->activated_providers = sk_OSSL_PROVIDER_new_null();
+                    sk_OSSL_PROVIDER_push(pcgbl->activated_providers, actual);
+                    ok = 1;
+                }
             }
+            if (!ok)
+                ossl_provider_free(prov);
         }
-
-        if (!ok)
-            ossl_provider_free(prov);
+        CRYPTO_THREAD_unlock(pcgbl->lock);
     } else {
         OSSL_PROVIDER_INFO entry;
 
diff --git a/test/build.info b/test/build.info
index af21e03255..dab5af4ebe 100644
--- a/test/build.info
+++ b/test/build.info
@@ -57,7 +57,7 @@ IF[{- !$disabled{tests} -}]
           context_internal_test aesgcmtest params_test evp_pkey_dparams_test \
           keymgmt_internal_test hexstr_test provider_status_test defltfips_test \
           bio_readbuffer_test user_property_test pkcs7_test upcallstest \
-          provfetchtest
+          provfetchtest prov_config_test
 
   IF[{- !$disabled{'deprecated-3.0'} -}]
     PROGRAMS{noinst}=enginetest
@@ -176,6 +176,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[provfetchtest]=../include ../apps/include
   DEPEND[provfetchtest]=../libcrypto.a libtestutil.a
 
+  SOURCE[prov_config_test]=prov_config_test.c
+  INCLUDE[prov_config_test]=../include ../apps/include
+  DEPEND[prov_config_test]=../libcrypto.a libtestutil.a
+
   SOURCE[evp_pkey_provided_test]=evp_pkey_provided_test.c
   INCLUDE[evp_pkey_provided_test]=../include ../apps/include
   DEPEND[evp_pkey_provided_test]=../libcrypto.a libtestutil.a
diff --git a/test/prov_config_test.c b/test/prov_config_test.c
new file mode 100644
index 0000000000..4b04211fa4
--- /dev/null
+++ b/test/prov_config_test.c
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2021 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <openssl/evp.h>
+#include "testutil.h"
+
+static char *configfile = NULL;
+
+/*
+ * Test to make sure there are no leaks or failures from loading the config
+ * file twice.
+ */
+static int test_double_config(void)
+{
+    OSSL_LIB_CTX *ctx = OSSL_LIB_CTX_new();
+    int testresult = 0;
+    EVP_MD *sha256 = NULL;
+
+    if (!TEST_ptr(configfile))
+        return 0;
+    if (!TEST_ptr(ctx))
+        return 0;
+
+    if (!TEST_true(OSSL_LIB_CTX_load_config(ctx, configfile)))
+        return 0;
+    if (!TEST_true(OSSL_LIB_CTX_load_config(ctx, configfile)))
+        return 0;
+
+    /* Check we can actually fetch something */
+    sha256 = EVP_MD_fetch(ctx, "SHA2-256", NULL);
+    if (!TEST_ptr(sha256))
+        goto err;
+
+    testresult = 1;
+ err:
+    EVP_MD_free(sha256);
+    OSSL_LIB_CTX_free(ctx);
+    return testresult;
+}
+
+OPT_TEST_DECLARE_USAGE("configfile\n")
+
+int setup_tests(void)
+{
+    if (!test_skip_common_options()) {
+        TEST_error("Error parsing test options\n");
+        return 0;
+    }
+
+    if (!TEST_ptr(configfile = test_get_argument(0)))
+        return 0;
+
+    ADD_TEST(test_double_config);
+    return 1;
+}
diff --git a/test/recipes/30-test_prov_config.t b/test/recipes/30-test_prov_config.t
new file mode 100644
index 0000000000..f97a26dbe9
--- /dev/null
+++ b/test/recipes/30-test_prov_config.t
@@ -0,0 +1,32 @@
+#! /usr/bin/env perl
+# Copyright 2021 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the Apache License 2.0 (the "License").  You may not use
+# this file except in compliance with the License.  You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+
+use OpenSSL::Test::Simple;
+use OpenSSL::Test qw/:DEFAULT srctop_file srctop_dir bldtop_dir/;
+use OpenSSL::Test::Utils;
+
+BEGIN {
+setup("test_prov_config");
+}
+
+use lib srctop_dir('Configurations');
+use lib bldtop_dir('.');
+
+my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0);
+
+plan tests => 2;
+
+ok(run(test(["prov_config_test", srctop_file("test", "default.cnf")])),
+    "running prov_config_test default.cnf");
+SKIP: {
+    skip "Skipping FIPS test in this build", 1 if $no_fips;
+
+    ok(run(test(["prov_config_test", srctop_file("test", "fips.cnf")])),
+       "running prov_config_test fips.cnf");
+}


More information about the openssl-commits mailing list