From 2fe0f94b2647ad762a845d1e11ed8f8e3614a60f Mon Sep 17 00:00:00 2001 From: Dmitry Belyavskiy Date: Thu, 8 Aug 2024 17:20:53 +0200 Subject: [PATCH] Fix PBMAC1 MAC verification in FIPS mode The check for fetchability PKCS12KDF doesn't make sense when we have a different MAC mechanism --- apps/pkcs12.c | 26 ++++++++++++++++++-------- test/recipes/80-test_pkcs12.t | 30 +++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 11 deletions(-) diff --git a/apps/pkcs12.c b/apps/pkcs12.c index cbe133742a8be..7ef4d586c3315 100644 --- a/apps/pkcs12.c +++ b/apps/pkcs12.c @@ -819,17 +819,27 @@ int pkcs12_main(int argc, char **argv) tsalt != NULL ? ASN1_STRING_length(tsalt) : 0L); } } + if (macver) { - EVP_KDF *pkcs12kdf; + const X509_ALGOR *macalgid; + const ASN1_OBJECT *macobj; - pkcs12kdf = EVP_KDF_fetch(app_get0_libctx(), "PKCS12KDF", - app_get0_propq()); - if (pkcs12kdf == NULL) { - BIO_printf(bio_err, "Error verifying PKCS12 MAC; no PKCS12KDF support.\n"); - BIO_printf(bio_err, "Use -nomacver if MAC verification is not required.\n"); - goto end; + PKCS12_get0_mac(NULL, &macalgid, NULL, NULL, p12); + X509_ALGOR_get0(&macobj, NULL, NULL, macalgid); + + if (OBJ_obj2nid(macobj) != NID_pbmac1) { + EVP_KDF *pkcs12kdf; + + pkcs12kdf = EVP_KDF_fetch(app_get0_libctx(), "PKCS12KDF", + app_get0_propq()); + if (pkcs12kdf == NULL) { + BIO_printf(bio_err, "Error verifying PKCS12 MAC; no PKCS12KDF support.\n"); + BIO_printf(bio_err, "Use -nomacver if MAC verification is not required.\n"); + goto end; + } + EVP_KDF_free(pkcs12kdf); } - EVP_KDF_free(pkcs12kdf); + /* If we enter empty password try no password first */ if (!mpass[0] && PKCS12_verify_mac(p12, NULL, 0)) { /* If mac and crypto pass the same set it to NULL too */ diff --git a/test/recipes/80-test_pkcs12.t b/test/recipes/80-test_pkcs12.t index c14ef94998cde..9fcfa96542930 100644 --- a/test/recipes/80-test_pkcs12.t +++ b/test/recipes/80-test_pkcs12.t @@ -9,7 +9,7 @@ use strict; use warnings; -use OpenSSL::Test qw/:DEFAULT srctop_file with/; +use OpenSSL::Test qw/:DEFAULT srctop_file bldtop_dir with/; use OpenSSL::Test::Utils; use Encode; @@ -54,7 +54,9 @@ if (eval { require Win32::API; 1; }) { } $ENV{OPENSSL_WIN32_UTF8}=1; -plan tests => 45; +my $no_fips = disabled('fips') || ($ENV{NO_FIPS} // 0); + +plan tests => $no_fips ? 45 : 51; # Test different PKCS#12 formats ok(run(test(["pkcs12_format_test"])), "test pkcs12 formats"); @@ -185,7 +187,7 @@ for my $instance (sort keys %pbmac1_tests) { "-inkey", srctop_file(@path, "cert-key-cert.pem"), "-in", srctop_file(@path, "cert-key-cert.pem"), "-passout", "pass:1234", - @$extra_args, + @$extra_args, "-out", "$pbmac1_id.p12"], stderr => "${pbmac1_id}_err.txt")), "test_export_pkcs12_${pbmac1_id}"); open DATA, "${pbmac1_id}_err.txt"; @@ -211,6 +213,28 @@ for my $file ("pbmac1_256_256.good.p12", "pbmac1_512_256.good.p12", "pbmac1_512_ "test pbmac1 pkcs12 file $file"); } + +unless ($no_fips) { + my $provpath = bldtop_dir("providers"); + my $provconf = srctop_file("test", "fips-and-base.cnf"); + my $provname = 'fips'; + my @prov = ("-provider-path", $provpath, + "-provider", $provname); + local $ENV{OPENSSL_CONF} = $provconf; + +# Test pbmac1 pkcs12 good files, RFC 9579 + for my $file ("pbmac1_256_256.good.p12", "pbmac1_512_256.good.p12", "pbmac1_512_512.good.p12") + { + my $path = srctop_file("test", "recipes", "80-test_pkcs12_data", $file); + ok(run(app(["openssl", "pkcs12", @prov, "-in", $path, "-password", "pass:1234", "-noenc"])), + "test pbmac1 pkcs12 file $file"); + + ok(run(app(["openssl", "pkcs12", @prov, "-in", $path, "-info", "-noout", + "-passin", "pass:1234"], stderr => "${file}_info.txt")), + "test_export_pkcs12_${file}_info"); + } +} + # Test pbmac1 pkcs12 bad files, RFC 9579 for my $file ("pbmac1_256_256.bad-iter.p12", "pbmac1_256_256.bad-salt.p12", "pbmac1_256_256.no-len.p12") {