Simplify states with hs_wait_t returns. Change-Id: Ie0014bf73625144503b649e84b43ca4b03a4df1f Reviewed-on: https://boringssl-review.googlesource.com/19704 Reviewed-by: Steven Valdez <svaldez@google.com> Commit-Queue: Steven Valdez <svaldez@google.com> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
diff --git a/ssl/handshake_client.cc b/ssl/handshake_client.cc index b1c48d8..37bf372 100644 --- a/ssl/handshake_client.cc +++ b/ssl/handshake_client.cc
@@ -175,7 +175,6 @@ enum ssl_client_hs_state_t { state_start_connect = 0, - state_send_client_hello, state_enter_early_data, state_read_hello_verify_request, state_read_server_hello, @@ -430,20 +429,9 @@ } static enum ssl_hs_wait_t do_start_connect(SSL_HANDSHAKE *hs) { - ssl_do_info_callback(hs->ssl, SSL_CB_HANDSHAKE_START, 1); - hs->state = state_send_client_hello; - return ssl_hs_ok; -} - -static enum ssl_hs_wait_t do_send_client_hello(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - /* The handshake buffer is reset on every ClientHello. Notably, in DTLS, we - * may send multiple ClientHellos if we receive HelloVerifyRequest. */ - if (!hs->transcript.Init()) { - OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR); - return ssl_hs_error; - } + ssl_do_info_callback(ssl, SSL_CB_HANDSHAKE_START, 1); /* Freeze the version range. */ if (!ssl_get_version_range(ssl, &hs->min_version, &hs->max_version)) { @@ -474,10 +462,7 @@ } } - /* If resending the ClientHello in DTLS after a HelloVerifyRequest, don't - * renegerate the client_random. The random must be reused. */ - if ((!SSL_is_dtls(ssl) || !ssl->d1->send_cookie) && - !RAND_bytes(ssl->s3->client_random, sizeof(ssl->s3->client_random))) { + if (!RAND_bytes(ssl->s3->client_random, sizeof(ssl->s3->client_random))) { return ssl_hs_error; } @@ -501,7 +486,7 @@ static enum ssl_hs_wait_t do_enter_early_data(SSL_HANDSHAKE *hs) { SSL *const ssl = hs->ssl; - if (SSL_is_dtls(ssl) && !ssl->d1->send_cookie) { + if (SSL_is_dtls(ssl)) { hs->state = state_read_hello_verify_request; return ssl_hs_ok; } @@ -542,7 +527,6 @@ } if (msg.type != DTLS1_MT_HELLO_VERIFY_REQUEST) { - ssl->d1->send_cookie = false; hs->state = state_read_server_hello; return ssl_hs_ok; } @@ -561,10 +545,19 @@ OPENSSL_memcpy(ssl->d1->cookie, CBS_data(&cookie), CBS_len(&cookie)); ssl->d1->cookie_len = CBS_len(&cookie); - ssl->d1->send_cookie = true; ssl->method->next_message(ssl); - hs->state = state_send_client_hello; - return ssl_hs_ok; + + /* DTLS resets the handshake buffer after HelloVerifyRequest. */ + if (!hs->transcript.Init()) { + return ssl_hs_error; + } + + if (!ssl_write_client_hello(hs)) { + return ssl_hs_error; + } + + hs->state = state_read_server_hello; + return ssl_hs_flush; } static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) { @@ -1733,9 +1726,6 @@ case state_start_connect: ret = do_start_connect(hs); break; - case state_send_client_hello: - ret = do_send_client_hello(hs); - break; case state_enter_early_data: ret = do_enter_early_data(hs); break; @@ -1823,8 +1813,6 @@ switch (state) { case state_start_connect: return "TLS client start_connect"; - case state_send_client_hello: - return "TLS client send_client_hello"; case state_enter_early_data: return "TLS client enter_early_data"; case state_read_hello_verify_request:
diff --git a/ssl/handshake_server.cc b/ssl/handshake_server.cc index b1df9f9..efbb7bd 100644 --- a/ssl/handshake_server.cc +++ b/ssl/handshake_server.cc
@@ -580,16 +580,10 @@ /* Determine whether we are doing session resumption. */ UniquePtr<SSL_SESSION> session; int tickets_supported = 0, renew_ticket = 0; - switch (ssl_get_prev_session(ssl, &session, &tickets_supported, &renew_ticket, - &client_hello)) { - case ssl_session_success: - break; - case ssl_session_error: - return ssl_hs_error; - case ssl_session_retry: - return ssl_hs_pending_session; - case ssl_session_ticket_retry: - return ssl_hs_pending_ticket; + enum ssl_hs_wait_t wait = ssl_get_prev_session( + ssl, &session, &tickets_supported, &renew_ticket, &client_hello); + if (wait != ssl_hs_ok) { + return wait; } if (session) {
diff --git a/ssl/internal.h b/ssl/internal.h index d3efdae..393cd5e 100644 --- a/ssl/internal.h +++ b/ssl/internal.h
@@ -1814,10 +1814,6 @@ }; struct DTLS1_STATE { - /* send_cookie is true if we are resending the ClientHello with a cookie from - * a HelloVerifyRequest. */ - bool send_cookie:1; - /* has_change_cipher_spec is true if we have received a ChangeCipherSpec from * the peer in this epoch. */ bool has_change_cipher_spec:1; @@ -2109,22 +2105,17 @@ void ssl_set_session(SSL *ssl, SSL_SESSION *session); -enum ssl_session_result_t { - ssl_session_success, - ssl_session_error, - ssl_session_retry, - ssl_session_ticket_retry, -}; - /* ssl_get_prev_session looks up the previous session based on |client_hello|. * On success, it sets |*out_session| to the session or nullptr if none was * found. If the session could not be looked up synchronously, it returns - * |ssl_session_retry| and should be called again. If a ticket could not be - * decrypted immediately it returns |ssl_session_ticket_retry| and should also - * be called again. Otherwise, it returns |ssl_session_error|. */ -enum ssl_session_result_t ssl_get_prev_session( - SSL *ssl, UniquePtr<SSL_SESSION> *out_session, int *out_tickets_supported, - int *out_renew_ticket, const SSL_CLIENT_HELLO *client_hello); + * |ssl_hs_pending_session| and should be called again. If a ticket could not be + * decrypted immediately it returns |ssl_hs_pending_ticket| and should also + * be called again. Otherwise, it returns |ssl_hs_error|. */ +enum ssl_hs_wait_t ssl_get_prev_session(SSL *ssl, + UniquePtr<SSL_SESSION> *out_session, + int *out_tickets_supported, + int *out_renew_ticket, + const SSL_CLIENT_HELLO *client_hello); /* The following flags determine which parts of the session are duplicated. */ #define SSL_SESSION_DUP_AUTH_ONLY 0x0
diff --git a/ssl/ssl_session.cc b/ssl/ssl_session.cc index 07329d0..e92f452 100644 --- a/ssl/ssl_session.cc +++ b/ssl/ssl_session.cc
@@ -660,13 +660,13 @@ /* ssl_lookup_session looks up |session_id| in the session cache and sets * |*out_session| to an |SSL_SESSION| object if found. */ -static enum ssl_session_result_t ssl_lookup_session( +static enum ssl_hs_wait_t ssl_lookup_session( SSL *ssl, UniquePtr<SSL_SESSION> *out_session, const uint8_t *session_id, size_t session_id_len) { out_session->reset(); if (session_id_len == 0 || session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH) { - return ssl_session_success; + return ssl_hs_ok; } UniquePtr<SSL_SESSION> session; @@ -695,12 +695,12 @@ session_id_len, ©)); if (!session) { - return ssl_session_success; + return ssl_hs_ok; } if (session.get() == SSL_magic_pending_session_ptr()) { session.release(); // This pointer is not actually owned. - return ssl_session_retry; + return ssl_hs_pending_session; } /* Increment reference count now if the session callback asks us to do so @@ -725,12 +725,14 @@ } *out_session = std::move(session); - return ssl_session_success; + return ssl_hs_ok; } -enum ssl_session_result_t ssl_get_prev_session( - SSL *ssl, UniquePtr<SSL_SESSION> *out_session, int *out_tickets_supported, - int *out_renew_ticket, const SSL_CLIENT_HELLO *client_hello) { +enum ssl_hs_wait_t ssl_get_prev_session(SSL *ssl, + UniquePtr<SSL_SESSION> *out_session, + int *out_tickets_supported, + int *out_renew_ticket, + const SSL_CLIENT_HELLO *client_hello) { /* This is used only by servers. */ assert(ssl->server); UniquePtr<SSL_SESSION> session; @@ -754,15 +756,15 @@ assert(!session); break; case ssl_ticket_aead_error: - return ssl_session_error; + return ssl_hs_error; case ssl_ticket_aead_retry: - return ssl_session_ticket_retry; + return ssl_hs_pending_ticket; } } else { /* The client didn't send a ticket, so the session ID is a real ID. */ - enum ssl_session_result_t lookup_ret = ssl_lookup_session( + enum ssl_hs_wait_t lookup_ret = ssl_lookup_session( ssl, &session, client_hello->session_id, client_hello->session_id_len); - if (lookup_ret != ssl_session_success) { + if (lookup_ret != ssl_hs_ok) { return lookup_ret; } } @@ -770,7 +772,7 @@ *out_session = std::move(session); *out_tickets_supported = tickets_supported; *out_renew_ticket = renew_ticket; - return ssl_session_success; + return ssl_hs_ok; } static int remove_session_lock(SSL_CTX *ctx, SSL_SESSION *session, int lock) {