From 81b7bc3979a556a2df616dfcdf3d39e670779639 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Thu, 12 Jan 2017 19:44:57 -0500 Subject: [PATCH] Trim unnecessary TLS 1.3 states. Large chunks of contiguous messages can now be sent in a row. Notably, the ServerHello flight involves a number of optional messages which can now be collapsed into straight-line code. BUG=72 Change-Id: I1429d22a12401aa0f811a04e495bd5d754c084a4 Reviewed-on: https://boringssl-review.googlesource.com/13226 Reviewed-by: Adam Langley --- ssl/tls13_client.c | 64 +++++++++++-------------- ssl/tls13_server.c | 116 ++++++++++++++++----------------------------- 2 files changed, 69 insertions(+), 111 deletions(-) diff --git a/ssl/tls13_client.c b/ssl/tls13_client.c index b31a6593..518c4dc8 100644 --- a/ssl/tls13_client.c +++ b/ssl/tls13_client.c @@ -40,8 +40,7 @@ enum client_hs_state_t { state_send_client_certificate, state_send_client_certificate_verify, state_complete_client_certificate_verify, - state_send_channel_id, - state_send_client_finished, + state_complete_second_flight, state_done, }; @@ -464,7 +463,7 @@ static enum ssl_hs_wait_t do_send_client_certificate(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; /* The peer didn't request a certificate. */ if (!hs->cert_request) { - hs->tls13_state = state_send_channel_id; + hs->tls13_state = state_complete_second_flight; return ssl_hs_ok; } @@ -496,13 +495,13 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL_HANDSHAKE *hs, SSL *const ssl = hs->ssl; /* Don't send CertificateVerify if there is no certificate. */ if (!ssl_has_certificate(ssl)) { - hs->tls13_state = state_send_channel_id; + hs->tls13_state = state_complete_second_flight; return ssl_hs_ok; } switch (tls13_prepare_certificate_verify(hs, is_first_run)) { case ssl_private_key_success: - hs->tls13_state = state_send_channel_id; + hs->tls13_state = state_complete_second_flight; return ssl_hs_ok; case ssl_private_key_retry: @@ -517,39 +516,35 @@ static enum ssl_hs_wait_t do_send_client_certificate_verify(SSL_HANDSHAKE *hs, return ssl_hs_error; } -static enum ssl_hs_wait_t do_send_channel_id(SSL_HANDSHAKE *hs) { +static enum ssl_hs_wait_t do_complete_second_flight(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (!ssl->s3->tlsext_channel_id_valid) { - hs->tls13_state = state_send_client_finished; - return ssl_hs_ok; + + /* Send a Channel ID assertion if necessary. */ + if (ssl->s3->tlsext_channel_id_valid) { + if (!ssl_do_channel_id_callback(ssl)) { + hs->tls13_state = state_complete_second_flight; + return ssl_hs_error; + } + + if (ssl->tlsext_channel_id_private == NULL) { + return ssl_hs_channel_id_lookup; + } + + CBB cbb, body; + if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) || + !tls1_write_channel_id(ssl, &body) || + !ssl_add_message_cbb(ssl, &cbb)) { + CBB_cleanup(&cbb); + return ssl_hs_error; + } } - if (!ssl_do_channel_id_callback(ssl)) { - return ssl_hs_error; - } - - if (ssl->tlsext_channel_id_private == NULL) { - return ssl_hs_channel_id_lookup; - } - - CBB cbb, body; - if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_CHANNEL_ID) || - !tls1_write_channel_id(ssl, &body) || - !ssl_add_message_cbb(ssl, &cbb)) { - CBB_cleanup(&cbb); - return ssl_hs_error; - } - - hs->tls13_state = state_send_client_finished; - return ssl_hs_ok; -} - -static enum ssl_hs_wait_t do_send_client_finished(SSL_HANDSHAKE *hs) { - SSL *const ssl = hs->ssl; + /* Send a Finished message. */ if (!tls13_prepare_finished(hs)) { return ssl_hs_error; } + /* Derive the final keys and enable them. */ if (!tls13_set_traffic_key(ssl, evp_aead_open, hs->server_traffic_secret_0, hs->hash_len) || !tls13_set_traffic_key(ssl, evp_aead_seal, hs->client_traffic_secret_0, @@ -600,11 +595,8 @@ enum ssl_hs_wait_t tls13_client_handshake(SSL_HANDSHAKE *hs) { case state_complete_client_certificate_verify: ret = do_send_client_certificate_verify(hs, 0 /* complete */); break; - case state_send_channel_id: - ret = do_send_channel_id(hs); - break; - case state_send_client_finished: - ret = do_send_client_finished(hs); + case state_complete_second_flight: + ret = do_complete_second_flight(hs); break; case state_done: ret = ssl_hs_ok; diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index f5bdf535..c7096fc5 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c @@ -40,9 +40,6 @@ enum server_hs_state_t { state_send_hello_retry_request, state_process_second_client_hello, state_send_server_hello, - state_send_encrypted_extensions, - state_send_certificate_request, - state_send_server_certificate, state_send_server_certificate_verify, state_complete_server_certificate_verify, state_send_server_finished, @@ -390,6 +387,8 @@ static enum ssl_hs_wait_t do_process_second_client_hello(SSL_HANDSHAKE *hs) { static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; + + /* Send a ServerHello. */ CBB cbb, body, extensions; if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_SERVER_HELLO) || !CBB_add_u16(&body, ssl->version) || @@ -413,39 +412,23 @@ static enum ssl_hs_wait_t do_send_server_hello(SSL_HANDSHAKE *hs) { goto err; } - hs->tls13_state = state_send_encrypted_extensions; - return ssl_hs_ok; - -err: - CBB_cleanup(&cbb); - return ssl_hs_error; -} - -static enum ssl_hs_wait_t do_send_encrypted_extensions(SSL_HANDSHAKE *hs) { - SSL *const ssl = hs->ssl; + /* Derive and enable the handshake traffic secrets. */ if (!tls13_derive_handshake_secrets(hs) || !tls13_set_traffic_key(ssl, evp_aead_open, hs->client_handshake_secret, hs->hash_len) || !tls13_set_traffic_key(ssl, evp_aead_seal, hs->server_handshake_secret, hs->hash_len)) { - return ssl_hs_error; + goto err; } - CBB cbb, body; + /* Send EncryptedExtensions. */ if (!ssl->method->init_message(ssl, &cbb, &body, SSL3_MT_ENCRYPTED_EXTENSIONS) || !ssl_add_serverhello_tlsext(hs, &body) || !ssl_add_message_cbb(ssl, &cbb)) { - CBB_cleanup(&cbb); - return ssl_hs_error; + goto err; } - hs->tls13_state = state_send_certificate_request; - return ssl_hs_ok; -} - -static enum ssl_hs_wait_t do_send_certificate_request(SSL_HANDSHAKE *hs) { - SSL *const ssl = hs->ssl; /* Determine whether to request a client certificate. */ hs->cert_request = !!(ssl->verify_mode & SSL_VERIFY_PEER); /* CertificateRequest may only be sent in non-resumption handshakes. */ @@ -453,38 +436,50 @@ static enum ssl_hs_wait_t do_send_certificate_request(SSL_HANDSHAKE *hs) { hs->cert_request = 0; } - if (!hs->cert_request) { - /* Skip this state. */ - hs->tls13_state = state_send_server_certificate; - return ssl_hs_ok; - } + /* Send a CertificateRequest, if necessary. */ + if (hs->cert_request) { + CBB sigalgs_cbb; + if (!ssl->method->init_message(ssl, &cbb, &body, + SSL3_MT_CERTIFICATE_REQUEST) || + !CBB_add_u8(&body, 0 /* no certificate_request_context. */)) { + goto err; + } - CBB cbb, body, sigalgs_cbb; - if (!ssl->method->init_message(ssl, &cbb, &body, - SSL3_MT_CERTIFICATE_REQUEST) || - !CBB_add_u8(&body, 0 /* no certificate_request_context. */)) { - goto err; - } + const uint16_t *sigalgs; + size_t num_sigalgs = tls12_get_verify_sigalgs(ssl, &sigalgs); + if (!CBB_add_u16_length_prefixed(&body, &sigalgs_cbb)) { + goto err; + } - const uint16_t *sigalgs; - size_t num_sigalgs = tls12_get_verify_sigalgs(ssl, &sigalgs); - if (!CBB_add_u16_length_prefixed(&body, &sigalgs_cbb)) { - goto err; - } + for (size_t i = 0; i < num_sigalgs; i++) { + if (!CBB_add_u16(&sigalgs_cbb, sigalgs[i])) { + goto err; + } + } - for (size_t i = 0; i < num_sigalgs; i++) { - if (!CBB_add_u16(&sigalgs_cbb, sigalgs[i])) { + if (!ssl_add_client_CA_list(ssl, &body) || + !CBB_add_u16(&body, 0 /* empty certificate_extensions. */) || + !ssl_add_message_cbb(ssl, &cbb)) { goto err; } } - if (!ssl_add_client_CA_list(ssl, &body) || - !CBB_add_u16(&body, 0 /* empty certificate_extensions. */) || - !ssl_add_message_cbb(ssl, &cbb)) { - goto err; + /* Send the server Certificate message, if necessary. */ + if (!ssl->s3->session_reused) { + if (!ssl_has_certificate(ssl)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATE_SET); + goto err; + } + + if (!tls13_prepare_certificate(hs)) { + goto err; + } + + hs->tls13_state = state_send_server_certificate_verify; + return ssl_hs_ok; } - hs->tls13_state = state_send_server_certificate; + hs->tls13_state = state_send_server_finished; return ssl_hs_ok; err: @@ -492,26 +487,6 @@ err: return ssl_hs_error; } -static enum ssl_hs_wait_t do_send_server_certificate(SSL_HANDSHAKE *hs) { - SSL *const ssl = hs->ssl; - if (ssl->s3->session_reused) { - hs->tls13_state = state_send_server_finished; - return ssl_hs_ok; - } - - if (!ssl_has_certificate(ssl)) { - OPENSSL_PUT_ERROR(SSL, SSL_R_NO_CERTIFICATE_SET); - return ssl_hs_error; - } - - if (!tls13_prepare_certificate(hs)) { - return ssl_hs_error; - } - - hs->tls13_state = state_send_server_certificate_verify; - return ssl_hs_ok; -} - static enum ssl_hs_wait_t do_send_server_certificate_verify(SSL_HANDSHAKE *hs, int is_first_run) { switch (tls13_prepare_certificate_verify(hs, is_first_run)) { @@ -714,15 +689,6 @@ enum ssl_hs_wait_t tls13_server_handshake(SSL_HANDSHAKE *hs) { case state_send_server_hello: ret = do_send_server_hello(hs); break; - case state_send_encrypted_extensions: - ret = do_send_encrypted_extensions(hs); - break; - case state_send_certificate_request: - ret = do_send_certificate_request(hs); - break; - case state_send_server_certificate: - ret = do_send_server_certificate(hs); - break; case state_send_server_certificate_verify: ret = do_send_server_certificate_verify(hs, 1 /* first run */); break;