Add an early return in TlsHandshaker::AdvanceHandshake if connection is closed after SSL_do_handshake. This is intended to fix https://bugs.chromium.org/p/chromium/issues/detail?id=1305526, however we don't have hard evidence to show when the connection is closed in the bug. Protected by FLAGS_quic_reloadable_flag_quic_tls_handshaker_check_connection_closed. PiperOrigin-RevId: 452390558
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 62407d4..916d885 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -23,6 +23,8 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_enable_mtu_discovery_at_server, false) // If true, QuicGsoBatchWriter will support release time if it is available and the process has the permission to do so. QUIC_FLAG(FLAGS_quic_restart_flag_quic_support_release_time_for_gso, false) +// If true, TlsHandshaker::AdvanceHandshake will check if connection is closed after SSL_do_handshake. +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_tls_handshaker_check_connection_closed, true) // If true, abort async QPACK header decompression in QuicSpdyStream::Reset() and in QuicSpdyStream::OnStreamReset(). QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_abort_qpack_on_stream_reset, true) // If true, ack frequency frame can be sent from server to client.
diff --git a/quiche/quic/core/tls_handshaker.cc b/quiche/quic/core/tls_handshaker.cc index 15b4d4f..0449c67 100644 --- a/quiche/quic/core/tls_handshaker.cc +++ b/quiche/quic/core/tls_handshaker.cc
@@ -85,7 +85,7 @@ } void TlsHandshaker::AdvanceHandshake() { - if (is_connection_closed_) { + if (is_connection_closed()) { return; } if (GetHandshakeState() >= HANDSHAKE_COMPLETE) { @@ -101,6 +101,13 @@ QUIC_VLOG(1) << ENDPOINT << "Continuing handshake"; int rv = SSL_do_handshake(ssl()); + if (GetQuicReloadableFlag(quic_tls_handshaker_check_connection_closed) && + is_connection_closed()) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_tls_handshaker_check_connection_closed, 1, + 2); + return; + } + // If SSL_do_handshake return success(1) and we are in early data, it is // possible that we have provided ServerHello to BoringSSL but it hasn't been // processed. Retry SSL_do_handshake once will advance the handshake more in @@ -109,6 +116,14 @@ if (rv == 1 && SSL_in_early_data(ssl())) { OnEnterEarlyData(); rv = SSL_do_handshake(ssl()); + + if (GetQuicReloadableFlag(quic_tls_handshaker_check_connection_closed) && + is_connection_closed()) { + QUIC_RELOADABLE_FLAG_COUNT_N(quic_tls_handshaker_check_connection_closed, + 2, 2); + return; + } + QUIC_VLOG(1) << ENDPOINT << "SSL_do_handshake returned when entering early data. After " << "retry, rv=" << rv @@ -120,7 +135,7 @@ // SSL_in_early_data should be false. // // In either case, it should not both return 1 and stay in early data. - if (rv == 1 && SSL_in_early_data(ssl()) && !is_connection_closed_) { + if (rv == 1 && SSL_in_early_data(ssl()) && !is_connection_closed()) { QUIC_BUG(quic_handshaker_stay_in_early_data) << "The original and the retry of SSL_do_handshake both returned " "success and in early data"; @@ -139,7 +154,7 @@ return; } if (ShouldCloseConnectionOnUnexpectedError(ssl_error) && - !is_connection_closed_) { + !is_connection_closed()) { QUIC_VLOG(1) << "SSL_do_handshake failed; SSL_get_error returns " << ssl_error; ERR_print_errors_fp(stderr);