EVP: Make the KEYEXCH implementation leaner

Because the algorithm to use is decided already when creating an
EVP_PKEY_CTX regardless of how it was created, it turns out that it's
unnecessary to provide the KEYEXCH method explicitly, and rather
always have it be fetched implicitly.

This means fewer changes for applications that want to use new key
exchange algorithms / implementations.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10305)

3 files changed, 41 insertions, 72 deletions

diff --git a/crypto/evp/evp_local.h b/crypto/evp/evp_local.h
index 9b208190fb..5795dcbe51 100644
--- a/crypto/evp/evp_local.h
+++ b/crypto/evp/evp_local.h
@@ -102,8 +102,6 @@ struct evp_keyexch_st {
     CRYPTO_REF_COUNT refcnt;
     CRYPTO_RWLOCK *lock;
 
-    EVP_KEYMGMT *keymgmt;
-
     OSSL_OP_keyexch_newctx_fn *newctx;
     OSSL_OP_keyexch_init_fn *init;
     OSSL_OP_keyexch_set_peer_fn *set_peer;
diff --git a/crypto/evp/exchange.c b/crypto/evp/exchange.c
index dfc309ecd7..6e1372e042 100644
--- a/crypto/evp/exchange.c
+++ b/crypto/evp/exchange.c
@@ -35,37 +35,17 @@ static EVP_KEYEXCH *evp_keyexch_new(OSSL_PROVIDER *prov)
 static void *evp_keyexch_from_dispatch(int name_id,
                                        const OSSL_DISPATCH *fns,
                                        OSSL_PROVIDER *prov,
-                                       void *vkeymgmt_data)
+                                       void *unused)
 {
-    /*
-     * Key exchange cannot work without a key, and key management
-     * from the same provider to manage its keys.  We therefore fetch
-     * a key management method using the same algorithm and properties
-     * and pass that down to evp_generic_fetch to be passed on to our
-     * evp_keyexch_from_dispatch, which will attach the key management
-     * method to the newly created key exchange method as long as the
-     * provider matches.
-     */
-    struct keymgmt_data_st *keymgmt_data = vkeymgmt_data;
-    EVP_KEYMGMT *keymgmt =
-        evp_keymgmt_fetch_by_number(keymgmt_data->ctx, name_id,
-                                    keymgmt_data->properties);
     EVP_KEYEXCH *exchange = NULL;
     int fncnt = 0, paramfncnt = 0;
 
-    if (keymgmt == NULL || EVP_KEYMGMT_provider(keymgmt) != prov) {
-        ERR_raise(ERR_LIB_EVP, EVP_R_NO_KEYMGMT_AVAILABLE);
-        goto err;
-    }
-
     if ((exchange = evp_keyexch_new(prov)) == NULL) {
         ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
         goto err;
     }
 
     exchange->name_id = name_id;
-    exchange->keymgmt = keymgmt;
-    keymgmt = NULL;              /* avoid double free on failure below */
 
     for (; fns->function_id != 0; fns++) {
         switch (fns->function_id) {
@@ -135,7 +115,6 @@ static void *evp_keyexch_from_dispatch(int name_id,
 
  err:
     EVP_KEYEXCH_free(exchange);
-    EVP_KEYMGMT_free(keymgmt);
     return NULL;
 }
 
@@ -147,7 +126,6 @@ void EVP_KEYEXCH_free(EVP_KEYEXCH *exchange)
         CRYPTO_DOWN_REF(&exchange->refcnt, &i, exchange->lock);
         if (i > 0)
             return;
-        EVP_KEYMGMT_free(exchange->keymgmt);
         ossl_provider_free(exchange->prov);
         CRYPTO_THREAD_lock_free(exchange->lock);
         OPENSSL_free(exchange);
@@ -171,70 +149,73 @@ EVP_KEYEXCH *EVP_KEYEXCH_fetch(OPENSSL_CTX *ctx, const char *algorithm,
                                const char *properties)
 {
     EVP_KEYEXCH *keyexch = NULL;
-    struct keymgmt_data_st keymgmt_data;
 
-    keymgmt_data.ctx = ctx;
-    keymgmt_data.properties = properties;
     keyexch = evp_generic_fetch(ctx, OSSL_OP_KEYEXCH, algorithm, properties,
-                                evp_keyexch_from_dispatch, &keymgmt_data,
+                                evp_keyexch_from_dispatch, NULL,
                                 (int (*)(void *))EVP_KEYEXCH_up_ref,
                                 (void (*)(void *))EVP_KEYEXCH_free);
 
     return keyexch;
 }
 
-int EVP_PKEY_derive_init_ex(EVP_PKEY_CTX *ctx, EVP_KEYEXCH *exchange)
+int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx)
 {
     int ret;
     void *provkey = NULL;
+    EVP_KEYEXCH *exchange = NULL;
+
+    if (ctx == NULL) {
+        EVPerr(0, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
+        return -2;
+    }
 
     evp_pkey_ctx_free_old_ops(ctx);
     ctx->operation = EVP_PKEY_OP_DERIVE;
 
-    if (ctx->engine != NULL)
+    if (ctx->engine != NULL || ctx->algorithm == NULL)
         goto legacy;
 
-    if (exchange != NULL) {
-        if (!EVP_KEYEXCH_up_ref(exchange))
-            goto err;
-    } else {
-        int nid = ctx->pkey != NULL ? ctx->pkey->type : ctx->pmeth->pkey_id;
+    /*
+     * Because we cleared out old ops, we shouldn't need to worry about
+     * checking if exchange is already there.  Keymgmt is a different
+     * matter, as it isn't tied to a specific EVP_PKEY op.
+     */
+    exchange = EVP_KEYEXCH_fetch(ctx->libctx, ctx->algorithm, ctx->propquery);
+    if (exchange != NULL && ctx->keymgmt == NULL) {
+        int name_id = EVP_KEYEXCH_number(exchange);
 
+        ctx->keymgmt =
+            evp_keymgmt_fetch_by_number(ctx->libctx, name_id, ctx->propquery);
+    }
+
+    if (ctx->keymgmt == NULL
+        || exchange == NULL
+        || (EVP_KEYMGMT_provider(ctx->keymgmt)
+            != EVP_KEYEXCH_provider(exchange))) {
         /*
-         * TODO(3.0): Check for legacy handling. Remove this once all all
-         * algorithms are moved to providers.
+         * We don't have the full support we need with provided methods,
+         * let's go see if legacy does.  Also, we don't need to free
+         * ctx->keymgmt here, as it's not necessarily tied to this
+         * operation.  It will be freed by EVP_PKEY_CTX_free().
          */
-        if (ctx->pkey != NULL) {
-            switch (ctx->pkey->type) {
-            case EVP_PKEY_DH:
-                break;
-            default:
-                goto legacy;
-            }
-            exchange = EVP_KEYEXCH_fetch(NULL, OBJ_nid2sn(nid), NULL);
-        } else {
-            goto legacy;
-        }
-
-        if (exchange == NULL) {
-            EVPerr(EVP_F_EVP_PKEY_DERIVE_INIT_EX, EVP_R_INITIALIZATION_ERROR);
-            goto err;
-        }
+        EVP_KEYEXCH_free(exchange);
+        goto legacy;
     }
 
+
     ctx->op.kex.exchange = exchange;
+
     if (ctx->pkey != NULL) {
-        provkey =
-            evp_keymgmt_export_to_provider(ctx->pkey, exchange->keymgmt, 0);
+        provkey = evp_keymgmt_export_to_provider(ctx->pkey, ctx->keymgmt, 0);
         if (provkey == NULL) {
-            EVPerr(EVP_F_EVP_PKEY_DERIVE_INIT_EX, EVP_R_INITIALIZATION_ERROR);
+            EVPerr(0, EVP_R_INITIALIZATION_ERROR);
             goto err;
         }
     }
     ctx->op.kex.exchprovctx = exchange->newctx(ossl_provider_ctx(exchange->prov));
     if (ctx->op.kex.exchprovctx == NULL) {
         /* The provider key can stay in the cache */
-        EVPerr(EVP_F_EVP_PKEY_DERIVE_INIT_EX, EVP_R_INITIALIZATION_ERROR);
+        EVPerr(0, EVP_R_INITIALIZATION_ERROR);
         goto err;
     }
     ret = exchange->init(ctx->op.kex.exchprovctx, provkey);
@@ -246,8 +227,7 @@ int EVP_PKEY_derive_init_ex(EVP_PKEY_CTX *ctx, EVP_KEYEXCH *exchange)
 
  legacy:
     if (ctx == NULL || ctx->pmeth == NULL || ctx->pmeth->derive == NULL) {
-        EVPerr(EVP_F_EVP_PKEY_DERIVE_INIT_EX,
-               EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
+        EVPerr(0, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
         return -2;
     }
 
@@ -259,11 +239,6 @@ int EVP_PKEY_derive_init_ex(EVP_PKEY_CTX *ctx, EVP_KEYEXCH *exchange)
     return ret;
 }
 
-int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx)
-{
-    return EVP_PKEY_derive_init_ex(ctx, NULL);
-}
-
 int EVP_PKEY_derive_set_peer(EVP_PKEY_CTX *ctx, EVP_PKEY *peer)
 {
     int ret;
@@ -284,8 +259,7 @@ int EVP_PKEY_derive_set_peer(EVP_PKEY_CTX *ctx, EVP_PKEY *peer)
         return -2;
     }
 
-    provkey =
-        evp_keymgmt_export_to_provider(peer, ctx->op.kex.exchange->keymgmt, 0);
+    provkey = evp_keymgmt_export_to_provider(peer, ctx->keymgmt, 0);
     if (provkey == NULL) {
         EVPerr(EVP_F_EVP_PKEY_DERIVE_SET_PEER, ERR_R_INTERNAL_ERROR);
         return 0;
@@ -402,13 +376,9 @@ void EVP_KEYEXCH_do_all_provided(OPENSSL_CTX *libctx,
                                  void (*fn)(EVP_KEYEXCH *keyexch, void *arg),
                                  void *arg)
 {
-    struct keymgmt_data_st keymgmt_data;
-
-    keymgmt_data.ctx = libctx;
-    keymgmt_data.properties = NULL;
     evp_generic_do_all(libctx, OSSL_OP_KEYEXCH,
                        (void (*)(void *, void *))fn, arg,
-                       evp_keyexch_from_dispatch, &keymgmt_data,
+                       evp_keyexch_from_dispatch, NULL,
                        (void (*)(void *))EVP_KEYEXCH_free);
 }
 
diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c
index d547e5a69d..cc0707e052 100644
--- a/crypto/evp/pmeth_lib.c
+++ b/crypto/evp/pmeth_lib.c
@@ -482,6 +482,7 @@ void EVP_PKEY_CTX_free(EVP_PKEY_CTX *ctx)
         ctx->pmeth->cleanup(ctx);
 
     evp_pkey_ctx_free_old_ops(ctx);
+    EVP_KEYMGMT_free(ctx->keymgmt);
 
     EVP_PKEY_free(ctx->pkey);
     EVP_PKEY_free(ctx->peerkey);