From 14a684bfb091b12aa3094a6097932f76f799990a Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Wed, 18 Sep 2019 17:27:10 +0100 Subject: Make sure we only run the self tests once Fixes #9909 Reviewed-by: Paul Dale Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/9939) --- providers/fips/build.info | 2 +- providers/fips/fipsprov.c | 2 +- providers/fips/selftest.c | 105 +++++++++++++++++++++++++++++++++++++++++++--- providers/fips/selftest.h | 2 +- 4 files changed, 102 insertions(+), 9 deletions(-) (limited to 'providers') diff --git a/providers/fips/build.info b/providers/fips/build.info index 4dfbb4623a..12ca452073 100644 --- a/providers/fips/build.info +++ b/providers/fips/build.info @@ -1,3 +1,3 @@ SOURCE[../fips]=fipsprov.c selftest.c -INCLUDE[../fips]=../implementations/include ../common/include +INCLUDE[../fips]=../implementations/include ../common/include ../.. diff --git a/providers/fips/fipsprov.c b/providers/fips/fipsprov.c index a12163fa97..12c471f325 100644 --- a/providers/fips/fipsprov.c +++ b/providers/fips/fipsprov.c @@ -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; } diff --git a/providers/fips/selftest.c b/providers/fips/selftest.c index d954073d64..ad7dab2021 100644 --- a/providers/fips/selftest.c +++ b/providers/fips/selftest.c @@ -10,11 +10,21 @@ #include #include #include +#include +#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 #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 */ @@ -24,8 +34,67 @@ #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 +/* + * 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; } diff --git a/providers/fips/selftest.h b/providers/fips/selftest.h index 230d448b1d..a56e42c7ab 100644 --- a/providers/fips/selftest.h +++ b/providers/fips/selftest.h @@ -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); -- cgit v1.2.3