[openssl] master update

tomas at openssl.org tomas at openssl.org
Mon Oct 25 12:33:21 UTC 2021


The branch master has been updated
       via  7e35458b511f042d9a37d49227b01096c444e575 (commit)
       via  e0c5184a56b6580127b39774f9e4e0f2caef696e (commit)
       via  bf585c9c071ec606ebb4606e749e63354140ca30 (commit)
      from  ef2fb64f9dfde1965cb0b8a5f8765c4f467c1604 (commit)


- Log -----------------------------------------------------------------
commit 7e35458b511f042d9a37d49227b01096c444e575
Author: Tomas Mraz <tomas at openssl.org>
Date:   Fri Oct 22 14:22:57 2021 +0200

    X509_PUBKEY_dup: Do not just up-ref the EVP_PKEY
    
    We try EVP_PKEY_dup() and if it fails we re-decode it using the
    legacy method as provided keys should be duplicable.
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16648)

commit e0c5184a56b6580127b39774f9e4e0f2caef696e
Author: Tomas Mraz <tomas at openssl.org>
Date:   Thu Oct 21 19:06:55 2021 +0200

    X509_dup: Avoid duplicating the embedded EVP_PKEY
    
    The EVP_PKEY will be recreated from scratch which is OK.
    
    Fixes #16606
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16648)

commit bf585c9c071ec606ebb4606e749e63354140ca30
Author: Tomas Mraz <tomas at openssl.org>
Date:   Wed Sep 22 17:24:09 2021 +0200

    tests: Add test for X509_dup with ENGINE based key
    
    Reviewed-by: Matt Caswell <matt at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/16648)

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

Summary of changes:
 crypto/x509/x_pubkey.c        | 20 ++++++++--
 crypto/x509/x_x509.c          | 19 +--------
 test/enginetest.c             | 89 +++++++++++++++++++++++++++++++++++++++++++
 test/evp_extra_test.c         |  2 +-
 test/recipes/30-test_engine.t |  7 +++-
 5 files changed, 114 insertions(+), 23 deletions(-)

diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c
index 0c07c39a1f..bc90ddd89b 100644
--- a/crypto/x509/x_pubkey.c
+++ b/crypto/x509/x_pubkey.c
@@ -289,14 +289,28 @@ X509_PUBKEY *X509_PUBKEY_dup(const X509_PUBKEY *a)
             || (pubkey->algor = X509_ALGOR_dup(a->algor)) == NULL
             || (pubkey->public_key = ASN1_BIT_STRING_new()) == NULL
             || !ASN1_BIT_STRING_set(pubkey->public_key,
-                                    a->public_key->data, a->public_key->length)
-            || (a->pkey != NULL && !EVP_PKEY_up_ref(a->pkey))) {
+                                    a->public_key->data,
+                                    a->public_key->length)) {
         x509_pubkey_ex_free((ASN1_VALUE **)&pubkey,
                             ASN1_ITEM_rptr(X509_PUBKEY_INTERNAL));
         ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    pubkey->pkey = a->pkey;
+
+    if (a->pkey != NULL) {
+        ERR_set_mark();
+        pubkey->pkey = EVP_PKEY_dup(a->pkey);
+        if (pubkey->pkey == NULL) {
+            pubkey->flag_force_legacy = 1;
+            if (x509_pubkey_decode(&pubkey->pkey, pubkey) <= 0) {
+                x509_pubkey_ex_free((ASN1_VALUE **)&pubkey,
+                                    ASN1_ITEM_rptr(X509_PUBKEY_INTERNAL));
+                ERR_clear_last_mark();
+                return NULL;
+            }
+        }
+        ERR_pop_to_mark();
+    }
     return pubkey;
 }
 
diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c
index d14de0e77e..010578b19a 100644
--- a/crypto/x509/x_x509.c
+++ b/crypto/x509/x_x509.c
@@ -104,23 +104,6 @@ static int x509_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
 
             if (!ossl_x509_set0_libctx(ret, old->libctx, old->propq))
                 return 0;
-            if (old->cert_info.key != NULL) {
-                EVP_PKEY *pkey = X509_PUBKEY_get0(old->cert_info.key);
-
-                if (pkey != NULL) {
-                    pkey = EVP_PKEY_dup(pkey);
-                    if (pkey == NULL) {
-                        ERR_raise(ERR_LIB_X509, ERR_R_MALLOC_FAILURE);
-                        return 0;
-                    }
-                    if (!X509_PUBKEY_set(&ret->cert_info.key, pkey)) {
-                        EVP_PKEY_free(pkey);
-                        ERR_raise(ERR_LIB_X509, ERR_R_INTERNAL_ERROR);
-                        return 0;
-                    }
-                    EVP_PKEY_free(pkey);
-                }
-            }
         }
         break;
     case ASN1_OP_GET0_LIBCTX:
@@ -130,6 +113,7 @@ static int x509_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
             *libctx = ret->libctx;
         }
         break;
+
     case ASN1_OP_GET0_PROPQ:
         {
             const char **propq = exarg;
@@ -137,6 +121,7 @@ static int x509_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
             *propq = ret->propq;
         }
         break;
+
     default:
         break;
     }
diff --git a/test/enginetest.c b/test/enginetest.c
index 4c4aeb9b8d..04e61743a1 100644
--- a/test/enginetest.c
+++ b/test/enginetest.c
@@ -23,6 +23,7 @@
 # include <openssl/engine.h>
 # include <openssl/rsa.h>
 # include <openssl/err.h>
+# include <openssl/x509.h>
 
 static void display_engine_list(void)
 {
@@ -352,6 +353,80 @@ static int test_redirect(void)
     OPENSSL_free(tmp);
     return to_return;
 }
+
+static int test_x509_dup_w_engine(void)
+{
+    ENGINE *e = NULL;
+    X509 *cert = NULL, *dupcert = NULL;
+    X509_PUBKEY *pubkey, *duppubkey = NULL;
+    int ret = 0;
+    BIO *b = NULL;
+    RSA_METHOD *rsameth = NULL;
+
+    if (!TEST_ptr(b = BIO_new_file(test_get_argument(0), "r"))
+        || !TEST_ptr(cert = PEM_read_bio_X509(b, NULL, NULL, NULL)))
+        goto err;
+
+    /* Dup without an engine */
+    if (!TEST_ptr(dupcert = X509_dup(cert)))
+        goto err;
+    X509_free(dupcert);
+    dupcert = NULL;
+
+    if (!TEST_ptr(pubkey = X509_get_X509_PUBKEY(cert))
+        || !TEST_ptr(duppubkey = X509_PUBKEY_dup(pubkey))
+        || !TEST_ptr_ne(duppubkey, pubkey)
+        || !TEST_ptr_ne(X509_PUBKEY_get0(duppubkey), X509_PUBKEY_get0(pubkey)))
+        goto err;
+
+    X509_PUBKEY_free(duppubkey);
+    duppubkey = NULL;
+
+    X509_free(cert);
+    cert = NULL;
+
+    /* Create a test ENGINE */
+    if (!TEST_ptr(e = ENGINE_new())
+            || !TEST_true(ENGINE_set_id(e, "Test dummy engine"))
+            || !TEST_true(ENGINE_set_name(e, "Test dummy engine")))
+        goto err;
+
+    if (!TEST_ptr(rsameth = RSA_meth_dup(RSA_get_default_method())))
+        goto err;
+
+    ENGINE_set_RSA(e, rsameth);
+
+    if (!TEST_true(ENGINE_set_default_RSA(e)))
+        goto err;
+
+    if (!TEST_int_ge(BIO_seek(b, 0), 0)
+        || !TEST_ptr(cert = PEM_read_bio_X509(b, NULL, NULL, NULL)))
+        goto err;
+
+    /* Dup with an engine set on the key */
+    if (!TEST_ptr(dupcert = X509_dup(cert)))
+        goto err;
+
+    if (!TEST_ptr(pubkey = X509_get_X509_PUBKEY(cert))
+        || !TEST_ptr(duppubkey = X509_PUBKEY_dup(pubkey))
+        || !TEST_ptr_ne(duppubkey, pubkey)
+        || !TEST_ptr_ne(X509_PUBKEY_get0(duppubkey), X509_PUBKEY_get0(pubkey)))
+        goto err;
+
+    ret = 1;
+
+ err:
+    X509_free(cert);
+    X509_free(dupcert);
+    X509_PUBKEY_free(duppubkey);
+    if (e != NULL) {
+        ENGINE_unregister_RSA(e);
+        ENGINE_free(e);
+    }
+    RSA_meth_free(rsameth);
+    BIO_free(b);
+    return ret;
+}
 #endif
 
 int global_init(void)
@@ -363,13 +438,27 @@ int global_init(void)
     return OPENSSL_init_crypto(OPENSSL_INIT_NO_LOAD_CONFIG, NULL);
 }
 
+OPT_TEST_DECLARE_USAGE("certfile\n")
+
 int setup_tests(void)
 {
 #ifdef OPENSSL_NO_ENGINE
     TEST_note("No ENGINE support");
 #else
+    int n;
+
+    if (!test_skip_common_options()) {
+        TEST_error("Error parsing test options\n");
+        return 0;
+    }
+
+    n = test_get_argument_count();
+    if (n == 0)
+        return 0;
+
     ADD_TEST(test_engines);
     ADD_TEST(test_redirect);
+    ADD_TEST(test_x509_dup_w_engine);
 #endif
     return 1;
 }
diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index 2c604daae9..58c4a590e7 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -2278,7 +2278,7 @@ static int test_X509_PUBKEY_dup(void)
 
     if (!TEST_ptr(X509_PUBKEY_get0(xq))
             || !TEST_ptr(X509_PUBKEY_get0(xp))
-            || !TEST_ptr_eq(X509_PUBKEY_get0(xq), X509_PUBKEY_get0(xp)))
+            || !TEST_ptr_ne(X509_PUBKEY_get0(xq), X509_PUBKEY_get0(xp)))
         goto done;
 
     X509_PUBKEY_free(xq);
diff --git a/test/recipes/30-test_engine.t b/test/recipes/30-test_engine.t
index 57a2479b04..88db8ec9a7 100644
--- a/test/recipes/30-test_engine.t
+++ b/test/recipes/30-test_engine.t
@@ -10,13 +10,16 @@
 use strict;
 use warnings;
 
-use OpenSSL::Test;
+use OpenSSL::Test qw/:DEFAULT srctop_file/;
 use OpenSSL::Test::Utils;
 
 setup("test_engine");
 
+my @path = qw(test certs);
+
 plan skip_all => "engines are deprecated"
     if disabled('deprecated-3.0');
 
 plan tests => 1;
-ok(run(test(["enginetest"])), "running enginetest");
+ok(run(test(["enginetest", srctop_file(@path, "root-cert.pem")])),
+   "running enginetest");


More information about the openssl-commits mailing list