commit b95c7e53d7d4376dbba18c3e3bbba99d66e3fbff
author David Benjamin <davidben@google.com> Fri Jul 22 16:28:22 2022 -0700
committer Boringssl LUCI CQ <boringssl-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jul 25 18:53:51 2022 +0000
tree 86ba92eb76e01ff43ab6c1ecbd2dbf87386b90b2
parent 64bf8c50a327217c5f56ba6a4d19ba98b1a07788

Fix up book-keeping between the write buffer and pending writes.

Writing application data goes through three steps:

1. Encrypt the data into the write buffer.
2. Flush the write buffer to the network.
3. Report to SSL_write's caller that the write succeeded.

In principle, steps 2 and 3 are done together, but it is possible that
BoringSSL needs to write something, but we are not in the middle of
servicing an SSL_write call. Then we must perform (2) but cannot perform
(3).

TLS 1.3 0-RTT on a client introduces a case like this. Suppose we write
some 0-RTT data, but it is blocked on the network. Meanwhile, the
application tries to read from the socket (protocols like HTTP/2 read
and write concurrently). We discover ServerHello..Finished and must then
respond with EndOfEarlyData..Finished. But to write, we must flush the
current write buffer.

To fix this, https://boringssl-review.googlesource.com/14164 split (2)
and (3) more explicitly. The write buffer may be flushed to the network
at any point, but the wpend_* book-keeping is separate. It represents
whether (3) is done. As part of that, we introduced a wpend_pending
boolean to track whether there was pending data.

This introduces an interesting corner case. We now keep NewSessionTicket
messages buffered until the next SSL_write. (KeyUpdate ACKs are
implemented similarly.) Suppose the caller calls SSL_write(nullptr, 0)
to flush the NewSessionTicket and this hits EWOULDBLOCK. We'll track a
zero-length pending write in wpend_*! A future attempt to write non-zero
data would then violate the moving buffer check. This is strange because
we don't build records for zero-length application writes in the first
place.

Instead, wpend_pending should have been wpend_tot > 0. Remove that and
rearrange the code to check that properly. Also remove wpend_ret as it
has the same data as wpend_tot.

Change-Id: I58c23842cd55e8a8dfbb1854b61278b108b5c7ea
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53546
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>