Ensure access to FIPS_state and rate_limit is appropriately locked

These variables can be accessed concurrently from multiple threads so
we ensure that we properly lock them before read or write.

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13987)
This commit is contained in:
Matt Caswell 2021-01-27 15:51:48 +00:00
parent 04b9435a99
commit 4099460514

View File

@ -47,18 +47,19 @@
static int FIPS_conditional_error_check = 1; static int FIPS_conditional_error_check = 1;
static int FIPS_state = FIPS_STATE_INIT; static int FIPS_state = FIPS_STATE_INIT;
static CRYPTO_RWLOCK *self_test_lock = NULL; static CRYPTO_RWLOCK *self_test_lock = NULL;
static CRYPTO_RWLOCK *fips_state_lock = NULL;
static unsigned char fixed_key[32] = { FIPS_KEY_ELEMENTS }; static unsigned char fixed_key[32] = { FIPS_KEY_ELEMENTS };
static CRYPTO_ONCE fips_self_test_init = CRYPTO_ONCE_STATIC_INIT; static CRYPTO_ONCE fips_self_test_init = CRYPTO_ONCE_STATIC_INIT;
DEFINE_RUN_ONCE_STATIC(do_fips_self_test_init) DEFINE_RUN_ONCE_STATIC(do_fips_self_test_init)
{ {
/* /*
* This lock gets freed in platform specific ways that may occur after we * These locks get freed in platform specific ways that may occur after we
* do mem leak checking. If we don't know how to free it for a particular * do mem leak checking. If we don't know how to free it for a particular
* platform then we just leak it deliberately. So we temporarily disable the * platform then we just leak it deliberately.
* mem leak checking while we allocate this.
*/ */
self_test_lock = CRYPTO_THREAD_lock_new(); self_test_lock = CRYPTO_THREAD_lock_new();
fips_state_lock = CRYPTO_THREAD_lock_new();
return self_test_lock != NULL; return self_test_lock != NULL;
} }
@ -150,6 +151,7 @@ DEP_INIT_ATTRIBUTE void init(void)
DEP_FINI_ATTRIBUTE void cleanup(void) DEP_FINI_ATTRIBUTE void cleanup(void)
{ {
CRYPTO_THREAD_lock_free(self_test_lock); CRYPTO_THREAD_lock_free(self_test_lock);
CRYPTO_THREAD_lock_free(fips_state_lock);
} }
#endif #endif
@ -212,6 +214,13 @@ err:
return ret; return ret;
} }
static void set_fips_state(int state)
{
CRYPTO_THREAD_write_lock(fips_state_lock);
FIPS_state = state;
CRYPTO_THREAD_unlock(fips_state_lock);
}
/* This API is triggered either on loading of the FIPS module or on demand */ /* This API is triggered either on loading of the FIPS module or on demand */
int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test) int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test)
{ {
@ -227,9 +236,9 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test)
if (!RUN_ONCE(&fips_self_test_init, do_fips_self_test_init)) if (!RUN_ONCE(&fips_self_test_init, do_fips_self_test_init))
return 0; return 0;
CRYPTO_THREAD_read_lock(self_test_lock); CRYPTO_THREAD_read_lock(fips_state_lock);
loclstate = FIPS_state; loclstate = FIPS_state;
CRYPTO_THREAD_unlock(self_test_lock); CRYPTO_THREAD_unlock(fips_state_lock);
if (loclstate == FIPS_STATE_RUNNING) { if (loclstate == FIPS_STATE_RUNNING) {
if (!on_demand_test) if (!on_demand_test)
@ -240,17 +249,23 @@ int SELF_TEST_post(SELF_TEST_POST_PARAMS *st, int on_demand_test)
} }
CRYPTO_THREAD_write_lock(self_test_lock); CRYPTO_THREAD_write_lock(self_test_lock);
CRYPTO_THREAD_read_lock(fips_state_lock);
if (FIPS_state == FIPS_STATE_RUNNING) { if (FIPS_state == FIPS_STATE_RUNNING) {
CRYPTO_THREAD_unlock(fips_state_lock);
if (!on_demand_test) { if (!on_demand_test) {
CRYPTO_THREAD_unlock(self_test_lock); CRYPTO_THREAD_unlock(self_test_lock);
return 1; return 1;
} }
FIPS_state = FIPS_STATE_SELFTEST; set_fips_state(FIPS_STATE_SELFTEST);
} else if (FIPS_state != FIPS_STATE_SELFTEST) { } else if (FIPS_state != FIPS_STATE_SELFTEST) {
CRYPTO_THREAD_unlock(fips_state_lock);
CRYPTO_THREAD_unlock(self_test_lock); CRYPTO_THREAD_unlock(self_test_lock);
ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_STATE); ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_STATE);
return 0; return 0;
} else {
CRYPTO_THREAD_unlock(fips_state_lock);
} }
if (st == NULL if (st == NULL
|| st->module_checksum_data == NULL) { || st->module_checksum_data == NULL) {
ERR_raise(ERR_LIB_PROV, PROV_R_MISSING_CONFIG_DATA); ERR_raise(ERR_LIB_PROV, PROV_R_MISSING_CONFIG_DATA);
@ -328,7 +343,7 @@ end:
(*st->bio_free_cb)(bio_module); (*st->bio_free_cb)(bio_module);
} }
if (ok) if (ok)
FIPS_state = FIPS_STATE_RUNNING; set_fips_state(FIPS_STATE_RUNNING);
else else
ossl_set_error_state(OSSL_SELF_TEST_TYPE_NONE); ossl_set_error_state(OSSL_SELF_TEST_TYPE_NONE);
CRYPTO_THREAD_unlock(self_test_lock); CRYPTO_THREAD_unlock(self_test_lock);
@ -346,7 +361,7 @@ void ossl_set_error_state(const char *type)
int cond_test = (type != NULL && strcmp(type, OSSL_SELF_TEST_TYPE_PCT) == 0); int cond_test = (type != NULL && strcmp(type, OSSL_SELF_TEST_TYPE_PCT) == 0);
if (!cond_test || (FIPS_conditional_error_check == 1)) { if (!cond_test || (FIPS_conditional_error_check == 1)) {
FIPS_state = FIPS_STATE_ERROR; set_fips_state(FIPS_STATE_ERROR);
ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_ENTERING_ERROR_STATE); ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_ENTERING_ERROR_STATE);
} else { } else {
ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_CONDITIONAL_ERROR); ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_CONDITIONAL_ERROR);
@ -355,15 +370,20 @@ void ossl_set_error_state(const char *type)
int ossl_prov_is_running(void) int ossl_prov_is_running(void)
{ {
const int res = FIPS_state == FIPS_STATE_RUNNING int res;
|| FIPS_state == FIPS_STATE_SELFTEST;
static unsigned int rate_limit = 0; static unsigned int rate_limit = 0;
if (res) { if (!CRYPTO_THREAD_read_lock(fips_state_lock))
rate_limit = 0; return 0;
} else if (FIPS_state == FIPS_STATE_ERROR) { res = FIPS_state == FIPS_STATE_RUNNING
|| FIPS_state == FIPS_STATE_SELFTEST;
if (FIPS_state == FIPS_STATE_ERROR) {
CRYPTO_THREAD_unlock(fips_state_lock);
if (!CRYPTO_THREAD_write_lock(fips_state_lock))
return 0;
if (rate_limit++ < FIPS_ERROR_REPORTING_RATE_LIMIT) if (rate_limit++ < FIPS_ERROR_REPORTING_RATE_LIMIT)
ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_IN_ERROR_STATE); ERR_raise(ERR_LIB_PROV, PROV_R_FIPS_MODULE_IN_ERROR_STATE);
} }
CRYPTO_THREAD_unlock(fips_state_lock);
return res; return res;
} }