Set mark and pop error in d2i_PrivateKey_ex
authorDaniel Bevenius <daniel.bevenius@gmail.com>
Mon, 5 Oct 2020 06:14:29 +0000 (08:14 +0200)
committerTomas Mraz <tmraz@fedoraproject.org>
Thu, 8 Oct 2020 15:53:21 +0000 (17:53 +0200)
This commit sets the error mark before calling old_priv_decode and if
old_priv_decode returns false, and if EVP_PKCS82PKEY is successful, the
errors are popped to the previously set mark.

The motivation for this is an issue we found when linking Node.js
against OpenSSL 3.0. Details can be found in the link below and the
test case provided in this commit attempts cover this.

Refs: https://github.com/danbev/learning-libcrypto#asn1-wrong-tag-issue
Refs: https://github.com/nodejs/node/issues/29817

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13073)

crypto/asn1/d2i_pr.c
test/evp_extra_test2.c

index 838ce25b907b54002801cd9e3d4da5d79207d58f..b478112349d76d582ecdf70d2464f7bc20794a8d 100644 (file)
@@ -45,6 +45,7 @@ EVP_PKEY *d2i_PrivateKey_ex(int type, EVP_PKEY **a, const unsigned char **pp,
         goto err;
     }
 
+    ERR_set_mark();
     if (!ret->ameth->old_priv_decode ||
         !ret->ameth->old_priv_decode(ret, &p, length)) {
         if (ret->ameth->priv_decode != NULL
@@ -52,20 +53,28 @@ EVP_PKEY *d2i_PrivateKey_ex(int type, EVP_PKEY **a, const unsigned char **pp,
             EVP_PKEY *tmp;
             PKCS8_PRIV_KEY_INFO *p8 = NULL;
             p8 = d2i_PKCS8_PRIV_KEY_INFO(NULL, &p, length);
-            if (p8 == NULL)
+            if (p8 == NULL) {
+                ERR_clear_last_mark();
                 goto err;
+            }
             tmp = EVP_PKCS82PKEY_ex(p8, libctx, propq);
             PKCS8_PRIV_KEY_INFO_free(p8);
-            if (tmp == NULL)
+            if (tmp == NULL) {
+                ERR_clear_last_mark();
                 goto err;
+            }
             EVP_PKEY_free(ret);
             ret = tmp;
+            ERR_pop_to_mark();
             if (EVP_PKEY_type(type) != EVP_PKEY_base_id(ret))
                 goto err;
         } else {
+            ERR_clear_last_mark();
             ASN1err(0, ERR_R_ASN1_LIB);
             goto err;
         }
+    } else {
+      ERR_clear_last_mark();
     }
     *pp = p;
     if (a != NULL)
index 63380f878a100f2efd4ae842f9cb6b277dd40b7f..0667a82647b62fca09df6b595beeda0e553e8f84 100644 (file)
@@ -15,6 +15,7 @@
  */
 
 #include <openssl/evp.h>
+#include <openssl/pem.h>
 #include <openssl/provider.h>
 #include "testutil.h"
 #include "internal/nelem.h"
@@ -248,6 +249,27 @@ static int test_alternative_default(void)
     return ok;
 }
 
+static int test_d2i_PrivateKey_ex(void) {
+    int ok;
+    OSSL_PROVIDER *provider;
+    BIO *key_bio;
+    EVP_PKEY* pkey;
+    ok = 0;
+
+    provider = OSSL_PROVIDER_load(NULL, "default");
+    key_bio = BIO_new_mem_buf((&keydata[0])->kder, (&keydata)[0]->size);
+
+    ok = TEST_ptr(pkey = PEM_read_bio_PrivateKey(key_bio, NULL, NULL, NULL));
+    TEST_int_eq(ERR_peek_error(), 0);
+    test_openssl_errors();
+
+    EVP_PKEY_free(pkey);
+    BIO_free(key_bio);
+    OSSL_PROVIDER_unload(provider);
+
+    return ok;
+}
+
 int setup_tests(void)
 {
     mainctx = OPENSSL_CTX_new();
@@ -264,6 +286,7 @@ int setup_tests(void)
 
     ADD_TEST(test_alternative_default);
     ADD_ALL_TESTS(test_d2i_AutoPrivateKey_ex, OSSL_NELEM(keydata));
+    ADD_TEST(test_d2i_PrivateKey_ex);
 
     return 1;
 }