commit fae0964b3d44e94ca2a2d21f86e61dabe683d130
author Bob Beck <bbe@google.com> Tue Mar 12 19:31:09 2024 +0000
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Mar 13 15:52:19 2024 +0000
tree 32afb7984ef813e6381f34f1f7bb46f45d3a0780
parent 356d37861f5772e2d87ef443f61f33c020e52b04

Re-remove unnecesary stat calls from by_dir.c

After examining consumer test code and discussion with davidben,
the stat here serves only to get out of this code without having
an error on the error stack when the file does not exist, which is
then interpreted as the CA or CRL does not exist. Instead, we
simply attempt to open the files, and if it does not work for
any reason, clear the error that was set.

This changes us to treat any failure in finding a CA or CRL using
the by directory lookup as if the file was just not present. This
ensures a consistent behaviour with the error returned from the
verification code. We don't differentiate between the file not existing
or other errors such as garbage in the file.

Fixed: 708
Change-Id: I1eee01282cde803fb7c9b52003da3dfbd5ba9e33
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66967
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>

crypto/x509/by_dir.c

diff --git a/crypto/x509/by_dir.c b/crypto/x509/by_dir.c
index 16d2dfb..5829de4 100644
--- a/crypto/x509/by_dir.c
+++ b/crypto/x509/by_dir.c
@@ -66,12 +66,6 @@
 #include "../internal.h"
 #include "internal.h"
 
-#if !defined(OPENSSL_NO_POSIX_IO)
-#include <sys/stat.h>
-#include <sys/types.h>
-#endif
-
-
 typedef struct lookup_dir_hashes_st {
   uint32_t hash;
   int suffix;
@@ -317,31 +311,22 @@
       for (;;) {
         snprintf(b->data, b->max, "%s/%08" PRIx32 ".%s%d", ent->dir, h, postfix,
                  k);
-#if !defined(OPENSSL_NO_POSIX_IO)
-#if defined(_WIN32) && !defined(stat)
-#define stat _stat
-#endif
-        {
-          // TODO(crbug.com/boringssl/708): This call should be redundant with
-          // |X509_load_*_file| failing below, but it turns out to be
-          // load-bearing. Clean this up.
-          struct stat st;
-          if (stat(b->data, &st) < 0) {
-            break;
-          }
-        }
-#endif
-        // found one.
         if (type == X509_LU_X509) {
           if ((X509_load_cert_file(xl, b->data, ent->dir_type)) == 0) {
+            // Don't expose the lower level error, All of these boil
+            // down to "we could not find a CA".
+            ERR_clear_error();
             break;
           }
         } else if (type == X509_LU_CRL) {
           if ((X509_load_crl_file(xl, b->data, ent->dir_type)) == 0) {
+            // Don't expose the lower level error, All of these boil
+            // down to "we could not find a CRL".
+            ERR_clear_error();
             break;
           }
         }
-        // else case will caught higher up
+        // The lack of a CA or CRL will be caught higher up
         k++;
       }