Add support for mac-less password-base PKCS12 files to PKCS12_parse API.

Fixes openssl#17720.

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

diff --git a/crypto/pkcs12/p12_kiss.c b/crypto/pkcs12/p12_kiss.c
index ed1105cee42b4b602551a61d5d303d6bc762d94c..6d9990007787dc021243efcfdaf9dabc38642ce6 100644 (file)
--- a/crypto/pkcs12/p12_kiss.c
+++ b/crypto/pkcs12/p12_kiss.c
@@ -49,27 +49,28 @@ int PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
     }
 
     /* Check the mac */
-
-    /*
-     * If password is zero length or NULL then try verifying both cases to
-     * determine which password is correct. The reason for this is that under
-     * PKCS#12 password based encryption no password and a zero length
-     * password are two different things...
-     */
-
-    if (pass == NULL || *pass == '\0') {
-        if (!PKCS12_mac_present(p12)
-            || PKCS12_verify_mac(p12, NULL, 0))
-            pass = NULL;
-        else if (PKCS12_verify_mac(p12, "", 0))
-            pass = "";
-        else {
+    if (PKCS12_mac_present(p12)) {
+        /*
+         * If password is zero length or NULL then try verifying both cases to
+         * determine which password is correct. The reason for this is that under
+         * PKCS#12 password based encryption no password and a zero length
+         * password are two different things...
+         */
+        if (pass == NULL || *pass == '\0') {
+            if (PKCS12_verify_mac(p12, NULL, 0))
+                pass = NULL;
+            else if (PKCS12_verify_mac(p12, "", 0))
+                pass = "";
+            else {
+                ERR_raise(ERR_LIB_PKCS12, PKCS12_R_MAC_VERIFY_FAILURE);
+                goto err;
+            }
+        } else if (!PKCS12_verify_mac(p12, pass, -1)) {
             ERR_raise(ERR_LIB_PKCS12, PKCS12_R_MAC_VERIFY_FAILURE);
             goto err;
         }
-    } else if (!PKCS12_verify_mac(p12, pass, -1)) {
-        ERR_raise(ERR_LIB_PKCS12, PKCS12_R_MAC_VERIFY_FAILURE);
-        goto err;
+    } else if (pass == NULL || *pass == '\0') {
+        pass = NULL;
     }
 
     /* If needed, allocate stack for other certificates */

diff --git a/test/build.info b/test/build.info
index 70b35dcb7319854b135b79cd108cb9521704e66f..8736f03153d6b0df86af90de7833b4d349fe73e4 100644 (file)
--- a/test/build.info
+++ b/test/build.info
@@ -33,7 +33,7 @@ IF[{- !$disabled{tests} -}]
   PROGRAMS{noinst}= \
           confdump \
           versions \
-          aborttest test_test pkcs12_format_test \
+          aborttest test_test pkcs12_format_test pkcs12_api_test \
           sanitytest rsa_complex exdatatest bntest \
           ecstresstest gmdifftest pbelutest \
           destest mdc2test sha_test \
@@ -286,6 +286,10 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[pkcs12_format_test]=../include ../apps/include
   DEPEND[pkcs12_format_test]=../libcrypto libtestutil.a
 
+  SOURCE[pkcs12_api_test]=pkcs12_api_test.c helpers/pkcs12.c
+  INCLUDE[pkcs12_api_test]=../include ../apps/include
+  DEPEND[pkcs12_api_test]=../libcrypto libtestutil.a
+
   SOURCE[pkcs7_test]=pkcs7_test.c
   INCLUDE[pkcs7_test]=../include ../apps/include
   DEPEND[pkcs7_test]=../libcrypto libtestutil.a

diff --git a/test/pkcs12_api_test.c b/test/pkcs12_api_test.c
new file mode 100644 (file)
index 0000000..f364870
--- /dev/null
+++ b/test/pkcs12_api_test.c
@@ -0,0 +1,169 @@
+/*
+ * Copyright 2022 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License").  You may not use
+ * this file except in compliance with the License.  You can obtain a copy
+ * in the file LICENSE in the source distribution or at
+ * https://www.openssl.org/source/license.html
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+
+#include "internal/nelem.h"
+
+#include <openssl/pkcs12.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/pem.h>
+
+#include "testutil.h"
+#include "helpers/pkcs12.h"
+
+static OSSL_LIB_CTX *testctx = NULL;
+static OSSL_PROVIDER *nullprov = NULL;
+static OSSL_PROVIDER *deflprov = NULL;
+
+static int test_null_args(void)
+{
+    return TEST_false(PKCS12_parse(NULL, NULL, NULL, NULL, NULL));
+}
+
+static PKCS12 *PKCS12_load(const char *fpath)
+{
+    BIO *bio = NULL;
+    PKCS12 *p12 = NULL;
+
+    bio = BIO_new_file(fpath, "r");
+    if (!TEST_ptr(bio))
+        goto err;
+
+    p12 = PKCS12_init(NID_pkcs7_data);
+    if (!TEST_ptr(p12))
+        goto err;
+
+    if (!TEST_true(p12 == d2i_PKCS12_bio(bio, &p12)))
+        goto err;
+
+    BIO_free(bio);
+
+    return p12;
+
+err:
+    BIO_free(bio);
+    PKCS12_free(p12);
+    return NULL;
+}
+
+static const char *in_file = NULL;
+static const char *in_pass = "";
+static int has_key = 0;
+static int has_cert = 0;
+static int has_ca = 0;
+static int pkcs12_parse_test(void)
+{
+    int ret = 0;
+    PKCS12 *p12 = NULL;
+    EVP_PKEY *key = NULL;
+    X509 *cert = NULL;
+    STACK_OF(X509) *ca = NULL;
+
+    if (in_file != NULL) {
+        p12 = PKCS12_load(in_file);
+        if (!TEST_ptr(p12))
+            goto err;
+
+        if (!TEST_true(PKCS12_parse(p12, in_pass, &key, &cert, &ca)))
+            goto err;
+
+        if ((has_key && !TEST_ptr(key)) || (!has_key && !TEST_ptr_null(key)))
+            goto err;
+        if ((has_cert && !TEST_ptr(cert)) || (!has_cert && !TEST_ptr_null(cert)))
+            goto err;
+        if ((has_ca && !TEST_ptr(ca)) || (!has_ca && !TEST_ptr_null(ca)))
+            goto err;
+    }
+
+    ret = 1;
+err:
+    PKCS12_free(p12);
+    EVP_PKEY_free(key);
+    X509_free(cert);
+    OSSL_STACK_OF_X509_free(ca);
+    return TEST_true(ret);
+}
+
+typedef enum OPTION_choice {
+    OPT_ERR = -1,
+    OPT_EOF = 0,
+    OPT_IN_FILE,
+    OPT_IN_PASS,
+    OPT_IN_HAS_KEY,
+    OPT_IN_HAS_CERT,
+    OPT_IN_HAS_CA,
+    OPT_LEGACY,
+    OPT_TEST_ENUM
+} OPTION_CHOICE;
+
+const OPTIONS *test_get_options(void)
+{
+    static const OPTIONS options[] = {
+        OPT_TEST_OPTIONS_DEFAULT_USAGE,
+        { "in",   OPT_IN_FILE,   '<', "PKCS12 input file" },
+        { "pass",   OPT_IN_PASS,   's', "PKCS12 input file password" },
+        { "has-key",   OPT_IN_HAS_KEY,  'n', "Whether the input file does contain an user key" },
+        { "has-cert",   OPT_IN_HAS_CERT, 'n', "Whether the input file does contain an user certificate" },
+        { "has-ca",   OPT_IN_HAS_CA,   'n', "Whether the input file does contain other certificate" },
+        { "legacy",  OPT_LEGACY,  '-', "Test the legacy APIs" },
+        { NULL }
+    };
+    return options;
+}
+
+int setup_tests(void)
+{
+    OPTION_CHOICE o;
+
+    while ((o = opt_next()) != OPT_EOF) {
+        switch (o) {
+        case OPT_IN_FILE:
+            in_file = opt_arg();
+            break;
+        case OPT_IN_PASS:
+            in_pass = opt_arg();
+            break;
+        case OPT_LEGACY:
+            break;
+        case OPT_IN_HAS_KEY:
+            has_key = opt_int_arg();
+            break;
+        case OPT_IN_HAS_CERT:
+            has_cert = opt_int_arg();
+            break;
+        case OPT_IN_HAS_CA:
+            has_ca = opt_int_arg();
+            break;
+        case OPT_TEST_CASES:
+            break;
+        default:
+            return 0;
+        }
+    }
+
+    deflprov = OSSL_PROVIDER_load(testctx, "default");
+    if (!TEST_ptr(deflprov))
+        return 0;
+
+    ADD_TEST(test_null_args);
+    ADD_TEST(pkcs12_parse_test);
+
+    return 1;
+}
+
+void cleanup_tests(void)
+{
+    OSSL_PROVIDER_unload(nullprov);
+    OSSL_PROVIDER_unload(deflprov);
+    OSSL_LIB_CTX_free(testctx);
+}

diff --git a/test/recipes/80-test_pkcs12.t b/test/recipes/80-test_pkcs12.t
index 759cc57118746b0c4201461eec474c09d48a2743..4601bbfdb16d3a7ec9dbb9277525036e3634ce71 100644 (file)
--- a/test/recipes/80-test_pkcs12.t
+++ b/test/recipes/80-test_pkcs12.t
@@ -54,7 +54,7 @@ if (eval { require Win32::API; 1; }) {
 }
 $ENV{OPENSSL_WIN32_UTF8}=1;
 
-plan tests => 13;
+plan tests => 21;
 
 # Test different PKCS#12 formats
 ok(run(test(["pkcs12_format_test"])), "test pkcs12 formats");
@@ -80,6 +80,7 @@ my $outfile2 = "out2.p12";
 my $outfile3 = "out3.p12";
 my $outfile4 = "out4.p12";
 my $outfile5 = "out5.p12";
+my $outfile6 = "out6.p12";
 
 # Test the -chain option with -untrusted
 ok(run(app(["openssl", "pkcs12", "-export", "-chain",
@@ -148,4 +149,66 @@ ok(grep(/subject=CN\s*=\s*server.example/, @pkcs12info) == 1,
 # Test that the expected friendly name is present in the output
 ok(grep(/testname/, @pkcs12info) == 1, "test friendly name in output");
 
+# Test export of PEM file with both cert and key, without password.
+# -nomac necessary to avoid legacy provider requirement
+{
+    ok(run(app(["openssl", "pkcs12", "-export",
+            "-inkey", srctop_file(@path, "cert-key-cert.pem"),
+            "-in", srctop_file(@path, "cert-key-cert.pem"),
+            "-passout", "pass:",
+            "-nomac", "-out", $outfile6], stderr => "outerr6.txt")),
+    "test_export_pkcs12_cert_key_cert_no_pass");
+    open DATA, "outerr6.txt";
+    my @match = grep /:error:/, <DATA>;
+    close DATA;
+    ok(scalar @match > 0 ? 0 : 1, "test_export_pkcs12_outerr6_empty");
+}
+
+# Tests for pkcs12_parse
+ok(run(test(["pkcs12_api_test",
+             "-in", $outfile1,
+             "-has-ca", 1,
+             ])), "Test pkcs12_parse()");
+
+SKIP: {
+    skip "Skipping PKCS#12 parse test because DES is disabled in this build", 1
+        if disabled("des");
+    ok(run(test(["pkcs12_api_test",
+                 "-in", $outfile2,
+                 "-pass", "v3-certs",
+                 "-has-ca", 1,
+                 ])), "Test pkcs12_parse()");
+}
+
+SKIP: {
+    skip "Skipping PKCS#12 parse test because the required algorithms are disabled", 1
+        if disabled("des") || disabled("rc2") || disabled("legacy");
+    ok(run(test(["pkcs12_api_test",
+                 "-in", $outfile3,
+                 "-pass", "v3-certs",
+                 "-has-ca", 1,
+                 ])), "Test pkcs12_parse()");
+}
+
+ok(run(test(["pkcs12_api_test",
+             "-in", $outfile4,
+             "-pass", "v3-certs",
+             "-has-ca", 1,
+             "-has-key", 1,
+             "-has-cert", 1,
+             ])), "Test pkcs12_parse()");
+
+ok(run(test(["pkcs12_api_test",
+             "-in", $outfile5,
+             "-has-ca", 1,
+             ])), "Test pkcs12_parse()");
+
+ok(run(test(["pkcs12_api_test",
+             "-in", $outfile6,
+             "-pass", "",
+             "-has-ca", 1,
+             "-has-key", 1,
+             "-has-cert", 1,
+             ])), "Test pkcs12_parse()");
+
 SetConsoleOutputCP($savedcp) if (defined($savedcp));