From 4b9e90f42a367a880af2dae6f6c4b455a0d2c0f4 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 17 Mar 2020 14:37:47 +0100 Subject: [PATCH] EVP: fetch the EVP_KEYMGMT earlier 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 (Merged from https://github.com/openssl/openssl/pull/11343) --- crypto/evp/exchange.c | 2 +- crypto/evp/p_lib.c | 21 +++++++++++++-------- crypto/evp/pmeth_gn.c | 22 +++------------------- crypto/evp/pmeth_lib.c | 23 ++++++++++++++--------- crypto/evp/signature.c | 2 +- include/crypto/evp.h | 8 +++----- 6 files changed, 35 insertions(+), 43 deletions(-) diff --git a/crypto/evp/exchange.c b/crypto/evp/exchange.c index ec5ba03f09..3e66e721d5 100644 --- a/crypto/evp/exchange.c +++ b/crypto/evp/exchange.c @@ -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; /* diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index 3012790cee..a2bb2b7190 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -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); } diff --git a/crypto/evp/pmeth_gn.c b/crypto/evp/pmeth_gn.c index 03f1426d85..1bf95af2ac 100644 --- a/crypto/evp/pmeth_gn.c +++ b/crypto/evp/pmeth_gn.c @@ -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: diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c index b8f4f740f0..f5e1131f06 100644 --- a/crypto/evp/pmeth_lib.c +++ b/crypto/evp/pmeth_lib.c @@ -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; diff --git a/crypto/evp/signature.c b/crypto/evp/signature.c index acbe76592f..1f5e570ff8 100644 --- a/crypto/evp/signature.c +++ b/crypto/evp/signature.c @@ -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; /* diff --git a/include/crypto/evp.h b/include/crypto/evp.h index c9d3075b82..2e0322fa98 100644 --- a/include/crypto/evp.h +++ b/include/crypto/evp.h @@ -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 { -- 2.34.1