Replumbing: re-implement error reporting for providers
authorRichard Levitte <levitte@openssl.org>
Tue, 18 Jun 2019 09:18:31 +0000 (11:18 +0200)
committerRichard Levitte <levitte@openssl.org>
Tue, 2 Jul 2019 15:02:02 +0000 (17:02 +0200)
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 <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9174)

crypto/provider_core.c
include/openssl/core_numbers.h

index cb136c421e0c414474a86468721ca5eeb67c4597..7b15f58c0a8dbd50a12a84032a92d56297a9bfbf 100644 (file)
@@ -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_;
index 8807942606b1fd0f121ccd45a538673bafd0c047..ff50636ab5d5a5a6e39831e65337d0827ecb9100 100644 (file)
@@ -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 */