tree 0be7e768538d0aea7482aad1a3b014ea1cc23e6e
parent a792f8804773f9c6c8fa55a8d9a502d56bd79b2b
author David Benjamin <davidben@google.com> 1709761715 -0500
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> 1710194110 +0000

Add BIO_FP_TEXT

This CL allows us to reduce the patch set on CPython.

BIO_new_fp is the FILE* analog of BIO_new_fd. However, it behaves very
strangely w.r.t. Windows file translation modes. Instead of simply
inheriting the FILE* as the caller constructed it, it unconditionally
overrides the file's translation mode!

This is surprising. Moreover, if you change the mode without flushing
the file, weird things happen, as Windows documentation discusses:
https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/setmode?view=msvc-170

This leaks all the way up to calling code, because callers need to pass
a matching BIO_FP_TEXT to the FILE* they made. To be source-compatible
with such callers, notably CPython, we need to at least provide
BIO_FP_TEXT.

I first tried to fully match OpenSSL's semantics, but OpenSSL's
semantics are quite dangerous. Code tested on POSIX, calling
BIO_new_fp(some_file, BIO_NOCLOSE), without much thought, is subtly
broken on Windows. It will change the mode of any file passed into it to
binary!

Our own code runs into this. BIO_new_file internally calls BIO_new_fp.
In OpenSSL, they need to re-parse the mode string and figure out the
right flag. ASN1_STRING_print_ex_fp doesn't even know which is the right
one. In OpenSSL, they actually call fwrite manually. We wrap it in a BIO
and then use the BIO version, because it makes no sense to not use the
abstraction we already have lying around. But that is incompatible with
OpenSSL's semantics.

So instead I've opted to make BIO_FP_TEXT switch the mode, but no flag
just leaves the mode alone. This is slightly OpenSSL-incompatible
because this code will work in OpenSSL, but continue to not work in
BoringSSL:

  // Oops, I actually wanted binary but forgot to use "rb"
  FILE *f = fopen("blah", "r");
  // But bio fixed it for me!
  BIO *bio = BIO_new_fp(f, BIO_NOCLOSE);

But callers should have passed "rb" if they wanted binary. This is also
preexisting and no one has noticed. I think it's far more likely that
applications *aren't* expecting BIO_new_fp to secretly change the input
FILE's mode. If we ever need to, we can adopt OpenSSL's semantics and
then add BIO_FP_LEAVE_MY_FILE_ALONE. But those are worse defaults.

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