X509_PUBKEY_dup: Do not just up-ref the EVP_PKEY
authorTomas Mraz <tomas@openssl.org>
Fri, 22 Oct 2021 12:22:57 +0000 (14:22 +0200)
committerTomas Mraz <tomas@openssl.org>
Mon, 25 Oct 2021 12:32:43 +0000 (14:32 +0200)
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@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16648)

crypto/x509/x_pubkey.c
test/enginetest.c
test/evp_extra_test.c

index 0c07c39a1f284d3d48632de3e7aa2c880219b22c..bc90ddd89b4952dd751ef3f7f4c391f6a84600fc 100644 (file)
@@ -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;
 }
 
index d865488770326a63db42aa70e9b9679d0d121435..04e61743a1b05aba7321aec8ba758f795e4ed67b 100644 (file)
@@ -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)
 {
@@ -357,6 +358,7 @@ 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;
@@ -370,6 +372,16 @@ static int test_x509_dup_w_engine(void)
         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;
 
@@ -395,11 +407,18 @@ static int test_x509_dup_w_engine(void)
     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);
index 2c604daae99a59b6e05b28aee4e88db08f13f972..58c4a590e7303e0ce2c107c88deb3a54ed7ecd6e 100644 (file)
@@ -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);