Fix some misunderstandings in our providers' main modules

This started with adding forward declarations of all provider side
interface functions, and fixing all compiler errors.

Furthermore, diminish the faulty assumption that the provider context
is and always will be just a library context.  That means adding a
teardown function in all providers that aren't necessarily built into
libcrypto.

Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11777)

diff --git a/providers/build.info b/providers/build.info
index aae9115dd8b7bdc57932b0fa8442dde7c8a0be5b..b7eef405215194982f212614594dbf60aa0e6eb4 100644 (file)
--- a/providers/build.info
+++ b/providers/build.info
@@ -149,7 +149,7 @@ IF[{- !$disabled{legacy} -}]
   # Common things that are valid no matter what form the Legacy provider
   # takes.
   SOURCE[$LEGACYGOAL]=legacyprov.c
-  INCLUDE[$LEGACYGOAL]=../include implementations/include
+  INCLUDE[$LEGACYGOAL]=../include implementations/include common/include
 ENDIF
 
 #

diff --git a/providers/defltprov.c b/providers/defltprov.c
index f26654abf726bc9d82944dabad0e842c3e81065b..baea34ac04872a9002194bd58eb2c185cc51da30 100644 (file)
--- a/providers/defltprov.c
+++ b/providers/defltprov.c
@@ -15,11 +15,20 @@
 #include <openssl/core_names.h>
 #include <openssl/params.h>
 #include "prov/bio.h"
+#include "prov/provider_ctx.h"
 #include "prov/providercommon.h"
 #include "prov/implementations.h"
 #include "prov/provider_util.h"
 #include "internal/nelem.h"
 
+/*
+ * Forward declarations to ensure that interface functions are correctly
+ * defined.
+ */
+static OSSL_provider_gettable_params_fn deflt_gettable_params;
+static OSSL_provider_get_params_fn deflt_get_params;
+static OSSL_provider_query_operation_fn deflt_query;
+
 #define ALGC(NAMES, FUNC, CHECK) { { NAMES, "provider=default", FUNC }, CHECK }
 #define ALG(NAMES, FUNC) ALGC(NAMES, FUNC, NULL)
 
@@ -35,12 +44,12 @@ static const OSSL_PARAM deflt_param_types[] = {
     OSSL_PARAM_END
 };
 
-static const OSSL_PARAM *deflt_gettable_params(const OSSL_PROVIDER *prov)
+static const OSSL_PARAM *deflt_gettable_params(void *provctx)
 {
     return deflt_param_types;
 }
 
-static int deflt_get_params(const OSSL_PROVIDER *prov, OSSL_PARAM params[])
+static int deflt_get_params(void *provctx, OSSL_PARAM params[])
 {
     OSSL_PARAM *p;
 
@@ -500,8 +509,7 @@ static const OSSL_ALGORITHM deflt_serializer[] = {
     { NULL, NULL, NULL }
 };
 
-static const OSSL_ALGORITHM *deflt_query(OSSL_PROVIDER *prov,
-                                         int operation_id,
+static const OSSL_ALGORITHM *deflt_query(void *provctx, int operation_id,
                                          int *no_cache)
 {
     *no_cache = 0;

diff --git a/providers/fips/fipsprov.c b/providers/fips/fipsprov.c
index faf74831ebd48d00b6d2b8e21276c57748088fe2..1ed475e1f56dc14d303f669bdfd41b691121e9bd 100644 (file)
--- a/providers/fips/fipsprov.c
+++ b/providers/fips/fipsprov.c
@@ -34,6 +34,15 @@
 #include "prov/provider_util.h"
 #include "self_test.h"
 
+/*
+ * Forward declarations to ensure that interface functions are correctly
+ * defined.
+ */
+static OSSL_provider_teardown_fn fips_teardown;
+static OSSL_provider_gettable_params_fn fips_gettable_params;
+static OSSL_provider_get_params_fn fips_get_params;
+static OSSL_provider_query_operation_fn fips_query;
+
 #define ALGC(NAMES, FUNC, CHECK) { { NAMES, "provider=fips,fips=yes", FUNC }, CHECK }
 #define ALG(NAMES, FUNC) ALGC(NAMES, FUNC, NULL)
 
@@ -127,9 +136,8 @@ static OSSL_PARAM core_params[] =
 };
 
 /* TODO(3.0): To be removed */
-static int dummy_evp_call(void *provctx)
+static int dummy_evp_call(OPENSSL_CTX *libctx)
 {
-    OPENSSL_CTX *libctx = PROV_LIBRARY_CONTEXT_OF(provctx);
     EVP_MD_CTX *ctx = EVP_MD_CTX_new();
     EVP_MD *sha256 = EVP_MD_fetch(libctx, "SHA256", NULL);
     EVP_KDF *kdf = EVP_KDF_fetch(libctx, OSSL_KDF_NAME_PBKDF2, NULL);
@@ -208,12 +216,12 @@ static int dummy_evp_call(void *provctx)
     return ret;
 }
 
-static const OSSL_PARAM *fips_gettable_params(const OSSL_PROVIDER *prov)
+static const OSSL_PARAM *fips_gettable_params(void *provctx)
 {
     return fips_param_types;
 }
 
-static int fips_get_params(const OSSL_PROVIDER *prov, OSSL_PARAM params[])
+static int fips_get_params(void *provctx, OSSL_PARAM params[])
 {
     OSSL_PARAM *p;
 
@@ -479,9 +487,8 @@ static const OSSL_ALGORITHM fips_keymgmt[] = {
     { NULL, NULL, NULL }
 };
 
-static const OSSL_ALGORITHM *fips_query(OSSL_PROVIDER *prov,
-                                         int operation_id,
-                                         int *no_cache)
+static const OSSL_ALGORITHM *fips_query(void *provctx, int operation_id,
+                                        int *no_cache)
 {
     *no_cache = 0;
     switch (operation_id) {
@@ -506,13 +513,14 @@ static const OSSL_ALGORITHM *fips_query(OSSL_PROVIDER *prov,
     return NULL;
 }
 
+static void fips_teardown(void *provctx)
+{
+    OPENSSL_CTX_free(PROV_LIBRARY_CONTEXT_OF(provctx));
+}
+
 /* Functions we provide to the core */
 static const OSSL_DISPATCH fips_dispatch_table[] = {
-    /*
-     * To release our resources we just need to free the OPENSSL_CTX so we just
-     * use OPENSSL_CTX_free directly as our teardown function
-     */
-    { OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))OPENSSL_CTX_free },
+    { OSSL_FUNC_PROVIDER_TEARDOWN, (void (*)(void))fips_teardown },
     { OSSL_FUNC_PROVIDER_GETTABLE_PARAMS, (void (*)(void))fips_gettable_params },
     { OSSL_FUNC_PROVIDER_GET_PARAMS, (void (*)(void))fips_get_params },
     { OSSL_FUNC_PROVIDER_QUERY_OPERATION, (void (*)(void))fips_query },
@@ -532,7 +540,7 @@ int OSSL_provider_init(const OSSL_PROVIDER *provider,
                        void **provctx)
 {
     FIPS_GLOBAL *fgbl;
-    OPENSSL_CTX *ctx;
+    OPENSSL_CTX *libctx;
     OSSL_self_test_cb_fn *stcbfn = NULL;
     OSSL_core_get_library_context_fn *c_get_libctx = NULL;
 
@@ -639,35 +647,34 @@ int OSSL_provider_init(const OSSL_PROVIDER *provider,
         return 0;
 
     /*  Create a context. */
-    if ((ctx = OPENSSL_CTX_new()) == NULL)
-        return 0;
-    if ((fgbl = openssl_ctx_get_data(ctx, OPENSSL_CTX_FIPS_PROV_INDEX,
-                                     &fips_prov_ossl_ctx_method)) == NULL) {
-        OPENSSL_CTX_free(ctx);
+    if ((libctx = OPENSSL_CTX_new()) == NULL)
         return 0;
-    }
+    *provctx = libctx;
+
+    if ((fgbl = openssl_ctx_get_data(libctx, OPENSSL_CTX_FIPS_PROV_INDEX,
+                                     &fips_prov_ossl_ctx_method)) == NULL)
+        goto err;
 
     fgbl->prov = provider;
 
-    selftest_params.libctx = PROV_LIBRARY_CONTEXT_OF(ctx);
-    if (!SELF_TEST_post(&selftest_params, 0)) {
-        OPENSSL_CTX_free(ctx);
-        return 0;
-    }
+    selftest_params.libctx = libctx;
+    if (!SELF_TEST_post(&selftest_params, 0))
+        goto err;
 
     /*
      * TODO(3.0): Remove me. This is just a dummy call to demonstrate making
      * EVP calls from within the FIPS module.
      */
-    if (!dummy_evp_call(ctx)) {
-        OPENSSL_CTX_free(ctx);
-        return 0;
-    }
+    if (!dummy_evp_call(libctx))
+        goto err;
 
     *out = fips_dispatch_table;
-    *provctx = ctx;
 
     return 1;
+ err:
+    fips_teardown(*provctx);
+    *provctx = NULL;
+    return 0;
 }
 
 /*
@@ -750,9 +757,17 @@ int ERR_pop_to_mark(void)
     return c_pop_error_to_mark(NULL);
 }
 
-const OSSL_PROVIDER *FIPS_get_provider(OPENSSL_CTX *ctx)
+/*
+ * This must take a library context, since it's called from the depths
+ * of crypto/initthread.c code, where it's (correctly) assumed that the
+ * passed caller argument is an OPENSSL_CTX pointer (since the same routine
+ * is also called from other parts of libcrypto, which all pass around a
+ * OPENSSL_CTX pointer)
+ */
+const OSSL_PROVIDER *FIPS_get_provider(OPENSSL_CTX *libctx)
 {
-    FIPS_GLOBAL *fgbl = openssl_ctx_get_data(ctx, OPENSSL_CTX_FIPS_PROV_INDEX,
+    FIPS_GLOBAL *fgbl = openssl_ctx_get_data(libctx,
+                                             OPENSSL_CTX_FIPS_PROV_INDEX,
                                              &fips_prov_ossl_ctx_method);
 
     if (fgbl == NULL)

diff --git a/providers/legacyprov.c b/providers/legacyprov.c
index ca9109389349a13cde0441a01cf8745223ea4dfb..07b505a8ac5c3a4a8d08cc0d24409110b0451bb8 100644 (file)
--- a/providers/legacyprov.c
+++ b/providers/legacyprov.c
@@ -13,8 +13,17 @@
 #include <openssl/core_numbers.h>
 #include <openssl/core_names.h>
 #include <openssl/params.h>
+#include "prov/provider_ctx.h"
 #include "prov/implementations.h"
 
+/*
+ * Forward declarations to ensure that interface functions are correctly
+ * defined.
+ */
+static OSSL_provider_gettable_params_fn legacy_gettable_params;
+static OSSL_provider_get_params_fn legacy_get_params;
+static OSSL_provider_query_operation_fn legacy_query;
+
 #define ALG(NAMES, FUNC) { NAMES, "provider=legacy", FUNC }
 
 #ifdef STATIC_LEGACY
@@ -27,19 +36,19 @@ static OSSL_core_gettable_params_fn *c_gettable_params = NULL;
 static OSSL_core_get_params_fn *c_get_params = NULL;
 
 /* Parameters we provide to the core */
-static const OSSL_ITEM legacy_param_types[] = {
-    { OSSL_PARAM_UTF8_PTR, OSSL_PROV_PARAM_NAME },
-    { OSSL_PARAM_UTF8_PTR, OSSL_PROV_PARAM_VERSION },
-    { OSSL_PARAM_UTF8_PTR, OSSL_PROV_PARAM_BUILDINFO },
-    { 0, NULL }
+static const OSSL_PARAM legacy_param_types[] = {
+    OSSL_PARAM_DEFN(OSSL_PROV_PARAM_NAME, OSSL_PARAM_UTF8_PTR, NULL, 0),
+    OSSL_PARAM_DEFN(OSSL_PROV_PARAM_VERSION, OSSL_PARAM_UTF8_PTR, NULL, 0),
+    OSSL_PARAM_DEFN(OSSL_PROV_PARAM_BUILDINFO, OSSL_PARAM_UTF8_PTR, NULL, 0),
+    OSSL_PARAM_END
 };
 
-static const OSSL_ITEM *legacy_gettable_params(const OSSL_PROVIDER *prov)
+static const OSSL_PARAM *legacy_gettable_params(void *provctx)
 {
     return legacy_param_types;
 }
 
-static int legacy_get_params(const OSSL_PROVIDER *prov, OSSL_PARAM params[])
+static int legacy_get_params(void *provctx, OSSL_PARAM params[])
 {
     OSSL_PARAM *p;
 
@@ -133,8 +142,7 @@ static const OSSL_ALGORITHM legacy_ciphers[] = {
     { NULL, NULL, NULL }
 };
 
-static const OSSL_ALGORITHM *legacy_query(OSSL_PROVIDER *prov,
-                                          int operation_id,
+static const OSSL_ALGORITHM *legacy_query(void *provctx, int operation_id,
                                           int *no_cache)
 {
     *no_cache = 0;