From 49c6434673ca5e9413062851979cf6ed126c9f1c Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Wed, 24 Jul 2019 13:37:42 +0200 Subject: [PATCH] Refactor provider support for reporting errors The core now supplies its own versions of ERR_new(), ERR_set_debug() and ERR_vset_error(). This should suffice for a provider to have any OpenSSL compatible functionlity it desires. The main difference between the ERR functions and the core counterparts is that the core counterparts take an OSSL_PROVIDER parameter instead of the library number. That way, providers do not need to know what number they have been assigned, that information stays in the core. Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/9452) --- crypto/property/property_parse.c | 53 ++++++++++------------- crypto/provider_core.c | 72 +++++++++++++++++++------------- doc/man7/provider-base.pod | 58 +++++++++++++++++-------- include/openssl/core_numbers.h | 20 +++++---- 4 files changed, 119 insertions(+), 84 deletions(-) diff --git a/crypto/property/property_parse.c b/crypto/property/property_parse.c index 0b4dcfc8aa..c17b0ddefc 100644 --- a/crypto/property/property_parse.c +++ b/crypto/property/property_parse.c @@ -91,8 +91,8 @@ static int parse_name(OPENSSL_CTX *ctx, const char *t[], int create, for (;;) { if (!ossl_isalpha(*s)) { - PROPerr(PROP_F_PARSE_NAME, PROP_R_NOT_AN_IDENTIFIER); - ERR_add_error_data(2, "HERE-->", *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_IDENTIFIER, + "HERE-->%s", *t); return 0; } do { @@ -112,8 +112,7 @@ static int parse_name(OPENSSL_CTX *ctx, const char *t[], int create, } name[i] = '\0'; if (err) { - PROPerr(PROP_F_PARSE_NAME, PROP_R_NAME_TOO_LONG); - ERR_add_error_data(2, "HERE-->", *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NAME_TOO_LONG, "HERE-->%s", *t); return 0; } *t = skip_space(s); @@ -132,8 +131,8 @@ static int parse_number(const char *t[], PROPERTY_DEFINITION *res) v = v * 10 + (*s++ - '0'); } while (ossl_isdigit(*s)); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { - PROPerr(PROP_F_PARSE_NUMBER, PROP_R_NOT_A_DECIMAL_DIGIT); - ERR_add_error_data(2, "HERE-->", *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_A_DECIMAL_DIGIT, + "HERE-->%s", *t); return 0; } *t = skip_space(s); @@ -157,8 +156,8 @@ static int parse_hex(const char *t[], PROPERTY_DEFINITION *res) v += ossl_tolower(*s) - 'a'; } while (ossl_isxdigit(*++s)); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { - PROPerr(PROP_F_PARSE_HEX, PROP_R_NOT_AN_HEXADECIMAL_DIGIT); - ERR_add_error_data(2, "HERE-->", *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_HEXADECIMAL_DIGIT, + "HERE-->%s", *t); return 0; } *t = skip_space(s); @@ -178,8 +177,8 @@ static int parse_oct(const char *t[], PROPERTY_DEFINITION *res) v = (v << 3) + (*s - '0'); } while (ossl_isdigit(*++s) && *s != '9' && *s != '8'); if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { - PROPerr(PROP_F_PARSE_OCT, PROP_R_NOT_AN_OCTAL_DIGIT); - ERR_add_error_data(2, "HERE-->", *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_OCTAL_DIGIT, + "HERE-->%s", *t); return 0; } *t = skip_space(s); @@ -204,18 +203,13 @@ static int parse_string(OPENSSL_CTX *ctx, const char *t[], char delim, s++; } if (*s == '\0') { - char buf[2] = { 0, 0 }; - - PROPerr(PROP_F_PARSE_STRING, - PROP_R_NO_MATCHING_STRING_DELIMETER); - buf[0] = delim; - ERR_add_error_data(3, "HERE-->", buf, *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NO_MATCHING_STRING_DELIMETER, + "HERE-->%c%s", delim, *t); return 0; } v[i] = '\0'; if (err) { - PROPerr(PROP_F_PARSE_STRING, PROP_R_STRING_TOO_LONG); - ERR_add_error_data(2, "HERE-->", *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_STRING_TOO_LONG, "HERE-->%s", *t); } else { res->v.str_val = ossl_property_value(ctx, v, create); } @@ -242,14 +236,13 @@ static int parse_unquoted(OPENSSL_CTX *ctx, const char *t[], s++; } if (!ossl_isspace(*s) && *s != '\0' && *s != ',') { - PROPerr(PROP_F_PARSE_UNQUOTED, PROP_R_NOT_AN_ASCII_CHARACTER); - ERR_add_error_data(2, "HERE-->", s); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NOT_AN_ASCII_CHARACTER, + "HERE-->%s", s); return 0; } v[i] = 0; if (err) { - PROPerr(PROP_F_PARSE_UNQUOTED, PROP_R_STRING_TOO_LONG); - ERR_add_error_data(2, "HERE-->", *t); + ERR_raise_data(ERR_LIB_PROP, PROP_R_STRING_TOO_LONG, "HERE-->%s", *t); } else { res->v.str_val = ossl_property_value(ctx, v, create); } @@ -358,14 +351,14 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn) goto err; prop->oper = PROPERTY_OPER_EQ; if (prop->name_idx == 0) { - PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_PARSE_FAILED); - ERR_add_error_data(2, "Unknown name HERE-->", start); + ERR_raise_data(ERR_LIB_PROP, PROP_R_PARSE_FAILED, + "Unknown name HERE-->%s", start); goto err; } if (match_ch(&s, '=')) { if (!parse_value(ctx, &s, prop, 1)) { - PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_NO_VALUE); - ERR_add_error_data(2, "HERE-->", start); + ERR_raise_data(ERR_LIB_PROP, PROP_R_NO_VALUE, + "HERE-->%s", start); goto err; } } else { @@ -380,8 +373,8 @@ OSSL_PROPERTY_LIST *ossl_parse_property(OPENSSL_CTX *ctx, const char *defn) done = !match_ch(&s, ','); } if (*s != '\0') { - PROPerr(PROP_F_OSSL_PARSE_PROPERTY, PROP_R_TRAILING_CHARACTERS); - ERR_add_error_data(2, "HERE-->", s); + ERR_raise_data(ERR_LIB_PROP, PROP_R_TRAILING_CHARACTERS, + "HERE-->%s", s); goto err; } res = stack_to_property_list(sk); @@ -442,8 +435,8 @@ skip_value: done = !match_ch(&s, ','); } if (*s != '\0') { - PROPerr(PROP_F_OSSL_PARSE_QUERY, PROP_R_TRAILING_CHARACTERS); - ERR_add_error_data(2, "HERE-->", s); + ERR_raise_data(ERR_LIB_PROP, PROP_R_TRAILING_CHARACTERS, + "HERE-->%s", s); goto err; } res = stack_to_property_list(sk); diff --git a/crypto/provider_core.c b/crypto/provider_core.c index 385a632653..803406d7f7 100644 --- a/crypto/provider_core.c +++ b/crypto/provider_core.c @@ -225,9 +225,8 @@ OSSL_PROVIDER *ossl_provider_new(OPENSSL_CTX *libctx, const char *name, if ((prov = ossl_provider_find(libctx, name)) != NULL) { /* refcount +1 */ ossl_provider_free(prov); /* refcount -1 */ - CRYPTOerr(CRYPTO_F_OSSL_PROVIDER_NEW, - CRYPTO_R_PROVIDER_ALREADY_EXISTS); - ERR_add_error_data(2, "name=", name); + ERR_raise_data(ERR_LIB_CRYPTO, CRYPTO_R_PROVIDER_ALREADY_EXISTS, NULL, + "name=%s", name); return NULL; } @@ -438,8 +437,8 @@ static int provider_activate(OSSL_PROVIDER *prov) if (prov->init_function == NULL || !prov->init_function(prov, core_dispatch, &provider_dispatch, &prov->provctx)) { - CRYPTOerr(CRYPTO_F_PROVIDER_ACTIVATE, ERR_R_INIT_FAIL); - ERR_add_error_data(2, "name=", prov->name); + ERR_raise_data(ERR_LIB_CRYPTO, ERR_R_INIT_FAIL, NULL, + "name=%s", prov->name); #ifndef FIPS_MODE DSO_free(prov->module); prov->module = NULL; @@ -730,6 +729,21 @@ static const OSSL_PARAM param_types[] = { OSSL_PARAM_END }; +/* + * Forward declare all the functions that are provided aa dispatch. + * This ensures that the compiler will complain if they aren't defined + * with the correct signature. + */ +static OSSL_core_get_param_types_fn core_get_param_types; +static OSSL_core_get_params_fn core_get_params; +static OSSL_core_thread_start_fn core_thread_start; +static OSSL_core_get_library_context_fn core_get_libctx; +#ifndef FIPS_MODE +static OSSL_core_new_error_fn core_new_error; +static OSSL_core_set_error_debug_fn core_set_error_debug; +static OSSL_core_vset_error_fn core_vset_error; +#endif + static const OSSL_PARAM *core_get_param_types(const OSSL_PROVIDER *prov) { return param_types; @@ -758,7 +772,6 @@ static int core_get_params(const OSSL_PROVIDER *prov, OSSL_PARAM params[]) return 1; } -static OSSL_core_get_library_context_fn core_get_libctx; /* Check */ static OPENSSL_CTX *core_get_libctx(const OSSL_PROVIDER *prov) { return prov->libctx; @@ -776,8 +789,26 @@ static int core_thread_start(const OSSL_PROVIDER *prov, * ones. */ #ifndef FIPS_MODE -static void core_put_error(const OSSL_PROVIDER *prov, - uint32_t reason, const char *file, int line) +/* + * TODO(3.0) These error functions 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_new_error(const OSSL_PROVIDER *prov) +{ + ERR_new(); +} + +static void core_set_error_debug(const OSSL_PROVIDER *prov, + const char *file, int line, const char *func) +{ + ERR_set_debug(file, line, func); +} + +static void core_vset_error(const OSSL_PROVIDER *prov, + uint32_t reason, const char *fmt, va_list args) { /* * If the uppermost 8 bits are non-zero, it's an OpenSSL library @@ -785,27 +816,11 @@ static void core_put_error(const OSSL_PROVIDER *prov, * 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); + ERR_vset_error(ERR_GET_LIB(reason), ERR_GET_REASON(reason), fmt, args); } else { - ERR_PUT_error(prov->error_lib, 0, (int)reason, file, line); + ERR_vset_error(prov->error_lib, (int)reason, fmt, args); } } - -/* - * 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); -} #endif /* @@ -818,8 +833,9 @@ static const OSSL_DISPATCH core_dispatch_[] = { { OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT, (void (*)(void))core_get_libctx }, { OSSL_FUNC_CORE_THREAD_START, (void (*)(void))core_thread_start }, #ifndef FIPS_MODE - { OSSL_FUNC_CORE_PUT_ERROR, (void (*)(void))core_put_error }, - { OSSL_FUNC_CORE_ADD_ERROR_VDATA, (void (*)(void))core_add_error_vdata }, + { OSSL_FUNC_CORE_NEW_ERROR, (void (*)(void))core_new_error }, + { OSSL_FUNC_CORE_SET_ERROR_DEBUG, (void (*)(void))core_set_error_debug }, + { OSSL_FUNC_CORE_VSET_ERROR, (void (*)(void))core_vset_error }, #endif { OSSL_FUNC_CRYPTO_MALLOC, (void (*)(void))CRYPTO_malloc }, diff --git a/doc/man7/provider-base.pod b/doc/man7/provider-base.pod index e8e5d28560..aa1a3d634b 100644 --- a/doc/man7/provider-base.pod +++ b/doc/man7/provider-base.pod @@ -20,11 +20,12 @@ provider-base int core_get_params(const OSSL_PROVIDER *prov, OSSL_PARAM params[]); int core_thread_start(const OSSL_PROVIDER *prov, OSSL_thread_stop_handler_fn handfn); - void core_put_error(const OSSL_PROVIDER *prov, - uint32_t reason, const char *file, int line); - void core_add_error_vdata(const OSSL_PROVIDER *prov, - int num, va_list args); OPENSSL_CTX *core_get_library_context(const OSSL_PROVIDER *prov); + void core_new_error(const OSSL_PROVIDER *prov); + void core_set_error_debug(const OSSL_PROVIDER *prov, + const char *file, int line, const char *func); + void core_vset_error(const OSSL_PROVIDER *prov, + uint32_t reason, const char *fmt, va_list args); /* * Some OpenSSL functionality is directly offered to providers via @@ -89,9 +90,10 @@ provider): core_get_param_types OSSL_FUNC_CORE_GET_PARAM_TYPES core_get_params OSSL_FUNC_CORE_GET_PARAMS core_thread_start OSSL_FUNC_CORE_THREAD_START - core_put_error OSSL_FUNC_CORE_PUT_ERROR - core_add_error_vdata OSSL_FUNC_CORE_ADD_ERROR_VDATA core_get_library_context OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT + core_new_error OSSL_FUNC_CORE_NEW_ERROR + core_set_error_debug OSSL_FUNC_CORE_SET_ERROR_DEBUG + core_set_error OSSL_FUNC_CORE_SET_ERROR CRYPTO_malloc OSSL_FUNC_CRYPTO_MALLOC CRYPTO_zalloc OSSL_FUNC_CRYPTO_ZALLOC CRYPTO_memdup OSSL_FUNC_CRYPTO_MEMDUP @@ -129,25 +131,47 @@ parameters. =for comment core_thread_start() TBA -core_put_error() is used to report an error back to the core, with +core_get_library_context() retrieves the library context in which the +B object I is stored. +This may sometimes be useful if the provider wishes to store a +reference to its context in the same library context. + +core_new_error(), core_set_error_debug() and core_set_error() are +building blocks for reporting an error back to the core, with reference to the provider object I. + +=over 4 + +=item core_new_error() + +allocates a new thread specific error record. + +This corresponds to the OpenSSL function L. + +=item core_set_error_debug() + +sets debugging information in the current thread specific error +record. +The debugging information includes the name of the file I, the +line I and the function name I where the error occured. + +This corresponds to the OpenSSL function L. + +=item core_set_error() + +sets the I for the error, along with any addition data. The I is a number defined by the provider and used to index the reason strings table that's returned by provider_get_reason_strings(). +The additional data is given as a format string I and a set of +arguments I, which are treated in the same manner as with +BIO_vsnprintf(). I and I may also be passed to indicate exactly where the error occured or was reported. -This corresponds to the OpenSSL function L. -core_add_error_vdata() is used to add additional text data to an -error already reported with core_put_error(). -It takes I strings in a B and concatenates them. -Provider authors will have to write the corresponding variadic -argument function. +This corresponds to the OpenSSL function L. -core_get_library_context() retrieves the library context in which the -B object I is stored. -This may sometimes be useful if the provider wishes to store a -reference to its context in the same library context. +=back CRYPTO_malloc(), CRYPTO_zalloc(), CRYPTO_memdup(), CRYPTO_strdup(), CRYPTO_strndup(), CRYPTO_free(), CRYPTO_clear_free(), diff --git a/include/openssl/core_numbers.h b/include/openssl/core_numbers.h index 3428ab59d9..0bbe92709c 100644 --- a/include/openssl/core_numbers.h +++ b/include/openssl/core_numbers.h @@ -66,17 +66,19 @@ OSSL_CORE_MAKE_FUNC(int,core_get_params,(const OSSL_PROVIDER *prov, # 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, - (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,(const OSSL_PROVIDER *prov, - int num, va_list args)) -# define OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT 6 +# define OSSL_FUNC_CORE_GET_LIBRARY_CONTEXT 4 OSSL_CORE_MAKE_FUNC(OPENSSL_CTX *,core_get_library_context, (const OSSL_PROVIDER *prov)) - +# define OSSL_FUNC_CORE_NEW_ERROR 5 +OSSL_CORE_MAKE_FUNC(void,core_new_error,(const OSSL_PROVIDER *prov)) +# define OSSL_FUNC_CORE_SET_ERROR_DEBUG 6 +OSSL_CORE_MAKE_FUNC(void,core_set_error_debug, + (const OSSL_PROVIDER *prov, + const char *file, int line, const char *func)) +# define OSSL_FUNC_CORE_VSET_ERROR 7 +OSSL_CORE_MAKE_FUNC(void,core_vset_error, + (const OSSL_PROVIDER *prov, + uint32_t reason, const char *fmt, va_list args)) /* Memory allocation, freeing, clearing. */ #define OSSL_FUNC_CRYPTO_MALLOC 10 -- 2.25.1