From da747958c5db57dbe22c015d058be9db8a90f8f9 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 27 May 2019 16:31:27 +0100 Subject: [PATCH] Tell the FIPS provider about thread stop events The RAND code needs to know about threads stopping in order to cleanup local thread data. Therefore we add a callback for libcrypto to tell providers about such events. Reviewed-by: Richard Levitte (Merged from https://github.com/openssl/openssl/pull/9040) --- crypto/async/async.c | 6 +- crypto/build.info | 4 +- crypto/context.c | 4 +- crypto/err/err.c | 4 +- crypto/include/internal/cryptlib_int.h | 8 +- crypto/initthread.c | 148 ++++++++++++------ crypto/provider_core.c | 9 +- crypto/rand/drbg_lib.c | 3 +- include/internal/cryptlib.h | 3 +- include/openssl/core.h | 13 ++ include/openssl/core_numbers.h | 9 +- .../common/include/internal/providercommon.h | 10 ++ providers/fips/fipsprov.c | 63 +++++++- 13 files changed, 219 insertions(+), 65 deletions(-) diff --git a/crypto/async/async.c b/crypto/async/async.c index 785ed06906..bcb0030e52 100644 --- a/crypto/async/async.c +++ b/crypto/async/async.c @@ -30,7 +30,7 @@ static CRYPTO_THREAD_LOCAL ctxkey; static CRYPTO_THREAD_LOCAL poolkey; -static void async_delete_thread_state(OPENSSL_CTX *ctx); +static void async_delete_thread_state(void *arg); static async_ctx *async_ctx_new(void) { @@ -376,8 +376,8 @@ err: return 0; } -/* OPENSSL_CTX ignored for now */ -static void async_delete_thread_state(OPENSSL_CTX *ctx) +/* TODO(3.0): arg ignored for now */ +static void async_delete_thread_state(void *arg) { async_pool *pool = (async_pool *)CRYPTO_THREAD_get_local(&poolkey); diff --git a/crypto/build.info b/crypto/build.info index cb8457a657..e64a8de12d 100644 --- a/crypto/build.info +++ b/crypto/build.info @@ -67,8 +67,8 @@ SOURCE[../providers/fips]=$CORE_COMMON # Central utilities $UTIL_COMMON=\ cryptlib.c mem.c mem_sec.c params.c bsearch.c ex_data.c o_str.c \ - ctype.c threads_pthread.c threads_win.c threads_none.c context.c \ - sparse_array.c $CPUIDASM + ctype.c threads_pthread.c threads_win.c threads_none.c initthread.c \ + context.c sparse_array.c $CPUIDASM $UTIL_DEFINE=$CPUIDDEF SOURCE[../libcrypto]=$UTIL_COMMON \ diff --git a/crypto/context.c b/crypto/context.c index d441c8b4e5..cc1ec0e664 100644 --- a/crypto/context.c +++ b/crypto/context.c @@ -7,7 +7,7 @@ * https://www.openssl.org/source/license.html */ -#include "internal/cryptlib.h" +#include "internal/cryptlib_int.h" #include "internal/thread_once.h" struct openssl_ctx_onfree_list_st { @@ -80,6 +80,8 @@ static int context_deinit(OPENSSL_CTX *ctx) if (ctx == NULL) return 1; + ossl_ctx_thread_stop(ctx); + onfree = ctx->onfreelist; while (onfree != NULL) { onfree->fn(ctx); diff --git a/crypto/err/err.c b/crypto/err/err.c index 2559e76baf..196f782b17 100644 --- a/crypto/err/err.c +++ b/crypto/err/err.c @@ -688,8 +688,8 @@ const char *ERR_reason_error_string(unsigned long e) return ((p == NULL) ? NULL : p->string); } -/* OPENSSL_CTX ignored for now */ -static void err_delete_thread_state(OPENSSL_CTX *ctx) +/* TODO(3.0): arg ignored for now */ +static void err_delete_thread_state(void *arg) { ERR_STATE *state = CRYPTO_THREAD_get_local(&err_thread_local); if (state == NULL) diff --git a/crypto/include/internal/cryptlib_int.h b/crypto/include/internal/cryptlib_int.h index 0e7a0d41d2..05fad2bf1b 100644 --- a/crypto/include/internal/cryptlib_int.h +++ b/crypto/include/internal/cryptlib_int.h @@ -7,16 +7,16 @@ * https://www.openssl.org/source/license.html */ +#include #include "internal/cryptlib.h" /* This file is not scanned by mkdef.pl, whereas cryptlib.h is */ -typedef void (*ossl_thread_stop_handler_fn)(OPENSSL_CTX *ctx); -int ossl_init_thread_start(OPENSSL_CTX *ctx, - ossl_thread_stop_handler_fn handfn); +int ossl_init_thread_start(void *arg, + OSSL_thread_stop_handler_fn handfn); int init_thread(void); void cleanup_thread(void); -void fips_thread_stop(OPENSSL_CTX *ctx); +void ossl_ctx_thread_stop(void *arg); /* * OPENSSL_INIT flags. The primary list of these is in crypto.h. Flags below diff --git a/crypto/initthread.c b/crypto/initthread.c index 74a5f4815a..124fdccd6e 100644 --- a/crypto/initthread.c +++ b/crypto/initthread.c @@ -8,16 +8,53 @@ */ #include +#include #include "internal/cryptlib_int.h" +#include "internal/providercommon.h" + +#ifdef FIPS_MODE +/* + * Thread aware code may want to be told about thread stop events. We register + * to hear about those thread stop events when we see a new thread has started. + * We call the ossl_init_thread_start function to do that. In the FIPS provider + * we have our own copy of ossl_init_thread_start, which cascades notifications + * about threads stopping from libcrypto to all the code in the FIPS provider + * that needs to know about it. + * + * The FIPS provider tells libcrypto about which threads it is interested in + * by calling "c_thread_start" which is a function pointer created during + * provider initialisation (i.e. OSSL_init_provider). + */ +extern OSSL_core_thread_start_fn *c_thread_start; +#endif typedef struct thread_event_handler_st THREAD_EVENT_HANDLER; struct thread_event_handler_st { - OPENSSL_CTX *ctx; - ossl_thread_stop_handler_fn handfn; + void *arg; + OSSL_thread_stop_handler_fn handfn; THREAD_EVENT_HANDLER *next; }; -static void ossl_init_thread_stop(THREAD_EVENT_HANDLER **hands); +static void ossl_init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands); + +static THREAD_EVENT_HANDLER ** +ossl_init_get_thread_local(CRYPTO_THREAD_LOCAL *local, int alloc, int keep) +{ + THREAD_EVENT_HANDLER **hands = CRYPTO_THREAD_get_local(local); + + if (alloc) { + if (hands == NULL + && (hands = OPENSSL_zalloc(sizeof(*hands))) != NULL + && !CRYPTO_THREAD_set_local(local, hands)) { + OPENSSL_free(hands); + return NULL; + } + } else if (!keep) { + CRYPTO_THREAD_set_local(local, NULL); + } + + return hands; +} #ifndef FIPS_MODE /* @@ -41,12 +78,12 @@ static union { static void ossl_init_thread_destructor(void *hands) { - ossl_init_thread_stop((THREAD_EVENT_HANDLER **)hands); + ossl_init_thread_stop(NULL, (THREAD_EVENT_HANDLER **)hands); + OPENSSL_free(hands); } int init_thread(void) { - if (!CRYPTO_THREAD_init_local(&destructor_key.value, ossl_init_thread_destructor)) return 0; @@ -60,39 +97,39 @@ void cleanup_thread(void) destructor_key.sane = -1; } -static THREAD_EVENT_HANDLER **ossl_init_get_thread_local(int alloc) +void OPENSSL_thread_stop(void) { - THREAD_EVENT_HANDLER **hands = - CRYPTO_THREAD_get_local(&destructor_key.value); - - if (alloc) { - if (hands == NULL - && (hands = OPENSSL_zalloc(sizeof(*hands))) != NULL - && !CRYPTO_THREAD_set_local(&destructor_key.value, hands)) { - OPENSSL_free(hands); - return NULL; - } - } else { - CRYPTO_THREAD_set_local(&destructor_key.value, NULL); + if (destructor_key.sane != -1) { + THREAD_EVENT_HANDLER **hands + = ossl_init_get_thread_local(&destructor_key.value, 0, 0); + ossl_init_thread_stop(NULL, hands); + OPENSSL_free(hands); } - - return hands; } -void OPENSSL_thread_stop(void) +void ossl_ctx_thread_stop(void *arg) { - if (destructor_key.sane != -1) - ossl_init_thread_stop(ossl_init_get_thread_local(0)); + if (destructor_key.sane != -1) { + THREAD_EVENT_HANDLER **hands + = ossl_init_get_thread_local(&destructor_key.value, 0, 1); + ossl_init_thread_stop(arg, hands); + } } + #else + static void *thread_event_ossl_ctx_new(OPENSSL_CTX *libctx) { THREAD_EVENT_HANDLER **hands = NULL; - CRYPTO_THREAD_LOCAL *tlocal = OPENSSL_zalloc(sizeof(CRYPTO_THREAD_LOCAL)); + CRYPTO_THREAD_LOCAL *tlocal = OPENSSL_zalloc(sizeof(*tlocal)); if (tlocal == NULL) return NULL; + if (!CRYPTO_THREAD_init_local(tlocal, NULL)) { + goto err; + } + hands = OPENSSL_zalloc(sizeof(*hands)); if (hands == NULL) goto err; @@ -107,14 +144,8 @@ static void *thread_event_ossl_ctx_new(OPENSSL_CTX *libctx) return NULL; } -static void thread_event_ossl_ctx_free(void *vtlocal) +static void thread_event_ossl_ctx_free(void *tlocal) { - CRYPTO_THREAD_LOCAL *tlocal = vtlocal; - THREAD_EVENT_HANDLER **hands = CRYPTO_THREAD_get_local(tlocal); - - if (hands != NULL) - ossl_init_thread_stop(hands); - OPENSSL_free(tlocal); } @@ -123,18 +154,24 @@ static const OPENSSL_CTX_METHOD thread_event_ossl_ctx_method = { thread_event_ossl_ctx_free, }; -void fips_thread_stop(OPENSSL_CTX *ctx) +void ossl_ctx_thread_stop(void *arg) { THREAD_EVENT_HANDLER **hands; + OPENSSL_CTX *ctx = arg; + CRYPTO_THREAD_LOCAL *local + = openssl_ctx_get_data(ctx, OPENSSL_CTX_THREAD_EVENT_HANDLER_INDEX, + &thread_event_ossl_ctx_method); - hands = openssl_ctx_get_data(ctx, OPENSSL_CTX_THREAD_EVENT_HANDLER_INDEX, - &thread_event_ossl_ctx_method); - if (hands != NULL) - ossl_init_thread_stop(hands); + if (local == NULL) + return; + hands = ossl_init_get_thread_local(local, 0, 0); + ossl_init_thread_stop(arg, hands); + OPENSSL_free(hands); } #endif /* FIPS_MODE */ -static void ossl_init_thread_stop(THREAD_EVENT_HANDLER **hands) + +static void ossl_init_thread_stop(void *arg, THREAD_EVENT_HANDLER **hands) { THREAD_EVENT_HANDLER *curr, *prev = NULL; @@ -144,28 +181,34 @@ static void ossl_init_thread_stop(THREAD_EVENT_HANDLER **hands) curr = *hands; while (curr != NULL) { - curr->handfn(curr->ctx); + if (arg != NULL && curr->arg != arg) { + curr = curr->next; + continue; + } + curr->handfn(curr->arg); prev = curr; curr = curr->next; + if (prev == *hands) + *hands = curr; OPENSSL_free(prev); } - - OPENSSL_free(hands); } -int ossl_init_thread_start(OPENSSL_CTX *ctx, ossl_thread_stop_handler_fn handfn) +int ossl_init_thread_start(void *arg, OSSL_thread_stop_handler_fn handfn) { THREAD_EVENT_HANDLER **hands; THREAD_EVENT_HANDLER *hand; - #ifdef FIPS_MODE + OPENSSL_CTX *ctx = arg; + /* * In FIPS mode the list of THREAD_EVENT_HANDLERs is unique per combination * of OPENSSL_CTX and thread. This is because in FIPS mode each OPENSSL_CTX * gets informed about thread stop events individually. */ - hands = openssl_ctx_get_data(ctx, OPENSSL_CTX_THREAD_EVENT_HANDLER_INDEX, - &thread_event_ossl_ctx_method); + CRYPTO_THREAD_LOCAL *local + = openssl_ctx_get_data(ctx, OPENSSL_CTX_THREAD_EVENT_HANDLER_INDEX, + &thread_event_ossl_ctx_method); #else /* * Outside of FIPS mode the list of THREAD_EVENT_HANDLERs is unique per @@ -173,18 +216,31 @@ int ossl_init_thread_start(OPENSSL_CTX *ctx, ossl_thread_stop_handler_fn handfn) * thread stop events globally, so we have to ensure all affected * OPENSSL_CTXs are informed. */ - hands = ossl_init_get_thread_local(1); + CRYPTO_THREAD_LOCAL *local = &destructor_key.value; #endif + hands = ossl_init_get_thread_local(local, 1, 0); if (hands == NULL) return 0; +#ifdef FIPS_MODE + if (*hands == NULL) { + /* + * We've not yet registered any handlers for this thread. We need to get + * 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_provider(ctx), ossl_ctx_thread_stop)) + return 0; + } +#endif + hand = OPENSSL_malloc(sizeof(*hand)); if (hand == NULL) return 0; hand->handfn = handfn; - hand->ctx = ctx; + hand->arg = arg; hand->next = *hands; *hands = hand; diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 62b5bd413f..10948ce271 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -11,7 +11,7 @@ #include #include #include -#include "internal/cryptlib.h" +#include "internal/cryptlib_int.h" #include "internal/nelem.h" #include "internal/thread_once.h" #include "internal/provider.h" @@ -667,10 +667,17 @@ static OPENSSL_CTX *core_get_libctx(const OSSL_PROVIDER *prov) return prov->libctx; } +static int core_thread_start(const OSSL_PROVIDER *prov, + OSSL_thread_stop_handler_fn handfn) +{ + return ossl_init_thread_start(prov->provctx, handfn); +} + static const OSSL_DISPATCH core_dispatch_[] = { { OSSL_FUNC_CORE_GET_PARAM_TYPES, (void (*)(void))core_get_param_types }, { OSSL_FUNC_CORE_GET_PARAMS, (void (*)(void))core_get_params }, { OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT, (void (*)(void))core_get_libctx }, + { OSSL_FUNC_CORE_THREAD_START, (void (*)(void))core_thread_start }, { OSSL_FUNC_CORE_PUT_ERROR, (void (*)(void))ERR_put_error }, { OSSL_FUNC_CORE_ADD_ERROR_VDATA, (void (*)(void))ERR_add_error_vdata }, { 0, NULL } diff --git a/crypto/rand/drbg_lib.c b/crypto/rand/drbg_lib.c index 79d986ef48..5d6ea1e26e 100644 --- a/crypto/rand/drbg_lib.c +++ b/crypto/rand/drbg_lib.c @@ -1145,8 +1145,9 @@ err: return NULL; } -static void drbg_delete_thread_state(OPENSSL_CTX *ctx) +static void drbg_delete_thread_state(void *arg) { + OPENSSL_CTX *ctx = arg; DRBG_GLOBAL *dgbl = drbg_get_global(ctx); RAND_DRBG *drbg; diff --git a/include/internal/cryptlib.h b/include/internal/cryptlib.h index c40bb2601d..f7bd06b539 100644 --- a/include/internal/cryptlib.h +++ b/include/internal/cryptlib.h @@ -150,7 +150,8 @@ typedef struct ossl_ex_data_global_st { # define OPENSSL_CTX_DRBG_NONCE_INDEX 6 # define OPENSSL_CTX_RAND_CRNGT_INDEX 7 # define OPENSSL_CTX_THREAD_EVENT_HANDLER_INDEX 8 -# define OPENSSL_CTX_MAX_INDEXES 9 +# define OPENSSL_CTX_FIPS_PROV_INDEX 9 +# define OPENSSL_CTX_MAX_INDEXES 10 typedef struct openssl_ctx_method { void *(*new_func)(OPENSSL_CTX *ctx); diff --git a/include/openssl/core.h b/include/openssl/core.h index cf4d3f41de..f59695703e 100644 --- a/include/openssl/core.h +++ b/include/openssl/core.h @@ -143,6 +143,19 @@ struct ossl_param_st { */ # define OSSL_PARAM_OCTET_PTR 7 +/* + * Typedef for the thread stop handling callback. Used both internally and by + * providers. + * + * Providers may register for notifications about threads stopping by + * registering a callback to hear about such events. Providers register the + * callback using the OSSL_FUNC_CORE_THREAD_START function in the |in| dispatch + * table passed to OSSL_provider_init(). The arg passed back to a provider will + * be the provider side context object. + */ +typedef void (*OSSL_thread_stop_handler_fn)(void *arg); + + /*- * Provider entry point * -------------------- diff --git a/include/openssl/core_numbers.h b/include/openssl/core_numbers.h index 370e0590c2..e1f02001be 100644 --- a/include/openssl/core_numbers.h +++ b/include/openssl/core_numbers.h @@ -58,12 +58,15 @@ OSSL_CORE_MAKE_FUNC(const OSSL_ITEM *, # define OSSL_FUNC_CORE_GET_PARAMS 2 OSSL_CORE_MAKE_FUNC(int,core_get_params,(const OSSL_PROVIDER *prov, const OSSL_PARAM params[])) -# define OSSL_FUNC_CORE_PUT_ERROR 3 +# define OSSL_FUNC_CORE_THREAD_START 3 +OSSL_CORE_MAKE_FUNC(int,core_thread_start,(const OSSL_PROVIDER *prov, + OSSL_thread_stop_handler_fn handfn)) +# define OSSL_FUNC_CORE_PUT_ERROR 4 OSSL_CORE_MAKE_FUNC(void,core_put_error,(int lib, int func, int reason, const char *file, int line)) -# define OSSL_FUNC_CORE_ADD_ERROR_VDATA 4 +# define OSSL_FUNC_CORE_ADD_ERROR_VDATA 5 OSSL_CORE_MAKE_FUNC(void,core_add_error_vdata,(int num, va_list args)) -# define OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT 5 +# define OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT 6 OSSL_CORE_MAKE_FUNC(OPENSSL_CTX *,core_get_library_context, (const OSSL_PROVIDER *prov)) diff --git a/providers/common/include/internal/providercommon.h b/providers/common/include/internal/providercommon.h index e69de29bb2..663d9c6183 100644 --- a/providers/common/include/internal/providercommon.h +++ b/providers/common/include/internal/providercommon.h @@ -0,0 +1,10 @@ +/* + * Copyright 2019 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 + */ + +const OSSL_PROVIDER *FIPS_get_provider(OPENSSL_CTX *ctx); diff --git a/providers/fips/fipsprov.c b/providers/fips/fipsprov.c index 51246d5499..9f9b4289ac 100644 --- a/providers/fips/fipsprov.c +++ b/providers/fips/fipsprov.c @@ -22,13 +22,44 @@ #include "internal/evp_int.h" #include "internal/provider_algs.h" #include "internal/provider_ctx.h" +#include "internal/providercommon.h" +/* + * TODO(3.0): Should these be stored in the provider side provctx? Could they + * ever be different from one init to the next? Unfortunately we can't do this + * at the moment because c_put_error/c_add_error_vdata do not provide us with + * the OPENSSL_CTX as a parameter. + */ /* Functions provided by the core */ static OSSL_core_get_param_types_fn *c_get_param_types = NULL; static OSSL_core_get_params_fn *c_get_params = NULL; +extern OSSL_core_thread_start_fn *c_thread_start; +OSSL_core_thread_start_fn *c_thread_start = NULL; static OSSL_core_put_error_fn *c_put_error = NULL; static OSSL_core_add_error_vdata_fn *c_add_error_vdata = NULL; +typedef struct fips_global_st { + const OSSL_PROVIDER *prov; +} FIPS_GLOBAL; + +static void *fips_prov_ossl_ctx_new(OPENSSL_CTX *libctx) +{ + FIPS_GLOBAL *fgbl = OPENSSL_zalloc(sizeof(*fgbl)); + + return fgbl; +} + +static void fips_prov_ossl_ctx_free(void *fgbl) +{ + OPENSSL_free(fgbl); +} + +static const OPENSSL_CTX_METHOD fips_prov_ossl_ctx_method = { + fips_prov_ossl_ctx_new, + fips_prov_ossl_ctx_free, +}; + + /* Parameters we provide to the core */ static const OSSL_ITEM fips_param_types[] = { { OSSL_PARAM_UTF8_PTR, OSSL_PROV_PARAM_NAME }, @@ -184,7 +215,19 @@ int OSSL_provider_init(const OSSL_PROVIDER *provider, const OSSL_DISPATCH **out, void **provctx) { - OPENSSL_CTX *ctx; + FIPS_GLOBAL *fgbl; + OPENSSL_CTX *ctx = OPENSSL_CTX_new(); + + if (ctx == NULL) + return 0; + + fgbl = openssl_ctx_get_data(ctx, OPENSSL_CTX_FIPS_PROV_INDEX, + &fips_prov_ossl_ctx_method); + + if (fgbl == NULL) + goto err; + + fgbl->prov = provider; for (; in->function_id != 0; in++) { switch (in->function_id) { @@ -194,6 +237,9 @@ int OSSL_provider_init(const OSSL_PROVIDER *provider, case OSSL_FUNC_CORE_GET_PARAMS: c_get_params = OSSL_get_core_get_params(in); break; + case OSSL_FUNC_CORE_THREAD_START: + c_thread_start = OSSL_get_core_thread_start(in); + break; case OSSL_FUNC_CORE_PUT_ERROR: c_put_error = OSSL_get_core_put_error(in); break; @@ -224,6 +270,10 @@ int OSSL_provider_init(const OSSL_PROVIDER *provider, } return 1; + + err: + OPENSSL_CTX_free(ctx); + return 0; } /* @@ -290,3 +340,14 @@ void ERR_add_error_vdata(int num, va_list args) { c_add_error_vdata(num, args); } + +const OSSL_PROVIDER *FIPS_get_provider(OPENSSL_CTX *ctx) +{ + FIPS_GLOBAL *fgbl = openssl_ctx_get_data(ctx, OPENSSL_CTX_FIPS_PROV_INDEX, + &fips_prov_ossl_ctx_method); + + if (fgbl == NULL) + return NULL; + + return fgbl->prov; +} -- 2.25.1