From 977e95b912138d02bae86d829a990d81c2bbcca0 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Wed, 27 Jan 2021 11:07:38 +0100 Subject: [PATCH] EVP: Fix evp_pkey_ctx_store_cached_data() to handle provider backed EVP_PKEY_CTX It assumed there would always be a non-NULL ctx->pmeth, leading to a crash when that isn't the case. Since it needs to check 'keytype' when that one isn't -1, we also add a corresponding check for the provider backed EVP_PKEY_CTX case. Reviewed-by: Dmitry Belyavskiy (Merged from https://github.com/openssl/openssl/pull/13973) --- crypto/evp/p_lib.c | 89 ++++++++++++++++++++++-------------------- crypto/evp/pmeth_lib.c | 29 +++++++++++++- include/crypto/evp.h | 1 + 3 files changed, 75 insertions(+), 44 deletions(-) diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index 21ce51d573..558f378168 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -906,53 +906,58 @@ int EVP_PKEY_base_id(const EVP_PKEY *pkey) } #ifndef FIPS_MODULE +/* + * These hard coded cases are pure hackery to get around the fact + * that names in crypto/objects/objects.txt are a mess. There is + * no "EC", and "RSA" leads to the NID for 2.5.8.1.1, an OID that's + * fallen out in favor of { pkcs-1 1 }, i.e. 1.2.840.113549.1.1.1, + * the NID of which is used for EVP_PKEY_RSA. Strangely enough, + * "DSA" is accurate... but still, better be safe and hard-code + * names that we know. + * On a similar topic, EVP_PKEY_type(EVP_PKEY_SM2) will result in + * EVP_PKEY_EC, because of aliasing. + * TODO Clean this away along with all other #legacy support. + */ +static const OSSL_ITEM standard_name2type[] = { + { EVP_PKEY_RSA, "RSA" }, + { EVP_PKEY_RSA_PSS, "RSA-PSS" }, + { EVP_PKEY_EC, "EC" }, + { EVP_PKEY_ED25519, "ED25519" }, + { EVP_PKEY_ED448, "ED448" }, + { EVP_PKEY_X25519, "X25519" }, + { EVP_PKEY_X448, "X448" }, + { EVP_PKEY_SM2, "SM2" }, + { EVP_PKEY_DH, "DH" }, + { EVP_PKEY_DHX, "X9.42 DH" }, + { EVP_PKEY_DHX, "DHX" }, + { EVP_PKEY_DSA, "DSA" }, +}; + int evp_pkey_name2type(const char *name) { - /* - * These hard coded cases are pure hackery to get around the fact - * that names in crypto/objects/objects.txt are a mess. There is - * no "EC", and "RSA" leads to the NID for 2.5.8.1.1, an OID that's - * fallen out in favor of { pkcs-1 1 }, i.e. 1.2.840.113549.1.1.1, - * the NID of which is used for EVP_PKEY_RSA. Strangely enough, - * "DSA" is accurate... but still, better be safe and hard-code - * names that we know. - * On a similar topic, EVP_PKEY_type(EVP_PKEY_SM2) will result in - * EVP_PKEY_EC, because of aliasing. - * TODO Clean this away along with all other #legacy support. - */ - int type = NID_undef; + int type; + size_t i; - if (strcasecmp(name, "RSA") == 0) - type = EVP_PKEY_RSA; - else if (strcasecmp(name, "RSA-PSS") == 0) - type = EVP_PKEY_RSA_PSS; - else if (strcasecmp(name, "EC") == 0) - type = EVP_PKEY_EC; - else if (strcasecmp(name, "ED25519") == 0) - type = EVP_PKEY_ED25519; - else if (strcasecmp(name, "ED448") == 0) - type = EVP_PKEY_ED448; - else if (strcasecmp(name, "X25519") == 0) - type = EVP_PKEY_X25519; - else if (strcasecmp(name, "X448") == 0) - type = EVP_PKEY_X448; - else if (strcasecmp(name, "SM2") == 0) - type = EVP_PKEY_SM2; - else if (strcasecmp(name, "DH") == 0) - type = EVP_PKEY_DH; - else if (strcasecmp(name, "X9.42 DH") == 0) - type = EVP_PKEY_DHX; - else if (strcasecmp(name, "DHX") == 0) - type = EVP_PKEY_DHX; - else if (strcasecmp(name, "DSA") == 0) - type = EVP_PKEY_DSA; + for (i = 0; i < OSSL_NELEM(standard_name2type); i++) { + if (strcasecmp(name, standard_name2type[i].ptr) == 0) + return (int)standard_name2type[i].id; + } - if (type == NID_undef) - type = EVP_PKEY_type(OBJ_sn2nid(name)); - if (type == NID_undef) - type = EVP_PKEY_type(OBJ_ln2nid(name)); + if ((type = EVP_PKEY_type(OBJ_sn2nid(name))) != NID_undef) + return type; + return EVP_PKEY_type(OBJ_ln2nid(name)); +} - return type; +const char *evp_pkey_type2name(int type) +{ + size_t i; + + for (i = 0; i < OSSL_NELEM(standard_name2type); i++) { + if (type == (int)standard_name2type[i].id) + return standard_name2type[i].ptr; + } + + return OBJ_nid2sn(type); } #endif diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c index bc58ea367c..91d892ec34 100644 --- a/crypto/evp/pmeth_lib.c +++ b/crypto/evp/pmeth_lib.c @@ -1684,8 +1684,33 @@ static int evp_pkey_ctx_store_cached_data(EVP_PKEY_CTX *ctx, int cmd, const char *name, const void *data, size_t data_len) { - if ((keytype != -1 && ctx->pmeth->pkey_id != keytype) - || ((optype != -1) && !(ctx->operation & optype))) { + if (keytype != -1) { + switch (evp_pkey_ctx_state(ctx)) { + case EVP_PKEY_STATE_PROVIDER: + if (ctx->keymgmt == NULL) { + ERR_raise(ERR_LIB_EVP, EVP_R_COMMAND_NOT_SUPPORTED); + return -2; + } + if (!EVP_KEYMGMT_is_a(ctx->keymgmt, + evp_pkey_type2name(keytype))) { + ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_OPERATION); + return -1; + } + break; + case EVP_PKEY_STATE_UNKNOWN: + case EVP_PKEY_STATE_LEGACY: + if (ctx->pmeth == NULL) { + ERR_raise(ERR_LIB_EVP, EVP_R_COMMAND_NOT_SUPPORTED); + return -2; + } + if (ctx->pmeth->pkey_id != keytype) { + ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_OPERATION); + return -1; + } + break; + } + } + if (optype != -1 && (ctx->operation & optype) == 0) { ERR_raise(ERR_LIB_EVP, EVP_R_INVALID_OPERATION); return -1; } diff --git a/include/crypto/evp.h b/include/crypto/evp.h index 8e86bb94df..7b3c4bfd2f 100644 --- a/include/crypto/evp.h +++ b/include/crypto/evp.h @@ -827,6 +827,7 @@ int evp_pkey_ctx_get_params_strict(EVP_PKEY_CTX *ctx, OSSL_PARAM *params); EVP_MD_CTX *evp_md_ctx_new_ex(EVP_PKEY *pkey, const ASN1_OCTET_STRING *id, OSSL_LIB_CTX *libctx, const char *propq); int evp_pkey_name2type(const char *name); +const char *evp_pkey_type2name(int type); int evp_pkey_ctx_set1_id_prov(EVP_PKEY_CTX *ctx, const void *id, int len); int evp_pkey_ctx_get1_id_prov(EVP_PKEY_CTX *ctx, void *id);