From 707af294a8af53160bd94c1dc788f8587092d997 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 10 Mar 2017 17:47:18 -0500 Subject: [PATCH] Support asynchronous ticket decryption with TLS 1.3. This shuffles a bit of the code around session resumption in TLS 1.3 to make the async point cleaner to inject. It also fills in cipher and tlsext_hostname more uniformly. Filling in the cipher on resumption is a no-op as SSL_SESSION_dup already copies it, but avoids confusion should we ever implement TLS 1.3's laxer cipher matching on the server. Not filling in tlsext_hostname on resumption was an oversight; the relevant check isn't whether we are resuming but whether we have a fresh SSL_SESSION to fill things into. Change-Id: Ic02eb079ff228ce4a4d3e0de7445e18cd367e8b2 Reviewed-on: https://boringssl-review.googlesource.com/14205 Commit-Queue: David Benjamin Reviewed-by: Adam Langley --- ssl/internal.h | 3 +- ssl/ssl_test.cc | 10 +- ssl/t1_lib.c | 27 +---- ssl/tls13_both.c | 5 + ssl/tls13_server.c | 268 +++++++++++++++++++++++++++------------------ 5 files changed, 180 insertions(+), 133 deletions(-) diff --git a/ssl/internal.h b/ssl/internal.h index b93a3e4b..99980d84 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -929,6 +929,7 @@ enum ssl_hs_wait_t { ssl_hs_x509_lookup, ssl_hs_channel_id_lookup, ssl_hs_private_key_operation, + ssl_hs_pending_ticket, }; struct ssl_handshake_st { @@ -1161,7 +1162,7 @@ int ssl_ext_key_share_add_serverhello(SSL_HANDSHAKE *hs, CBB *out); int ssl_ext_pre_shared_key_parse_serverhello(SSL_HANDSHAKE *hs, uint8_t *out_alert, CBS *contents); int ssl_ext_pre_shared_key_parse_clienthello( - SSL_HANDSHAKE *hs, SSL_SESSION **out_session, CBS *out_binders, + SSL_HANDSHAKE *hs, CBS *out_ticket, CBS *out_binders, uint32_t *out_obfuscated_ticket_age, uint8_t *out_alert, CBS *contents); int ssl_ext_pre_shared_key_add_serverhello(SSL_HANDSHAKE *hs, CBB *out); diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc index 719bdabc..6b150e8c 100644 --- a/ssl/ssl_test.cc +++ b/ssl/ssl_test.cc @@ -3427,6 +3427,7 @@ TEST_P(TicketAEADMethodTest, Resume) { SSL_CTX_set_session_cache_mode(server_ctx.get(), SSL_SESS_CACHE_BOTH); SSL_CTX_set_current_time_cb(client_ctx.get(), FrozenTimeCallback); SSL_CTX_set_current_time_cb(server_ctx.get(), FrozenTimeCallback); + SSL_CTX_sess_set_new_cb(client_ctx.get(), SaveLastSession); SSL_CTX_set_ticket_aead_method(server_ctx.get(), &kSSLTestTicketMethod); @@ -3447,10 +3448,13 @@ TEST_P(TicketAEADMethodTest, Resume) { EXPECT_FALSE(SSL_session_reused(client.get())); EXPECT_FALSE(SSL_session_reused(server.get())); - SSL_SESSION *session = SSL_get_session(client.get()); + // Run the read loop to account for post-handshake tickets in TLS 1.3. + SSL_read(client.get(), nullptr, 0); + + bssl::UniquePtr session = std::move(g_last_session); ConnectClientAndServerWithTicketMethod(&client, &server, client_ctx.get(), server_ctx.get(), retry_count, - failure_mode, session); + failure_mode, session.get()); switch (failure_mode) { case ssl_test_ticket_aead_ok: ASSERT_TRUE(client); @@ -3473,7 +3477,7 @@ TEST_P(TicketAEADMethodTest, Resume) { INSTANTIATE_TEST_CASE_P( TicketAEADMethodTests, TicketAEADMethodTest, testing::Combine( - testing::Values(TLS1_2_VERSION /*, TLS1_3_VERSION TODO: enable */), + testing::Values(TLS1_2_VERSION, TLS1_3_VERSION), testing::Values(0, 1, 2), testing::Values(ssl_test_ticket_aead_ok, ssl_test_ticket_aead_seal_fail, diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index e3d0a9e3..759d87bf 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1958,13 +1958,12 @@ int ssl_ext_pre_shared_key_parse_serverhello(SSL_HANDSHAKE *hs, } int ssl_ext_pre_shared_key_parse_clienthello( - SSL_HANDSHAKE *hs, SSL_SESSION **out_session, CBS *out_binders, + SSL_HANDSHAKE *hs, CBS *out_ticket, CBS *out_binders, uint32_t *out_obfuscated_ticket_age, uint8_t *out_alert, CBS *contents) { - SSL *const ssl = hs->ssl; /* We only process the first PSK identity since we don't support pure PSK. */ - CBS identities, ticket, binders; + CBS identities, binders; if (!CBS_get_u16_length_prefixed(contents, &identities) || - !CBS_get_u16_length_prefixed(&identities, &ticket) || + !CBS_get_u16_length_prefixed(&identities, out_ticket) || !CBS_get_u32(&identities, out_obfuscated_ticket_age) || !CBS_get_u16_length_prefixed(contents, &binders) || CBS_len(&binders) == 0 || @@ -2011,26 +2010,6 @@ int ssl_ext_pre_shared_key_parse_clienthello( return 0; } - /* TODO(svaldez): Check that the ticket_age is valid when attempting to use - * the PSK for 0-RTT. http://crbug.com/boringssl/113 */ - - /* TLS 1.3 session tickets are renewed separately as part of the - * NewSessionTicket. */ - int unused_renew; - switch (ssl_process_ticket(ssl, out_session, &unused_renew, CBS_data(&ticket), - CBS_len(&ticket), NULL, 0)) { - case ssl_ticket_aead_success: - break; - case ssl_ticket_aead_ignore_ticket: - assert(*out_session == NULL); - break; - case ssl_ticket_aead_retry: - /* TODO: async tickets for TLS 1.3. */ - case ssl_ticket_aead_error: - *out_alert = SSL_AD_INTERNAL_ERROR; - return 0; - } - return 1; } diff --git a/ssl/tls13_both.c b/ssl/tls13_both.c index fc7e2c91..62439235 100644 --- a/ssl/tls13_both.c +++ b/ssl/tls13_both.c @@ -79,6 +79,11 @@ int tls13_handshake(SSL_HANDSHAKE *hs) { hs->wait = ssl_hs_ok; return -1; + case ssl_hs_pending_ticket: + ssl->rwstate = SSL_PENDING_TICKET; + hs->wait = ssl_hs_ok; + return -1; + case ssl_hs_ok: break; } diff --git a/ssl/tls13_server.c b/ssl/tls13_server.c index 4136a693..9c8d1a15 100644 --- a/ssl/tls13_server.c +++ b/ssl/tls13_server.c @@ -36,6 +36,7 @@ static const size_t kMaxEarlyDataAccepted = 14336; enum server_hs_state_t { state_select_parameters = 0, + state_select_session, state_send_hello_retry_request, state_process_second_client_hello, state_send_server_hello, @@ -134,6 +135,8 @@ static const SSL_CIPHER *choose_tls13_cipher( } static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) { + /* At this point, most ClientHello extensions have already been processed by + * the common handshake logic. Resolve the remaining non-PSK parameters. */ SSL *const ssl = hs->ssl; SSL_CLIENT_HELLO client_hello; @@ -152,6 +155,14 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) { return ssl_hs_error; } + /* HTTP/2 negotiation depends on the cipher suite, so ALPN negotiation was + * deferred. Complete it now. */ + uint8_t alert = SSL_AD_DECODE_ERROR; + if (!ssl_negotiate_alpn(hs, &alert, &client_hello)) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); + return ssl_hs_error; + } + /* The PRF hash is now known. Set up the key schedule and hash the * ClientHello. */ if (!tls13_init_key_schedule(hs) || @@ -159,123 +170,159 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) { return ssl_hs_error; } + hs->tls13_state = state_select_session; + return ssl_hs_ok; +} + +static enum ssl_ticket_aead_result_t select_session( + SSL_HANDSHAKE *hs, uint8_t *out_alert, SSL_SESSION **out_session, + int32_t *out_ticket_age_skew, const SSL_CLIENT_HELLO *client_hello) { + SSL *const ssl = hs->ssl; + *out_session = NULL; + + /* Decode the ticket if we agreed on a PSK key exchange mode. */ + CBS pre_shared_key; + if (!hs->accept_psk_mode || + !ssl_client_hello_get_extension(client_hello, &pre_shared_key, + TLSEXT_TYPE_pre_shared_key)) { + return ssl_ticket_aead_ignore_ticket; + } + + /* Verify that the pre_shared_key extension is the last extension in + * ClientHello. */ + if (CBS_data(&pre_shared_key) + CBS_len(&pre_shared_key) != + client_hello->extensions + client_hello->extensions_len) { + OPENSSL_PUT_ERROR(SSL, SSL_R_PRE_SHARED_KEY_MUST_BE_LAST); + *out_alert = SSL_AD_ILLEGAL_PARAMETER; + return ssl_ticket_aead_error; + } + + CBS ticket, binders; + uint32_t client_ticket_age; + if (!ssl_ext_pre_shared_key_parse_clienthello(hs, &ticket, &binders, + &client_ticket_age, out_alert, + &pre_shared_key)) { + return ssl_ticket_aead_error; + } + + /* TLS 1.3 session tickets are renewed separately as part of the + * NewSessionTicket. */ + int unused_renew; + SSL_SESSION *session = NULL; + enum ssl_ticket_aead_result_t ret = + ssl_process_ticket(ssl, &session, &unused_renew, CBS_data(&ticket), + CBS_len(&ticket), NULL, 0); + switch (ret) { + case ssl_ticket_aead_success: + break; + case ssl_ticket_aead_error: + *out_alert = SSL_AD_INTERNAL_ERROR; + return ret; + default: + return ret; + } + + if (!ssl_session_is_resumable(hs, session) || + /* Historically, some TLS 1.3 tickets were missing ticket_age_add. */ + !session->ticket_age_add_valid) { + SSL_SESSION_free(session); + return ssl_ticket_aead_ignore_ticket; + } + + /* Recover the client ticket age and convert to seconds. */ + client_ticket_age -= session->ticket_age_add; + client_ticket_age /= 1000; + + struct OPENSSL_timeval now; + ssl_get_current_time(ssl, &now); + + /* Compute the server ticket age in seconds. */ + assert(now.tv_sec >= session->time); + uint64_t server_ticket_age = now.tv_sec - session->time; + + /* To avoid overflowing |hs->ticket_age_skew|, we will not resume + * 68-year-old sessions. */ + if (server_ticket_age > INT32_MAX) { + SSL_SESSION_free(session); + return ssl_ticket_aead_ignore_ticket; + } + + /* TODO(davidben,svaldez): Measure this value to decide on tolerance. For + * now, accept all values. https://crbug.com/boringssl/113. */ + *out_ticket_age_skew = + (int32_t)client_ticket_age - (int32_t)server_ticket_age; + + /* Check the PSK binder. */ + if (!tls13_verify_psk_binder(hs, session, &binders)) { + SSL_SESSION_free(session); + *out_alert = SSL_AD_DECRYPT_ERROR; + return ssl_ticket_aead_error; + } + + *out_session = session; + return ssl_ticket_aead_success; +} + +static enum ssl_hs_wait_t do_select_session(SSL_HANDSHAKE *hs) { + SSL *const ssl = hs->ssl; + SSL_CLIENT_HELLO client_hello; + if (!ssl_client_hello_init(ssl, &client_hello, ssl->init_msg, + ssl->init_num)) { + OPENSSL_PUT_ERROR(SSL, SSL_R_CLIENTHELLO_PARSE_FAILED); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECODE_ERROR); + return ssl_hs_error; + } - /* Decode the ticket if we agree on a PSK key exchange mode. */ uint8_t alert = SSL_AD_DECODE_ERROR; SSL_SESSION *session = NULL; - uint32_t client_ticket_age = 0; - CBS pre_shared_key, binders; - if (hs->accept_psk_mode && - ssl_client_hello_get_extension(&client_hello, &pre_shared_key, - TLSEXT_TYPE_pre_shared_key)) { - /* Verify that the pre_shared_key extension is the last extension in - * ClientHello. */ - if (CBS_data(&pre_shared_key) + CBS_len(&pre_shared_key) != - client_hello.extensions + client_hello.extensions_len) { - OPENSSL_PUT_ERROR(SSL, SSL_R_PRE_SHARED_KEY_MUST_BE_LAST); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER); - return ssl_hs_error; - } - - if (!ssl_ext_pre_shared_key_parse_clienthello(hs, &session, &binders, - &client_ticket_age, &alert, - &pre_shared_key)) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); - return ssl_hs_error; - } - } - - if (session != NULL && - (!ssl_session_is_resumable(hs, session) || - /* Historically, some TLS 1.3 tickets were missing ticket_age_add. */ - !session->ticket_age_add_valid)) { - SSL_SESSION_free(session); - session = NULL; - } - - if (session != NULL) { - /* Recover the client ticket age and convert to seconds. */ - client_ticket_age -= session->ticket_age_add; - client_ticket_age /= 1000; - - struct OPENSSL_timeval now; - ssl_get_current_time(ssl, &now); - - /* Compute the server ticket age in seconds. */ - assert(now.tv_sec >= session->time); - uint64_t server_ticket_age = now.tv_sec - session->time; - - /* To avoid overflowing |hs->ticket_age_skew|, we will not resume - * 68-year-old sessions. */ - if (server_ticket_age > INT32_MAX) { - SSL_SESSION_free(session); - session = NULL; - } else { - /* TODO(davidben,svaldez): Measure this value to decide on tolerance. For - * now, accept all values. https://crbug.com/boringssl/113. */ - ssl->s3->ticket_age_skew = - (int32_t)client_ticket_age - (int32_t)server_ticket_age; - } - } - - /* Set up the new session, either using the original one as a template or - * creating a fresh one. */ - if (session == NULL) { - if (!ssl_get_new_session(hs, 1 /* server */)) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return ssl_hs_error; - } - - hs->new_session->cipher = hs->new_cipher; - - /* On new sessions, stash the SNI value in the session. */ - if (hs->hostname != NULL) { - OPENSSL_free(hs->new_session->tlsext_hostname); - hs->new_session->tlsext_hostname = BUF_strdup(hs->hostname); - if (hs->new_session->tlsext_hostname == NULL) { + switch (select_session(hs, &alert, &session, &ssl->s3->ticket_age_skew, + &client_hello)) { + case ssl_ticket_aead_ignore_ticket: + assert(session == NULL); + if (!ssl_get_new_session(hs, 1 /* server */)) { ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return ssl_hs_error; } - } - } else { - /* Check the PSK binder. */ - if (!tls13_verify_psk_binder(hs, session, &binders)) { - SSL_SESSION_free(session); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_DECRYPT_ERROR); - return ssl_hs_error; - } + break; - /* Only authentication information carries over in TLS 1.3. */ - hs->new_session = SSL_SESSION_dup(session, SSL_SESSION_DUP_AUTH_ONLY); - if (hs->new_session == NULL) { + case ssl_ticket_aead_success: + /* Carry over authentication information from the previous handshake into + * a fresh session. */ + hs->new_session = SSL_SESSION_dup(session, SSL_SESSION_DUP_AUTH_ONLY); + SSL_SESSION_free(session); + if (hs->new_session == NULL) { + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + return ssl_hs_error; + } + + ssl->s3->session_reused = 1; + + /* Resumption incorporates fresh key material, so refresh the timeout. */ + ssl_session_renew_timeout(ssl, hs->new_session, + ssl->session_ctx->session_psk_dhe_timeout); + break; + + case ssl_ticket_aead_error: + ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); + return ssl_hs_error; + + case ssl_ticket_aead_retry: + hs->tls13_state = state_select_session; + return ssl_hs_pending_ticket; + } + + /* Record connection properties in the new session. */ + hs->new_session->cipher = hs->new_cipher; + + if (hs->hostname != NULL) { + OPENSSL_free(hs->new_session->tlsext_hostname); + hs->new_session->tlsext_hostname = BUF_strdup(hs->hostname); + if (hs->new_session->tlsext_hostname == NULL) { ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); return ssl_hs_error; } - ssl->s3->session_reused = 1; - SSL_SESSION_free(session); - - /* Resumption incorporates fresh key material, so refresh the timeout. */ - ssl_session_renew_timeout(ssl, hs->new_session, - ssl->session_ctx->session_psk_dhe_timeout); } - if (ssl->ctx->dos_protection_cb != NULL && - ssl->ctx->dos_protection_cb(&client_hello) == 0) { - /* Connection rejected for DOS reasons. */ - OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED); - ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); - return ssl_hs_error; - } - - /* HTTP/2 negotiation depends on the cipher suite, so ALPN negotiation was - * deferred. Complete it now. */ - alert = SSL_AD_DECODE_ERROR; - if (!ssl_negotiate_alpn(hs, &alert, &client_hello)) { - ssl3_send_alert(ssl, SSL3_AL_FATAL, alert); - return ssl_hs_error; - } - - /* Store the initial negotiated ALPN in the session. */ if (ssl->s3->alpn_selected != NULL) { hs->new_session->early_alpn = BUF_memdup(ssl->s3->alpn_selected, ssl->s3->alpn_selected_len); @@ -286,6 +333,14 @@ static enum ssl_hs_wait_t do_select_parameters(SSL_HANDSHAKE *hs) { hs->new_session->early_alpn_len = ssl->s3->alpn_selected_len; } + if (ssl->ctx->dos_protection_cb != NULL && + ssl->ctx->dos_protection_cb(&client_hello) == 0) { + /* Connection rejected for DOS reasons. */ + OPENSSL_PUT_ERROR(SSL, SSL_R_CONNECTION_REJECTED); + ssl3_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR); + return ssl_hs_error; + } + /* Incorporate the PSK into the running secret. */ if (ssl->s3->session_reused) { if (!tls13_advance_key_schedule(hs, hs->new_session->master_key, @@ -648,6 +703,9 @@ enum ssl_hs_wait_t tls13_server_handshake(SSL_HANDSHAKE *hs) { case state_select_parameters: ret = do_select_parameters(hs); break; + case state_select_session: + ret = do_select_session(hs); + break; case state_send_hello_retry_request: ret = do_send_hello_retry_request(hs); break;