[openssl] master update

Dr. Paul Dale pauli at openssl.org
Sun May 23 23:41:27 UTC 2021


The branch master has been updated
       via  bca0ffe8b3f229b47a515048505736708e38095b (commit)
       via  c9732f095363251131e6e6a4cbbe45deea285ed0 (commit)
       via  b3135f56a663711a1234e3d419aaae5fe6915f7f (commit)
       via  ec91f1ae199aaef7d4edd00741e0ccc5630871b8 (commit)
       via  235776b2c70fa2e283ea9fb47daf2cab4bc2309a (commit)
      from  b6f0f050fd6e40286eb33fcdf28507b0f9b79b26 (commit)


- Log -----------------------------------------------------------------
commit bca0ffe8b3f229b47a515048505736708e38095b
Author: Pauli <pauli at openssl.org>
Date:   Fri May 21 08:54:07 2021 +1000

    doc: update core_thread_start() documentation
    
    It is now passed an arugment to pass to the callback
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15278)

commit c9732f095363251131e6e6a4cbbe45deea285ed0
Author: Matt Caswell <matt at openssl.org>
Date:   Thu May 20 11:52:56 2021 +0100

    Fix a memleak in the FIPS provider
    
    If the DRBG is used within the scope of the FIPS OSSL_provider_init
    function then it attempts to register a thread callback via c_thread_start.
    However the implementation of c_thread_start assumed that the provider's
    provctx was already present. However because OSSL_provider_init is still
    running it was actually NULL. This means the thread callback fail to work
    correctly and a memory leak resulted.
    
    Instead of having c_thread_start use the provctx as the callback argument
    we change the definition of c_thread_start to have an explicit callback
    argument to use.
    
    Reviewed-by: Paul Dale <pauli at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15278)

commit b3135f56a663711a1234e3d419aaae5fe6915f7f
Author: Pauli <pauli at openssl.org>
Date:   Mon May 17 12:59:19 2021 +1000

    test: fix typo in comment in threadstest.c
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15278)

commit ec91f1ae199aaef7d4edd00741e0ccc5630871b8
Author: Pauli <pauli at openssl.org>
Date:   Tue May 18 17:54:43 2021 +1000

    core: condition out more in FIPS builds
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15278)

commit 235776b2c70fa2e283ea9fb47daf2cab4bc2309a
Author: Pauli <pauli at openssl.org>
Date:   Fri May 14 15:41:14 2021 +1000

    test: add test case to reliably reproduce RAND leak during POST
    
    The FIPS provider leaks a RAND if the POST is run at initialisation time.
    This test case reliably reproduces this event.
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15278)

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

Summary of changes:
 crypto/initthread.c             | 19 +++++++---
 crypto/provider_core.c          |  5 ++-
 doc/man7/provider-base.pod      | 12 +++---
 include/crypto/cryptlib.h       |  2 +-
 include/internal/cryptlib.h     |  4 +-
 include/openssl/core_dispatch.h |  3 +-
 test/build.info                 |  6 ++-
 test/recipes/90-test_threads.t  | 24 +++++++++++-
 test/threadstest.c              | 69 +---------------------------------
 test/threadstest.h              | 82 +++++++++++++++++++++++++++++++++++++++++
 test/threadstest_fips.c         | 49 ++++++++++++++++++++++++
 11 files changed, 191 insertions(+), 84 deletions(-)
 create mode 100644 test/threadstest.h
 create mode 100644 test/threadstest_fips.c

diff --git a/crypto/initthread.c b/crypto/initthread.c
index fec3213047..d86e280fc1 100644
--- a/crypto/initthread.c
+++ b/crypto/initthread.c
@@ -33,7 +33,9 @@ extern OSSL_FUNC_core_thread_start_fn *c_thread_start;
 
 typedef struct thread_event_handler_st THREAD_EVENT_HANDLER;
 struct thread_event_handler_st {
+#ifndef FIPS_MODULE
     const void *index;
+#endif
     void *arg;
     OSSL_thread_stop_handler_fn handfn;
     THREAD_EVENT_HANDLER *next;
@@ -235,12 +237,12 @@ void OPENSSL_thread_stop(void)
     }
 }
 
-void ossl_ctx_thread_stop(void *arg)
+void ossl_ctx_thread_stop(OSSL_LIB_CTX *ctx)
 {
     if (destructor_key.sane != -1) {
         THREAD_EVENT_HANDLER **hands
             = init_get_thread_local(&destructor_key.value, 0, 1);
-        init_thread_stop(arg, hands);
+        init_thread_stop(ctx, hands);
     }
 }
 
@@ -283,10 +285,14 @@ static const OSSL_LIB_CTX_METHOD thread_event_ossl_ctx_method = {
     thread_event_ossl_ctx_free,
 };
 
-void ossl_ctx_thread_stop(void *arg)
+static void ossl_arg_thread_stop(void *arg)
+{
+    ossl_ctx_thread_stop((OSSL_LIB_CTX *)arg);
+}
+
+void ossl_ctx_thread_stop(OSSL_LIB_CTX *ctx)
 {
     THREAD_EVENT_HANDLER **hands;
-    OSSL_LIB_CTX *ctx = PROV_LIBCTX_OF(arg);
     CRYPTO_THREAD_LOCAL *local
         = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_THREAD_EVENT_HANDLER_INDEX,
                                 &thread_event_ossl_ctx_method);
@@ -365,7 +371,8 @@ int ossl_init_thread_start(const void *index, void *arg,
          * libcrypto to tell us about later thread stop events. c_thread_start
          * is a callback to libcrypto defined in fipsprov.c
          */
-        if (!c_thread_start(FIPS_get_core_handle(ctx), ossl_ctx_thread_stop))
+        if (!c_thread_start(FIPS_get_core_handle(ctx), ossl_arg_thread_stop,
+                            ctx))
             return 0;
     }
 #endif
@@ -376,7 +383,9 @@ int ossl_init_thread_start(const void *index, void *arg,
 
     hand->handfn = handfn;
     hand->arg = arg;
+#ifndef FIPS_MODULE
     hand->index = index;
+#endif
     hand->next = *hands;
     *hands = hand;
 
diff --git a/crypto/provider_core.c b/crypto/provider_core.c
index 512a16ee66..ca2bfdb8fa 100644
--- a/crypto/provider_core.c
+++ b/crypto/provider_core.c
@@ -1603,7 +1603,8 @@ static OPENSSL_CORE_CTX *core_get_libctx(const OSSL_CORE_HANDLE *handle)
 }
 
 static int core_thread_start(const OSSL_CORE_HANDLE *handle,
-                             OSSL_thread_stop_handler_fn handfn)
+                             OSSL_thread_stop_handler_fn handfn,
+                             void *arg)
 {
     /*
      * We created this object originally and we know it is actually an
@@ -1611,7 +1612,7 @@ static int core_thread_start(const OSSL_CORE_HANDLE *handle,
      */
     OSSL_PROVIDER *prov = (OSSL_PROVIDER *)handle;
 
-    return ossl_init_thread_start(prov, prov->provctx, handfn);
+    return ossl_init_thread_start(prov, arg, handfn);
 }
 
 /*
diff --git a/doc/man7/provider-base.pod b/doc/man7/provider-base.pod
index 10ad301fb4..292752afe9 100644
--- a/doc/man7/provider-base.pod
+++ b/doc/man7/provider-base.pod
@@ -21,7 +21,8 @@ provider-base
 
  typedef void (*OSSL_thread_stop_handler_fn)(void *arg);
  int core_thread_start(const OSSL_CORE_HANDLE *handle,
-                       OSSL_thread_stop_handler_fn handfn);
+                       OSSL_thread_stop_handler_fn handfn,
+                       void *arg);
 
  OPENSSL_CORE_CTX *core_get_libctx(const OSSL_CORE_HANDLE *handle);
  void core_new_error(const OSSL_CORE_HANDLE *handle);
@@ -192,13 +193,14 @@ core_get_params() retrieves parameters from the core for the given I<handle>.
 See L</Core parameters> below for a description of currently known
 parameters.
 
-The core_thread_start() function informs the core that the provider has started
+The core_thread_start() function informs the core that the provider has stated
 an interest in the current thread. The core will inform the provider when the
 thread eventually stops. It must be passed the I<handle> for this provider, as
 well as a callback I<handfn> which will be called when the thread stops. The
-callback will subsequently be called from the thread that is stopping and gets
-passed the provider context as an argument. This may be useful to perform thread
-specific clean up such as freeing thread local variables.
+callback will subsequently be called, with the supplied argument I<arg>, from
+the thread that is stopping and gets passed the provider context as an
+argument. This may be useful to perform thread specific clean up such as
+freeing thread local variables.
 
 core_get_libctx() retrieves the library context in which the library
 object for the current provider is stored, accessible through the I<handle>.
diff --git a/include/crypto/cryptlib.h b/include/crypto/cryptlib.h
index 1e58663b4f..39a956bfd3 100644
--- a/include/crypto/cryptlib.h
+++ b/include/crypto/cryptlib.h
@@ -21,7 +21,7 @@ int ossl_init_thread_start(const void *index, void *arg,
 int ossl_init_thread_deregister(void *index);
 int ossl_init_thread(void);
 void ossl_cleanup_thread(void);
-void ossl_ctx_thread_stop(void *arg);
+void ossl_ctx_thread_stop(OSSL_LIB_CTX *ctx);
 
 /*
  * OPENSSL_INIT flags. The primary list of these is in crypto.h. Flags below
diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h
index 3499025fa1..1291299b6e 100644
--- a/include/internal/cryptlib.h
+++ b/include/internal/cryptlib.h
@@ -155,7 +155,9 @@ typedef struct ossl_ex_data_global_st {
 # define OSSL_LIB_CTX_DRBG_INDEX                     5
 # define OSSL_LIB_CTX_DRBG_NONCE_INDEX               6
 # define OSSL_LIB_CTX_RAND_CRNGT_INDEX               7
-# define OSSL_LIB_CTX_THREAD_EVENT_HANDLER_INDEX     8
+# ifdef FIPS_MODULE
+#  define OSSL_LIB_CTX_THREAD_EVENT_HANDLER_INDEX    8
+# endif
 # define OSSL_LIB_CTX_FIPS_PROV_INDEX                9
 # define OSSL_LIB_CTX_ENCODER_STORE_INDEX           10
 # define OSSL_LIB_CTX_DECODER_STORE_INDEX           11
diff --git a/include/openssl/core_dispatch.h b/include/openssl/core_dispatch.h
index 458cbb1c9e..2a46c10123 100644
--- a/include/openssl/core_dispatch.h
+++ b/include/openssl/core_dispatch.h
@@ -68,7 +68,8 @@ OSSL_CORE_MAKE_FUNC(int,core_get_params,(const OSSL_CORE_HANDLE *prov,
                                          OSSL_PARAM params[]))
 # define OSSL_FUNC_CORE_THREAD_START           3
 OSSL_CORE_MAKE_FUNC(int,core_thread_start,(const OSSL_CORE_HANDLE *prov,
-                                           OSSL_thread_stop_handler_fn handfn))
+                                           OSSL_thread_stop_handler_fn handfn,
+                                           void *arg))
 # define OSSL_FUNC_CORE_GET_LIBCTX             4
 OSSL_CORE_MAKE_FUNC(OPENSSL_CORE_CTX *,core_get_libctx,
                     (const OSSL_CORE_HANDLE *prov))
diff --git a/test/build.info b/test/build.info
index 58d75be5d4..183c972602 100644
--- a/test/build.info
+++ b/test/build.info
@@ -47,7 +47,7 @@ IF[{- !$disabled{tests} -}]
           bio_callback_test bio_memleak_test bio_core_test param_build_test \
           bioprinttest sslapitest dtlstest sslcorrupttest \
           bio_enc_test pkey_meth_test pkey_meth_kdf_test evp_kdf_test uitest \
-          cipherbytes_test \
+          cipherbytes_test threadstest_fips \
           asn1_encode_test asn1_decode_test asn1_string_table_test \
           x509_time_test x509_dup_cert_test x509_check_cert_pkey_test \
           recordlentest drbgtest rand_status_test sslbuffertest \
@@ -271,6 +271,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[threadstest]=../include ../apps/include
   DEPEND[threadstest]=../libcrypto libtestutil.a
 
+  SOURCE[threadstest_fips]=threadstest_fips.c
+  INCLUDE[threadstest_fips]=../include ../apps/include
+  DEPEND[threadstest_fips]=../libcrypto libtestutil.a
+
   SOURCE[afalgtest]=afalgtest.c
   INCLUDE[afalgtest]=../include ../apps/include
   DEPEND[afalgtest]=../libcrypto libtestutil.a
diff --git a/test/recipes/90-test_threads.t b/test/recipes/90-test_threads.t
index a841a4b2f5..651fa805d5 100644
--- a/test/recipes/90-test_threads.t
+++ b/test/recipes/90-test_threads.t
@@ -23,7 +23,7 @@ my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0);
 my $config_path = abs_path(srctop_file("test", $no_fips ? "default.cnf"
                                                         : "default-and-fips.cnf"));
 
-plan tests => 1;
+plan tests => 2;
 
 if ($no_fips) {
     ok(run(test(["threadstest", "-config", $config_path, data_dir()])),
@@ -32,3 +32,25 @@ if ($no_fips) {
     ok(run(test(["threadstest", "-fips", "-config", $config_path, data_dir()])),
        "running test_threads with FIPS");
 }
+
+# Merge the configuration files into one filtering the contents so the failure
+# condition is reproducable.  A working FIPS configuration without the install
+# status is required.
+
+open CFGBASE, '<', $config_path;
+open CFGINC, '<', bldtop_file('/providers/fipsmodule.cnf');
+open CFGOUT, '>', 'thread.cnf';
+
+while (<CFGBASE>) {
+    print CFGOUT unless m/^[.]include/;
+}
+close CFGBASE;
+print CFGOUT "\n\n";
+while (<CFGINC>) {
+    print CFGOUT unless m/^install-status/;
+}
+close CFGINC;
+close CFGOUT;
+
+$ENV{OPENSSL_CONF} = 'thread.cnf';
+ok(run(test(["threadstest_fips"])), "running test_threads_fips");
diff --git a/test/threadstest.c b/test/threadstest.c
index 359b330024..cb9817aa28 100644
--- a/test/threadstest.c
+++ b/test/threadstest.c
@@ -20,77 +20,12 @@
 #include <openssl/aes.h>
 #include <openssl/rsa.h>
 #include "testutil.h"
+#include "threadstest.h"
 
 static int do_fips = 0;
 static char *privkey;
 static char *config_file = NULL;
 
-#if !defined(OPENSSL_THREADS) || defined(CRYPTO_TDEBUG)
-
-typedef unsigned int thread_t;
-
-static int run_thread(thread_t *t, void (*f)(void))
-{
-    f();
-    return 1;
-}
-
-static int wait_for_thread(thread_t thread)
-{
-    return 1;
-}
-
-#elif defined(OPENSSL_SYS_WINDOWS)
-
-typedef HANDLE thread_t;
-
-static DWORD WINAPI thread_run(LPVOID arg)
-{
-    void (*f)(void);
-
-    *(void **) (&f) = arg;
-
-    f();
-    return 0;
-}
-
-static int run_thread(thread_t *t, void (*f)(void))
-{
-    *t = CreateThread(NULL, 0, thread_run, *(void **) &f, 0, NULL);
-    return *t != NULL;
-}
-
-static int wait_for_thread(thread_t thread)
-{
-    return WaitForSingleObject(thread, INFINITE) == 0;
-}
-
-#else
-
-typedef pthread_t thread_t;
-
-static void *thread_run(void *arg)
-{
-    void (*f)(void);
-
-    *(void **) (&f) = arg;
-
-    f();
-    return NULL;
-}
-
-static int run_thread(thread_t *t, void (*f)(void))
-{
-    return pthread_create(t, NULL, thread_run, *(void **) &f) == 0;
-}
-
-static int wait_for_thread(thread_t thread)
-{
-    return pthread_join(thread, NULL) == 0;
-}
-
-#endif
-
 static int test_lock(void)
 {
     CRYPTO_RWLOCK *lock = CRYPTO_THREAD_lock_new();
@@ -431,7 +366,7 @@ static void thread_provider_load_unload(void)
  * Test 2: Simple fetch worker
  * Test 3: Worker downgrading a shared EVP_PKEY
  * Test 4: Worker using a shared EVP_PKEY
- * Test 5: Workder loading and unloading a provider
+ * Test 5: Worker loading and unloading a provider
  */
 static int test_multi(int idx)
 {
diff --git a/test/threadstest.h b/test/threadstest.h
new file mode 100644
index 0000000000..8bdedd7052
--- /dev/null
+++ b/test/threadstest.h
@@ -0,0 +1,82 @@
+/*
+ * 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
+ */
+
+#if defined(_WIN32)
+# include <windows.h>
+#endif
+
+#include <string.h>
+#include "testutil.h"
+
+#if !defined(OPENSSL_THREADS) || defined(CRYPTO_TDEBUG)
+
+typedef unsigned int thread_t;
+
+static int run_thread(thread_t *t, void (*f)(void))
+{
+    f();
+    return 1;
+}
+
+static int wait_for_thread(thread_t thread)
+{
+    return 1;
+}
+
+#elif defined(OPENSSL_SYS_WINDOWS)
+
+typedef HANDLE thread_t;
+
+static DWORD WINAPI thread_run(LPVOID arg)
+{
+    void (*f)(void);
+
+    *(void **) (&f) = arg;
+
+    f();
+    return 0;
+}
+
+static int run_thread(thread_t *t, void (*f)(void))
+{
+    *t = CreateThread(NULL, 0, thread_run, *(void **) &f, 0, NULL);
+    return *t != NULL;
+}
+
+static int wait_for_thread(thread_t thread)
+{
+    return WaitForSingleObject(thread, INFINITE) == 0;
+}
+
+#else
+
+typedef pthread_t thread_t;
+
+static void *thread_run(void *arg)
+{
+    void (*f)(void);
+
+    *(void **) (&f) = arg;
+
+    f();
+    return NULL;
+}
+
+static int run_thread(thread_t *t, void (*f)(void))
+{
+    return pthread_create(t, NULL, thread_run, *(void **) &f) == 0;
+}
+
+static int wait_for_thread(thread_t thread)
+{
+    return pthread_join(thread, NULL) == 0;
+}
+
+#endif
+
diff --git a/test/threadstest_fips.c b/test/threadstest_fips.c
new file mode 100644
index 0000000000..b38221d4ed
--- /dev/null
+++ b/test/threadstest_fips.c
@@ -0,0 +1,49 @@
+/*
+ * 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
+ */
+
+#if defined(_WIN32)
+# include <windows.h>
+#endif
+
+#include "testutil.h"
+#include "threadstest.h"
+
+static int success;
+
+static void thread_fips_rand_fetch(void)
+{
+    EVP_MD *md;
+
+    if (!TEST_true(md = EVP_MD_fetch(NULL, "SHA2-256", NULL)))
+        success = 0;
+    EVP_MD_free(md);
+}
+
+static int test_fips_rand_leak(void)
+{
+    thread_t thread;
+
+    success = 1;
+
+    if (!TEST_true(run_thread(&thread, thread_fips_rand_fetch)))
+        return 0;
+    if (!TEST_true(wait_for_thread(thread)))
+        return 0;
+    return TEST_true(success);
+}
+
+int setup_tests(void)
+{
+    /*
+     * This test MUST be run first.  Once the default library context is set
+     * up, this test will always pass.
+     */
+    ADD_TEST(test_fips_rand_leak);
+    return 1;
+}


More information about the openssl-commits mailing list