tree fbedda1b582e83356ada748313da38a5d1e4ca5f
parent 6ac867026a49ffda6b4d6c7fab675642041dbf07
author David Benjamin <davidben@google.com> 1513712115 -0500
committer David Benjamin <davidben@google.com> 1513896947 +0000

Fix several issues with tls-boringssl.c.

An earlier CL added IPPS support via BoringSSL, but the change had many
issues. This fixes the following:

- random() with srandom(time(NULL)) is not an acceptable source of
  entropy for a cryptographic library. Fortunately, BoringSSL ignores
  calls to RAND_seed anyway, so delete it all.

- ERR_error_string(NULL) is not thread-safe. (We've really got to get
  rid of that function, but there are a lot of callers to clear
  through.) Use ERR_error_string_n with a local buffer.

- Using the version-specific APIs disables TLS 1.2. Instead, use
  TLS_method (client and server distinctions on methods are ignored in
  BoringSSL), and configure the minimum protocol version accordingly.

- SSL 3.0 is gone. Ignore _HTTP_TLS_ALLOW_SSL3 altogether. The old code
  called SSLv3_*_method() which always fail in BoringSSL and, were they
  not to fail, would have disabled all secure versions of TLS!

- The SSL_set_tlsext_host_name call was guarded by a
  HAVE_SSL_SET_TLSEXT_HOST_NAME, but config.h was not updated. Remove
  the guard.

- Server support was not added, so make the operation actually fail.
  Remove the commented out code (which wouldn't work as calling
  SSL_CTX_* functions after SSL_new doesn't do anything).

- The code to call SSL_connect vs SSL_accept had a typo and only called
  SSL_accept. In case someone wants server support in the future, use
  the generic SSL_do_handshake which is equivalent, provided the caller
  uses SSL_set_{connect,accept}_state.

This also cleans a couple things up:

- SSL_load_error_strings is a no-op in BoringSSL. SSL_library_init is
  also a no-op on Android, but there do exist configuations where it is
  not, so I've left it in.

- SSL_write returns int, not ssize_t. The casts are unnecessary.

- Extracting the SSL_CTX from the SSL to free it is weird. One can just
  free it earlier. The SSL owns a reference to the SSL_CTX and will
  do the rest for you.

- Delete some unused functions whose comments even still say "gnutls" on
  them.

IMPORTANT: This does NOT fix the following:

- This file does not verify peer certificates at all. This means any
  network attacker could use a different certificate and break the
  connection anyway. I do not know how printer certificates are
  typically checked or how Android's trust store is set up, so someone
  with more domain knowledge may need to help out here. (Are printer
  certs typically checked at all? [0] suggests yes, amazingly.)

[0] https://support.microsoft.com/en-us/help/2021626/when-attempting-to-add-an-ipp-printer-over-https--you-receive-an-error

Test: mma. Additionally tested by Mopria folks. See review comments.
Change-Id: Ife007038290ff79f3413179a26c0d40c1bb2c85b