Test errors from a provider can still be accessed after unload

Providers can create errors that may refer to const strings within the
provider module itself. If the provider gets unloaded we need to be sure
that we can still access the errors in the error stack.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14213)

diff --git a/test/build.info b/test/build.info
index 5bf35dcb10d9a45c3fec0f53979123abc6200fea..3b0fa12d20eeba7e794f103a3331d29cf4927b47 100644 (file)
--- a/test/build.info
+++ b/test/build.info
@@ -762,17 +762,17 @@ IF[{- !$disabled{tests} -}]
   PROGRAMS{noinst}=provider_internal_test
   DEFINE[provider_internal_test]=PROVIDER_INIT_FUNCTION_NAME=p_test_init
   SOURCE[provider_internal_test]=provider_internal_test.c p_test.c
-  INCLUDE[provider_internal_test]=../include ../apps/include
+  INCLUDE[provider_internal_test]=../include ../apps/include ..
   DEPEND[provider_internal_test]=../libcrypto.a libtestutil.a
   PROGRAMS{noinst}=provider_test
   DEFINE[provider_test]=PROVIDER_INIT_FUNCTION_NAME=p_test_init
   SOURCE[provider_test]=provider_test.c p_test.c
-  INCLUDE[provider_test]=../include ../apps/include
+  INCLUDE[provider_test]=../include ../apps/include ..
   DEPEND[provider_test]=../libcrypto.a libtestutil.a
   IF[{- !$disabled{module} -}]
     MODULES{noinst}=p_test
     SOURCE[p_test]=p_test.c
-    INCLUDE[p_test]=../include
+    INCLUDE[p_test]=../include ..
     IF[{- defined $target{shared_defflag} -}]
       SOURCE[p_test]=p_test.ld
       GENERATE[p_test.ld]=../util/providers.num

diff --git a/test/p_test.c b/test/p_test.c
index dfd62ebd83911bbbb50ef84b80e639f849011506..57597086aa9ce57eef5dd2b1648bbf3c259642f8 100644 (file)
--- a/test/p_test.c
+++ b/test/p_test.c
@@ -26,11 +26,22 @@
 # define OSSL_provider_init PROVIDER_INIT_FUNCTION_NAME
 #endif
 
+#include "e_os.h"
 #include <openssl/core.h>
 #include <openssl/core_dispatch.h>
+#include <openssl/err.h>
+
+typedef struct p_test_ctx {
+    char *thisfile;
+    char *thisfunc;
+    const OSSL_CORE_HANDLE *handle;
+} P_TEST_CTX;
 
 static OSSL_FUNC_core_gettable_params_fn *c_gettable_params = NULL;
 static OSSL_FUNC_core_get_params_fn *c_get_params = NULL;
+static OSSL_FUNC_core_new_error_fn *c_new_error;
+static OSSL_FUNC_core_set_error_debug_fn *c_set_error_debug;
+static OSSL_FUNC_core_vset_error_fn *c_vset_error;
 
 /* Tell the core what params we provide and what type they are */
 static const OSSL_PARAM p_param_types[] = {
@@ -42,15 +53,17 @@ static const OSSL_PARAM p_param_types[] = {
 static OSSL_FUNC_provider_gettable_params_fn p_gettable_params;
 static OSSL_FUNC_provider_get_params_fn p_get_params;
 static OSSL_FUNC_provider_get_reason_strings_fn p_get_reason_strings;
+static OSSL_FUNC_provider_teardown_fn p_teardown;
 
 static const OSSL_PARAM *p_gettable_params(void *_)
 {
     return p_param_types;
 }
 
-static int p_get_params(void *vhand, OSSL_PARAM params[])
+static int p_get_params(void *provctx, OSSL_PARAM params[])
 {
-    const OSSL_CORE_HANDLE *hand = vhand;
+    P_TEST_CTX *ctx = (P_TEST_CTX *)provctx;
+    const OSSL_CORE_HANDLE *hand = ctx->handle;
     OSSL_PARAM *p = params;
     int ok = 1;
 
@@ -101,6 +114,14 @@ static int p_get_params(void *vhand, OSSL_PARAM params[])
     return ok;
 }
 
+static void p_set_error(int lib, int reason, const char *file, int line,
+                        const char *func)
+{
+    c_new_error(NULL);
+    c_set_error_debug(NULL, file, line, func);
+    c_vset_error(NULL, ERR_PACK(lib, 0, reason), NULL, NULL);
+}
+
 static const OSSL_ITEM *p_get_reason_strings(void *_)
 {
     static const OSSL_ITEM reason_strings[] = {
@@ -116,6 +137,7 @@ static const OSSL_DISPATCH p_test_table[] = {
     { OSSL_FUNC_PROVIDER_GET_PARAMS, (void (*)(void))p_get_params },
     { OSSL_FUNC_PROVIDER_GET_REASON_STRINGS,
         (void (*)(void))p_get_reason_strings},
+    { OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))p_teardown },
     { 0, NULL }
 };
 
@@ -124,6 +146,8 @@ int OSSL_provider_init(const OSSL_CORE_HANDLE *handle,
                        const OSSL_DISPATCH **out,
                        void **provctx)
 {
+    P_TEST_CTX *ctx;
+
     for (; in->function_id != 0; in++) {
         switch (in->function_id) {
         case OSSL_FUNC_CORE_GETTABLE_PARAMS:
@@ -132,15 +156,54 @@ int OSSL_provider_init(const OSSL_CORE_HANDLE *handle,
         case OSSL_FUNC_CORE_GET_PARAMS:
             c_get_params = OSSL_FUNC_core_get_params(in);
             break;
+        case OSSL_FUNC_CORE_NEW_ERROR:
+            c_new_error = OSSL_FUNC_core_new_error(in);
+            break;
+        case OSSL_FUNC_CORE_SET_ERROR_DEBUG:
+            c_set_error_debug = OSSL_FUNC_core_set_error_debug(in);
+            break;
+        case OSSL_FUNC_CORE_VSET_ERROR:
+            c_vset_error = OSSL_FUNC_core_vset_error(in);
+            break;
         default:
             /* Just ignore anything we don't understand */
             break;
         }
     }
 
-    /* Because we use this in get_params, we need to pass it back */
-    *provctx = (void *)handle;
-
+    /*
+     * We want to test that libcrypto doesn't use the file and func pointers
+     * that we provide to it via c_set_error_debug beyond the time that they
+     * are valid for. Therefore we dynamically allocate these strings now and
+     * free them again when the provider is torn down. If anything tries to
+     * use those strings after that point there will be a use-after-free and
+     * asan will complain (and hence the tests will fail).
+     * This file isn't linked against libcrypto, so we use malloc and strdup
+     * instead of OPENSSL_malloc and OPENSSL_strdup
+     */
+    ctx = malloc(sizeof(*ctx));
+    if (ctx == NULL)
+        return 0;
+    ctx->thisfile = strdup(OPENSSL_FILE);
+    ctx->thisfunc = strdup(OPENSSL_FUNC);
+    ctx->handle = handle;
+
+    /*
+     * Set a spurious error to check error handling works correctly. This will
+     * be ignored
+     */
+    p_set_error(ERR_LIB_PROV, 1, ctx->thisfile, OPENSSL_LINE, ctx->thisfunc);
+
+    *provctx = (void *)ctx;
     *out = p_test_table;
     return 1;
 }
+
+static void p_teardown(void *provctx)
+{
+    P_TEST_CTX *ctx = (P_TEST_CTX *)provctx;
+
+    free(ctx->thisfile);
+    free(ctx->thisfunc);
+    free(ctx);
+}

diff --git a/test/provider_test.c b/test/provider_test.c
index acb9f2000ea5e9e676fe734cfe29fab23d020f76..7406bb4318dadefdc8bd5c449791f597a5f66f07 100644 (file)
--- a/test/provider_test.c
+++ b/test/provider_test.c
@@ -19,41 +19,79 @@ static OSSL_PARAM greeting_request[] = {
     { NULL, 0, NULL, 0, 0 }
 };
 
-static int test_provider(const char *name)
+static int test_provider(OSSL_LIB_CTX **libctx, const char *name)
 {
     OSSL_PROVIDER *prov = NULL;
     const char *greeting = NULL;
     char expected_greeting[256];
+    int ok = 0;
+    long err;
 
     BIO_snprintf(expected_greeting, sizeof(expected_greeting),
                  "Hello OpenSSL %.20s, greetings from %s!",
                  OPENSSL_VERSION_STR, name);
 
-    return
-        TEST_ptr(prov = OSSL_PROVIDER_load(NULL, name))
-        && TEST_true(OSSL_PROVIDER_get_params(prov, greeting_request))
-        && TEST_ptr(greeting = greeting_request[0].data)
-        && TEST_size_t_gt(greeting_request[0].data_size, 0)
-        && TEST_str_eq(greeting, expected_greeting)
-        && TEST_true(OSSL_PROVIDER_unload(prov));
+    if (!TEST_ptr(prov = OSSL_PROVIDER_load(*libctx, name))
+            || !TEST_true(OSSL_PROVIDER_get_params(prov, greeting_request))
+            || !TEST_ptr(greeting = greeting_request[0].data)
+            || !TEST_size_t_gt(greeting_request[0].data_size, 0)
+            || !TEST_str_eq(greeting, expected_greeting)
+            || !TEST_true(OSSL_PROVIDER_unload(prov)))
+        goto err;
+
+    prov = NULL;
+
+    /*
+     * We must free the libctx to force the provider to really be unloaded from
+     * memory
+     */
+    OSSL_LIB_CTX_free(*libctx);
+    *libctx = NULL;
+
+    /* Make sure we got the error we were expecting */
+    err = ERR_peek_last_error();
+    if (!TEST_int_gt(err, 0)
+            || !TEST_int_eq(ERR_GET_REASON(err), 1))
+        goto err;
+
+    /* We print out all the data to make sure it can still be accessed */
+    ERR_print_errors_fp(stderr);
+    ok = 1;
+ err:
+    OSSL_PROVIDER_unload(prov);
+    OSSL_LIB_CTX_free(*libctx);
+    *libctx = NULL;
+    return ok;
 }
 
 static int test_builtin_provider(void)
 {
+    OSSL_LIB_CTX *libctx = OSSL_LIB_CTX_new();
     const char *name = "p_test_builtin";
+    int ok;
 
-    return
-        TEST_true(OSSL_PROVIDER_add_builtin(NULL, name,
-                                            PROVIDER_INIT_FUNCTION_NAME))
-        && test_provider(name);
+    ok =
+        TEST_ptr(libctx)
+        && TEST_true(OSSL_PROVIDER_add_builtin(libctx, name,
+                                               PROVIDER_INIT_FUNCTION_NAME))
+        && test_provider(&libctx, name);
+
+    OSSL_LIB_CTX_free(libctx);
+
+    return ok;
 }
 
 #ifndef NO_PROVIDER_MODULE
 static int test_loaded_provider(void)
 {
+    OSSL_LIB_CTX *libctx = OSSL_LIB_CTX_new();
     const char *name = "p_test";
 
-    return test_provider(name);
+    if (!TEST_ptr(libctx))
+        return 0;
+
+    /* test_provider will free libctx as part of the test */
+    return test_provider(&libctx, name);
 }
 #endif