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)

1 files changed, 25 insertions, 12 deletions

diff --git a/apps/lib/apps.c b/apps/lib/apps.c
index f53f1b2003..7c1015737d 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: