EVP: Make the SIGNATURE 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 SIGNATURE method explicitly, and rather
always have it be fetched implicitly.

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

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

3 files changed, 101 insertions, 139 deletions

diff --git a/crypto/evp/evp_local.h b/crypto/evp/evp_local.h
index 5795dcbe51..259805db80 100644
--- a/crypto/evp/evp_local.h
+++ b/crypto/evp/evp_local.h
@@ -118,8 +118,6 @@ struct evp_signature_st {
     CRYPTO_REF_COUNT refcnt;
     CRYPTO_RWLOCK *lock;
 
-    EVP_KEYMGMT *keymgmt;
-
     OSSL_OP_signature_newctx_fn *newctx;
     OSSL_OP_signature_sign_init_fn *sign_init;
     OSSL_OP_signature_sign_fn *sign;
diff --git a/crypto/evp/m_sigver.c b/crypto/evp/m_sigver.c
index c02325cf6b..7a21f680b9 100644
--- a/crypto/evp/m_sigver.c
+++ b/crypto/evp/m_sigver.c
@@ -27,9 +27,10 @@ static int update(EVP_MD_CTX *ctx, const void *data, size_t datalen)
 static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
                           const EVP_MD *type, const char *mdname,
                           const char *props, ENGINE *e, EVP_PKEY *pkey,
-                          EVP_SIGNATURE *signature, int ver)
+                          int ver)
 {
     EVP_PKEY_CTX *locpctx = NULL;
+    EVP_SIGNATURE *signature = NULL;
     void *provkey = NULL;
     int ret;
 
@@ -43,51 +44,69 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
         ctx->provctx = NULL;
     }
 
-    if (ctx->pctx == NULL) {
+    if (ctx->pctx == NULL)
         ctx->pctx = EVP_PKEY_CTX_new(pkey, e);
-        if (ctx->pctx == NULL)
-            return 0;
-    } else if (pkey != NULL) {
-        if (!EVP_PKEY_up_ref(pkey))
-            return 0;
-        EVP_PKEY_free(ctx->pctx->pkey);
-        ctx->pctx->pkey = pkey;
-    }
+    if (ctx->pctx == NULL)
+        return 0;
+
     locpctx = ctx->pctx;
     evp_pkey_ctx_free_old_ops(locpctx);
-    if (locpctx->pkey == NULL)
-        goto legacy;
 
-    if (e != NULL || locpctx->engine != NULL)
+    if (locpctx->algorithm == NULL)
         goto legacy;
 
-    if (signature != NULL) {
-        if (!EVP_SIGNATURE_up_ref(signature))
-            goto err;
-    } else {
-        /*
-         * TODO(3.0): Check for legacy handling. Remove this once all all
-         * algorithms are moved to providers.
-         */
-        switch (locpctx->pkey->type) {
-        case NID_dsa:
-            break;
-        default:
-            goto legacy;
+    if (mdname == NULL) {
+        if (type != NULL) {
+            mdname = EVP_MD_name(type);
+        } else if (pkey != NULL) {
+            /*
+             * TODO(v3.0) work out a better way for EVP_PKEYs with no legacy
+             * component.
+             */
+            if (pkey->pkey.ptr != NULL) {
+                int def_nid;
+                if (EVP_PKEY_get_default_digest_nid(pkey, &def_nid) > 0)
+                    mdname = OBJ_nid2sn(def_nid);
+            }
         }
-        signature
-            = EVP_SIGNATURE_fetch(NULL, OBJ_nid2sn(locpctx->pkey->type), NULL);
+    }
 
-        if (signature == NULL) {
-            ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
-            goto err;
-        }
+    /*
+     * Because we cleared out old ops, we shouldn't need to worry about
+     * checking if signature is already there.  Keymgmt is a different
+     * matter, as it isn't tied to a specific EVP_PKEY op.
+     */
+    signature = EVP_SIGNATURE_fetch(locpctx->libctx, locpctx->algorithm,
+                                    locpctx->propquery);
+    if (signature != NULL && locpctx->keymgmt == NULL) {
+        int name_id = EVP_SIGNATURE_number(signature);
+
+        locpctx->keymgmt =
+            evp_keymgmt_fetch_by_number(locpctx->libctx, name_id,
+                                        locpctx->propquery);
     }
-    locpctx->operation = ver ? EVP_PKEY_OP_VERIFYCTX
-                             : EVP_PKEY_OP_SIGNCTX;
+
+    if (locpctx->keymgmt == NULL
+        || signature == NULL
+        || (EVP_KEYMGMT_provider(locpctx->keymgmt)
+            != EVP_SIGNATURE_provider(signature))) {
+        /*
+         * 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().
+         */
+        EVP_SIGNATURE_free(signature);
+        goto legacy;
+    }
+
+    /* No more legacy from here down to legacy: */
 
     locpctx->op.sig.signature = signature;
 
+    locpctx->operation = ver ? EVP_PKEY_OP_VERIFYCTX
+                             : EVP_PKEY_OP_SIGNCTX;
+
     locpctx->op.sig.sigprovctx
         = signature->newctx(ossl_provider_ctx(signature->prov));
     if (locpctx->op.sig.sigprovctx == NULL) {
@@ -95,14 +114,13 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
         goto err;
     }
     provkey =
-        evp_keymgmt_export_to_provider(locpctx->pkey, signature->keymgmt, 0);
+        evp_keymgmt_export_to_provider(locpctx->pkey, locpctx->keymgmt, 0);
     if (provkey == NULL) {
         ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
         goto err;
     }
 
-    if (mdname == NULL) {
-        mdname = EVP_MD_name(type);
+    if (type != NULL) {
         ctx->reqdigest = type;
     } else {
         /*
@@ -111,11 +129,8 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
          * man pages), i.e. the ref count is not updated so the EVP_MD should
          * not be used beyound the lifetime of the EVP_MD_CTX.
          */
-        ctx->reqdigest
-            = ctx->fetched_digest
-            = EVP_MD_fetch(
-                ossl_provider_library_context(EVP_SIGNATURE_provider(signature)),
-                mdname, props);
+        ctx->reqdigest = ctx->fetched_digest =
+            EVP_MD_fetch(locpctx->libctx, mdname, props);
     }
 
     if (ver) {
@@ -123,15 +138,15 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
             ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
             goto err;
         }
-        ret = signature->digest_verify_init(locpctx->op.sig.sigprovctx, mdname,
-                                            props, provkey);
+        ret = signature->digest_verify_init(locpctx->op.sig.sigprovctx,
+                                            mdname, props, provkey);
     } else {
         if (signature->digest_sign_init == NULL) {
             ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
             goto err;
         }
-        ret = signature->digest_sign_init(locpctx->op.sig.sigprovctx, mdname,
-                                          props, provkey);
+        ret = signature->digest_sign_init(locpctx->op.sig.sigprovctx,
+                                          mdname, props, provkey);
     }
 
     return ret ? 1 : 0;
@@ -197,31 +212,28 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
 }
 
 int EVP_DigestSignInit_ex(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
-                          const char *mdname, const char *props, EVP_PKEY *pkey,
-                          EVP_SIGNATURE *signature)
+                          const char *mdname, const char *props, EVP_PKEY *pkey)
 {
-    return do_sigver_init(ctx, pctx, NULL, mdname, props, NULL, pkey, signature,
-                          0);
+    return do_sigver_init(ctx, pctx, NULL, mdname, props, NULL, pkey, 0);
 }
 
 int EVP_DigestSignInit(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
                        const EVP_MD *type, ENGINE *e, EVP_PKEY *pkey)
 {
-    return do_sigver_init(ctx, pctx, type, NULL, NULL, e, pkey, NULL, 0);
+    return do_sigver_init(ctx, pctx, type, NULL, NULL, e, pkey, 0);
 }
 
 int EVP_DigestVerifyInit_ex(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
                             const char *mdname, const char *props,
-                            EVP_PKEY *pkey, EVP_SIGNATURE *signature)
+                            EVP_PKEY *pkey)
 {
-    return do_sigver_init(ctx, pctx, NULL, mdname, props, NULL, pkey, signature,
-                          1);
+    return do_sigver_init(ctx, pctx, NULL, mdname, props, NULL, pkey, 1);
 }
 
 int EVP_DigestVerifyInit(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
                          const EVP_MD *type, ENGINE *e, EVP_PKEY *pkey)
 {
-    return do_sigver_init(ctx, pctx, type, NULL, NULL, e, pkey, NULL, 1);
+    return do_sigver_init(ctx, pctx, type, NULL, NULL, e, pkey, 1);
 }
 #endif /* FIPS_MDOE */
 
diff --git a/crypto/evp/pmeth_fn.c b/crypto/evp/pmeth_fn.c
index d06edb218b..4a8ece5fac 100644
--- a/crypto/evp/pmeth_fn.c
+++ b/crypto/evp/pmeth_fn.c
@@ -35,39 +35,19 @@ static EVP_SIGNATURE *evp_signature_new(OSSL_PROVIDER *prov)
 static void *evp_signature_from_dispatch(int name_id,
                                          const OSSL_DISPATCH *fns,
                                          OSSL_PROVIDER *prov,
-                                         void *vkeymgmt_data)
+                                         void *unused)
 {
-    /*
-     * Signature functions 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_signature_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_SIGNATURE *signature = NULL;
     int ctxfncnt = 0, signfncnt = 0, verifyfncnt = 0, verifyrecfncnt = 0;
     int digsignfncnt = 0, digverifyfncnt = 0;
     int gparamfncnt = 0, sparamfncnt = 0, gmdparamfncnt = 0, smdparamfncnt = 0;
 
-    if (keymgmt == NULL || EVP_KEYMGMT_provider(keymgmt) != prov) {
-        ERR_raise(ERR_LIB_EVP, EVP_R_NO_KEYMGMT_AVAILABLE);
-        goto err;
-    }
-
     if ((signature = evp_signature_new(prov)) == NULL) {
         ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
         goto err;
     }
 
     signature->name_id = name_id;
-    signature->keymgmt = keymgmt;
-    keymgmt = NULL;              /* avoid double free on failure below */
 
     for (; fns->function_id != 0; fns++) {
         switch (fns->function_id) {
@@ -263,7 +243,6 @@ static void *evp_signature_from_dispatch(int name_id,
     return signature;
  err:
     EVP_SIGNATURE_free(signature);
-    EVP_KEYMGMT_free(keymgmt);
     return NULL;
 }
 
@@ -275,7 +254,6 @@ void EVP_SIGNATURE_free(EVP_SIGNATURE *signature)
         CRYPTO_DOWN_REF(&signature->refcnt, &i, signature->lock);
         if (i > 0)
             return;
-        EVP_KEYMGMT_free(signature->keymgmt);
         ossl_provider_free(signature->prov);
         CRYPTO_THREAD_lock_free(signature->lock);
         OPENSSL_free(signature);
@@ -298,16 +276,8 @@ OSSL_PROVIDER *EVP_SIGNATURE_provider(const EVP_SIGNATURE *signature)
 EVP_SIGNATURE *EVP_SIGNATURE_fetch(OPENSSL_CTX *ctx, const char *algorithm,
                                    const char *properties)
 {
-    struct keymgmt_data_st keymgmt_data;
-
-    /*
-     * A signature operation cannot work without a key, so we need key
-     * management from the same provider to manage its keys.
-     */
-    keymgmt_data.ctx = ctx;
-    keymgmt_data.properties = properties;
     return evp_generic_fetch(ctx, OSSL_OP_SIGNATURE, algorithm, properties,
-                             evp_signature_from_dispatch, &keymgmt_data,
+                             evp_signature_from_dispatch, NULL,
                              (int (*)(void *))EVP_SIGNATURE_up_ref,
                              (void (*)(void *))EVP_SIGNATURE_free);
 }
@@ -327,13 +297,9 @@ void EVP_SIGNATURE_do_all_provided(OPENSSL_CTX *libctx,
                                               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_SIGNATURE,
                        (void (*)(void *, void *))fn, arg,
-                       evp_signature_from_dispatch, &keymgmt_data,
+                       evp_signature_from_dispatch, NULL,
                        (void (*)(void *))EVP_SIGNATURE_free);
 }
 
@@ -346,11 +312,11 @@ void EVP_SIGNATURE_names_do_all(const EVP_SIGNATURE *signature,
         evp_names_do_all(signature->prov, signature->name_id, fn, data);
 }
 
-static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, EVP_SIGNATURE *signature,
-                                   int operation)
+static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, int operation)
 {
     int ret = 0;
     void *provkey = NULL;
+    EVP_SIGNATURE *signature = NULL;
 
     if (ctx == NULL) {
         EVPerr(0, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
@@ -360,41 +326,42 @@ static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, EVP_SIGNATURE *signature,
     evp_pkey_ctx_free_old_ops(ctx);
     ctx->operation = operation;
 
-    if (ctx->engine != NULL)
+    if (ctx->algorithm == NULL)
         goto legacy;
 
-    if (signature != NULL) {
-        if (!EVP_SIGNATURE_up_ref(signature))
-            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 signature is already there.  Keymgmt is a different
+     * matter, as it isn't tied to a specific EVP_PKEY op.
+     */
+    signature = EVP_SIGNATURE_fetch(ctx->libctx, ctx->algorithm,
+                                    ctx->propquery);
+    if (signature != NULL && ctx->keymgmt == NULL) {
+        int name_id = EVP_SIGNATURE_number(signature);
+
+        ctx->keymgmt = evp_keymgmt_fetch_by_number(ctx->libctx, name_id,
+                                                   ctx->propquery);
+    }
 
+    if (ctx->keymgmt == NULL
+        || signature == NULL
+        || (EVP_KEYMGMT_provider(ctx->keymgmt)
+            != EVP_SIGNATURE_provider(signature))) {
         /*
-         * 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 NID_dsa:
-                break;
-            default:
-                goto legacy;
-            }
-            signature = EVP_SIGNATURE_fetch(NULL, OBJ_nid2sn(nid), NULL);
-        } else {
-            goto legacy;
-        }
-
-        if (signature == NULL) {
-            EVPerr(0, EVP_R_INITIALIZATION_ERROR);
-            goto err;
-        }
+        EVP_SIGNATURE_free(signature);
+        goto legacy;
     }
 
     ctx->op.sig.signature = signature;
+
     if (ctx->pkey != NULL) {
         provkey =
-            evp_keymgmt_export_to_provider(ctx->pkey, signature->keymgmt, 0);
+            evp_keymgmt_export_to_provider(ctx->pkey, ctx->keymgmt, 0);
         if (provkey == NULL) {
             EVPerr(0, EVP_R_INITIALIZATION_ERROR);
             goto err;
@@ -483,14 +450,9 @@ static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, EVP_SIGNATURE *signature,
     return ret;
 }
 
-int EVP_PKEY_sign_init_ex(EVP_PKEY_CTX *ctx, EVP_SIGNATURE *signature)
-{
-    return evp_pkey_signature_init(ctx, signature, EVP_PKEY_OP_SIGN);
-}
-
 int EVP_PKEY_sign_init(EVP_PKEY_CTX *ctx)
 {
-    return evp_pkey_signature_init(ctx, NULL, EVP_PKEY_OP_SIGN);
+    return evp_pkey_signature_init(ctx, EVP_PKEY_OP_SIGN);
 }
 
 int EVP_PKEY_sign(EVP_PKEY_CTX *ctx,
@@ -527,14 +489,9 @@ int EVP_PKEY_sign(EVP_PKEY_CTX *ctx,
         return ctx->pmeth->sign(ctx, sig, siglen, tbs, tbslen);
 }
 
-int EVP_PKEY_verify_init_ex(EVP_PKEY_CTX *ctx, EVP_SIGNATURE *signature)
-{
-    return evp_pkey_signature_init(ctx, signature, EVP_PKEY_OP_VERIFY);
-}
-
 int EVP_PKEY_verify_init(EVP_PKEY_CTX *ctx)
 {
-    return evp_pkey_signature_init(ctx, NULL, EVP_PKEY_OP_VERIFY);
+    return evp_pkey_signature_init(ctx, EVP_PKEY_OP_VERIFY);
 }
 
 int EVP_PKEY_verify(EVP_PKEY_CTX *ctx,
@@ -569,14 +526,9 @@ int EVP_PKEY_verify(EVP_PKEY_CTX *ctx,
     return ctx->pmeth->verify(ctx, sig, siglen, tbs, tbslen);
 }
 
-int EVP_PKEY_verify_recover_init_ex(EVP_PKEY_CTX *ctx, EVP_SIGNATURE *signature)
-{
-    return evp_pkey_signature_init(ctx, signature, EVP_PKEY_OP_VERIFYRECOVER);
-}
-
 int EVP_PKEY_verify_recover_init(EVP_PKEY_CTX *ctx)
 {
-    return evp_pkey_signature_init(ctx, NULL, EVP_PKEY_OP_VERIFYRECOVER);
+    return evp_pkey_signature_init(ctx, EVP_PKEY_OP_VERIFYRECOVER);
 }
 
 int EVP_PKEY_verify_recover(EVP_PKEY_CTX *ctx,