commit | 9734e4453bd755562e40388fc7e6d36933b10edc | [log] [tgz] |
---|---|---|
author | David Benjamin <davidben@google.com> | Tue Jun 15 13:58:12 2021 -0400 |
committer | Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> | Wed Jun 16 21:10:48 2021 +0000 |
tree | b39e5f88fa111e370073d42b2263a3e87860a6b1 | |
parent | e9c5d72c09e01a0f71f30f7c3454e5e7f8711476 [diff] |
More reliably report handshake errors through SSL_write. This CL fixes a couple of things. First, we never tested that SSL_write refuses to write application data after a fatal alert, so add some tests here. With those tests, we can revise some of this logic: Next, this removes the write_shutdown check in SSL_write and instead relies on the lower-level versions of the check in the write_app_data, etc., hooks. This improves error-reporting on handshake errors: We generally try to make SSL_do_handshake errors sticky, analogous to handshakeErr in the Go implementation. SSL_write and SSL_read both implicitly call SSL_do_handshake. Callers driving the two in parallel will naturally call SSL_do_handshake twice. Since the error effectively applies to both operations, we save and replay handshake errors (hs->error). Handshake errors typically come with sending alerts, which also sets write_shutdown so we don't try to send more data over the channel. Checking this early in SSL_write means we don't get a chance to replay the handshake error. So this CL defers it, and the test ensures we still ultimately get it right. Finally, https://crbug.com/1078515 is a particular incarnation of this. If the server enables 0-RTT and then reverts to TLS 1.2, clients need to catch the error and retry. There, deferring the SSL_write check isn't sufficient, because the can_early_write bit removes the write path's dependency on the handshake, so we don't call into SSL_do_handshake at all. For now, I've made this error path clear can_early_write. I suspect we want it to apply to all handshake errors, though it's weird because the handshake error is effectively a read error in 0-RTT. We don't currently replay record decryption failures at SSL_write, even though those also send a fatal alert and thus break all subsequent writes. Bug: chromium:1078515 Change-Id: Icdfae6a8f2e7c1b1c921068dca244795a670807f Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/48065 Commit-Queue: David Benjamin <davidben@google.com> Reviewed-by: Adam Langley <agl@google.com>
BoringSSL is a fork of OpenSSL that is designed to meet Google's needs.
Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don't recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.
Programs ship their own copies of BoringSSL when they use it and we update everything as needed when deciding to make API changes. This allows us to mostly avoid compromises in the name of compatibility. It works for us, but it may not work for you.
BoringSSL arose because Google used OpenSSL for many years in various ways and, over time, built up a large number of patches that were maintained while tracking upstream OpenSSL. As Google's product portfolio became more complex, more copies of OpenSSL sprung up and the effort involved in maintaining all these patches in multiple places was growing steadily.
Currently BoringSSL is the SSL library in Chrome/Chromium, Android (but it's not part of the NDK) and a number of other apps/programs.
Project links:
There are other files in this directory which might be helpful: