Make sure we only run the self tests once
authorMatt Caswell <matt@openssl.org>
Wed, 18 Sep 2019 16:27:10 +0000 (17:27 +0100)
committerMatt Caswell <matt@openssl.org>
Fri, 29 Nov 2019 16:14:44 +0000 (16:14 +0000)
Fixes #9909

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

include/internal/thread_once.h
providers/fips/build.info
providers/fips/fipsprov.c
providers/fips/selftest.c
providers/fips/selftest.h

index 0b38ade6c662ceb9d4f8cc058f0bbb18c2bde60d..177974f0209f1e6f27f8f6403b07741dba3a86d5 100644 (file)
@@ -15,7 +15,7 @@
  * OPENSSL_CTX object. In this way data will get cleaned up correctly when the
  * module gets unloaded.
  */
-#ifndef FIPS_MODE
+#if !defined(FIPS_MODE) || defined(ALLOW_RUN_ONCE_IN_FIPS)
 /*
  * DEFINE_RUN_ONCE: Define an initialiser function that should be run exactly
  * once. It takes no arguments and returns and int result (1 for success or
index 4dfbb4623a346f3bbb82251e470b8dbc615757a7..12ca452073661134d3f580cc73a186577640fecb 100644 (file)
@@ -1,3 +1,3 @@
 
 SOURCE[../fips]=fipsprov.c selftest.c
-INCLUDE[../fips]=../implementations/include ../common/include
+INCLUDE[../fips]=../implementations/include ../common/include ../..
index a12163fa97a439fdf7d8b9f4a18997636df984da..12c471f32594685c42f67ce2230418ae352c8b81 100644 (file)
@@ -548,7 +548,7 @@ int OSSL_provider_init(const OSSL_PROVIDER *provider,
     fgbl->prov = provider;
 
     selftest_params.libctx = PROV_LIBRARY_CONTEXT_OF(ctx);
-    if (!SELF_TEST_post(&selftest_params)) {
+    if (!SELF_TEST_post(&selftest_params, 0)) {
         OPENSSL_CTX_free(ctx);
         return 0;
     }
index d954073d647e303d955391d35cc074a78600d55e..ad7dab2021b9ff6b338557bd63eb027079a154e0 100644 (file)
 #include <string.h>
 #include <openssl/evp.h>
 #include <openssl/params.h>
+#include <openssl/crypto.h>
+#include "e_os.h"
+/*
+ * We're cheating here. Normally we don't allow RUN_ONCE usage inside the FIPS
+ * module because all such initialisation should be associated with an
+ * individual OPENSSL_CTX. That doesn't work with the self test though because
+ * it should be run once regardless of the number of OPENSSL_CTXs we have.
+ */
+#define ALLOW_RUN_ONCE_IN_FIPS
+#include <internal/thread_once.h>
 #include "selftest.h"
 
 #define FIPS_STATE_INIT     0
-#define FIPS_STATE_RUNNING  1
-#define FIPS_STATE_SELFTEST 2
+#define FIPS_STATE_SELFTEST 1
+#define FIPS_STATE_RUNNING  2
 #define FIPS_STATE_ERROR    3
 
 /* The size of a temp buffer used to read in data */
 #define DIGEST_NAME "SHA256"
 
 static int FIPS_state = FIPS_STATE_INIT;
+static CRYPTO_RWLOCK *self_test_lock = NULL;
 static unsigned char fixed_key[32] = { 0 };
 
+static CRYPTO_ONCE fips_self_test_init = CRYPTO_ONCE_STATIC_INIT;
+DEFINE_RUN_ONCE_STATIC(do_fips_self_test_init)
+{
+    self_test_lock = CRYPTO_THREAD_lock_new();
+
+    return self_test_lock != NULL;
+}
+
+/*
+ * This is the Default Entry Point (DEP) code. Every platform must have a DEP.
+ * See FIPS 140-2 IG 9.10
+ *
+ * If we're run on a platform where we don't know how to define the DEP then
+ * the self-tests will never get triggered (FIPS_state never moves to
+ * FIPS_STATE_SELFTEST). This will be detected as an error when SELF_TEST_post()
+ * is called from OSSL_provider_init(), and so the fips module will be unusable
+ * on those platforms.
+ */
+#if defined(_WIN32) || defined(__CYGWIN__)
+# ifdef __CYGWIN__
+/* pick DLL_[PROCESS|THREAD]_[ATTACH|DETACH] definitions */
+#  include <windows.h>
+/*
+ * this has side-effect of _WIN32 getting defined, which otherwise is
+ * mutually exclusive with __CYGWIN__...
+ */
+# endif
+
+BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved);
+BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD fdwReason, LPVOID lpvReserved)
+{
+    switch (fdwReason) {
+    case DLL_PROCESS_ATTACH:
+        FIPS_state = FIPS_STATE_SELFTEST;
+        break;
+    case DLL_PROCESS_DETACH:
+        CRYPTO_THREAD_lock_free(self_test_lock);
+        break;
+    default:
+        break;
+    }
+    return TRUE;
+}
+#elif defined(__GNUC__)
+
+static __attribute__((constructor)) void init(void)
+{
+    FIPS_state = FIPS_STATE_SELFTEST;
+}
+
+
+static __attribute__((destructor)) void cleanup(void)
+{
+    CRYPTO_THREAD_lock_free(self_test_lock);
+}
+
+#endif
+
 /*
  * Calculate the HMAC SHA256 of data read using a BIO and read_cb, and verify
  * the result matches the expected value.
@@ -79,19 +148,42 @@ err:
 }
 
 /* This API is triggered either on loading of the FIPS module or on demand */
-int SELF_TEST_post(SELF_TEST_POST_PARAMS *st)
+int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test)
 {
     int ok = 0;
     int kats_already_passed = 0;
-    int on_demand_test = (FIPS_state != FIPS_STATE_INIT);
     long checksum_len;
     BIO *bio_module = NULL, *bio_indicator = NULL;
     unsigned char *module_checksum = NULL;
     unsigned char *indicator_checksum = NULL;
+    int loclstate;
+
+    if (!RUN_ONCE(&fips_self_test_init, do_fips_self_test_init))
+        return 0;
+
+    CRYPTO_THREAD_read_lock(self_test_lock);
+    loclstate = FIPS_state;
+    CRYPTO_THREAD_unlock(self_test_lock);
 
+    if (loclstate == FIPS_STATE_RUNNING) {
+        if (!on_demand_test)
+            return 1;
+    } else if (loclstate != FIPS_STATE_SELFTEST) {
+        return 0;
+    }
+
+    CRYPTO_THREAD_write_lock(self_test_lock);
+    if (FIPS_state == FIPS_STATE_RUNNING) {
+        if (!on_demand_test) {
+            CRYPTO_THREAD_unlock(self_test_lock);
+            return 1;
+        }
+        FIPS_state = FIPS_STATE_SELFTEST;
+    } else if (FIPS_state != FIPS_STATE_SELFTEST) {
+        CRYPTO_THREAD_unlock(self_test_lock);
+        return 0;
+    }
     if (st == NULL
-            || FIPS_state == FIPS_STATE_ERROR
-            || FIPS_state == FIPS_STATE_SELFTEST
             || st->module_checksum_data == NULL)
         goto end;
 
@@ -146,6 +238,7 @@ end:
         (*st->bio_free_cb)(bio_module);
     }
     FIPS_state = ok ? FIPS_STATE_RUNNING : FIPS_STATE_ERROR;
+    CRYPTO_THREAD_unlock(self_test_lock);
 
     return ok;
 }
index 230d448b1d700ccc1c6aefb8074ed65268ed2b19..a56e42c7ab99548bdbf78fd67034312b00d54369 100644 (file)
@@ -29,4 +29,4 @@ typedef struct self_test_post_params_st {
 
 } SELF_TEST_POST_PARAMS;
 
-int SELF_TEST_post(SELF_TEST_POST_PARAMS *st);
+int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test);