Remove the push argument to ssl_init_wbio_buffer.
Having bbio be tri-state (not allocated, allocated but not active, and
allocated and active) is confusing.
The extra state is only used in the client handshake, where ClientHello is
special-cased to not go through the buffer while everything else is. This dates
to OpenSSL's initial commit and doesn't seem to do much. I do not believe it
can affect renego as the buffer only affects writes; although OpenSSL accepted
interleave on read (though this logic predates it slightly), it never sent
application data while it believed a handshake was active. The handshake would
always be driven to completion first.
My guess is this was to save a copy since the ClientHello is a one-message
flight so it wouldn't need to be buffered? This is probably not worth the extra
variation in the state. (Especially with the DTLS state machine going through
ClientHello twice and pushing the BIO in between the two. Though I suspect that
was a mistake in itself. If the optimization guess is correct, there was no
need to do that.)
Change-Id: I6726f866e16ee7213cab0c3e6abb133981444d47
Reviewed-on: https://boringssl-review.googlesource.com/7873
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 6f381cf..76e2e93 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -209,13 +209,11 @@
buf = NULL;
}
- if (!ssl_init_wbio_buffer(ssl, 0)) {
+ if (!ssl_init_wbio_buffer(ssl)) {
ret = -1;
goto end;
}
- /* don't push the buffering BIO quite yet */
-
if (!ssl3_init_handshake_buffer(ssl)) {
OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);
ret = -1;
@@ -233,14 +231,9 @@
if (ret <= 0) {
goto end;
}
- ssl->state = SSL3_ST_CR_SRVR_HELLO_A;
+ ssl->s3->tmp.next_state = SSL3_ST_CR_SRVR_HELLO_A;
+ ssl->state = SSL3_ST_CW_FLUSH;
ssl->init_num = 0;
-
- /* turn on buffering for the next lot of output */
- if (ssl->bbio != ssl->wbio) {
- ssl->wbio = BIO_push(ssl->bbio, ssl->wbio);
- }
-
break;
case SSL3_ST_CR_SRVR_HELLO_A: