x509/by_file.c: fix unreachable and redundant code

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Tom Cosgrove <tom.cosgrove@arm.com>
Reviewed-by: Todd Short <todd.short@me.com>
(Merged from https://github.com/openssl/openssl/pull/21545)

diff --git a/crypto/x509/by_file.c b/crypto/x509/by_file.c
index 811b840ff100bdce5e2dbbe7d949f5061cf11b75..450bbba0537b6a20adf923e0cae58254804b4f19 100644 (file)
--- a/crypto/x509/by_file.c
+++ b/crypto/x509/by_file.c
@@ -1,5 +1,5 @@
 /*
- * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2023 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
@@ -58,15 +58,13 @@ static int by_file_ctrl_ex(X509_LOOKUP *ctx, int cmd, const char *argp,
             if (file)
                 ok = (X509_load_cert_crl_file_ex(ctx, file, X509_FILETYPE_PEM,
                                                  libctx, propq) != 0);
-
             else
                 ok = (X509_load_cert_crl_file_ex(
                          ctx, X509_get_default_cert_file(),
                          X509_FILETYPE_PEM, libctx, propq) != 0);
 
-            if (!ok) {
+            if (!ok)
                 ERR_raise(ERR_LIB_X509, X509_R_LOADING_DEFAULTS);
-            }
         } else {
             if (argl == X509_FILETYPE_PEM)
                 ok = (X509_load_cert_crl_file_ex(ctx, argp, X509_FILETYPE_PEM,
@@ -89,22 +87,17 @@ static int by_file_ctrl(X509_LOOKUP *ctx, int cmd,
 int X509_load_cert_file_ex(X509_LOOKUP *ctx, const char *file, int type,
                            OSSL_LIB_CTX *libctx, const char *propq)
 {
-    int ret = 0;
     BIO *in = NULL;
-    int i, count = 0;
+    int count = 0;
     X509 *x = NULL;
 
     in = BIO_new(BIO_s_file());
 
     if ((in == NULL) || (BIO_read_filename(in, file) <= 0)) {
-        ERR_raise(ERR_LIB_X509, ERR_R_SYS_LIB);
+        ERR_raise(ERR_LIB_X509, ERR_R_BIO_LIB);
         goto err;
     }
 
-    if (type != X509_FILETYPE_PEM && type != X509_FILETYPE_ASN1) {
-        ERR_raise(ERR_LIB_X509, X509_R_BAD_X509_FILETYPE);
-        goto err;
-    }
     x = X509_new_ex(libctx, propq);
     if (x == NULL) {
         ERR_raise(ERR_LIB_X509, ERR_R_ASN1_LIB);
@@ -121,34 +114,36 @@ int X509_load_cert_file_ex(X509_LOOKUP *ctx, const char *file, int type,
                     break;
                 } else {
                     ERR_clear_last_mark();
+                    if (count == 0) {
+                        ERR_raise(ERR_LIB_X509, X509_R_NO_CERTIFICATE_FOUND);
+                    } else {
+                        ERR_raise(ERR_LIB_X509, ERR_R_PEM_LIB);
+                        count = 0;
+                    }
                     goto err;
                 }
             }
             ERR_clear_last_mark();
-            i = X509_STORE_add_cert(ctx->store_ctx, x);
-            if (!i)
+            if (!X509_STORE_add_cert(ctx->store_ctx, x)) {
+                count = 0;
                 goto err;
+            }
             count++;
-            X509_free(x);
-            x = NULL;
         }
-        ret = count;
     } else if (type == X509_FILETYPE_ASN1) {
         if (d2i_X509_bio(in, &x) == NULL) {
-            ERR_raise(ERR_LIB_X509, ERR_R_ASN1_LIB);
+            ERR_raise(ERR_LIB_X509, X509_R_NO_CERTIFICATE_FOUND);
             goto err;
         }
-        i = X509_STORE_add_cert(ctx->store_ctx, x);
-        if (!i)
-            goto err;
-        ret = i;
+        count = X509_STORE_add_cert(ctx->store_ctx, x);
+    } else {
+        ERR_raise(ERR_LIB_X509, X509_R_BAD_X509_FILETYPE);
+        goto err;
     }
-    if (ret == 0)
-        ERR_raise(ERR_LIB_X509, X509_R_NO_CERTIFICATE_FOUND);
  err:
     X509_free(x);
     BIO_free(in);
-    return ret;
+    return count;
 }
 
 int X509_load_cert_file(X509_LOOKUP *ctx, const char *file, int type)
@@ -158,15 +153,14 @@ int X509_load_cert_file(X509_LOOKUP *ctx, const char *file, int type)
 
 int X509_load_crl_file(X509_LOOKUP *ctx, const char *file, int type)
 {
-    int ret = 0;
     BIO *in = NULL;
-    int i, count = 0;
+    int count = 0;
     X509_CRL *x = NULL;
 
     in = BIO_new(BIO_s_file());
 
     if ((in == NULL) || (BIO_read_filename(in, file) <= 0)) {
-        ERR_raise(ERR_LIB_X509, ERR_R_SYS_LIB);
+        ERR_raise(ERR_LIB_X509, ERR_R_BIO_LIB);
         goto err;
     }
 
@@ -179,71 +173,73 @@ int X509_load_crl_file(X509_LOOKUP *ctx, const char *file, int type)
                     ERR_clear_error();
                     break;
                 } else {
-                    ERR_raise(ERR_LIB_X509, ERR_R_PEM_LIB);
+                    if (count == 0) {
+                        ERR_raise(ERR_LIB_X509, X509_R_NO_CRL_FOUND);
+                    } else {
+                        ERR_raise(ERR_LIB_X509, ERR_R_PEM_LIB);
+                        count = 0;
+                    }
                     goto err;
                 }
             }
-            i = X509_STORE_add_crl(ctx->store_ctx, x);
-            if (!i)
+            if (!X509_STORE_add_crl(ctx->store_ctx, x)) {
+                count = 0;
                 goto err;
+            }
             count++;
-            X509_CRL_free(x);
-            x = NULL;
         }
-        ret = count;
     } else if (type == X509_FILETYPE_ASN1) {
         x = d2i_X509_CRL_bio(in, NULL);
         if (x == NULL) {
-            ERR_raise(ERR_LIB_X509, ERR_R_ASN1_LIB);
+            ERR_raise(ERR_LIB_X509, X509_R_NO_CRL_FOUND);
             goto err;
         }
-        i = X509_STORE_add_crl(ctx->store_ctx, x);
-        if (!i)
-            goto err;
-        ret = i;
+        count = X509_STORE_add_crl(ctx->store_ctx, x);
     } else {
         ERR_raise(ERR_LIB_X509, X509_R_BAD_X509_FILETYPE);
         goto err;
     }
-    if (ret == 0)
-        ERR_raise(ERR_LIB_X509, X509_R_NO_CRL_FOUND);
  err:
     X509_CRL_free(x);
     BIO_free(in);
-    return ret;
+    return count;
 }
 
 int X509_load_cert_crl_file_ex(X509_LOOKUP *ctx, const char *file, int type,
                                OSSL_LIB_CTX *libctx, const char *propq)
 {
-    STACK_OF(X509_INFO) *inf;
-    X509_INFO *itmp;
-    BIO *in;
+    STACK_OF(X509_INFO) *inf = NULL;
+    X509_INFO *itmp = NULL;
+    BIO *in = NULL;
     int i, count = 0;
 
     if (type != X509_FILETYPE_PEM)
         return X509_load_cert_file_ex(ctx, file, type, libctx, propq);
     in = BIO_new_file(file, "r");
-    if (!in) {
-        ERR_raise(ERR_LIB_X509, ERR_R_SYS_LIB);
+    if (in == NULL) {
+        ERR_raise(ERR_LIB_X509, ERR_R_BIO_LIB);
         return 0;
     }
     inf = PEM_X509_INFO_read_bio_ex(in, NULL, NULL, "", libctx, propq);
     BIO_free(in);
-    if (!inf) {
+    if (inf == NULL) {
         ERR_raise(ERR_LIB_X509, ERR_R_PEM_LIB);
         return 0;
     }
     for (i = 0; i < sk_X509_INFO_num(inf); i++) {
         itmp = sk_X509_INFO_value(inf, i);
         if (itmp->x509) {
-            if (!X509_STORE_add_cert(ctx->store_ctx, itmp->x509))
+            if (!X509_STORE_add_cert(ctx->store_ctx, itmp->x509)) {
+                count = 0;
                 goto err;
+            }
             count++;
         }
         if (itmp->crl) {
-            if (!X509_STORE_add_crl(ctx->store_ctx, itmp->crl))
+            if (!X509_STORE_add_crl(ctx->store_ctx, itmp->crl)) {
+                count = 0;
                 goto err;
+            }
             count++;
         }
     }