Don't fallback to legacy in DigestSignInit/DigestVerifyInit too easily
authorMatt Caswell <matt@openssl.org>
Tue, 28 Jul 2020 15:47:03 +0000 (16:47 +0100)
committerMatt Caswell <matt@openssl.org>
Thu, 30 Jul 2020 08:28:01 +0000 (09:28 +0100)
The only reason we should fallback to legacy codepaths in DigestSignInit/
DigestVerifyInit, is if we have an engine, or we have a legacy algorithm
that does not (yet) have a provider based equivalent (e.g. SM2, HMAC, etc).
Currently we were falling back even if we have a suitable key manager but
the export of the key fails. This might be for legitimate reasons (e.g.
we only have the FIPS provider, but we're trying to export a brainpool key).
In those circumstances we don't want to fallback to the legacy code.

Therefore we tighten then checks for falling back to legacy. Eventually this
particular fallback can be removed entirely (once all legacy algorithms have
provider based key managers).

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/12550)

crypto/evp/m_sigver.c

index 44e7cab1afe99fe2b272fcdb45d2c71f8b3c5f13..8d37f19d6c205843450e1e3134a2135dedf8d693 100644 (file)
@@ -85,13 +85,25 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
 
     /*
      * Ensure that the key is provided, either natively, or as a cached export.
-     *  If not, go legacy
      */
     tmp_keymgmt = locpctx->keymgmt;
     provkey = evp_pkey_export_to_provider(locpctx->pkey, locpctx->libctx,
                                           &tmp_keymgmt, locpctx->propquery);
-    if (provkey == NULL)
-        goto legacy;
+    if (provkey == NULL) {
+        /*
+         * If we couldn't find a keymgmt at all try legacy.
+         * TODO(3.0): Once all legacy algorithms (SM2, HMAC etc) have provider
+         * based implementations this fallback shouldn't be necessary. Either
+         * we have an ENGINE based implementation (in which case we should have
+         * already fallen back in the test above here), or we don't have the
+         * provider based implementation loaded (in which case this is an
+         * application config error)
+         */
+        if (locpctx->keymgmt == NULL)
+            goto legacy;
+        ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
+        goto err;
+    }
     if (!EVP_KEYMGMT_up_ref(tmp_keymgmt)) {
         ERR_clear_last_mark();
         ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);