Consistently open files in binary mode on Windows

BIO_*_filename, in upstream OpenSSL, opens in binary mode on Windows,
not text mode. We seem to have lost those ifdefs in the fork. But since
C mandates the 'b' suffix (POSIX just ignores it), apply it consistently
to all OSes for simplicity.

This fixes X509_FILETYPE_ASN1 in X509_STORE's file-based machinery on
Windows.

Also fix the various BIO_new_file calls to all specify binary mode.
Looking through them, I don't think any of them care (they're all
parsing PEM), but let's just apply it across the board so we don't have
to think about this.

Update-Note: BIO_read_filename, etc., now open in binary mode on
Windows. This matches OpenSSL behavior.

Change-Id: I7e555085d5c66ad2f205b476d0317570075bbadb
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/66009
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>

2 files changed, 32 insertions, 8 deletions

diff --git a/crypto/bio/bio_test.cc b/crypto/bio/bio_test.cc
index 7115b98..65324d0 100644
--- a/crypto/bio/bio_test.cc
+++ b/crypto/bio/bio_test.cc
@@ -645,19 +645,25 @@ TEST(BIOTest, Gets) {
         SCOPED_TRACE("file");
 
         // Test |BIO_new_file|.
-        bssl::UniquePtr<BIO> bio(BIO_new_file(file.path().c_str(), "r"));
+        bssl::UniquePtr<BIO> bio(BIO_new_file(file.path().c_str(), "rb"));
         ASSERT_TRUE(bio);
         check_bio_gets(bio.get());
 
+        // Test |BIO_read_filename|.
+        bio.reset(BIO_new(BIO_s_file()));
+        ASSERT_TRUE(bio);
+        ASSERT_TRUE(BIO_read_filename(bio.get(), file.path().c_str()));
+        check_bio_gets(bio.get());
+
         // Test |BIO_NOCLOSE|.
-        ScopedFILE file_obj = file.Open("r");
+        ScopedFILE file_obj = file.Open("rb");
         ASSERT_TRUE(file_obj);
         bio.reset(BIO_new_fp(file_obj.get(), BIO_NOCLOSE));
         ASSERT_TRUE(bio);
         check_bio_gets(bio.get());
 
         // Test |BIO_CLOSE|.
-        file_obj = file.Open("r");
+        file_obj = file.Open("rb");
         ASSERT_TRUE(file_obj);
         bio.reset(BIO_new_fp(file_obj.get(), BIO_CLOSE));
         ASSERT_TRUE(bio);
@@ -700,6 +706,24 @@ TEST(BIOTest, Gets) {
   EXPECT_EQ(c, 'a');
 }
 
+// Test that, on Windows, |BIO_read_filename| opens files in binary mode.
+TEST(BIOTest, BinaryMode) {
+  if (SkipTempFileTests()) {
+    GTEST_SKIP();
+  }
+
+  TemporaryFile file;
+  ASSERT_TRUE(file.Init("\r\n"));
+
+  // Reading from the file should give back the exact bytes we put in.
+  bssl::UniquePtr<BIO> bio(BIO_new(BIO_s_file()));
+  ASSERT_TRUE(bio);
+  ASSERT_TRUE(BIO_read_filename(bio.get(), file.path().c_str()));
+  char buf[2];
+  ASSERT_EQ(2, BIO_read(bio.get(), buf, 2));
+  EXPECT_EQ(Bytes(buf, 2), Bytes("\r\n"));
+}
+
 // Run through the tests twice, swapping |bio1| and |bio2|, for symmetry.
 class BIOPairTest : public testing::TestWithParam<bool> {};
 
diff --git a/crypto/bio/file.c b/crypto/bio/file.c
index dd11e50..9b2a6ca 100644
--- a/crypto/bio/file.c
+++ b/crypto/bio/file.c
@@ -206,16 +206,16 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) {
       const char *mode;
       if (num & BIO_FP_APPEND) {
         if (num & BIO_FP_READ) {
-          mode = "a+";
+          mode = "ab+";
         } else {
-          mode = "a";
+          mode = "ab";
         }
       } else if ((num & BIO_FP_READ) && (num & BIO_FP_WRITE)) {
-        mode = "r+";
+        mode = "rb+";
       } else if (num & BIO_FP_WRITE) {
-        mode = "w";
+        mode = "wb";
       } else if (num & BIO_FP_READ) {
-        mode = "r";
+        mode = "rb";
       } else {
         OPENSSL_PUT_ERROR(BIO, BIO_R_BAD_FOPEN_MODE);
         ret = 0;