From 42bfeb362338857321ef146e8eb743463b3e8ccb Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 2 Feb 2017 23:28:14 -0500 Subject: [PATCH] Remove an unnecessary TLS 1.2 ClientHello state. The version negotiation logic was a little bizarrely wedged in the middle of the state machine. (We don't support server renegotiation, so have_version is always false here.) BUG=128 Change-Id: I9448dce374004b92e8bd5172c36a4e0eea51619c Reviewed-on: https://boringssl-review.googlesource.com/13561 Commit-Queue: David Benjamin Reviewed-by: Adam Langley --- include/openssl/ssl3.h | 1 - ssl/handshake_server.c | 17 ++++++----------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h index 256b2c50..84051ff2 100644 --- a/include/openssl/ssl3.h +++ b/include/openssl/ssl3.h @@ -344,7 +344,6 @@ OPENSSL_COMPILE_ASSERT( #define SSL3_ST_SR_CLNT_HELLO_B (0x111 | SSL_ST_ACCEPT) #define SSL3_ST_SR_CLNT_HELLO_C (0x112 | SSL_ST_ACCEPT) #define SSL3_ST_SR_CLNT_HELLO_D (0x113 | SSL_ST_ACCEPT) -#define SSL3_ST_SR_CLNT_HELLO_E (0x114 | SSL_ST_ACCEPT) /* write to client */ #define SSL3_ST_SW_HELLO_REQ_A (0x120 | SSL_ST_ACCEPT) #define SSL3_ST_SW_HELLO_REQ_B (0x121 | SSL_ST_ACCEPT) diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c index 80250302..80b28eda 100644 --- a/ssl/handshake_server.c +++ b/ssl/handshake_server.c @@ -230,7 +230,6 @@ int ssl3_accept(SSL_HANDSHAKE *hs) { case SSL3_ST_SR_CLNT_HELLO_B: case SSL3_ST_SR_CLNT_HELLO_C: case SSL3_ST_SR_CLNT_HELLO_D: - case SSL3_ST_SR_CLNT_HELLO_E: ret = ssl3_get_client_hello(hs); if (hs->state == SSL_ST_TLS13) { break; @@ -541,6 +540,7 @@ int ssl_client_cipher_list_contains_cipher(const SSL_CLIENT_HELLO *client_hello, static int negotiate_version(SSL_HANDSHAKE *hs, uint8_t *out_alert, const SSL_CLIENT_HELLO *client_hello) { SSL *const ssl = hs->ssl; + assert(!ssl->s3->have_version); uint16_t min_version, max_version; if (!ssl_get_version_range(ssl, &min_version, &max_version)) { *out_alert = SSL_AD_PROTOCOL_VERSION; @@ -853,23 +853,18 @@ static int ssl3_get_client_hello(SSL_HANDSHAKE *hs) { /* fallthrough */; } } - hs->state = SSL3_ST_SR_CLNT_HELLO_C; - } - /* Negotiate the protocol version if we have not done so yet. */ - if (!ssl->s3->have_version) { if (!negotiate_version(hs, &al, &client_hello)) { goto f_err; } if (ssl3_protocol_version(ssl) >= TLS1_3_VERSION) { + /* Jump to the TLS 1.3 state machine. */ hs->state = SSL_ST_TLS13; hs->do_tls13_handshake = tls13_server_handshake; return 1; } - } - if (hs->state == SSL3_ST_SR_CLNT_HELLO_C) { /* Load the client random. */ if (client_hello.random_len != SSL3_RANDOM_SIZE) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); @@ -892,10 +887,10 @@ static int ssl3_get_client_hello(SSL_HANDSHAKE *hs) { goto err; } - hs->state = SSL3_ST_SR_CLNT_HELLO_D; + hs->state = SSL3_ST_SR_CLNT_HELLO_C; } - if (hs->state == SSL3_ST_SR_CLNT_HELLO_D) { + if (hs->state == SSL3_ST_SR_CLNT_HELLO_C) { /* Call |cert_cb| to update server certificates if required. */ if (ssl->cert->cert_cb != NULL) { int rv = ssl->cert->cert_cb(ssl, ssl->cert->cert_cb_arg); @@ -924,10 +919,10 @@ static int ssl3_get_client_hello(SSL_HANDSHAKE *hs) { goto f_err; } - hs->state = SSL3_ST_SR_CLNT_HELLO_E; + hs->state = SSL3_ST_SR_CLNT_HELLO_D; } - assert(hs->state == SSL3_ST_SR_CLNT_HELLO_E); + assert(hs->state == SSL3_ST_SR_CLNT_HELLO_D); /* Determine whether we are doing session resumption. */ int tickets_supported = 0, renew_ticket = 0;