From: Daniel Fiala Date: Sun, 13 Mar 2022 05:56:13 +0000 (+0100) Subject: Add support for mac-less password-base PKCS12 files to PKCS12_parse API. X-Git-Tag: openssl-3.2.0-alpha1~2818 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cfd24cde81aa5f63dba41ddcde0fa3c5d64e1db0;p=thirdparty%2Fopenssl.git Add support for mac-less password-base PKCS12 files to PKCS12_parse API. Fixes openssl#17720. Reviewed-by: Paul Dale Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/17882) --- diff --git a/crypto/pkcs12/p12_kiss.c b/crypto/pkcs12/p12_kiss.c index ed1105cee42..6d999000778 100644 --- 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 70b35dcb731..8736f03153d 100644 --- 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 index 00000000000..f3648706f69 --- /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 +#include +#include + +#include "internal/nelem.h" + +#include +#include +#include +#include + +#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 759cc571187..4601bbfdb16 100644 --- 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:/, ; + 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));