[openssl] master update

Matt Caswell matt at openssl.org
Tue Oct 19 15:33:42 UTC 2021


The branch master has been updated
       via  0299094c52ddb66f9a22cfff4e7d70c139112832 (commit)
       via  86c15ba87488f88e6191f098ff154f79ce91847b (commit)
       via  f7d6868d0d48fedd5d9daad0c3e0cbcaef423ff3 (commit)
       via  a0cbc2d222743fc4ffd276b97bd5f8aeacf01122 (commit)
      from  a98b26588b683eb024ab81f3bb3549c43acd5188 (commit)


- Log -----------------------------------------------------------------
commit 0299094c52ddb66f9a22cfff4e7d70c139112832
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Oct 15 16:30:45 2021 +0100

    Add tests for ENGINE problems
    
    Add some tests which would have caught the issues fixed in the previous
    3 commits related to engine handling.
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16846)

commit 86c15ba87488f88e6191f098ff154f79ce91847b
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Oct 15 16:28:53 2021 +0100

    Update provider_util.c to correctly handle ENGINE references
    
    provider_util.c failed to free ENGINE references when clearing a cipher
    or a digest. Additionally ciphers and digests were not copied correctly,
    which would lead to double-frees if it were not for the previously
    mentioned leaks.
    
    Fixes #16845
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16846)

commit f7d6868d0d48fedd5d9daad0c3e0cbcaef423ff3
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Oct 15 16:23:31 2021 +0100

    Ensure pkey_set_type handles ENGINE references correctly
    
    pkey_set_type should not consume the ENGINE references that may be
    passed to it.
    
    Fixes #16757
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16846)

commit a0cbc2d222743fc4ffd276b97bd5f8aeacf01122
Author: Matt Caswell <matt at openssl.org>
Date:   Fri Oct 15 16:06:28 2021 +0100

    Make sure EVP_CIPHER_CTX_copy works with the dasync engine
    
    Ciphers in the daysnc engine were failing to copy their context properly
    in the event of EVP_CIPHER_CTX_copy() because they did not define the
    flag EVP_CIPH_CUSTOM_FLAG
    
    Fixes #16844
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16846)

-----------------------------------------------------------------------

Summary of changes:
 crypto/evp/p_lib.c               |   8 ++-
 engines/e_dasync.c               |  25 +++++--
 providers/common/provider_util.c |  31 +++++++-
 test/evp_extra_test.c            | 148 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 205 insertions(+), 7 deletions(-)

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index 61cfe1efb9..aabd92d555 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -1554,7 +1554,6 @@ static int pkey_set_type(EVP_PKEY *pkey, ENGINE *e, int type, const char *str,
          */
         if (keymgmt == NULL)
             pkey->ameth = ameth;
-        pkey->engine = e;
 
         /*
          * The EVP_PKEY_ASN1_METHOD |pkey_id| retains its legacy key purpose
@@ -1570,6 +1569,13 @@ static int pkey_set_type(EVP_PKEY *pkey, ENGINE *e, int type, const char *str,
         } else {
             pkey->type = EVP_PKEY_KEYMGMT;
         }
+# ifndef OPENSSL_NO_ENGINE
+        if (eptr == NULL && e != NULL && !ENGINE_init(e)) {
+            ERR_raise(ERR_LIB_EVP, EVP_R_INITIALIZATION_ERROR);
+            return 0;
+        }
+# endif
+        pkey->engine = e;
 #endif
     }
     return 1;
diff --git a/engines/e_dasync.c b/engines/e_dasync.c
index b775d59a2c..5a303a9f85 100644
--- a/engines/e_dasync.c
+++ b/engines/e_dasync.c
@@ -268,7 +268,8 @@ static int bind_dasync(ENGINE *e)
             || !EVP_CIPHER_meth_set_flags(_hidden_aes_128_cbc,
                                           EVP_CIPH_FLAG_DEFAULT_ASN1
                                           | EVP_CIPH_CBC_MODE
-                                          | EVP_CIPH_FLAG_PIPELINE)
+                                          | EVP_CIPH_FLAG_PIPELINE
+                                          | EVP_CIPH_CUSTOM_COPY)
             || !EVP_CIPHER_meth_set_init(_hidden_aes_128_cbc,
                                          dasync_aes128_init_key)
             || !EVP_CIPHER_meth_set_do_cipher(_hidden_aes_128_cbc,
@@ -293,7 +294,8 @@ static int bind_dasync(ENGINE *e)
                                             EVP_CIPH_CBC_MODE
                                           | EVP_CIPH_FLAG_DEFAULT_ASN1
                                           | EVP_CIPH_FLAG_AEAD_CIPHER
-                                          | EVP_CIPH_FLAG_PIPELINE)
+                                          | EVP_CIPH_FLAG_PIPELINE
+                                          | EVP_CIPH_CUSTOM_COPY)
             || !EVP_CIPHER_meth_set_init(_hidden_aes_128_cbc_hmac_sha1,
                                          dasync_aes128_cbc_hmac_sha1_init_key)
             || !EVP_CIPHER_meth_set_do_cipher(_hidden_aes_128_cbc_hmac_sha1,
@@ -580,7 +582,8 @@ static int dasync_sha1_final(EVP_MD_CTX *ctx, unsigned char *md)
 /* Cipher helper functions */
 
 static int dasync_cipher_ctrl_helper(EVP_CIPHER_CTX *ctx, int type, int arg,
-                                     void *ptr, int aeadcapable)
+                                     void *ptr, int aeadcapable,
+                                     const EVP_CIPHER *ciph)
 {
     int ret;
     struct dasync_pipeline_ctx *pipe_ctx =
@@ -590,6 +593,18 @@ static int dasync_cipher_ctrl_helper(EVP_CIPHER_CTX *ctx, int type, int arg,
         return 0;
 
     switch (type) {
+        case EVP_CTRL_COPY:
+            {
+                size_t sz = EVP_CIPHER_impl_ctx_size(ciph);
+                void *inner_cipher_data = OPENSSL_malloc(sz);
+
+                if (inner_cipher_data == NULL)
+                    return -1;
+                memcpy(inner_cipher_data, pipe_ctx->inner_cipher_data, sz);
+                pipe_ctx->inner_cipher_data = inner_cipher_data;
+            }
+            break;
+
         case EVP_CTRL_SET_PIPELINE_OUTPUT_BUFS:
             pipe_ctx->numpipes = arg;
             pipe_ctx->outbufs = (unsigned char **)ptr;
@@ -744,7 +759,7 @@ static int dasync_cipher_cleanup_helper(EVP_CIPHER_CTX *ctx,
 static int dasync_aes128_cbc_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
                                   void *ptr)
 {
-    return dasync_cipher_ctrl_helper(ctx, type, arg, ptr, 0);
+    return dasync_cipher_ctrl_helper(ctx, type, arg, ptr, 0, EVP_aes_128_cbc());
 }
 
 static int dasync_aes128_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
@@ -772,7 +787,7 @@ static int dasync_aes128_cbc_cleanup(EVP_CIPHER_CTX *ctx)
 static int dasync_aes128_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type,
                                              int arg, void *ptr)
 {
-    return dasync_cipher_ctrl_helper(ctx, type, arg, ptr, 1);
+    return dasync_cipher_ctrl_helper(ctx, type, arg, ptr, 1, EVP_aes_128_cbc_hmac_sha1());
 }
 
 static int dasync_aes128_cbc_hmac_sha1_init_key(EVP_CIPHER_CTX *ctx,
diff --git a/providers/common/provider_util.c b/providers/common/provider_util.c
index fcfbab632d..58d4db3379 100644
--- a/providers/common/provider_util.c
+++ b/providers/common/provider_util.c
@@ -26,6 +26,9 @@ void ossl_prov_cipher_reset(PROV_CIPHER *pc)
     EVP_CIPHER_free(pc->alloc_cipher);
     pc->alloc_cipher = NULL;
     pc->cipher = NULL;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    ENGINE_finish(pc->engine);
+#endif
     pc->engine = NULL;
 }
 
@@ -33,6 +36,12 @@ int ossl_prov_cipher_copy(PROV_CIPHER *dst, const PROV_CIPHER *src)
 {
     if (src->alloc_cipher != NULL && !EVP_CIPHER_up_ref(src->alloc_cipher))
         return 0;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    if (src->engine != NULL && !ENGINE_init(src->engine)) {
+        EVP_CIPHER_free(src->alloc_cipher);
+        return 0;
+    }
+#endif
     dst->engine = src->engine;
     dst->cipher = src->cipher;
     dst->alloc_cipher = src->alloc_cipher;
@@ -52,6 +61,9 @@ static int load_common(const OSSL_PARAM params[], const char **propquery,
         *propquery = p->data;
     }
 
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    ENGINE_finish(*engine);
+#endif
     *engine = NULL;
     /* Inside the FIPS module, we don't support legacy ciphers */
 #if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
@@ -59,10 +71,18 @@ static int load_common(const OSSL_PARAM params[], const char **propquery,
     if (p != NULL) {
         if (p->data_type != OSSL_PARAM_UTF8_STRING)
             return 0;
-        ENGINE_finish(*engine);
+        /* Get a structural reference */
         *engine = ENGINE_by_id(p->data);
         if (*engine == NULL)
             return 0;
+        /* Get a functional reference */
+        if (!ENGINE_init(*engine)) {
+            ENGINE_free(*engine);
+            *engine = NULL;
+            return 0;
+        }
+        /* Free the structural reference */
+        ENGINE_free(*engine);
     }
 #endif
     return 1;
@@ -122,6 +142,9 @@ void ossl_prov_digest_reset(PROV_DIGEST *pd)
     EVP_MD_free(pd->alloc_md);
     pd->alloc_md = NULL;
     pd->md = NULL;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    ENGINE_finish(pd->engine);
+#endif
     pd->engine = NULL;
 }
 
@@ -129,6 +152,12 @@ int ossl_prov_digest_copy(PROV_DIGEST *dst, const PROV_DIGEST *src)
 {
     if (src->alloc_md != NULL && !EVP_MD_up_ref(src->alloc_md))
         return 0;
+#if !defined(FIPS_MODULE) && !defined(OPENSSL_NO_ENGINE)
+    if (src->engine != NULL && !ENGINE_init(src->engine)) {
+        EVP_MD_free(src->alloc_md);
+        return 0;
+    }
+#endif
     dst->engine = src->engine;
     dst->md = src->md;
     dst->alloc_md = src->alloc_md;
diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index 83f8902d24..baa93beb11 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -30,6 +30,7 @@
 #include <openssl/aes.h>
 #include <openssl/decoder.h>
 #include <openssl/rsa.h>
+#include <openssl/engine.h>
 #include "testutil.h"
 #include "internal/nelem.h"
 #include "internal/sizes.h"
@@ -3854,6 +3855,141 @@ static int test_evp_md_cipher_meth(void)
 
     return testresult;
 }
+
+# ifndef OPENSSL_NO_DYNAMIC_ENGINE
+/* Test we can create a signature keys with an associated ENGINE */
+static int test_signatures_with_engine(int tst)
+{
+    ENGINE *e;
+    const char *engine_id = "dasync";
+    EVP_PKEY *pkey = NULL;
+    const unsigned char badcmackey[] = { 0x00, 0x01 };
+    const unsigned char cmackey[] = {
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+        0x0c, 0x0d, 0x0e, 0x0f
+    };
+    const unsigned char ed25519key[] = {
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+        0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+        0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f
+    };
+    const unsigned char msg[] = { 0x00, 0x01, 0x02, 0x03 };
+    int testresult = 0;
+    EVP_MD_CTX *ctx = NULL;
+    unsigned char *mac = NULL;
+    size_t maclen = 0;
+    int ret;
+
+    if (!TEST_ptr(e = ENGINE_by_id(engine_id)))
+        return 0;
+
+    if (!TEST_true(ENGINE_init(e))) {
+        ENGINE_free(e);
+        return 0;
+    }
+
+    switch (tst) {
+    case 0:
+        pkey = EVP_PKEY_new_CMAC_key(e, cmackey, sizeof(cmackey),
+                                     EVP_aes_128_cbc());
+        break;
+    case 1:
+        pkey = EVP_PKEY_new_CMAC_key(e, badcmackey, sizeof(badcmackey),
+                                     EVP_aes_128_cbc());
+        break;
+    case 2:
+        pkey = EVP_PKEY_new_raw_private_key(EVP_PKEY_ED25519, e, ed25519key,
+                                            sizeof(ed25519key));
+        break;
+    default:
+        TEST_error("Invalid test case");
+        goto err;
+    }
+    if (!TEST_ptr(pkey))
+        goto err;
+
+    if (!TEST_ptr(ctx = EVP_MD_CTX_new()))
+        goto err;
+
+    ret = EVP_DigestSignInit(ctx, NULL, tst == 2 ? NULL : EVP_sha256(), NULL,
+                             pkey);
+    if (tst == 0) {
+        if (!TEST_true(ret))
+            goto err;
+
+        if (!TEST_true(EVP_DigestSignUpdate(ctx, msg, sizeof(msg)))
+                || !TEST_true(EVP_DigestSignFinal(ctx, NULL, &maclen)))
+            goto err;
+
+        if (!TEST_ptr(mac = OPENSSL_malloc(maclen)))
+            goto err;
+
+        if (!TEST_true(EVP_DigestSignFinal(ctx, mac, &maclen)))
+            goto err;
+    } else {
+        /* We used a bad key. We expect a failure here */
+        if (!TEST_false(ret))
+            goto err;
+    }
+
+    testresult = 1;
+ err:
+    EVP_MD_CTX_free(ctx);
+    OPENSSL_free(mac);
+    EVP_PKEY_free(pkey);
+    ENGINE_finish(e);
+    ENGINE_free(e);
+
+    return testresult;
+}
+
+static int test_cipher_with_engine(void)
+{
+    ENGINE *e;
+    const char *engine_id = "dasync";
+    const unsigned char keyiv[] = {
+        0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b,
+        0x0c, 0x0d, 0x0e, 0x0f
+    };
+    const unsigned char msg[] = { 0x00, 0x01, 0x02, 0x03 };
+    int testresult = 0;
+    EVP_CIPHER_CTX *ctx = NULL, *ctx2 = NULL;
+    unsigned char buf[AES_BLOCK_SIZE];
+    int len = 0;
+
+    if (!TEST_ptr(e = ENGINE_by_id(engine_id)))
+        return 0;
+
+    if (!TEST_true(ENGINE_init(e))) {
+        ENGINE_free(e);
+        return 0;
+    }
+
+    if (!TEST_ptr(ctx = EVP_CIPHER_CTX_new())
+            || !TEST_ptr(ctx2 = EVP_CIPHER_CTX_new()))
+        goto err;
+
+    if (!TEST_true(EVP_EncryptInit_ex(ctx, EVP_aes_128_cbc(), e, keyiv, keyiv)))
+        goto err;
+
+    /* Copy the ctx, and complete the operation with the new ctx */
+    if (!TEST_true(EVP_CIPHER_CTX_copy(ctx2, ctx)))
+        goto err;
+
+    if (!TEST_true(EVP_EncryptUpdate(ctx2, buf, &len, msg, sizeof(msg)))
+            || !TEST_true(EVP_EncryptFinal_ex(ctx2, buf + len, &len)))
+        goto err;
+
+    testresult = 1;
+ err:
+    EVP_CIPHER_CTX_free(ctx);
+    EVP_CIPHER_CTX_free(ctx2);
+    ENGINE_finish(e);
+    ENGINE_free(e);
+
+    return testresult;
+}
+# endif /* OPENSSL_NO_DYNAMIC_ENGINE */
 #endif /* OPENSSL_NO_DEPRECATED_3_0 */
 
 typedef enum OPTION_choice {
@@ -3980,6 +4116,18 @@ int setup_tests(void)
 #ifndef OPENSSL_NO_DEPRECATED_3_0
     ADD_ALL_TESTS(test_custom_pmeth, 12);
     ADD_TEST(test_evp_md_cipher_meth);
+
+# ifndef OPENSSL_NO_DYNAMIC_ENGINE
+    /* Tests only support the default libctx */
+    if (testctx == NULL) {
+#  ifndef OPENSSL_NO_EC
+        ADD_ALL_TESTS(test_signatures_with_engine, 3);
+#  else
+        ADD_ALL_TESTS(test_signatures_with_engine, 2);
+#  endif
+        ADD_TEST(test_cipher_with_engine);
+    }
+# endif
 #endif
 
     return 1;


More information about the openssl-commits mailing list