From: Richard Levitte Date: Fri, 10 Jan 2020 16:50:03 +0000 (+0100) Subject: EVP: clear error when falling back from failed EVP_KEYMGMT_fetch() X-Git-Tag: openssl-3.0.0-alpha1~632 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=0b9dd3842f9bbe7a7c290187056a96827d848f52 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 (Merged from https://github.com/openssl/openssl/pull/10803) --- diff --git a/crypto/evp/exchange.c b/crypto/evp/exchange.c index 9bb241fe14..1f9e39be4c 100644 --- 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 20c0a1ad59..79099b1e35 100644 --- 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 19bc56c654..34cde67668 100644 --- 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 0adf59be2a..7c6828b3b3 100644 --- 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)