From 6ebc2f56f04ac2738d3b9bfc732063ad8f51e75d Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 18 Jun 2019 11:18:31 +0200 Subject: [PATCH] Replumbing: re-implement error reporting for providers The idea is that providers should only have to report a reason code. The library code is considered to be libcrypto internal, and are allocated dynamically and automatically for providers on creation. We reserve the upper 8 bits of the reason code for internal OpenSSL use. This allows our own providers to report errors in form of a packed number that includes library number, function number and reason number. With this, a provider can potentially use any reason number it wants from 1 to 16777216, although the current error semantics really only allow 1 to 4095 (because only the lower 12 bits are currently considered an actual reason code by the ERR subsystem). A provider can provide a reason string table in form of an array of ERR_STRING_DATA, with each item containing just the reason code and the associated string, with the dispatch function numbered OSSL_FUNC_PROVIDER_GET_REASON_STRINGS matching the type OSSL_provider_get_reason_strings_fn. If available, libcrypto will call that function on provider activation. Reviewed-by: Matt Caswell (Merged from https://github.com/openssl/openssl/pull/9174) --- crypto/provider_core.c | 102 ++++++++++++++++++++++++++++++++- include/openssl/core_numbers.h | 11 +++- 2 files changed, 108 insertions(+), 5 deletions(-) diff --git a/crypto/provider_core.c b/crypto/provider_core.c index cb136c421e..7b15f58c0a 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -49,6 +49,10 @@ struct ossl_provider_st { STACK_OF(INFOPAIR) *parameters; OPENSSL_CTX *libctx; /* The library context this instance is in */ struct provider_store_st *store; /* The store this instance belongs to */ + int error_lib; /* ERR library number, one for each provider */ +#ifndef OPENSSL_NO_ERR + ERR_STRING_DATA *error_strings; /* Copy of what the provider gives us */ +#endif /* Provider side functions */ OSSL_provider_teardown_fn *teardown; @@ -123,6 +127,7 @@ static void *provider_store_new(OPENSSL_CTX *ctx) } prov->libctx = ctx; prov->store = store; + prov->error_lib = ERR_get_next_error_library(); if(p->is_fallback) ossl_provider_set_fallback(prov); } @@ -233,6 +238,7 @@ OSSL_PROVIDER *ossl_provider_new(OPENSSL_CTX *libctx, const char *name, } else { prov->libctx = libctx; prov->store = store; + prov->error_lib = ERR_get_next_error_library(); } CRYPTO_THREAD_unlock(store->lock); @@ -274,6 +280,15 @@ void ossl_provider_free(OSSL_PROVIDER *prov) #endif if (prov->teardown != NULL) prov->teardown(prov->provctx); +#ifndef OPENSSL_NO_ERR +# ifndef FIPS_MODE + if (prov->error_strings != NULL) { + ERR_unload_strings(prov->error_lib, prov->error_strings); + OPENSSL_free(prov->error_strings); + prov->error_strings = NULL; + } +# endif +#endif prov->flag_initialized = 0; } @@ -352,6 +367,9 @@ static const OSSL_DISPATCH *core_dispatch; /* Define further down */ static int provider_activate(OSSL_PROVIDER *prov) { const OSSL_DISPATCH *provider_dispatch = NULL; +#ifndef OPENSSL_NO_ERR + OSSL_provider_get_reason_strings_fn *p_get_reason_strings = NULL; +#endif if (prov->flag_initialized) return 1; @@ -435,8 +453,57 @@ static int provider_activate(OSSL_PROVIDER *prov) prov->query_operation = OSSL_get_provider_query_operation(provider_dispatch); break; +#ifndef OPENSSL_NO_ERR + case OSSL_FUNC_PROVIDER_GET_REASON_STRINGS: + p_get_reason_strings = + OSSL_get_provider_get_reason_strings(provider_dispatch); + break; +#endif + } + } + +#ifndef OPENSSL_NO_ERR + if (p_get_reason_strings != NULL) { + const OSSL_ITEM *reasonstrings = p_get_reason_strings(prov->provctx); + size_t cnt, cnt2; + + /* + * ERR_load_strings() handles ERR_STRING_DATA rather than OSSL_ITEM, + * although they are essentially the same type. + * Furthermore, ERR_load_strings() patches the array's error number + * with the error library number, so we need to make a copy of that + * array either way. + */ + cnt = 1; /* One for the terminating item */ + while (reasonstrings[cnt].id != 0) { + if (ERR_GET_LIB(reasonstrings[cnt].id) != 0) + return 0; + cnt++; + } + + /* Allocate one extra item for the "library" name */ + prov->error_strings = + OPENSSL_zalloc(sizeof(ERR_STRING_DATA) * (cnt + 1)); + if (prov->error_strings == NULL) + return 0; + + /* + * Set the "library" name. + */ + prov->error_strings[0].error = ERR_PACK(prov->error_lib, 0, 0); + prov->error_strings[0].string = prov->name; + /* + * Copy reasonstrings item 0..cnt-1 to prov->error_trings positions + * 1..cnt. + */ + for (cnt2 = 1; cnt2 <= cnt; cnt2++) { + prov->error_strings[cnt2].error = (int)reasonstrings[cnt2-1].id; + prov->error_strings[cnt2].string = reasonstrings[cnt2-1].ptr; } + + ERR_load_strings(prov->error_lib, prov->error_strings); } +#endif /* With this flag set, this provider has become fully "loaded". */ prov->flag_initialized = 1; @@ -675,13 +742,44 @@ static int core_thread_start(const OSSL_PROVIDER *prov, return ossl_init_thread_start(prov, prov->provctx, handfn); } +static void core_put_error(const OSSL_PROVIDER *prov, + uint32_t reason, const char *file, int line) +{ + /* + * If the uppermost 8 bits are non-zero, it's an OpenSSL library + * error and will be treated as such. Otherwise, it's a new style + * provider error and will be treated as such. + */ + if (ERR_GET_LIB(reason) != 0) { + ERR_PUT_error(ERR_GET_LIB(reason), + ERR_GET_FUNC(reason), + ERR_GET_REASON(reason), + file, line); + } else { + ERR_PUT_error(prov->error_lib, 0, (int)reason, file, line); + } +} + +/* + * TODO(3.0) This, as well as core_put_error above, should use |prov| + * to select the proper library context to report in the correct error + * stack, at least if error stacks become tied to the library context. + * We cannot currently do that since there's no support for it in the + * ERR subsystem. + */ +static void core_add_error_vdata(const OSSL_PROVIDER *prov, + int num, va_list args) +{ + ERR_add_error_vdata(num, args); +} + 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 }, + { OSSL_FUNC_CORE_PUT_ERROR, (void (*)(void))core_put_error }, + { OSSL_FUNC_CORE_ADD_ERROR_VDATA, (void (*)(void))core_add_error_vdata }, { 0, NULL } }; static const OSSL_DISPATCH *core_dispatch = core_dispatch_; diff --git a/include/openssl/core_numbers.h b/include/openssl/core_numbers.h index 8807942606..ff50636ab5 100644 --- a/include/openssl/core_numbers.h +++ b/include/openssl/core_numbers.h @@ -62,10 +62,12 @@ OSSL_CORE_MAKE_FUNC(int,core_get_params,(const OSSL_PROVIDER *prov, 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)) +OSSL_CORE_MAKE_FUNC(void,core_put_error, + (const OSSL_PROVIDER *prov, + uint32_t reason, const char *file, int line)) # define OSSL_FUNC_CORE_ADD_ERROR_VDATA 5 -OSSL_CORE_MAKE_FUNC(void,core_add_error_vdata,(int num, va_list args)) +OSSL_CORE_MAKE_FUNC(void,core_add_error_vdata,(const OSSL_PROVIDER *prov, + int num, va_list args)) # define OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT 6 OSSL_CORE_MAKE_FUNC(OPENSSL_CTX *,core_get_library_context, (const OSSL_PROVIDER *prov)) @@ -83,6 +85,9 @@ OSSL_CORE_MAKE_FUNC(int,provider_get_params,(void *provctx, # define OSSL_FUNC_PROVIDER_QUERY_OPERATION 1027 OSSL_CORE_MAKE_FUNC(const OSSL_ALGORITHM *,provider_query_operation, (void *provctx, int operation_id, const int *no_store)) +# define OSSL_FUNC_PROVIDER_GET_REASON_STRINGS 1028 +OSSL_CORE_MAKE_FUNC(const OSSL_ITEM *,provider_get_reason_strings, + (void *provctx)) /* Digests */ -- 2.34.1