[openssl] OpenSSL_1_1_1-stable update

Matt Caswell matt at openssl.org
Thu Jan 13 11:49:06 UTC 2022


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 <matt at 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 <pauli at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/17472)

commit 93dd7ab35f6ccfb8bde7a7a6e38ea5817c5b54e2
Author: Matt Caswell <matt at 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 <pauli at openssl.org>
    Reviewed-by: Bernd Edlinger <bernd.edlinger at 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);


More information about the openssl-commits mailing list