The branch OpenSSL_1_1_1-stable has been updated via 4c5c2a5efbc315d7926cafbd5a19044ee3e087fa (commit) via 93dd7ab35f6ccfb8bde7a7a6e38ea5817c5b54e2 (commit) from 5e7098e11581b6b3a4083a1c17889ed817e8ac22 (commit)
- Log ----------------------------------------------------------------- commit 4c5c2a5efbc315d7926cafbd5a19044ee3e087fa Author: Matt Caswell <m...@openssl.org> Date: Wed Dec 29 16:39:11 2021 +0000 Add a test for a custom digest created via EVP_MD_meth_new() We check that the init and cleanup functions for the custom method are called as expected. Based on an original reproducer by Dmitry Belyavsky from issue #17149. Reviewed-by: Paul Dale <pa...@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlin...@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/17472) commit 93dd7ab35f6ccfb8bde7a7a6e38ea5817c5b54e2 Author: Matt Caswell <m...@openssl.org> Date: Fri Dec 10 17:17:27 2021 +0000 Fix a leak in EVP_DigestInit_ex() If an EVP_MD_CTX is reused then memory allocated and stored in md_data can be leaked unless the EVP_MD's cleanup function is called. Fixes #17149 Reviewed-by: Paul Dale <pa...@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlin...@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/17472) ----------------------------------------------------------------------- Summary of changes: crypto/evp/digest.c | 32 +++++++++++++-------- test/evp_extra_test.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 11 deletions(-) diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c index d1bfa274ca..41ecdd8e5a 100644 --- a/crypto/evp/digest.c +++ b/crypto/evp/digest.c @@ -15,6 +15,22 @@ #include "crypto/evp.h" #include "evp_local.h" + +static void cleanup_old_md_data(EVP_MD_CTX *ctx, int force) +{ + if (ctx->digest != NULL) { + if (ctx->digest->cleanup != NULL + && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_CLEANED)) + ctx->digest->cleanup(ctx); + if (ctx->md_data != NULL && ctx->digest->ctx_size > 0 + && (!EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_REUSE) + || force)) { + OPENSSL_clear_free(ctx->md_data, ctx->digest->ctx_size); + ctx->md_data = NULL; + } + } +} + /* This call frees resources associated with the context */ int EVP_MD_CTX_reset(EVP_MD_CTX *ctx) { @@ -25,13 +41,8 @@ int EVP_MD_CTX_reset(EVP_MD_CTX *ctx) * Don't assume ctx->md_data was cleaned in EVP_Digest_Final, because * sometimes only copies of the context are ever finalised. */ - if (ctx->digest && ctx->digest->cleanup - && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_CLEANED)) - ctx->digest->cleanup(ctx); - if (ctx->digest && ctx->digest->ctx_size && ctx->md_data - && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_REUSE)) { - OPENSSL_clear_free(ctx->md_data, ctx->digest->ctx_size); - } + cleanup_old_md_data(ctx, 0); + /* * pctx should be freed by the user of EVP_MD_CTX * if EVP_MD_CTX_FLAG_KEEP_PKEY_CTX is set @@ -76,6 +87,7 @@ int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl) if (ctx->engine && ctx->digest && (type == NULL || (type->type == ctx->digest->type))) goto skip_to_init; + if (type) { /* * Ensure an ENGINE left lying around from last time is cleared (the @@ -119,10 +131,8 @@ int EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl) } #endif if (ctx->digest != type) { - if (ctx->digest && ctx->digest->ctx_size) { - OPENSSL_clear_free(ctx->md_data, ctx->digest->ctx_size); - ctx->md_data = NULL; - } + cleanup_old_md_data(ctx, 1); + ctx->digest = type; if (!(ctx->flags & EVP_MD_CTX_FLAG_NO_INIT) && type->ctx_size) { ctx->update = type->update; diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c index e4a0b180d7..538bff4659 100644 --- a/test/evp_extra_test.c +++ b/test/evp_extra_test.c @@ -1762,6 +1762,83 @@ static int test_EVP_PKEY_set1_DH(void) } #endif /* OPENSSL_NO_DH */ +typedef struct { + int data; +} custom_dgst_ctx; + +static int custom_md_init_called = 0; +static int custom_md_cleanup_called = 0; + +static int custom_md_init(EVP_MD_CTX *ctx) +{ + custom_dgst_ctx *p = EVP_MD_CTX_md_data(ctx); + + if (p == NULL) + return 0; + + custom_md_init_called++; + return 1; +} + +static int custom_md_cleanup(EVP_MD_CTX *ctx) +{ + custom_dgst_ctx *p = EVP_MD_CTX_md_data(ctx); + + if (p == NULL) + /* Nothing to do */ + return 1; + + custom_md_cleanup_called++; + return 1; +} + +static int test_custom_md_meth(void) +{ + EVP_MD_CTX *mdctx = NULL; + EVP_MD *tmp = NULL; + char mess[] = "Test Message\n"; + unsigned char md_value[EVP_MAX_MD_SIZE]; + unsigned int md_len; + int testresult = 0; + int nid; + + custom_md_init_called = custom_md_cleanup_called = 0; + + nid = OBJ_create("1.3.6.1.4.1.16604.998866.1", "custom-md", "custom-md"); + if (!TEST_int_ne(nid, NID_undef)) + goto err; + tmp = EVP_MD_meth_new(nid, NID_undef); + if (!TEST_ptr(tmp)) + goto err; + + if (!TEST_true(EVP_MD_meth_set_init(tmp, custom_md_init)) + || !TEST_true(EVP_MD_meth_set_cleanup(tmp, custom_md_cleanup)) + || !TEST_true(EVP_MD_meth_set_app_datasize(tmp, + sizeof(custom_dgst_ctx)))) + goto err; + + mdctx = EVP_MD_CTX_new(); + if (!TEST_ptr(mdctx) + /* + * Initing our custom md and then initing another md should + * result in the init and cleanup functions of the custom md + * from being called. + */ + || !TEST_true(EVP_DigestInit_ex(mdctx, tmp, NULL)) + || !TEST_true(EVP_DigestInit_ex(mdctx, EVP_sha256(), NULL)) + || !TEST_true(EVP_DigestUpdate(mdctx, mess, strlen(mess))) + || !TEST_true(EVP_DigestFinal_ex(mdctx, md_value, &md_len)) + || !TEST_int_eq(custom_md_init_called, 1) + || !TEST_int_eq(custom_md_cleanup_called, 1)) + goto err; + + testresult = 1; + err: + EVP_MD_CTX_free(mdctx); + EVP_MD_meth_free(tmp); + return testresult; +} + #if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) /* Test we can create a signature keys with an associated ENGINE */ static int test_signatures_with_engine(int tst) @@ -1965,6 +2042,7 @@ int setup_tests(void) ADD_ALL_TESTS(test_gcm_reinit, OSSL_NELEM(gcm_reinit_tests)); ADD_ALL_TESTS(test_evp_updated_iv, OSSL_NELEM(evp_updated_iv_tests)); + ADD_TEST(test_custom_md_meth); #if !defined(OPENSSL_NO_ENGINE) && !defined(OPENSSL_NO_DYNAMIC_ENGINE) # ifndef OPENSSL_NO_EC ADD_ALL_TESTS(test_signatures_with_engine, 3);