From 550f974a09942ace37cf3cf14021ea5e51e6dd11 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Tue, 3 Sep 2019 18:11:49 +0200 Subject: [PATCH] New function EVP_CIPHER_free() This function re-implements EVP_CIPHER_meth_free(), but has a name that isn't encumbered by legacy EVP_CIPHER construction functionality. We also refactor most of EVP_CIPHER_meth_new() into an internal evp_cipher_new() that's used when creating fetched methods. EVP_CIPHER_meth_new() and EVP_CIPHER_meth_free() are rewritten in terms of evp_cipher_new() and EVP_CIPHER_free(). This means that at any time, we can deprecate all the EVP_CIPHER_meth_ functions with no harmful consequence. Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/9758) --- apps/list.c | 2 +- crypto/evp/cmeth_lib.c | 41 +++++++----------------- crypto/evp/evp_enc.c | 53 +++++++++++++++++++++++++++---- crypto/evp/evp_locl.h | 1 + crypto/rand/drbg_ctr.c | 4 +-- doc/man3/EVP_CIPHER_meth_new.pod | 10 ++---- doc/man3/EVP_EncryptInit.pod | 22 +++++++++++-- include/openssl/evp.h | 3 +- providers/common/macs/cmac_prov.c | 4 +-- providers/common/macs/gmac_prov.c | 4 +-- test/evp_extra_test.c | 10 +++--- util/libcrypto.num | 1 + 12 files changed, 97 insertions(+), 58 deletions(-) diff --git a/apps/list.c b/apps/list.c index f3ef233c84..3e34228d1e 100644 --- a/apps/list.c +++ b/apps/list.c @@ -80,7 +80,7 @@ static void list_ciphers(void) EVP_CIPHER_CTX_settable_params(c), 4); } } - sk_EVP_CIPHER_pop_free(ciphers, EVP_CIPHER_meth_free); + sk_EVP_CIPHER_pop_free(ciphers, EVP_CIPHER_free); } static void list_md_fn(const EVP_MD *m, diff --git a/crypto/evp/cmeth_lib.c b/crypto/evp/cmeth_lib.c index 51c9b6ece2..34e85f6366 100644 --- a/crypto/evp/cmeth_lib.c +++ b/crypto/evp/cmeth_lib.c @@ -16,28 +16,29 @@ EVP_CIPHER *EVP_CIPHER_meth_new(int cipher_type, int block_size, int key_len) { - EVP_CIPHER *cipher = OPENSSL_zalloc(sizeof(EVP_CIPHER)); + EVP_CIPHER *cipher = evp_cipher_new(); if (cipher != NULL) { cipher->nid = cipher_type; cipher->block_size = block_size; cipher->key_len = key_len; - cipher->lock = CRYPTO_THREAD_lock_new(); - if (cipher->lock == NULL) { - OPENSSL_free(cipher); - return NULL; - } - cipher->refcnt = 1; } return cipher; } EVP_CIPHER *EVP_CIPHER_meth_dup(const EVP_CIPHER *cipher) { - EVP_CIPHER *to = EVP_CIPHER_meth_new(cipher->nid, cipher->block_size, - cipher->key_len); + EVP_CIPHER *to = NULL; - if (to != NULL) { + /* + * Non-legacy EVP_CIPHERs can't be duplicated like this. + * Use EVP_CIPHER_up_ref() instead. + */ + if (cipher->prov != NULL) + return NULL; + + if ((to = EVP_CIPHER_meth_new(cipher->nid, cipher->block_size, + cipher->key_len)) == NULL) { CRYPTO_RWLOCK *lock = to->lock; memcpy(to, cipher, sizeof(*to)); @@ -48,25 +49,7 @@ EVP_CIPHER *EVP_CIPHER_meth_dup(const EVP_CIPHER *cipher) void EVP_CIPHER_meth_free(EVP_CIPHER *cipher) { - if (cipher != NULL) { - int i; - - CRYPTO_DOWN_REF(&cipher->refcnt, &i, cipher->lock); - if (i > 0) - return; - ossl_provider_free(cipher->prov); - OPENSSL_free(cipher->name); - CRYPTO_THREAD_lock_free(cipher->lock); - OPENSSL_free(cipher); - } -} - -int EVP_CIPHER_up_ref(EVP_CIPHER *cipher) -{ - int ref = 0; - - CRYPTO_UP_REF(&cipher->refcnt, &ref, cipher->lock); - return 1; + EVP_CIPHER_free(cipher); } int EVP_CIPHER_meth_set_iv_length(EVP_CIPHER *cipher, int iv_len) diff --git a/crypto/evp/evp_enc.c b/crypto/evp/evp_enc.c index 142ffecfed..96dc83b2a0 100644 --- a/crypto/evp/evp_enc.c +++ b/crypto/evp/evp_enc.c @@ -35,7 +35,7 @@ int EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *ctx) ctx->provctx = NULL; } if (ctx->fetched_cipher != NULL) - EVP_CIPHER_meth_free(ctx->fetched_cipher); + EVP_CIPHER_free(ctx->fetched_cipher); memset(ctx, 0, sizeof(*ctx)); return 1; @@ -133,7 +133,7 @@ int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, || impl != NULL) { if (ctx->cipher == ctx->fetched_cipher) ctx->cipher = NULL; - EVP_CIPHER_meth_free(ctx->fetched_cipher); + EVP_CIPHER_free(ctx->fetched_cipher); ctx->fetched_cipher = NULL; goto legacy; } @@ -274,7 +274,7 @@ int EVP_CipherInit_ex(EVP_CIPHER_CTX *ctx, const EVP_CIPHER *cipher, return 0; } cipher = provciph; - EVP_CIPHER_meth_free(ctx->fetched_cipher); + EVP_CIPHER_free(ctx->fetched_cipher); ctx->fetched_cipher = provciph; #endif } @@ -1244,6 +1244,21 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in) return 1; } +EVP_CIPHER *evp_cipher_new(void) +{ + EVP_CIPHER *cipher = OPENSSL_zalloc(sizeof(EVP_CIPHER)); + + if (cipher != NULL) { + cipher->lock = CRYPTO_THREAD_lock_new(); + if (cipher->lock == NULL) { + OPENSSL_free(cipher); + return NULL; + } + cipher->refcnt = 1; + } + return cipher; +} + static void *evp_cipher_from_dispatch(const char *name, const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov, @@ -1252,9 +1267,9 @@ static void *evp_cipher_from_dispatch(const char *name, EVP_CIPHER *cipher = NULL; int fnciphcnt = 0, fnctxcnt = 0; - if ((cipher = EVP_CIPHER_meth_new(0, 0, 0)) == NULL + if ((cipher = evp_cipher_new()) == NULL || (cipher->name = OPENSSL_strdup(name)) == NULL) { - EVP_CIPHER_meth_free(cipher); + EVP_CIPHER_free(cipher); EVPerr(0, ERR_R_MALLOC_FAILURE); return NULL; } @@ -1361,7 +1376,7 @@ static void *evp_cipher_from_dispatch(const char *name, * functions, or a single "cipher" function. In all cases we need both * the "newctx" and "freectx" functions. */ - EVP_CIPHER_meth_free(cipher); + EVP_CIPHER_free(cipher); EVPerr(EVP_F_EVP_CIPHER_FROM_DISPATCH, EVP_R_INVALID_PROVIDER_FUNCTIONS); return NULL; } @@ -1379,7 +1394,7 @@ static int evp_cipher_up_ref(void *cipher) static void evp_cipher_free(void *cipher) { - EVP_CIPHER_meth_free(cipher); + EVP_CIPHER_free(cipher); } EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm, @@ -1393,6 +1408,30 @@ EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm, return cipher; } +int EVP_CIPHER_up_ref(EVP_CIPHER *cipher) +{ + int ref = 0; + + CRYPTO_UP_REF(&cipher->refcnt, &ref, cipher->lock); + return 1; +} + +void EVP_CIPHER_free(EVP_CIPHER *cipher) +{ + int i; + + if (cipher == NULL) + return; + + CRYPTO_DOWN_REF(&cipher->refcnt, &i, cipher->lock); + if (i > 0) + return; + ossl_provider_free(cipher->prov); + OPENSSL_free(cipher->name); + CRYPTO_THREAD_lock_free(cipher->lock); + OPENSSL_free(cipher); +} + void EVP_CIPHER_do_all_ex(OPENSSL_CTX *libctx, void (*fn)(EVP_CIPHER *mac, void *arg), void *arg) diff --git a/crypto/evp/evp_locl.h b/crypto/evp/evp_locl.h index 4342956531..87f54bdc5f 100644 --- a/crypto/evp/evp_locl.h +++ b/crypto/evp/evp_locl.h @@ -158,6 +158,7 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id, /* Internal structure constructors for fetched methods */ EVP_MD *evp_md_new(void); +EVP_CIPHER *evp_cipher_new(void); /* Helper functions to avoid duplicating code */ diff --git a/crypto/rand/drbg_ctr.c b/crypto/rand/drbg_ctr.c index 23e504bfac..28db4eed7f 100644 --- a/crypto/rand/drbg_ctr.c +++ b/crypto/rand/drbg_ctr.c @@ -354,7 +354,7 @@ static int drbg_ctr_uninstantiate(RAND_DRBG *drbg) { EVP_CIPHER_CTX_free(drbg->data.ctr.ctx); EVP_CIPHER_CTX_free(drbg->data.ctr.ctx_df); - EVP_CIPHER_meth_free(drbg->data.ctr.cipher); + EVP_CIPHER_free(drbg->data.ctr.cipher); OPENSSL_cleanse(&drbg->data.ctr, sizeof(drbg->data.ctr)); return 1; } @@ -392,7 +392,7 @@ int drbg_ctr_init(RAND_DRBG *drbg) if (cipher == NULL) return 0; - EVP_CIPHER_meth_free(ctr->cipher); + EVP_CIPHER_free(ctr->cipher); ctr->cipher = cipher; drbg->meth = &drbg_ctr_meth; diff --git a/doc/man3/EVP_CIPHER_meth_new.pod b/doc/man3/EVP_CIPHER_meth_new.pod index 3d4da9c04e..8a6a4b99de 100644 --- a/doc/man3/EVP_CIPHER_meth_new.pod +++ b/doc/man3/EVP_CIPHER_meth_new.pod @@ -10,7 +10,7 @@ EVP_CIPHER_meth_set_set_asn1_params, EVP_CIPHER_meth_set_get_asn1_params, EVP_CIPHER_meth_set_ctrl, EVP_CIPHER_meth_get_init, EVP_CIPHER_meth_get_do_cipher, EVP_CIPHER_meth_get_cleanup, EVP_CIPHER_meth_get_set_asn1_params, EVP_CIPHER_meth_get_get_asn1_params, -EVP_CIPHER_meth_get_ctrl, EVP_CIPHER_up_ref +EVP_CIPHER_meth_get_ctrl - Routines to build up EVP_CIPHER methods =head1 SYNOPSIS @@ -63,8 +63,6 @@ EVP_CIPHER_meth_get_ctrl, EVP_CIPHER_up_ref int type, int arg, void *ptr); - int EVP_CIPHER_up_ref(EVP_CIPHER *cipher); - =head1 DESCRIPTION The B type is a structure for symmetric cipher method @@ -226,8 +224,6 @@ EVP_CIPHER_meth_get_get_asn1_params() and EVP_CIPHER_meth_get_ctrl() are all used to retrieve the method data given with the EVP_CIPHER_meth_set_*() functions above. -EVP_CIPHER_up_ref() increments the reference count for an EVP_CIPHER structure. - =head1 RETURN VALUES EVP_CIPHER_meth_new() and EVP_CIPHER_meth_dup() return a pointer to a @@ -236,8 +232,6 @@ All EVP_CIPHER_meth_set_*() functions return 1. All EVP_CIPHER_meth_get_*() functions return pointers to their respective B function. -EVP_CIPHER_up_ref() returns 1 for success or 0 otherwise. - =head1 SEE ALSO L @@ -245,6 +239,8 @@ L =head1 HISTORY The functions described here were added in OpenSSL 1.1.0. +The B structure created with these functions became reference +counted in OpenSSL 3.0. =head1 COPYRIGHT diff --git a/doc/man3/EVP_EncryptInit.pod b/doc/man3/EVP_EncryptInit.pod index 7f69a23dd7..11d0250a0d 100644 --- a/doc/man3/EVP_EncryptInit.pod +++ b/doc/man3/EVP_EncryptInit.pod @@ -3,6 +3,8 @@ =head1 NAME EVP_CIPHER_fetch, +EVP_CIPHER_up_ref, +EVP_CIPHER_free, EVP_CIPHER_CTX_new, EVP_CIPHER_CTX_reset, EVP_CIPHER_CTX_free, @@ -67,6 +69,8 @@ EVP_CIPHER_do_all_ex EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm, const char *properties); + int EVP_CIPHER_up_ref(EVP_CIPHER *cipher); + void EVP_CIPHER_free(EVP_CIPHER *cipher); EVP_CIPHER_CTX *EVP_CIPHER_CTX_new(void); int EVP_CIPHER_CTX_reset(EVP_CIPHER_CTX *ctx); void EVP_CIPHER_CTX_free(EVP_CIPHER_CTX *ctx); @@ -150,13 +154,21 @@ EVP_CIPHER_do_all_ex The EVP cipher routines are a high level interface to certain symmetric ciphers. +The B type is a structure for cipher method implementation. + EVP_CIPHER_fetch() fetches the cipher implementation for the given B from any provider offering it, within the criteria given by the B. See L for further information. -The returned value must eventually be freed with -L. +The returned value must eventually be freed with EVP_CIPHER_free(). + +EVP_CIPHER_up_ref() increments the reference count for an B +structure. + +EVP_CIPHER_free() decrements the reference count for the B +structure. +If the reference count drops to 0 then the structure is freed. EVP_CIPHER_CTX_new() creates a cipher context. @@ -351,6 +363,8 @@ and the given I as argument. EVP_CIPHER_fetch() returns a pointer to a B for success and B for failure. +EVP_CIPHER_up_ref() returns 1 for success or 0 otherwise. + EVP_CIPHER_CTX_new() returns a pointer to a newly created B for success and B for failure. @@ -757,6 +771,10 @@ EVP_CIPHER_CTX_reset() appeared and EVP_CIPHER_CTX_cleanup() disappeared. EVP_CIPHER_CTX_init() remains as an alias for EVP_CIPHER_CTX_reset(). +The EVP_CIPHER_fetch(), EVP_CIPHER_free(), EVP_CIPHER_up_ref(), +EVP_CIPHER_CTX_set_params() and EVP_CIPHER_CTX_get_params() functions +were added in 3.0. + =head1 COPYRIGHT Copyright 2000-2018 The OpenSSL Project Authors. All Rights Reserved. diff --git a/include/openssl/evp.h b/include/openssl/evp.h index 6c59aae701..a0733b9697 100644 --- a/include/openssl/evp.h +++ b/include/openssl/evp.h @@ -191,7 +191,6 @@ int (*EVP_MD_meth_get_ctrl(const EVP_MD *md))(EVP_MD_CTX *ctx, int cmd, EVP_CIPHER *EVP_CIPHER_meth_new(int cipher_type, int block_size, int key_len); EVP_CIPHER *EVP_CIPHER_meth_dup(const EVP_CIPHER *cipher); void EVP_CIPHER_meth_free(EVP_CIPHER *cipher); -int EVP_CIPHER_up_ref(EVP_CIPHER *cipher); int EVP_CIPHER_meth_set_iv_length(EVP_CIPHER *cipher, int iv_len); int EVP_CIPHER_meth_set_flags(EVP_CIPHER *cipher, unsigned long flags); @@ -485,6 +484,8 @@ unsigned long EVP_CIPHER_flags(const EVP_CIPHER *cipher); int EVP_CIPHER_mode(const EVP_CIPHER *cipher); EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm, const char *properties); +int EVP_CIPHER_up_ref(EVP_CIPHER *cipher); +void EVP_CIPHER_free(EVP_CIPHER *cipher); const EVP_CIPHER *EVP_CIPHER_CTX_cipher(const EVP_CIPHER_CTX *ctx); int EVP_CIPHER_CTX_encrypting(const EVP_CIPHER_CTX *ctx); diff --git a/providers/common/macs/cmac_prov.c b/providers/common/macs/cmac_prov.c index 4dcdea6ebe..7c15bc77b0 100644 --- a/providers/common/macs/cmac_prov.c +++ b/providers/common/macs/cmac_prov.c @@ -76,7 +76,7 @@ static void cmac_free(void *vmacctx) if (macctx != NULL) { CMAC_CTX_free(macctx->ctx); - EVP_CIPHER_meth_free(macctx->alloc_cipher); + EVP_CIPHER_free(macctx->alloc_cipher); OPENSSL_free(macctx); } } @@ -208,7 +208,7 @@ static int cmac_set_ctx_params(void *vmacctx, const OSSL_PARAM params[]) propquery = p->data; } - EVP_CIPHER_meth_free(macctx->alloc_cipher); + EVP_CIPHER_free(macctx->alloc_cipher); macctx->tmpcipher = macctx->alloc_cipher = EVP_CIPHER_fetch(PROV_LIBRARY_CONTEXT_OF(macctx->provctx), diff --git a/providers/common/macs/gmac_prov.c b/providers/common/macs/gmac_prov.c index abd5baa106..22c8c95550 100644 --- a/providers/common/macs/gmac_prov.c +++ b/providers/common/macs/gmac_prov.c @@ -64,7 +64,7 @@ static void gmac_free(void *vmacctx) if (macctx != NULL) { EVP_CIPHER_CTX_free(macctx->ctx); - EVP_CIPHER_meth_free(macctx->alloc_cipher); + EVP_CIPHER_free(macctx->alloc_cipher); OPENSSL_free(macctx); } } @@ -222,7 +222,7 @@ static int gmac_set_ctx_params(void *vmacctx, const OSSL_PARAM params[]) propquery = p->data; } - EVP_CIPHER_meth_free(macctx->alloc_cipher); + EVP_CIPHER_free(macctx->alloc_cipher); macctx->cipher = macctx->alloc_cipher = NULL; macctx->cipher = macctx->alloc_cipher = diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c index eba7f64bab..1ad62d2f79 100644 --- a/test/evp_extra_test.c +++ b/test/evp_extra_test.c @@ -1317,8 +1317,8 @@ static int test_EVP_CIPHER_fetch(int tst) if (!TEST_true(EVP_CIPHER_up_ref(cipher))) goto err; /* Ref count should now be 2. Release both */ - EVP_CIPHER_meth_free(cipher); - EVP_CIPHER_meth_free(cipher); + EVP_CIPHER_free(cipher); + EVP_CIPHER_free(cipher); cipher = NULL; /* @@ -1336,7 +1336,7 @@ static int test_EVP_CIPHER_fetch(int tst) goto err; } - EVP_CIPHER_meth_free(cipher); + EVP_CIPHER_free(cipher); cipher = NULL; /* @@ -1355,7 +1355,7 @@ static int test_EVP_CIPHER_fetch(int tst) goto err; } - EVP_CIPHER_meth_free(cipher); + EVP_CIPHER_free(cipher); cipher = NULL; /* @@ -1381,7 +1381,7 @@ static int test_EVP_CIPHER_fetch(int tst) ret = 1; err: - EVP_CIPHER_meth_free(cipher); + EVP_CIPHER_free(cipher); OSSL_PROVIDER_unload(defltprov); OSSL_PROVIDER_unload(fipsprov); /* Not normally needed, but we would like to test that diff --git a/util/libcrypto.num b/util/libcrypto.num index 3ace3ad1b9..9f7b0fd7c6 100644 --- a/util/libcrypto.num +++ b/util/libcrypto.num @@ -4733,3 +4733,4 @@ EVP_MAC_gettable_params 4842 3_0_0 EXIST::FUNCTION: EVP_MAC_provider 4843 3_0_0 EXIST::FUNCTION: EVP_MAC_do_all_ex 4844 3_0_0 EXIST::FUNCTION: EVP_MD_free 4845 3_0_0 EXIST::FUNCTION: +EVP_CIPHER_free 4846 3_0_0 EXIST::FUNCTION: -- 2.25.1