EVP: clear error when falling back from failed EVP_KEYMGMT_fetch()

Since we're falling back to legacy, this isn't an error any more.
Among others the failed EVP_KEYMGMT_fetch() error shadows other errors
produced by the legacy code, which disrupts our test/evp_test runs.

We use the error stack mark to restore the error stack just right,
i.e. ERR_set_mark(), ERR_clear_last_mark() and ERR_pop_to_mark()

Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/10803)

diff --git a/crypto/evp/exchange.c b/crypto/evp/exchange.c
index 9bb241fe145e7c07a2d643cc46d06bf5362c49b7..1f9e39be4ce27eebacb93599527664505e9722a1 100644 (file)
--- a/crypto/evp/exchange.c
+++ b/crypto/evp/exchange.c
@@ -175,6 +175,12 @@ int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx)
     evp_pkey_ctx_free_old_ops(ctx);
     ctx->operation = EVP_PKEY_OP_DERIVE;
 
+    /*
+     * TODO when we stop falling back to legacy, this and the ERR_pop_to_mark()
+     * calls can be removed.
+     */
+    ERR_set_mark();
+
     if (ctx->engine != NULL || ctx->keytype == NULL)
         goto legacy;
 
@@ -185,6 +191,7 @@ int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx)
     if (provkey == NULL)
         goto legacy;
     if (!EVP_KEYMGMT_up_ref(tmp_keymgmt)) {
+        ERR_clear_last_mark();
         ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
         goto err;
     }
@@ -211,15 +218,21 @@ int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx)
         || (EVP_KEYMGMT_provider(ctx->keymgmt)
             != EVP_KEYEXCH_provider(exchange))) {
         /*
-         * 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().
+         * 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_KEYEXCH_free(exchange);
         goto legacy;
     }
 
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
+    /* No more legacy from here down to legacy: */
 
     ctx->op.kex.exchange = exchange;
     ctx->op.kex.exchprovctx = exchange->newctx(ossl_provider_ctx(exchange->prov));
@@ -236,6 +249,13 @@ int EVP_PKEY_derive_init(EVP_PKEY_CTX *ctx)
     return 0;
 
  legacy:
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
     if (ctx->pmeth == NULL || ctx->pmeth->derive == NULL) {
         EVPerr(0, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
         return -2;

diff --git a/crypto/evp/m_sigver.c b/crypto/evp/m_sigver.c
index 20c0a1ad597c70b1fb6d37908c89dbba1f5ecef3..79099b1e356818d57cbabb6cbe0fb9c2fa518f94 100644 (file)
--- a/crypto/evp/m_sigver.c
+++ b/crypto/evp/m_sigver.c
@@ -54,6 +54,12 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
     locpctx = ctx->pctx;
     evp_pkey_ctx_free_old_ops(locpctx);
 
+    /*
+     * TODO when we stop falling back to legacy, this and the ERR_pop_to_mark()
+     * calls can be removed.
+     */
+    ERR_set_mark();
+
     if (locpctx->keytype == NULL)
         goto legacy;
 
@@ -80,6 +86,7 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
     if (provkey == NULL)
         goto legacy;
     if (!EVP_KEYMGMT_up_ref(tmp_keymgmt)) {
+        ERR_clear_last_mark();
         ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
         goto err;
     }
@@ -108,15 +115,20 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
         || (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().
+         * 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;
     }
 
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
     /* No more legacy from here down to legacy: */
 
     locpctx->op.sig.signature = signature;
@@ -164,6 +176,13 @@ static int do_sigver_init(EVP_MD_CTX *ctx, EVP_PKEY_CTX **pctx,
     return 0;
 
  legacy:
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
     if (ctx->pctx->pmeth == NULL) {
         EVPerr(0, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
         return 0;

diff --git a/crypto/evp/pmeth_fn.c b/crypto/evp/pmeth_fn.c
index 19bc56c654d818137fd8344f22e8177fd38a2562..34cde676681e918a8136d2662488f4a9a8a39570 100644 (file)
--- a/crypto/evp/pmeth_fn.c
+++ b/crypto/evp/pmeth_fn.c
@@ -32,6 +32,12 @@ static int evp_pkey_asym_cipher_init(EVP_PKEY_CTX *ctx, int operation)
     evp_pkey_ctx_free_old_ops(ctx);
     ctx->operation = operation;
 
+    /*
+     * TODO when we stop falling back to legacy, this and the ERR_pop_to_mark()
+     * calls can be removed.
+     */
+    ERR_set_mark();
+
     if (ctx->keytype == NULL || ctx->engine != NULL)
         goto legacy;
 
@@ -41,7 +47,11 @@ static int evp_pkey_asym_cipher_init(EVP_PKEY_CTX *ctx, int operation)
                                      &tmp_keymgmt, ctx->propquery, 0);
     if (provkey == NULL)
         goto legacy;
-    EVP_KEYMGMT_up_ref(tmp_keymgmt);
+    if (!EVP_KEYMGMT_up_ref(tmp_keymgmt)) {
+        ERR_clear_last_mark();
+        ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
+        goto err;
+    }
     EVP_KEYMGMT_free(ctx->keymgmt);
     ctx->keymgmt = tmp_keymgmt;
 
@@ -67,15 +77,22 @@ static int evp_pkey_asym_cipher_init(EVP_PKEY_CTX *ctx, int operation)
         || (EVP_KEYMGMT_provider(ctx->keymgmt)
             != EVP_ASYM_CIPHER_provider(cipher))) {
         /*
-         * 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().
+         * 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_ASYM_CIPHER_free(cipher);
         goto legacy;
     }
 
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
+    /* No more legacy from here down to legacy: */
+
     ctx->op.ciph.cipher = cipher;
     ctx->op.ciph.ciphprovctx = cipher->newctx(ossl_provider_ctx(cipher->prov));
     if (ctx->op.ciph.ciphprovctx == NULL) {
@@ -114,6 +131,13 @@ static int evp_pkey_asym_cipher_init(EVP_PKEY_CTX *ctx, int operation)
     return 1;
 
  legacy:
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
     if (ctx->pmeth == NULL || ctx->pmeth->encrypt == NULL) {
         EVPerr(0, EVP_R_OPERATION_NOT_SUPPORTED_FOR_THIS_KEYTYPE);
         return -2;

diff --git a/crypto/evp/signature.c b/crypto/evp/signature.c
index 0adf59be2a444ad6cc5ede32e30292d7a56c79e5..7c6828b3b35068789d4e06e2350de2cd58499311 100644 (file)
--- a/crypto/evp/signature.c
+++ b/crypto/evp/signature.c
@@ -333,6 +333,12 @@ static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, int operation)
     evp_pkey_ctx_free_old_ops(ctx);
     ctx->operation = operation;
 
+    /*
+     * TODO when we stop falling back to legacy, this and the ERR_pop_to_mark()
+     * calls can be removed.
+     */
+    ERR_set_mark();
+
     if (ctx->keytype == NULL)
         goto legacy;
 
@@ -343,6 +349,7 @@ static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, int operation)
     if (provkey == NULL)
         goto legacy;
     if (!EVP_KEYMGMT_up_ref(tmp_keymgmt)) {
+        ERR_clear_last_mark();
         ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
         goto err;
     }
@@ -370,15 +377,22 @@ static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, int operation)
         || (EVP_KEYMGMT_provider(ctx->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().
+         * 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;
     }
 
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
+    /* No more legacy from here down to legacy: */
+
     ctx->op.sig.signature = signature;
     ctx->op.sig.sigprovctx = signature->newctx(ossl_provider_ctx(signature->prov));
     if (ctx->op.sig.sigprovctx == NULL) {
@@ -425,6 +439,13 @@ static int evp_pkey_signature_init(EVP_PKEY_CTX *ctx, int operation)
     return 1;
 
  legacy:
+    /*
+     * TODO remove this when legacy is gone
+     * If we don't have the full support we need with provided methods,
+     * let's go see if legacy does.
+     */
+    ERR_pop_to_mark();
+
     if (ctx->pmeth == NULL
             || (operation == EVP_PKEY_OP_SIGN && ctx->pmeth->sign == NULL)
             || (operation == EVP_PKEY_OP_VERIFY && ctx->pmeth->verify == NULL)