d2i_PrivateKey{,_ex}() and PEM_X509_INFO_read_bio_ex(): Fix handling of RSA/DSA/EC private key

This is needed to correct d2i_PrivateKey() after it was changed by commit 576892d7.
Reviewed-by: NTomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14647)

crypto/asn1/d2i_pr.c

@@ -29,7 +29,7 @@ d2i_PrivateKey_decoder(int keytype, EVP_PKEY **a, const unsigned char **pp,
 {
     OSSL_DECODER_CTX *dctx = NULL;
     size_t len = length;
-    EVP_PKEY *pkey = NULL;
+    EVP_PKEY *pkey = NULL, *bak_a = NULL;
     EVP_PKEY **ppkey = &pkey;
     const char *key_name = NULL;
     const char *input_structures[] = { "type-specific", "pkcs8", NULL };
@@ -40,15 +40,17 @@ d2i_PrivateKey_decoder(int keytype, EVP_PKEY **a, const unsigned char **pp,
         if (key_name == NULL)
             return NULL;
     }
-    if (a != NULL && *a != NULL)
-        ppkey = a;
 
     for (i = 0;  i < (int)OSSL_NELEM(input_structures); ++i) {
         const unsigned char *p = *pp;
 
+        if (a != NULL && (bak_a = *a) != NULL)
+            ppkey = a;
         dctx = OSSL_DECODER_CTX_new_for_pkey(ppkey, "DER",
                                              input_structures[i], key_name,
                                              EVP_PKEY_KEYPAIR, libctx, propq);
+        if (a != NULL)
+            *a = bak_a;
         if (dctx == NULL)
             return NULL;
 
@@ -56,8 +58,11 @@ d2i_PrivateKey_decoder(int keytype, EVP_PKEY **a, const unsigned char **pp,
         OSSL_DECODER_CTX_free(dctx);
         if (ret) {
             if (*ppkey != NULL
-                && evp_keymgmt_util_has(*ppkey, OSSL_KEYMGMT_SELECT_PRIVATE_KEY))
+                && evp_keymgmt_util_has(*ppkey, OSSL_KEYMGMT_SELECT_PRIVATE_KEY)) {
+                if (a != NULL)
+                    *a = *ppkey;
                 return *ppkey;
+            }
             *pp = p;
             goto err;
         }
@@ -76,7 +81,7 @@ d2i_PrivateKey_legacy(int keytype, EVP_PKEY **a, const unsigned char **pp,
     EVP_PKEY *ret;
     const unsigned char *p = *pp;
 
-    if ((a == NULL) || (*a == NULL)) {
+    if (a == NULL || *a == NULL) {
         if ((ret = EVP_PKEY_new()) == NULL) {
             ERR_raise(ERR_LIB_ASN1, ERR_R_EVP_LIB);
             return NULL;
@@ -127,7 +132,7 @@ d2i_PrivateKey_legacy(int keytype, EVP_PKEY **a, const unsigned char **pp,
     }
     *pp = p;
     if (a != NULL)
-        (*a) = ret;
+        *a = ret;
     return ret;
  err:
     if (a == NULL || *a != ret)
@@ -195,7 +200,7 @@ static EVP_PKEY *d2i_AutoPrivateKey_legacy(EVP_PKEY **a,
         if (ret == NULL)
             return NULL;
         *pp = p;
-        if (a) {
+        if (a != NULL) {
             *a = ret;
         }
         return ret;

crypto/pem/pem_info.c

@@ -209,7 +209,8 @@ STACK_OF(X509_INFO) *PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk,
                     goto err;
                 p = data;
                 if (ptype) {
-                    if (!d2i_PrivateKey(ptype, pp, &p, len)) {
+                    if (d2i_PrivateKey_ex(ptype, pp, &p, len,
+                                          libctx, propq) == NULL) {
                         ERR_raise(ERR_LIB_PEM, ERR_R_ASN1_LIB);
                         goto err;
                     }
@@ -217,7 +218,7 @@ STACK_OF(X509_INFO) *PEM_X509_INFO_read_bio_ex(BIO *bp, STACK_OF(X509_INFO) *sk,
                     ERR_raise(ERR_LIB_PEM, ERR_R_ASN1_LIB);
                     goto err;
                 }
-            } else {            /* encrypted RSA data */
+            } else {            /* encrypted key data */
                 if (!PEM_get_EVP_CIPHER_INFO(header, &xi->enc_cipher))
                     goto err;
                 xi->enc_data = (char *)data;

doc/man3/d2i_PrivateKey.pod

@@ -50,13 +50,16 @@ i2d_PrivateKey_fp
 =head1 DESCRIPTION
 
 d2i_PrivateKey_ex() decodes a private key using algorithm I<type>. It attempts
-to use any key specific format or PKCS#8 unencrypted PrivateKeyInfo format. The
-I<type> parameter should be a public key algorithm constant such as
+to use any key-specific format or PKCS#8 unencrypted PrivateKeyInfo format.
+The I<type> parameter should be a public key algorithm constant such as
 B<EVP_PKEY_RSA>. An error occurs if the decoded key does not match I<type>. Some
 private key decoding implementations may use cryptographic algorithms (for
 example to automatically derive the public key if it is not explicitly
 included in the encoding). In this case the supplied library context I<libctx>
 and property query string I<propq> are used.
+If successful and the I<a> parameter is not NULL the function assigns the
+returned B<EVP_PKEY> structure pointer to I<*a>, overwriting any previous value.
+
 d2i_PrivateKey() does the same as d2i_PrivateKey_ex() except that the default
 library context and property query string are used.
 d2i_PublicKey() does the same for public keys.
@@ -87,10 +90,6 @@ All these functions use DER format and unencrypted keys. Applications wishing
 to encrypt or decrypt private keys should use other functions such as
 d2i_PKCS8PrivateKey() instead.
 
-If the I<*a> is not NULL when calling d2i_PrivateKey() or d2i_AutoPrivateKey()
-(i.e. an existing structure is being reused) and the key format is PKCS#8
-then I<*a> will be freed and replaced on a successful call.
-
 To decode a key with type B<EVP_PKEY_EC>, d2i_PublicKey() requires I<*a> to be
 a non-NULL EVP_PKEY structure assigned an EC_KEY structure referencing the proper
 EC_GROUP.
@@ -100,7 +99,7 @@ EC_GROUP.
 The d2i_PrivateKey_ex(), d2i_PrivateKey(), d2i_AutoPrivateKey_ex(),
 d2i_AutoPrivateKey(), d2i_PrivateKey_ex_bio(), d2i_PrivateKey_bio(),
 d2i_PrivateKey_ex_fp(), d2i_PrivateKey_fp(), d2i_PublicKey(), d2i_KeyParams()
-and d2i_KeyParams_bio() functions return a valid B<EVP_KEY> structure or B<NULL>
+and d2i_KeyParams_bio() functions return a valid B<EVP_PKEY> structure or NULL
 if an error occurs. The error code can be obtained by calling
 L<ERR_get_error(3)>.
 

test/certs/ec_privkey_with_chain.pem

+Private Key for CN=Ca-ENROLLMENT-INTERMEDIATE-3
+-----BEGIN EC PRIVATE KEY-----
+MHcCAQEEIFGgYhBJYVKeQgTP0hsIv3NGTcG1+dooIFdRbEbCWrUvoAoGCCqGSM49
+AwEHoUQDQgAEYJfmnfC2iI6xjUarHNOY5TbNFD8MZVdb1PszPdbeuGs7hgiEcSWI
+hRjawFslN3XiubZeMPtN5nX8vudvtnNYVA==
+-----END EC PRIVATE KEY-----
+
+Subject: CN=Ca-ENROLLMENT-INTERMEDIATE-3
+Issuer: CN=Ca-ENROLLMENT-INTERMEDIATE-2
+Valid from Thu Sep 03 10:45:37 CEST 2020 to Sun Sep 01 10:45:37 CEST 2030
+Serial: 1599122797485
+-----BEGIN CERTIFICATE-----
+MIIB3zCCAYWgAwIBAgIGAXRTJXOtMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMMHENh
+LUVOUk9MTE1FTlQtSU5URVJNRURJQVRFLTIwHhcNMjAwOTAzMDg0NTM3WhcNMzAw
+OTAxMDg0NTM3WjAnMSUwIwYDVQQDDBxDYS1FTlJPTExNRU5ULUlOVEVSTUVESUFU
+RS0zMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEYJfmnfC2iI6xjUarHNOY5TbN
+FD8MZVdb1PszPdbeuGs7hgiEcSWIhRjawFslN3XiubZeMPtN5nX8vudvtnNYVKOB
+nDCBmTAdBgNVHQ4EFgQUpdEIxYuP40cHdbcVTKRsT5/1PEMwVAYDVR0jBE0wS4AU
+TfcTbSV0o6Zwb+Rg0fvscn3R97WhK6QpMCcxJTAjBgNVBAMMHENhLUVOUk9MTE1F
+TlQtSU5URVJNRURJQVRFLTGCBgF0UyVzpDASBgNVHRMBAf8ECDAGAQH/AgECMA4G
+A1UdDwEB/wQEAwIBhjAKBggqhkjOPQQDAgNIADBFAiEAmIyD1fuTtMTuJwSccOg2
+jR+7HX67yTx1ozZOOrAsdBACIAo14mDvZYrFUke3r69690gCbiNUEQgbhIwCLYTQ
+2qbo
+-----END CERTIFICATE-----
+
+Subject: CN=Ca-ENROLLMENT-INTERMEDIATE-2
+Issuer: CN=Ca-ENROLLMENT-INTERMEDIATE-1
+Valid from Thu Sep 03 10:45:37 CEST 2020 to Sun Sep 01 10:45:37 CEST 2030
+Serial: 1599122797476
+-----BEGIN CERTIFICATE-----
+MIIB1jCCAXugAwIBAgIGAXRTJXOkMAoGCCqGSM49BAMCMCcxJTAjBgNVBAMMHENh
+LUVOUk9MTE1FTlQtSU5URVJNRURJQVRFLTEwHhcNMjAwOTAzMDg0NTM3WhcNMzAw
+OTAxMDg0NTM3WjAnMSUwIwYDVQQDDBxDYS1FTlJPTExNRU5ULUlOVEVSTUVESUFU
+RS0yMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEU4USFKQ1laHIiT1hC+ynawpl
+GFrEqt51RroMKIJclV+y+V8PQIAOAIMIDvpuxmDsnLr/I1QZO5Ui8pZdX379F6OB
+kjCBjzAdBgNVHQ4EFgQUTfcTbSV0o6Zwb+Rg0fvscn3R97UwSgYDVR0jBEMwQYAU
+HSCEFJcZjBVN6QtcmyGcFap0KR2hIaQfMB0xGzAZBgNVBAMMEkNhLUVOUk9MTE1F
+TlQtUk9PVIIGAXRTJXOfMBIGA1UdEwEB/wQIMAYBAf8CAQMwDgYDVR0PAQH/BAQD
+AgGGMAoGCCqGSM49BAMCA0kAMEYCIQC8F6GxJoW9XiD8m/rEECipJntU3iVNstHk
+Mdyx/wWvEAIhAIbw3IddLmt4dt1ce+sweFzrYSuGMH3LVSoIs6XhRqHx
+-----END CERTIFICATE-----
+
+Subject: CN=Ca-ENROLLMENT-INTERMEDIATE-1
+Issuer: CN=Ca-ENROLLMENT-ROOT
+Valid from Thu Sep 03 10:45:37 CEST 2020 to Sun Sep 01 10:45:37 CEST 2030
+Serial: 1599122797471
+-----BEGIN CERTIFICATE-----
+MIIByjCCAXGgAwIBAgIGAXRTJXOfMAoGCCqGSM49BAMCMB0xGzAZBgNVBAMMEkNh
+LUVOUk9MTE1FTlQtUk9PVDAeFw0yMDA5MDMwODQ1MzdaFw0zMDA5MDEwODQ1Mzda
+MCcxJTAjBgNVBAMMHENhLUVOUk9MTE1FTlQtSU5URVJNRURJQVRFLTEwWTATBgcq
+hkjOPQIBBggqhkjOPQMBBwNCAAR9uWgfHScQFcB87LaQKvSFPhngP4hHIsFdh5cY
+7ji2HYNfrkl2uWLKJfMiOFT06c1byplGzyj0ju8VWNV5Tee7o4GSMIGPMB0GA1Ud
+DgQWBBQdIIQUlxmMFU3pC1ybIZwVqnQpHTBKBgNVHSMEQzBBgBScYjWnQ+g/rclT
+YjwpMptoEfes3KEhpB8wHTEbMBkGA1UEAwwSQ2EtRU5ST0xMTUVOVC1ST09UggYB
+dFMlc5owEgYDVR0TAQH/BAgwBgEB/wIBBDAOBgNVHQ8BAf8EBAMCAYYwCgYIKoZI
+zj0EAwIDRwAwRAIgRXLkNZAXLbhCyyL4614DuSm++fJ90A9JPU/uVpivz+MCIGlR
+G8F6eiU7ZeKPr/JON1BxLRXBZyI+Pfidj06Zvfvx
+-----END CERTIFICATE-----
+
+Subject: CN=Ca-ENROLLMENT-ROOT
+Issuer: CN=Ca-ENROLLMENT-ROOT
+Valid from Thu Sep 03 10:45:37 CEST 2020 to Sun Sep 01 10:45:37 CEST 2030
+Serial: 1599122797466
+-----BEGIN CERTIFICATE-----
+MIIBczCCARmgAwIBAgIGAXRTJXOaMAoGCCqGSM49BAMCMB0xGzAZBgNVBAMMEkNh
+LUVOUk9MTE1FTlQtUk9PVDAeFw0yMDA5MDMwODQ1MzdaFw0zMDA5MDEwODQ1Mzda
+MB0xGzAZBgNVBAMMEkNhLUVOUk9MTE1FTlQtUk9PVDBZMBMGByqGSM49AgEGCCqG
+SM49AwEHA0IABJd2nkxUVJqZ0NkEloOc3I7atQvkYmHg7UAOXp/QtwusVXfgG5lZ
+5qLayDuxlQNgcBDMilKBMnB2SNT+/IcQwEyjRTBDMB0GA1UdDgQWBBScYjWnQ+g/
+rclTYjwpMptoEfes3DASBgNVHRMBAf8ECDAGAQH/AgEKMA4GA1UdDwEB/wQEAwIB
+hjAKBggqhkjOPQQDAgNIADBFAiEAqGN70wgX6B1KU++k2inz04EPRTRqk5KLxHaW
+1jBXCbwCIGTNjmSi5J2mp+RL5UCP0ji41uPtwENC4mX4hJ+pOMIa
+-----END CERTIFICATE-----
+

test/recipes/60-test_x509_check_cert_pkey.t

@@ -12,35 +12,52 @@ use OpenSSL::Test::Utils;
 
 setup("test_x509_check_cert_pkey");
 
-plan tests => 6;
+plan tests => 9;
+
+sub src_file {
+    return srctop_file("test", "certs", shift);
+}
+
+sub test_PEM_X509_INFO_read {
+    my $file = shift;
+    my $num = shift;
+    ok(run(test(["x509_check_cert_pkey_test", src_file($file), $num])),
+        "test_PEM_X509_INFO_read $file");
+}
 
 # rsa
 ok(run(test(["x509_check_cert_pkey_test",
-             srctop_file("test", "certs", "servercert.pem"),
-             srctop_file("test", "certs", "serverkey.pem"), "cert", "ok"])));
+             src_file("servercert.pem"),
+             src_file("serverkey.pem"), "cert", "ok"])));
 # mismatched rsa
 ok(run(test(["x509_check_cert_pkey_test",
-             srctop_file("test", "certs", "servercert.pem"),
-             srctop_file("test", "certs", "wrongkey.pem"), "cert", "failed"])));
+             src_file("servercert.pem"),
+             src_file("wrongkey.pem"), "cert", "failed"])));
 SKIP: {
     skip "DSA disabled", 1, if disabled("dsa");
     # dsa
     ok(run(test(["x509_check_cert_pkey_test",
-		 srctop_file("test", "certs", "server-dsa-cert.pem"),
-		 srctop_file("test", "certs", "server-dsa-key.pem"), "cert", "ok"])));
+		 src_file("server-dsa-cert.pem"),
+		 src_file("server-dsa-key.pem"), "cert", "ok"])));
 }
 # ecc
 SKIP: {
-    skip "EC disabled", 1 if disabled("ec");
+    skip "EC disabled", 2 if disabled("ec");
     ok(run(test(["x509_check_cert_pkey_test",
-                 srctop_file("test", "certs", "server-ecdsa-cert.pem"),
-                 srctop_file("test", "certs", "server-ecdsa-key.pem"), "cert", "ok"])));
+                 src_file("server-ecdsa-cert.pem"),
+                 src_file("server-ecdsa-key.pem"), "cert", "ok"])));
+
+    test_PEM_X509_INFO_read("ec_privkey_with_chain.pem", "5");
+
 }
 # certificate request (rsa)
 ok(run(test(["x509_check_cert_pkey_test",
-             srctop_file("test", "certs", "x509-check.csr"),
-             srctop_file("test", "certs", "x509-check-key.pem"), "req", "ok"])));
+             src_file("x509-check.csr"),
+             src_file("x509-check-key.pem"), "req", "ok"])));
 # mismatched certificate request (rsa)
 ok(run(test(["x509_check_cert_pkey_test",
-             srctop_file("test", "certs", "x509-check.csr"),
-             srctop_file("test", "certs", "wrongkey.pem"), "req", "failed"])));
+             src_file("x509-check.csr"),
+             src_file("wrongkey.pem"), "req", "failed"])));
+
+test_PEM_X509_INFO_read("root-cert.pem", "1");
+test_PEM_X509_INFO_read("cyrillic_crl.utf8", "1");

test/x509_check_cert_pkey_test.c

@@ -106,15 +106,47 @@ failed:
     return ret;
 }
 
+static const char *file; /* path of a cert/CRL/key file in PEM format */
+static const char *num;  /* expected number of certs/CRLs/keys included */
+
+static int test_PEM_X509_INFO_read_bio(void)
+{
+    BIO *in;
+    STACK_OF(X509_INFO) *sk;
+    X509_INFO *it;
+    int i, count = 0;
+    int expected = 0;
+
+    if (!TEST_ptr((in = BIO_new_file(file, "r"))))
+        return 0;
+    sk = PEM_X509_INFO_read_bio(in, NULL, NULL, "");
+    BIO_free(in);
+    sscanf(num, "%d", &expected);
+    for (i = 0; i < sk_X509_INFO_num(sk); i++) {
+        it = sk_X509_INFO_value(sk, i);
+        if (it->x509 != NULL)
+            count++;
+        if (it->crl != NULL)
+            count++;
+        if (it->x_pkey != NULL)
+            count++;
+    }
+    sk_X509_INFO_pop_free(sk, X509_INFO_free);
+    return TEST_int_eq(count, expected);
+}
+
 const OPTIONS *test_get_options(void)
 {
     enum { OPT_TEST_ENUM };
     static const OPTIONS test_options[] = {
-        OPT_TEST_OPTIONS_WITH_EXTRA_USAGE("cert key type expected\n"),
+        OPT_TEST_OPTIONS_WITH_EXTRA_USAGE("cert key type expected\n"
+                                          "     or [options] file num\n"),
         { OPT_HELP_STR, 1, '-', "cert\tcertificate or CSR filename in PEM\n" },
         { OPT_HELP_STR, 1, '-', "key\tprivate key filename in PEM\n" },
         { OPT_HELP_STR, 1, '-', "type\t\tvalue must be 'cert' or 'req'\n" },
         { OPT_HELP_STR, 1, '-', "expected\tthe expected return value, either 'ok' or 'failed'\n" },
+        { OPT_HELP_STR, 1, '-', "file\tPEM format file containing certs, keys, and/OR CRLs\n" },
+        { OPT_HELP_STR, 1, '-', "num\texpected number of credentials to be loaded from file\n" },
         { NULL }
     };
     return test_options;
@@ -127,6 +159,14 @@ int setup_tests(void)
         return 0;
     }
 
+    if (test_get_argument_count() == 2) {
+        if (!TEST_ptr(file = test_get_argument(0))
+                || !TEST_ptr(num = test_get_argument(1)))
+            return 0;
+        ADD_TEST(test_PEM_X509_INFO_read_bio);
+        return 1;
+    }
+
     if (!TEST_ptr(c = test_get_argument(0))
             || !TEST_ptr(k = test_get_argument(1))
             || !TEST_ptr(t = test_get_argument(2))