From 045cc5590a214a0efb982d028a4f1f0e9dfe3314 Mon Sep 17 00:00:00 2001 From: Adam Langley Date: Wed, 16 Jul 2014 10:45:57 -0700 Subject: [PATCH] Don't delay CKX and Finished for False Start. Android never did this - they patched out the point in the code that set the SSL3_FLAGS_DELAY_CLIENT_FINISHED flag when doing False Start. Also, from the unittests it appears that NSS doesn't do this either. Thus this change brings BoringSSL into line with existing behaviour. SSL3_FLAGS_DELAY_CLIENT_FINISHED wasn't introduced with False Start, it's an option in vanilla OpenSSL. But I can't find anything that uses it and, since it's going to be untested, I've removed it completely in this change. Change-Id: I910537bfa35e74ab88778b83612cf5607d485969 Reviewed-on: https://boringssl-review.googlesource.com/1221 Reviewed-by: David Benjamin Reviewed-by: Adam Langley --- include/openssl/ssl3.h | 1 - ssl/d1_clnt.c | 6 ------ ssl/s3_clnt.c | 29 ++--------------------------- 3 files changed, 2 insertions(+), 34 deletions(-) diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 182edb312..a1610505d 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -339,7 +339,6 @@ typedef struct ssl3_buffer_st #define SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS 0x0001 -#define SSL3_FLAGS_DELAY_CLIENT_FINISHED 0x0002 #define SSL3_FLAGS_POP_BUFFER 0x0004 #define TLS1_FLAGS_TLS_PADDING_BUG 0x0008 #define TLS1_FLAGS_SKIP_CERT_VERIFY 0x0010 diff --git a/ssl/d1_clnt.c b/ssl/d1_clnt.c index 538aa60a7..63d31f2f7 100644 --- a/ssl/d1_clnt.c +++ b/ssl/d1_clnt.c @@ -454,12 +454,6 @@ int dtls1_connect(SSL *s) if (s->hit) { s->s3->tmp.next_state=SSL_ST_OK; - if (s->s3->flags & SSL3_FLAGS_DELAY_CLIENT_FINISHED) - { - s->state=SSL_ST_OK; - s->s3->flags|=SSL3_FLAGS_POP_BUFFER; - s->s3->delay_buf_pop_ret=0; - } } else { diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 77ea695f4..856843ba1 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -198,12 +198,6 @@ int ssl3_connect(SSL *s) s->in_handshake++; if (!SSL_in_init(s) || SSL_in_before(s)) SSL_clear(s); - if (SSL_get_mode(s) & SSL_MODE_HANDSHAKE_CUTTHROUGH) - { - /* Send app data along with CCS/Finished */ - s->s3->flags |= SSL3_FLAGS_DELAY_CLIENT_FINISHED; - } - for (;;) { state=s->state; @@ -483,12 +477,6 @@ int ssl3_connect(SSL *s) if (s->hit) { s->s3->tmp.next_state=SSL_ST_OK; - if (s->s3->flags & SSL3_FLAGS_DELAY_CLIENT_FINISHED) - { - s->state=SSL_ST_OK; - s->s3->flags|=SSL3_FLAGS_POP_BUFFER; - s->s3->delay_buf_pop_ret=0; - } } else { @@ -509,16 +497,7 @@ int ssl3_connect(SSL *s) && s->s3->previous_server_finished_len == 0 /* no cutthrough on renegotiation (would complicate the state machine) */ ) { - if (s->s3->flags & SSL3_FLAGS_DELAY_CLIENT_FINISHED) - { - s->state=SSL3_ST_CUTTHROUGH_COMPLETE; - s->s3->flags|=SSL3_FLAGS_POP_BUFFER; - s->s3->delay_buf_pop_ret=0; - } - else - { - s->s3->tmp.next_state=SSL3_ST_CUTTHROUGH_COMPLETE; - } + s->s3->tmp.next_state=SSL3_ST_CUTTHROUGH_COMPLETE; } else { @@ -581,11 +560,7 @@ int ssl3_connect(SSL *s) else s->state=SSL3_ST_CR_FINISHED_A; - /* SSL_write() will take care of flushing buffered data if - * DELAY_CLIENT_FINISHED is set. - */ - if (!(s->s3->flags & SSL3_FLAGS_DELAY_CLIENT_FINISHED)) - ssl_free_wbio_buffer(s); + ssl_free_wbio_buffer(s); ret = 1; goto end; /* break; */