From 2ef9a7ac5eb93c3f5460695c526968faf025b730 Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Mon, 17 Aug 2020 15:14:14 +0100 Subject: [PATCH] Improve code reuse in the provider MAC bridge We reuse concepts such as PROV_CIPHER, and make use of some common code in provider_util.c Reviewed-by: Paul Dale (Merged from https://github.com/openssl/openssl/pull/12637) --- crypto/evp/p_lib.c | 5 +- providers/common/include/prov/provider_util.h | 15 +++ providers/common/provider_util.c | 107 +++++++++++------- .../include/prov/macsignature.h | 5 +- providers/implementations/keymgmt/build.info | 4 +- .../keymgmt/mac_legacy_kmgmt.c | 96 ++++++++-------- .../implementations/signature/build.info | 4 +- .../implementations/signature/mac_legacy.c | 35 +++--- 8 files changed, 158 insertions(+), 113 deletions(-) diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c index a742f4c092..16c674d920 100644 --- a/crypto/evp/p_lib.c +++ b/crypto/evp/p_lib.c @@ -595,7 +595,7 @@ static EVP_PKEY *new_cmac_key_int(const unsigned char *priv, size_t len, # ifndef OPENSSL_NO_ENGINE const char *engine_id = e != NULL ? ENGINE_get_id(e) : NULL; # endif - OSSL_PARAM params[4], *p = params; + OSSL_PARAM params[5], *p = params; EVP_PKEY *pkey = NULL; EVP_PKEY_CTX *ctx; @@ -622,6 +622,9 @@ static EVP_PKEY *new_cmac_key_int(const unsigned char *priv, size_t len, (void *)priv, len); *p++ = OSSL_PARAM_construct_utf8_string(OSSL_PKEY_PARAM_CIPHER, (char *)cipher_name, 0); + if (propq != NULL) + *p++ = OSSL_PARAM_construct_utf8_string(OSSL_PKEY_PARAM_PROPERTIES, + (char *)propq, 0); # ifndef OPENSSL_NO_ENGINE if (engine_id != NULL) *p++ = OSSL_PARAM_construct_utf8_string(OSSL_PKEY_PARAM_ENGINE, diff --git a/providers/common/include/prov/provider_util.h b/providers/common/include/prov/provider_util.h index 9b5b983299..d964f832ad 100644 --- a/providers/common/include/prov/provider_util.h +++ b/providers/common/include/prov/provider_util.h @@ -78,6 +78,21 @@ int ossl_prov_digest_copy(PROV_DIGEST *dst, const PROV_DIGEST *src); const EVP_MD *ossl_prov_digest_md(const PROV_DIGEST *pd); ENGINE *ossl_prov_digest_engine(const PROV_DIGEST *pd); + +/* + * Set the various parameters on an EVP_MAC_CTX from the supplied arguments. + * If any of the supplied ciphername/mdname etc are NULL then the values + * from the supplied params (if non NULL) are used instead. + */ +int ossl_prov_set_macctx(EVP_MAC_CTX *macctx, + const OSSL_PARAM params[], + const char *ciphername, + const char *mdname, + const char *engine, + const char *properties, + const unsigned char *key, + size_t keylen); + /* MAC functions */ /* * Load an EVP_MAC_CTX* from the specified parameters with the specified diff --git a/providers/common/provider_util.c b/providers/common/provider_util.c index 1b02d70b78..1bd514221f 100644 --- a/providers/common/provider_util.c +++ b/providers/common/provider_util.c @@ -164,6 +164,72 @@ ENGINE *ossl_prov_digest_engine(const PROV_DIGEST *pd) return pd->engine; } +int ossl_prov_set_macctx(EVP_MAC_CTX *macctx, + const OSSL_PARAM params[], + const char *ciphername, + const char *mdname, + const char *engine, + const char *properties, + const unsigned char *key, + size_t keylen) +{ + const OSSL_PARAM *p; + OSSL_PARAM mac_params[6], *mp = mac_params; + + if (params != NULL) { + if (mdname == NULL) { + if ((p = OSSL_PARAM_locate_const(params, + OSSL_ALG_PARAM_DIGEST)) != NULL) { + if (p->data_type != OSSL_PARAM_UTF8_STRING) + return 0; + mdname = p->data; + } + } + if (ciphername == NULL) { + if ((p = OSSL_PARAM_locate_const(params, + OSSL_ALG_PARAM_CIPHER)) != NULL) { + if (p->data_type != OSSL_PARAM_UTF8_STRING) + return 0; + ciphername = p->data; + } + } + if (engine == NULL) { + if ((p = OSSL_PARAM_locate_const(params, OSSL_ALG_PARAM_ENGINE)) + != NULL) { + if (p->data_type != OSSL_PARAM_UTF8_STRING) + return 0; + engine = p->data; + } + } + } + + if (mdname != NULL) + *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, + (char *)mdname, 0); + if (ciphername != NULL) + *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_CIPHER, + (char *)ciphername, 0); + if (properties != NULL) + *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_PROPERTIES, + (char *)properties, 0); + +#if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE) + if (engine != NULL) + *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_ALG_PARAM_ENGINE, + (char *) engine, 0); +#endif + + if (key != NULL) + *mp++ = OSSL_PARAM_construct_octet_string(OSSL_MAC_PARAM_KEY, + (unsigned char *)key, + keylen); + + *mp = OSSL_PARAM_construct_end(); + + return EVP_MAC_CTX_set_params(macctx, mac_params); + +} + int ossl_prov_macctx_load_from_params(EVP_MAC_CTX **macctx, const OSSL_PARAM params[], const char *macname, @@ -172,7 +238,6 @@ int ossl_prov_macctx_load_from_params(EVP_MAC_CTX **macctx, OPENSSL_CTX *libctx) { const OSSL_PARAM *p; - OSSL_PARAM mac_params[5], *mp = mac_params; const char *properties = NULL; if (macname == NULL @@ -207,44 +272,8 @@ int ossl_prov_macctx_load_from_params(EVP_MAC_CTX **macctx, if (*macctx == NULL) return 1; - if (mdname == NULL) { - if ((p = OSSL_PARAM_locate_const(params, - OSSL_ALG_PARAM_DIGEST)) != NULL) { - if (p->data_type != OSSL_PARAM_UTF8_STRING) - return 0; - mdname = p->data; - } - } - if (ciphername == NULL) { - if ((p = OSSL_PARAM_locate_const(params, - OSSL_ALG_PARAM_CIPHER)) != NULL) { - if (p->data_type != OSSL_PARAM_UTF8_STRING) - return 0; - ciphername = p->data; - } - } - - if (mdname != NULL) - *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, - (char *)mdname, 0); - if (ciphername != NULL) - *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_CIPHER, - (char *)ciphername, 0); - if (properties != NULL) - *mp++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_PROPERTIES, - (char *)properties, 0); - -#if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE) - if ((p = OSSL_PARAM_locate_const(params, "engine")) != NULL) { - if (p->data_type != OSSL_PARAM_UTF8_STRING) - return 0; - *mp++ = OSSL_PARAM_construct_utf8_string("engine", - p->data, p->data_size); - } -#endif - *mp = OSSL_PARAM_construct_end(); - - if (EVP_MAC_CTX_set_params(*macctx, mac_params)) + if (ossl_prov_set_macctx(*macctx, params, ciphername, mdname, NULL, + properties, NULL, 0)) return 1; EVP_MAC_CTX_free(*macctx); diff --git a/providers/implementations/include/prov/macsignature.h b/providers/implementations/include/prov/macsignature.h index 57adf7d7ba..bec5c46fbe 100644 --- a/providers/implementations/include/prov/macsignature.h +++ b/providers/implementations/include/prov/macsignature.h @@ -10,6 +10,7 @@ #include #include #include "internal/refcount.h" +#include "prov/provider_util.h" struct mac_key_st { CRYPTO_RWLOCK *lock; @@ -17,8 +18,8 @@ struct mac_key_st { CRYPTO_REF_COUNT refcnt; unsigned char *priv_key; size_t priv_key_len; - char *cipher_name; - char *engine_name; + PROV_CIPHER cipher; + char *properties; int cmac; }; diff --git a/providers/implementations/keymgmt/build.info b/providers/implementations/keymgmt/build.info index a8ad5ae4f9..978cd706ae 100644 --- a/providers/implementations/keymgmt/build.info +++ b/providers/implementations/keymgmt/build.info @@ -6,7 +6,6 @@ $DSA_GOAL=../../libimplementations.a $EC_GOAL=../../libimplementations.a $ECX_GOAL=../../libimplementations.a $KDF_GOAL=../../libimplementations.a -$MAC_GOAL=../../libimplementations.a IF[{- !$disabled{dh} -}] SOURCE[$DH_GOAL]=dh_kmgmt.c @@ -38,4 +37,5 @@ SOURCE[../../libnonfips.a]=rsa_kmgmt.c SOURCE[$KDF_GOAL]=kdf_legacy_kmgmt.c -SOURCE[$MAC_GOAL]=mac_legacy_kmgmt.c \ No newline at end of file +SOURCE[../../libfips.a]=mac_legacy_kmgmt.c +SOURCE[../../libnonfips.a]=mac_legacy_kmgmt.c diff --git a/providers/implementations/keymgmt/mac_legacy_kmgmt.c b/providers/implementations/keymgmt/mac_legacy_kmgmt.c index fadf379f67..779240e036 100644 --- a/providers/implementations/keymgmt/mac_legacy_kmgmt.c +++ b/providers/implementations/keymgmt/mac_legacy_kmgmt.c @@ -7,6 +7,9 @@ * https://www.openssl.org/source/license.html */ +/* We need to use some engine deprecated APIs */ +#define OPENSSL_SUPPRESS_DEPRECATED + #include #include #include @@ -42,7 +45,7 @@ struct mac_gen_ctx { int selection; unsigned char *priv_key; size_t priv_key_len; - char *cipher_name; + PROV_CIPHER cipher; }; MAC_KEY *mac_key_new(OPENSSL_CTX *libctx, int cmac) @@ -76,8 +79,8 @@ void mac_key_free(MAC_KEY *mackey) return; OPENSSL_secure_clear_free(mackey->priv_key, mackey->priv_key_len); - OPENSSL_free(mackey->cipher_name); - OPENSSL_free(mackey->engine_name); + OPENSSL_free(mackey->properties); + ossl_prov_cipher_reset(&mackey->cipher); CRYPTO_THREAD_lock_free(mackey->lock); OPENSSL_free(mackey); } @@ -134,15 +137,16 @@ static int mac_match(const void *keydata1, const void *keydata2, int selection) if ((key1->priv_key == NULL && key2->priv_key != NULL) || (key1->priv_key != NULL && key2->priv_key == NULL) || key1->priv_key_len != key2->priv_key_len - || (key1->cipher_name == NULL && key2->cipher_name != NULL) - || (key1->cipher_name != NULL && key2->cipher_name == NULL)) + || (key1->cipher.cipher == NULL && key2->cipher.cipher != NULL) + || (key1->cipher.cipher != NULL && key2->cipher.cipher == NULL)) ok = 0; else ok = ok && (key1->priv_key == NULL /* implies key2->privkey == NULL */ || CRYPTO_memcmp(key1->priv_key, key2->priv_key, key1->priv_key_len) == 0); - if (key1->cipher_name != NULL) - ok = ok && (strcasecmp(key1->cipher_name, key2->cipher_name) == 0); + if (key1->cipher.cipher != NULL) + ok = ok && EVP_CIPHER_is_a(key1->cipher.cipher, + EVP_CIPHER_name(key2->cipher.cipher)); } return ok; } @@ -167,34 +171,27 @@ static int mac_key_fromdata(MAC_KEY *key, const OSSL_PARAM params[]) key->priv_key_len = p->data_size; } - if (key->cmac) { - p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_CIPHER); - if (p != NULL) { - if (p->data_type != OSSL_PARAM_UTF8_STRING) { - ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_INVALID_ARGUMENT); - return 0; - } - key->cipher_name = OPENSSL_strdup(p->data); - if (key->cipher_name == NULL) { - ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE); - return 0; - } + p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_PROPERTIES); + if (p != NULL) { + if (p->data_type != OSSL_PARAM_UTF8_STRING) { + ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_INVALID_ARGUMENT); + return 0; } - p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_ENGINE); - if (p != NULL) { - if (p->data_type != OSSL_PARAM_UTF8_STRING) { - ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_INVALID_ARGUMENT); - return 0; - } - key->engine_name = OPENSSL_strdup(p->data); - if (key->engine_name == NULL) { - ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE); - return 0; - } + OPENSSL_free(key->properties); + key->properties = OPENSSL_strdup(p->data); + if (key->properties == NULL) { + ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE); + return 0; } } - if (key->priv_key != NULL && (!key->cmac || key->cipher_name != NULL)) + if (key->cmac && !ossl_prov_cipher_load_from_params(&key->cipher, params, + key->libctx)) { + ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_INVALID_ARGUMENT); + return 0; + } + + if (key->priv_key != NULL) return 1; return 0; @@ -225,17 +222,19 @@ static int key_to_params(MAC_KEY *key, OSSL_PARAM_BLD *tmpl, key->priv_key, key->priv_key_len)) return 0; - if (key->cipher_name != NULL + if (key->cipher.cipher != NULL && !ossl_param_build_set_utf8_string(tmpl, params, OSSL_PKEY_PARAM_CIPHER, - key->cipher_name)) + EVP_CIPHER_name(key->cipher.cipher))) return 0; - if (key->engine_name != NULL +#if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE) + if (key->cipher.engine != NULL && !ossl_param_build_set_utf8_string(tmpl, params, OSSL_PKEY_PARAM_ENGINE, - key->engine_name)) + ENGINE_get_id(key->cipher.engine))) return 0; +#endif return 1; } @@ -272,6 +271,7 @@ err: static const OSSL_PARAM mac_key_types[] = { OSSL_PARAM_octet_string(OSSL_PKEY_PARAM_PRIV_KEY, NULL, 0), + OSSL_PARAM_utf8_string(OSSL_PKEY_PARAM_PROPERTIES, NULL, 0), OSSL_PARAM_END }; static const OSSL_PARAM *mac_imexport_types(int selection) @@ -285,6 +285,7 @@ static const OSSL_PARAM cmac_key_types[] = { OSSL_PARAM_octet_string(OSSL_PKEY_PARAM_PRIV_KEY, NULL, 0), OSSL_PARAM_utf8_string(OSSL_PKEY_PARAM_CIPHER, NULL, 0), OSSL_PARAM_utf8_string(OSSL_PKEY_PARAM_ENGINE, NULL, 0), + OSSL_PARAM_utf8_string(OSSL_PKEY_PARAM_PROPERTIES, NULL, 0), OSSL_PARAM_END }; static const OSSL_PARAM *cmac_imexport_types(int selection) @@ -385,22 +386,14 @@ static int mac_gen_set_params(void *genctx, const OSSL_PARAM params[]) static int cmac_gen_set_params(void *genctx, const OSSL_PARAM params[]) { struct mac_gen_ctx *gctx = genctx; - const OSSL_PARAM *p; if (!mac_gen_set_params(genctx, params)) return 0; - p = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_CIPHER); - if (p != NULL) { - if (p->data_type != OSSL_PARAM_UTF8_STRING) { - ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_INVALID_ARGUMENT); - return 0; - } - gctx->cipher_name = OPENSSL_strdup(p->data); - if (gctx->cipher_name == NULL) { - ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE); - return 0; - } + if (!ossl_prov_cipher_load_from_params(&gctx->cipher, params, + gctx->libctx)) { + ERR_raise(ERR_LIB_PROV, ERR_R_PASSED_INVALID_ARGUMENT); + return 0; } return 1; @@ -454,12 +447,15 @@ static void *mac_gen(void *genctx, OSSL_CALLBACK *cb, void *cbarg) * previously set in the gctx. Hopefully at some point in the future all * of this can be removed and we will only support the EVP_KDF APIs. */ + if (!ossl_prov_cipher_copy(&key->cipher, &gctx->cipher)) { + ERR_raise(ERR_LIB_PROV, ERR_R_INTERNAL_ERROR); + return NULL; + } + ossl_prov_cipher_reset(&gctx->cipher); key->priv_key = gctx->priv_key; key->priv_key_len = gctx->priv_key_len; - key->cipher_name = gctx->cipher_name; gctx->priv_key_len = 0; gctx->priv_key = NULL; - gctx->cipher_name = NULL; return key; } @@ -469,7 +465,7 @@ static void mac_gen_cleanup(void *genctx) struct mac_gen_ctx *gctx = genctx; OPENSSL_secure_clear_free(gctx->priv_key, gctx->priv_key_len); - OPENSSL_free(gctx->cipher_name); + ossl_prov_cipher_reset(&gctx->cipher); OPENSSL_free(gctx); } diff --git a/providers/implementations/signature/build.info b/providers/implementations/signature/build.info index 087da2f54f..5a813083b2 100644 --- a/providers/implementations/signature/build.info +++ b/providers/implementations/signature/build.info @@ -3,7 +3,6 @@ $DSA_GOAL=../../libimplementations.a $EC_GOAL=../../libimplementations.a -$MAC_GOAL=../../libimplementations.a IF[{- !$disabled{dsa} -}] SOURCE[$DSA_GOAL]=dsa.c @@ -22,4 +21,5 @@ DEPEND[rsa.o]=../../common/include/prov/der_rsa.h DEPEND[dsa.o]=../../common/include/prov/der_dsa.h DEPEND[ecdsa.o]=../../common/include/prov/der_ec.h -SOURCE[$MAC_GOAL]=mac_legacy.c \ No newline at end of file +SOURCE[../../libfips.a]=mac_legacy.c +SOURCE[../../libnonfips.a]=mac_legacy.c diff --git a/providers/implementations/signature/mac_legacy.c b/providers/implementations/signature/mac_legacy.c index eaf2c7566b..76ed3b5fdd 100644 --- a/providers/implementations/signature/mac_legacy.c +++ b/providers/implementations/signature/mac_legacy.c @@ -7,6 +7,9 @@ * https://www.openssl.org/source/license.html */ +/* We need to use some engine deprecated APIs */ +#define OPENSSL_SUPPRESS_DEPRECATED + #include #include #include @@ -77,7 +80,7 @@ MAC_NEWCTX(cmac, "CMAC") static int mac_digest_sign_init(void *vpmacctx, const char *mdname, void *vkey) { PROV_MAC_CTX *pmacctx = (PROV_MAC_CTX *)vpmacctx; - OSSL_PARAM params[5], *p = params; + const char *ciphername = NULL, *engine = NULL; if (pmacctx == NULL || vkey == NULL || !mac_key_up_ref(vkey)) return 0; @@ -86,22 +89,20 @@ static int mac_digest_sign_init(void *vpmacctx, const char *mdname, void *vkey) mac_key_free(pmacctx->key); pmacctx->key = vkey; - /* Read only so cast away of const is safe */ - if (mdname != NULL) - *p++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_DIGEST, - (char *)mdname, 0); - if (pmacctx->key->cipher_name != NULL) - *p++ = OSSL_PARAM_construct_utf8_string(OSSL_MAC_PARAM_CIPHER, - pmacctx->key->cipher_name, 0); - if (pmacctx->key->engine_name != NULL) - *p++ = OSSL_PARAM_construct_utf8_string(OSSL_ALG_PARAM_ENGINE, - pmacctx->key->engine_name, 0); - *p++ = OSSL_PARAM_construct_octet_string(OSSL_MAC_PARAM_KEY, - pmacctx->key->priv_key, - pmacctx->key->priv_key_len); - *p = OSSL_PARAM_construct_end(); - - if (!EVP_MAC_CTX_set_params(pmacctx->macctx, params)) + if (pmacctx->key->cipher.cipher != NULL) + ciphername = (char *)EVP_CIPHER_name(pmacctx->key->cipher.cipher); +#if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE) + if (pmacctx->key->cipher.engine != NULL) + engine = (char *)ENGINE_get_id(pmacctx->key->cipher.engine); +#endif + + if (!ossl_prov_set_macctx(pmacctx->macctx, NULL, + (char *)ciphername, + (char *)mdname, + (char *)engine, + pmacctx->key->properties, + pmacctx->key->priv_key, + pmacctx->key->priv_key_len)) return 0; if (!EVP_MAC_init(pmacctx->macctx)) -- 2.39.2