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@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16648)

(cherry picked from commit 7e35458b511f042d9a37d49227b01096c444e575)

3 files changed, 37 insertions, 4 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/test/enginetest.c b/test/enginetest.c
index d865488770..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)
 {
@@ -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);
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);