EVP: fetch the EVP_KEYMGMT earlier
authorRichard Levitte <levitte@openssl.org>
Tue, 17 Mar 2020 13:37:47 +0000 (14:37 +0100)
committerRichard Levitte <levitte@openssl.org>
Sat, 21 Mar 2020 08:28:11 +0000 (09:28 +0100)
Instead of fetching the EVP_KEYMGMT in the init for every different
operation, do it when creating the EVP_PKEY_CTX.

This allows certain control functions to be called between the
creation of the EVP_PKEY_CTX and the call of the operation's init
function.

Use case: EVP_PKEY_CTX_set1_id(), which is allowed to be called very
early with the legacy implementation, this should still be allowed
with provider implementations.

Reviewed-by: Paul Yang <kaishen.yy@antfin.com>
(Merged from https://github.com/openssl/openssl/pull/11343)

crypto/evp/exchange.c
crypto/evp/p_lib.c
crypto/evp/pmeth_gn.c
crypto/evp/pmeth_lib.c
crypto/evp/signature.c
include/crypto/evp.h

index ec5ba03..3e66e72 100644 (file)
@@ -197,7 +197,7 @@ int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx)
      */
     ERR_set_mark();
 
-    if (ctx->engine != NULL || ctx->keytype == NULL)
+    if (ctx->keymgmt == NULL)
         goto legacy;
 
     /*
index 3012790..a2bb2b7 100644 (file)
@@ -1105,13 +1105,15 @@ void *evp_pkey_export_to_provider(EVP_PKEY *pk, OPENSSL_CTX *libctx,
         *keymgmt = NULL;
     }
 
-    /* If no keymgmt was given or found, get a default keymgmt */
+    /*
+     * If no keymgmt was given or found, get a default keymgmt.  We do so by
+     * letting EVP_PKEY_CTX_new_from_pkey() do it for us, then we steal it.
+     */
     if (tmp_keymgmt == NULL) {
         EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new_from_pkey(libctx, pk, propquery);
 
-        if (ctx != NULL && ctx->keytype != NULL)
-            tmp_keymgmt = allocated_keymgmt =
-                EVP_KEYMGMT_fetch(ctx->libctx, ctx->keytype, propquery);
+        tmp_keymgmt = ctx->keymgmt;
+        ctx->keymgmt = NULL;
         EVP_PKEY_CTX_free(ctx);
     }
 
@@ -1249,14 +1251,17 @@ void *evp_pkey_upgrade_to_provider(EVP_PKEY *pk, OPENSSL_CTX *libctx,
         if (pk->ameth->export_to == NULL)
             return NULL;
 
-        /* If no keymgmt was given, get a default keymgmt */
+        /*
+         * If no keymgmt was given or found, get a default keymgmt.  We do
+         * so by letting EVP_PKEY_CTX_new_from_pkey() do it for us, then we
+         * steal it.
+         */
         if (tmp_keymgmt == NULL) {
             EVP_PKEY_CTX *ctx =
                 EVP_PKEY_CTX_new_from_pkey(libctx, pk, propquery);
 
-            if (ctx != NULL && ctx->keytype != NULL)
-                tmp_keymgmt = allocated_keymgmt =
-                    EVP_KEYMGMT_fetch(ctx->libctx, ctx->keytype, propquery);
+            tmp_keymgmt = ctx->keymgmt;
+            ctx->keymgmt = NULL;
             EVP_PKEY_CTX_free(ctx);
         }
 
index 03f1426..1bf95af 100644 (file)
@@ -30,22 +30,9 @@ static int gen_init(EVP_PKEY_CTX *ctx, int operation)
     evp_pkey_ctx_free_old_ops(ctx);
     ctx->operation = operation;
 
-    if (ctx->engine != NULL || ctx->keytype == NULL)
+    if (ctx->keymgmt == NULL || ctx->keymgmt->gen_init == NULL)
         goto legacy;
 
-    if (ctx->keymgmt == NULL) {
-        ctx->keymgmt =
-            EVP_KEYMGMT_fetch(ctx->libctx, ctx->keytype, ctx->propquery);
-        if (ctx->keymgmt == NULL
-            || ctx->keymgmt->gen_init == NULL) {
-            EVP_KEYMGMT_free(ctx->keymgmt);
-            ctx->keymgmt = NULL;
-            goto legacy;
-        }
-    }
-    if (ctx->keymgmt->gen_init == NULL)
-        goto not_supported;
-
     switch (operation) {
     case EVP_PKEY_OP_PARAMGEN:
         ctx->op.keymgmt.genctx =
@@ -156,7 +143,7 @@ int EVP_PKEY_gen(EVP_PKEY_CTX *ctx, EVP_PKEY **ppkey)
         return -1;
     }
 
-    if (ctx->keymgmt == NULL)
+    if (ctx->keymgmt == NULL || ctx->op.keymgmt.genctx == NULL)
         goto legacy;
 
     ret = 1;
@@ -309,13 +296,10 @@ static int fromdata_init(EVP_PKEY_CTX *ctx, int operation)
         goto not_supported;
 
     evp_pkey_ctx_free_old_ops(ctx);
-    ctx->operation = operation;
-    if (ctx->keymgmt == NULL)
-        ctx->keymgmt = EVP_KEYMGMT_fetch(ctx->libctx, ctx->keytype,
-                                         ctx->propquery);
     if (ctx->keymgmt == NULL)
         goto not_supported;
 
+    ctx->operation = operation;
     return 1;
 
  not_supported:
index b8f4f74..f5e1131 100644 (file)
@@ -138,12 +138,13 @@ EVP_PKEY_METHOD *EVP_PKEY_meth_new(int id, int flags)
 
 static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
                                  EVP_PKEY *pkey, ENGINE *e,
-                                 const char *name, const char *propquery,
+                                 const char *keytype, const char *propquery,
                                  int id)
 
 {
     EVP_PKEY_CTX *ret;
     const EVP_PKEY_METHOD *pmeth = NULL;
+    EVP_KEYMGMT *keymgmt = NULL;
 
     /*
      * When using providers, the context is bound to the algo implementation
@@ -160,11 +161,7 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
         /* If we have an engine, something went wrong somewhere... */
         if (!ossl_assert(e == NULL))
             return NULL;
-        name = evp_first_name(pkey->keymgmt->prov, pkey->keymgmt->name_id);
-        /*
-         * TODO: I wonder if the EVP_PKEY should have the name and propquery
-         * that were used when building it....  /RL
-         */
+        keytype = evp_first_name(pkey->keymgmt->prov, pkey->keymgmt->name_id);
         goto common;
     }
 #ifndef FIPS_MODE
@@ -187,10 +184,10 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
      * since that can only happen internally, it's safe to make an
      * assertion.
      */
-    if (!ossl_assert(e == NULL || name == NULL))
+    if (!ossl_assert(e == NULL || keytype == NULL))
         return NULL;
     if (e == NULL)
-        name = OBJ_nid2sn(id);
+        keytype = OBJ_nid2sn(id);
 
 # ifndef OPENSSL_NO_ENGINE
     if (e == NULL && pkey != NULL)
@@ -225,6 +222,13 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
     /* END legacy */
 #endif /* FIPS_MODE */
  common:
+    /*
+     * If there's no engine and there's a name, we try fetching a provider
+     * implementation.
+     */
+    if (e == NULL && keytype != NULL)
+        keymgmt = EVP_KEYMGMT_fetch(libctx, keytype, propquery);
+
     ret = OPENSSL_zalloc(sizeof(*ret));
     if (ret == NULL) {
 #if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODE)
@@ -234,8 +238,9 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
         return NULL;
     }
     ret->libctx = libctx;
-    ret->keytype = name;
     ret->propquery = propquery;
+    ret->keytype = keytype;
+    ret->keymgmt = keymgmt;
     ret->engine = e;
     ret->pmeth = pmeth;
     ret->operation = EVP_PKEY_OP_UNDEFINED;
index acbe765..1f5e570 100644 (file)
@@ -359,7 +359,7 @@ static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, int operation)
      */
     ERR_set_mark();
 
-    if (ctx->engine != NULL || ctx->keytype == NULL)
+    if (ctx->keymgmt == NULL)
         goto legacy;
 
     /*
index c9d3075..2e0322f 100644 (file)
@@ -23,14 +23,12 @@ struct evp_pkey_ctx_st {
     int operation;
 
     /*
-     * Library context, Key type name and properties associated
-     * with this context
+     * Library context, property query, keytype and keymgmt associated with
+     * this context
      */
     OPENSSL_CTX *libctx;
-    const char *keytype;
     const char *propquery;
-
-    /* cached key manager */
+    const char *keytype;
     EVP_KEYMGMT *keymgmt;
 
     union {