From: Matt Caswell <matt@openssl.org>
Date: Thu, 11 Feb 2021 16:32:58 +0000 (+0000)
Subject: Don't forget the type of thing we are loading
X-Git-Tag: openssl-3.0.0-alpha13~263
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3a2171f6aa0f72ca95210fa80d92214315d1e744;p=thirdparty%2Fopenssl.git

Don't forget the type of thing we are loading

The apps helper function load_key_certs_crls() is a general purpose
function for loading different types of objects from a given URI. It
sets up an OSSL_STORE and calls OSSL_STORE_expect() so that the store
knows what type of thing to expect to load. Unfortunately this wasn't
working and was always setting "expect" to 0 - which means "anything".

Fixes #13709

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14191)
---

diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index f53f1b20030..7c1015737d6 100644
--- a/apps/lib/apps.c
+++ b/apps/lib/apps.c
@@ -730,11 +730,11 @@ int load_key_certs_crls(const char *uri, int maybe_stdin,
         return 0;
     }
 
-    if (pcerts != NULL && *pcerts == NULL
-            && (*pcerts = sk_X509_new_null()) == NULL) {
-        BIO_printf(bio_err, "Out of memory loading");
-        goto end;
-    } else {
+    if (pcerts != NULL) {
+        if (*pcerts == NULL && (*pcerts = sk_X509_new_null()) == NULL) {
+            BIO_printf(bio_err, "Out of memory loading");
+            goto end;
+        }
         cnt_expectations++;
         expect = OSSL_STORE_INFO_CERT;
     }
@@ -743,11 +743,11 @@ int load_key_certs_crls(const char *uri, int maybe_stdin,
         cnt_expectations++;
         expect = OSSL_STORE_INFO_CRL;
     }
-    if (pcrls != NULL && *pcrls == NULL
-            && (*pcrls = sk_X509_CRL_new_null()) == NULL) {
-        BIO_printf(bio_err, "Out of memory loading");
-        goto end;
-    } else {
+    if (pcrls != NULL) {
+        if (*pcrls == NULL && (*pcrls = sk_X509_CRL_new_null()) == NULL) {
+            BIO_printf(bio_err, "Out of memory loading");
+            goto end;
+        }
         cnt_expectations++;
         expect = OSSL_STORE_INFO_CRL;
     }
@@ -787,8 +787,21 @@ int load_key_certs_crls(const char *uri, int maybe_stdin,
         OSSL_STORE_INFO *info = OSSL_STORE_load(ctx);
         int type, ok = 1;
 
-        if (info == NULL)
-            break;
+        /*
+         * This can happen (for example) if we attempt to load a file with
+         * multiple different types of things in it - but the thing we just
+         * tried to load wasn't one of the ones we wanted, e.g. if we're trying
+         * to load a certificate but the file has both the private key and the
+         * certificate in it. We just retry until eof.
+         */
+        if (info == NULL) {
+            if (OSSL_STORE_error(ctx)) {
+                ERR_print_errors(bio_err);
+                ERR_clear_error();
+            }
+            continue;
+        }
+
         type = OSSL_STORE_INFO_get_type(info);
         switch (type) {
         case OSSL_STORE_INFO_PKEY:
diff --git a/test/recipes/20-test_cli_fips.t b/test/recipes/20-test_cli_fips.t
index 364c9d2bdee..591b4970270 100644
--- a/test/recipes/20-test_cli_fips.t
+++ b/test/recipes/20-test_cli_fips.t
@@ -64,11 +64,27 @@ ok(run(app(['openssl', 'list', '-asymcipher-algorithms', '-verbose'])),
 ok(run(app(['openssl', 'list', '-key-managers', '-verbose', '-select', 'DSA' ])),
    "provider listing of one item in the keymanager");
 
+sub pubfrompriv {
+    my $prefix = shift;
+    my $key = shift;
+    my $pub_key = shift;
+    my $type = shift;
+
+    ok(run(app(['openssl', 'pkey',
+                '-in', $key,
+                '-pubout',
+                '-out', $pub_key])),
+        $prefix.': '."Create the public key with $type parameters");
+
+}
+
 my $tsignverify_count = 8;
 sub tsignverify {
     my $prefix = shift;
     my $fips_key = shift;
+    my $fips_pub_key = shift;
     my $nonfips_key = shift;
+    my $nonfips_pub_key = shift;
     my $fips_sigfile = $prefix.'.fips.sig';
     my $nonfips_sigfile = $prefix.'.nonfips.sig';
     my $sigfile = '';
@@ -88,7 +104,7 @@ sub tsignverify {
     $testtext = $prefix.': '.
         'Verify something with a FIPS key';
     ok(run(app(['openssl', 'dgst', '-sha256',
-                '-verify', $fips_key,
+                '-verify', $fips_pub_key,
                 '-signature', $sigfile,
                 $tbs_data])),
        $testtext);
@@ -97,7 +113,7 @@ sub tsignverify {
         'Verify a valid signature against the wrong data with a FIPS key'.
         ' (should fail)';
     ok(!run(app(['openssl', 'dgst', '-sha256',
-                 '-verify', $fips_key,
+                 '-verify', $fips_pub_key,
                  '-signature', $sigfile,
                  $bogus_data])),
        $testtext);
@@ -118,7 +134,7 @@ sub tsignverify {
         'Verify something with a non-FIPS key'.
         ' with the default provider';
     ok(run(app(['openssl', 'dgst', '-sha256',
-                '-verify', $nonfips_key,
+                '-verify', $nonfips_pub_key,
                 '-signature', $sigfile,
                 $tbs_data])),
        $testtext);
@@ -138,7 +154,7 @@ sub tsignverify {
         'Verify something with a non-FIPS key'.
         ' (should fail)';
     ok(!run(app(['openssl', 'dgst', '-sha256',
-                 '-verify', $nonfips_key,
+                 '-verify', $nonfips_pub_key,
                  '-signature', $sigfile,
                  $tbs_data])),
        $testtext);
@@ -147,7 +163,7 @@ sub tsignverify {
         'Verify a valid signature against the wrong data with a non-FIPS key'.
         ' (should fail)';
     ok(!run(app(['openssl', 'dgst', '-sha256',
-                 '-verify', $nonfips_key,
+                 '-verify', $nonfips_pub_key,
                  '-signature', $sigfile,
                  $bogus_data])),
        $testtext);
@@ -161,12 +177,14 @@ SKIP : {
         my $testtext_prefix = 'EC';
         my $a_fips_curve = 'prime256v1';
         my $fips_key = $testtext_prefix.'.fips.priv.pem';
+        my $fips_pub_key = $testtext_prefix.'.fips.pub.pem';
         my $a_nonfips_curve = 'brainpoolP256r1';
         my $nonfips_key = $testtext_prefix.'.nonfips.priv.pem';
+        my $nonfips_pub_key = $testtext_prefix.'.nonfips.pub.pem';
         my $testtext = '';
         my $curvename = '';
 
-        plan tests => 3 + $tsignverify_count;
+        plan tests => 5 + $tsignverify_count;
 
         $ENV{OPENSSL_CONF} = $defaultconf;
         $curvename = $a_nonfips_curve;
@@ -177,6 +195,8 @@ SKIP : {
                     '-out', $nonfips_key])),
            $testtext);
 
+        pubfrompriv($testtext_prefix, $nonfips_key, $nonfips_pub_key, "non-FIPS");
+
         $ENV{OPENSSL_CONF} = $fipsconf;
 
         $curvename = $a_fips_curve;
@@ -187,6 +207,8 @@ SKIP : {
                     '-out', $fips_key])),
            $testtext);
 
+        pubfrompriv($testtext_prefix, $fips_key, $fips_pub_key, "FIPS");
+
         $curvename = $a_nonfips_curve;
         $testtext = $testtext_prefix.': '.
             'Generate a key with a non-FIPS algorithm'.
@@ -196,7 +218,8 @@ SKIP : {
                      '-out', $testtext_prefix.'.'.$curvename.'.priv.pem'])),
            $testtext);
 
-        tsignverify($testtext_prefix, $fips_key, $nonfips_key);
+        tsignverify($testtext_prefix, $fips_key, $fips_pub_key, $nonfips_key,
+                    $nonfips_pub_key);
     };
 }
 
@@ -207,10 +230,12 @@ SKIP: {
     subtest RSA => sub {
         my $testtext_prefix = 'RSA';
         my $fips_key = $testtext_prefix.'.fips.priv.pem';
+        my $fips_pub_key = $testtext_prefix.'.fips.pub.pem';
         my $nonfips_key = $testtext_prefix.'.nonfips.priv.pem';
+        my $nonfips_pub_key = $testtext_prefix.'.nonfips.pub.pem';
         my $testtext = '';
 
-        plan tests => 3 + $tsignverify_count;
+        plan tests => 5 + $tsignverify_count;
 
         $ENV{OPENSSL_CONF} = $defaultconf;
         $testtext = $testtext_prefix.': '.
@@ -220,6 +245,8 @@ SKIP: {
                     '-out', $nonfips_key])),
            $testtext);
 
+        pubfrompriv($testtext_prefix, $nonfips_key, $nonfips_pub_key, "non-FIPS");
+
         $ENV{OPENSSL_CONF} = $fipsconf;
 
         $testtext = $testtext_prefix.': '.
@@ -229,6 +256,8 @@ SKIP: {
                     '-out', $fips_key])),
            $testtext);
 
+        pubfrompriv($testtext_prefix, $fips_key, $fips_pub_key, "FIPS");
+
         $testtext = $testtext_prefix.': '.
             'Generate a key with a non-FIPS algorithm'.
             ' (should fail)';
@@ -237,7 +266,8 @@ SKIP: {
                      '-out', $testtext_prefix.'.fail.priv.pem'])),
            $testtext);
 
-        tsignverify($testtext_prefix, $fips_key, $nonfips_key);
+        tsignverify($testtext_prefix, $fips_key, $fips_pub_key, $nonfips_key,
+                    $nonfips_pub_key);
     };
 }
 
@@ -248,12 +278,14 @@ SKIP : {
     subtest DSA => sub {
         my $testtext_prefix = 'DSA';
         my $fips_key = $testtext_prefix.'.fips.priv.pem';
+        my $fips_pub_key = $testtext_prefix.'.fips.pub.pem';
         my $nonfips_key = $testtext_prefix.'.nonfips.priv.pem';
+        my $nonfips_pub_key = $testtext_prefix.'.nonfips.pub.pem';
         my $testtext = '';
         my $fips_param = $testtext_prefix.'.fips.param.pem';
         my $nonfips_param = $testtext_prefix.'.nonfips.param.pem';
 
-        plan tests => 6 + $tsignverify_count;
+        plan tests => 8 + $tsignverify_count;
 
         $ENV{OPENSSL_CONF} = $defaultconf;
 
@@ -295,6 +327,8 @@ SKIP : {
                     '-out', $nonfips_key])),
            $testtext);
 
+        pubfrompriv($testtext_prefix, $nonfips_key, $nonfips_pub_key, "non-FIPS");
+
         $ENV{OPENSSL_CONF} = $fipsconf;
 
         $testtext = $testtext_prefix.': '.
@@ -305,6 +339,8 @@ SKIP : {
                     '-out', $fips_key])),
            $testtext);
 
+        pubfrompriv($testtext_prefix, $fips_key, $fips_pub_key, "FIPS");
+
         $testtext = $testtext_prefix.': '.
             'Generate a key with non-FIPS parameters'.
             ' (should fail)';
@@ -314,6 +350,7 @@ SKIP : {
                      '-out', $testtext_prefix.'.fail.priv.pem'])),
            $testtext);
 
-        tsignverify($testtext_prefix, $fips_key, $nonfips_key);
+        tsignverify($testtext_prefix, $fips_key, $fips_pub_key, $nonfips_key,
+                    $nonfips_pub_key);
     };
 }