EVP: Make the KEYEXCH implementation leaner
authorRichard Levitte <levitte@openssl.org>
Wed, 30 Oct 2019 17:03:07 +0000 (18:03 +0100)
committerRichard Levitte <levitte@openssl.org>
Tue, 5 Nov 2019 21:20:06 +0000 (22:20 +0100)
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)

crypto/evp/evp_local.h
crypto/evp/exchange.c
crypto/evp/pmeth_lib.c
doc/man3/EVP_PKEY_derive.pod
include/crypto/evp.h
include/openssl/evp.h
util/libcrypto.num

index 9b208190fbe387a82e0467c7494eb85c4f225200..5795dcbe51a1943ebdf70c48b309d98db01e07ba 100644 (file)
@@ -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;
index dfc309ecd777fbc95882deba92551141622c5745..6e1372e042f7a0a80ff9532a149ada58e42eca1f 100644 (file)
@@ -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);
 }
 
index d547e5a69dcb2866b5aa6adc43c570b1888dee75..cc0707e0521095a15ecb7c85e5eb83df9a094e67 100644 (file)
@@ -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);
index 832498ba76fde57711411b47af1b79f752d94e62..954a3501e844ca3a86cad06dd1d9ea80bcb9686d 100644 (file)
@@ -2,43 +2,34 @@
 
 =head1 NAME
 
-EVP_PKEY_derive_init, EVP_PKEY_derive_init_ex, EVP_PKEY_derive_set_peer,
-EVP_PKEY_derive - derive public key algorithm shared secret
+EVP_PKEY_derive_init, EVP_PKEY_derive_set_peer, EVP_PKEY_derive
+- derive public key algorithm shared secret
 
 =head1 SYNOPSIS
 
  #include <openssl/evp.h>
 
- int EVP_PKEY_derive_init_ex(EVP_PKEY_CTX *ctx, EVP_KEYEXCH *exchange);
  int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx);
  int EVP_PKEY_derive_set_peer(EVP_PKEY_CTX *ctx, EVP_PKEY *peer);
  int EVP_PKEY_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen);
 
 =head1 DESCRIPTION
 
-The EVP_PKEY_derive_init_ex() function initializes a public key algorithm
-context for shared secret derivation using the key exchange algorithm
-B<exchange>.
-The key exchange algorithm B<exchange> should be fetched using a call to
-L<EVP_KEYEXCH_fetch(3)>.
-The EVP_PKEY object associated with B<ctx> must be compatible with that
-algorithm.
-B<exchange> may be NULL in which case the EVP_KEYEXCH algorithm is fetched
-implicitly based on the type of EVP_PKEY associated with B<ctx>.
-See L<provider(7)/Implicit fetch> for more information about implict fetches.
-
-The EVP_PKEY_derive_init() function is the same as EVP_PKEY_derive_init_ex()
-except that the EVP_KEYEXCH algorithm is always implicitly fetched.
-
-The EVP_PKEY_derive_set_peer() function sets the peer key: this will normally
+EVP_PKEY_derive_init() initializes a public key algorithm context I<ctx> for
+shared secret derivation using the algorithm given when the context was created
+using L<EVP_PKEY_CTX_new(3)> or variants thereof.  The algorithm is used to
+fetch a B<EVP_KEYEXCH> method implicitly, see L<provider(7)/Implicit fetch> for
+more information about implict fetches.
+
+EVP_PKEY_derive_set_peer() sets the peer key: this will normally
 be a public key.
 
-The EVP_PKEY_derive() derives a shared secret using B<ctx>.
-If B<key> is B<NULL> then the maximum size of the output buffer is written to
-the B<keylen> parameter. If B<key> is not B<NULL> then before the call the
-B<keylen> parameter should contain the length of the B<key> buffer, if the call
-is successful the shared secret is written to B<key> and the amount of data
-written to B<keylen>.
+EVP_PKEY_derive() derives a shared secret using I<ctx>.
+If I<key> is NULL then the maximum size of the output buffer is written to the
+I<keylen> parameter. If I<key> is not NULL then before the call the I<keylen>
+parameter should contain the length of the I<key> buffer, if the call is
+successful the shared secret is written to I<key> and the amount of data
+written to I<keylen>.
 
 =head1 NOTES
 
index 32ae121eeadf92383d964f9b859d9c6793b3a1e2..dfbcf149de13440affa28489f11ed8a59a4d372c 100644 (file)
@@ -29,6 +29,9 @@ struct evp_pkey_ctx_st {
     const char *algorithm;
     const char *propquery;
 
+    /* cached key manager */
+    EVP_KEYMGMT *keymgmt;
+
     union {
         struct {
             EVP_KEYEXCH *exchange;
index baa1ce8c6c559b7ad5dfb01a7dc5080705d2f2fa..5de6f8b08eef0f79afc176cd8b67e9b1a03e863c 100644 (file)
@@ -1550,7 +1550,6 @@ int EVP_PKEY_decrypt(EVP_PKEY_CTX *ctx,
                      unsigned char *out, size_t *outlen,
                      const unsigned char *in, size_t inlen);
 
-int EVP_PKEY_derive_init_ex(EVP_PKEY_CTX *ctx, EVP_KEYEXCH *exchange);
 int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx);
 int EVP_PKEY_derive_set_peer(EVP_PKEY_CTX *ctx, EVP_PKEY *peer);
 int EVP_PKEY_derive(EVP_PKEY_CTX *ctx, unsigned char *key, size_t *keylen);
index 8a88b6e8bd3af8da20c76677e8517d09435d5a20..737a2aba98d4f56245b6a1051d4bbc52364af8dc 100644 (file)
@@ -4666,7 +4666,7 @@ BN_priv_rand_ex                         4782      3_0_0   EXIST::FUNCTION:
 BN_rand_range_ex                        4783   3_0_0   EXIST::FUNCTION:
 BN_priv_rand_range_ex                   4784   3_0_0   EXIST::FUNCTION:
 BN_generate_prime_ex2                   4785   3_0_0   EXIST::FUNCTION:
-EVP_PKEY_derive_init_ex                 4786   3_0_0   EXIST::FUNCTION:
+EVP_PKEY_derive_init_ex                 4786   3_0_0   NOEXIST::FUNCTION:
 EVP_KEYEXCH_free                        4787   3_0_0   EXIST::FUNCTION:
 EVP_KEYEXCH_up_ref                      4788   3_0_0   EXIST::FUNCTION:
 EVP_KEYEXCH_fetch                       4789   3_0_0   EXIST::FUNCTION: