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

This is needed to correct d2i_PrivateKey() after it was changed by commit 576892d78f80cf9a.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14647)

diff --git a/crypto/asn1/d2i_pr.c b/crypto/asn1/d2i_pr.c
index fb0ae08356113f6c14767dbb0cfd1dbdf25499fc..9d9c1898cb9f636e7493a413e80dc0240f13806e 100644 (file)
--- a/crypto/asn1/d2i_pr.c
+++ b/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;

diff --git a/crypto/pem/pem_info.c b/crypto/pem/pem_info.c
index 54e29ab41f2afedec92ca5932633458050225059..2714009103c48838295f3449e1132b8b2e72a952 100644 (file)
--- a/crypto/pem/pem_info.c
+++ b/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;

diff --git a/doc/man3/d2i_PrivateKey.pod b/doc/man3/d2i_PrivateKey.pod
index 4e918f14c612befdc2cc260e7b80ae14503f0ac2..c28aae817eec7137b7496b06f8051028f791e60b 100644 (file)
--- a/doc/man3/d2i_PrivateKey.pod
+++ b/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)>.
 

diff --git a/test/certs/ec_privkey_with_chain.pem b/test/certs/ec_privkey_with_chain.pem
new file mode 100644 (file)
index 0000000..fcdf42a
--- /dev/null
+++ b/test/certs/ec_privkey_with_chain.pem
@@ -0,0 +1,74 @@
+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-----
+

diff --git a/test/recipes/60-test_x509_check_cert_pkey.t b/test/recipes/60-test_x509_check_cert_pkey.t
index b6011ef76437d48d92a64b4dd506fe1fe9a8b4f0..2c0f2e40097ed4b95004f4d0005f01629b3f4fea 100644 (file)
--- a/test/recipes/60-test_x509_check_cert_pkey.t
+++ b/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");

diff --git a/test/x509_check_cert_pkey_test.c b/test/x509_check_cert_pkey_test.c
index cf26ed9228435046193900a43bb00379f3c46b8f..3e075e9bbe8d99b5cdb882ddc329b52121744f37 100644 (file)
--- a/test/x509_check_cert_pkey_test.c
+++ b/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))