Add more specific error codes for QUIC handshake failures. This is to help with debugging the QUIC_HANDSHAKE_FAILED issues with Kyber. Changing error code for handshake failure is safe give our recent experience from cl/678914877. PiperOrigin-RevId: 683401678
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc index 8732d4b..6ecfbd2 100644 --- a/quiche/quic/core/http/end_to_end_test.cc +++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -5306,7 +5306,8 @@ client_.reset(CreateQuicClient(client_writer_)); EXPECT_EQ("", client_->SendSynchronousRequest("/foo")); - EXPECT_THAT(client_->connection_error(), IsError(QUIC_HANDSHAKE_FAILED)); + EXPECT_THAT(client_->connection_error(), + IsError(QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE)); } // Regression test for b/116200989.
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc index 877bb31..b14d1e3 100644 --- a/quiche/quic/core/quic_dispatcher.cc +++ b/quiche/quic/core/quic_dispatcher.cc
@@ -515,7 +515,7 @@ packet_info.self_address, packet_info.peer_address, packet_info.destination_connection_id, packet_info.form, packet_info.version_flag, packet_info.use_length_prefix, - packet_info.version, QUIC_HANDSHAKE_FAILED, + packet_info.version, QUIC_HANDSHAKE_FAILED_REJECTING_ALL_CONNECTIONS, "Stop accepting new connections", quic::QuicTimeWaitListManager::SEND_STATELESS_RESET); // Time wait list will reject the packet correspondingly.. @@ -563,7 +563,8 @@ // |connection_close_error_code| is used if the final packet fate is // kFateTimeWait. - QuicErrorCode connection_close_error_code = QUIC_HANDSHAKE_FAILED; + QuicErrorCode connection_close_error_code = + QUIC_HANDSHAKE_FAILED_INVALID_CONNECTION; // If a fatal TLS alert was received when extracting Client Hello, // |tls_alert_error_detail| will be set and will be used as the error_details @@ -582,7 +583,8 @@ // Fatal TLS alert when parsing Client Hello. fate = kFateTimeWait; uint8_t tls_alert = *extract_chlo_result.tls_alert; - connection_close_error_code = TlsAlertToQuicErrorCode(tls_alert); + connection_close_error_code = TlsAlertToQuicErrorCode(tls_alert).value_or( + connection_close_error_code); tls_alert_error_detail = absl::StrCat("TLS handshake failure from dispatcher (", EncryptionLevelToString(ENCRYPTION_INITIAL), ") ", @@ -810,7 +812,7 @@ connection->version(), /*last_sent_packet_number=*/QuicPacketNumber(), helper_.get(), time_wait_list_manager_.get()); terminator.CloseConnection( - QUIC_HANDSHAKE_FAILED, + QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE, "Connection is closed by server before handshake confirmed", /*ietf_quic=*/true, connection->GetActiveServerConnectionIds()); return; @@ -1500,7 +1502,8 @@ self_address, peer_address, original_connection_id, IETF_QUIC_LONG_HEADER_PACKET, /*version_flag=*/true, version.HasLengthPrefixedConnectionIds(), version, - QUIC_HANDSHAKE_FAILED, "Connection ID collision, please retry", + QUIC_HANDSHAKE_FAILED_CID_COLLISION, + "Connection ID collision, please retry", QuicTimeWaitListManager::SEND_CONNECTION_CLOSE_PACKETS); // Caller is responsible for erasing the connection from the buffered store,
diff --git a/quiche/quic/core/quic_error_codes.cc b/quiche/quic/core/quic_error_codes.cc index b5bee26..3863584 100644 --- a/quiche/quic/core/quic_error_codes.cc +++ b/quiche/quic/core/quic_error_codes.cc
@@ -98,6 +98,10 @@ RETURN_STRING_LITERAL(QUIC_HANDSHAKE_FAILED); RETURN_STRING_LITERAL(QUIC_HANDSHAKE_FAILED_PACKETS_BUFFERED_TOO_LONG); RETURN_STRING_LITERAL(QUIC_HANDSHAKE_FAILED_INVALID_HOSTNAME); + RETURN_STRING_LITERAL(QUIC_HANDSHAKE_FAILED_REJECTING_ALL_CONNECTIONS); + RETURN_STRING_LITERAL(QUIC_HANDSHAKE_FAILED_INVALID_CONNECTION); + RETURN_STRING_LITERAL(QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE); + RETURN_STRING_LITERAL(QUIC_HANDSHAKE_FAILED_CID_COLLISION); RETURN_STRING_LITERAL(QUIC_CRYPTO_TAGS_OUT_OF_ORDER); RETURN_STRING_LITERAL(QUIC_CRYPTO_TOO_MANY_ENTRIES); RETURN_STRING_LITERAL(QUIC_CRYPTO_TOO_MANY_REJECTS); @@ -805,6 +809,14 @@ return {true, static_cast<uint64_t>(NO_IETF_QUIC_ERROR)}; case QUIC_HANDSHAKE_FAILED_INVALID_HOSTNAME: return {true, static_cast<uint64_t>(NO_IETF_QUIC_ERROR)}; + case QUIC_HANDSHAKE_FAILED_REJECTING_ALL_CONNECTIONS: + return {true, static_cast<uint64_t>(NO_IETF_QUIC_ERROR)}; + case QUIC_HANDSHAKE_FAILED_INVALID_CONNECTION: + return {true, static_cast<uint64_t>(NO_IETF_QUIC_ERROR)}; + case QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE: + return {true, static_cast<uint64_t>(NO_IETF_QUIC_ERROR)}; + case QUIC_HANDSHAKE_FAILED_CID_COLLISION: + return {true, static_cast<uint64_t>(NO_IETF_QUIC_ERROR)}; case QUIC_LAST_ERROR: return {false, static_cast<uint64_t>(QUIC_LAST_ERROR)}; } @@ -812,7 +824,7 @@ return {true, static_cast<uint64_t>(INTERNAL_ERROR)}; } -QuicErrorCode TlsAlertToQuicErrorCode(uint8_t desc) { +std::optional<QuicErrorCode> TlsAlertToQuicErrorCode(uint8_t desc) { switch (desc) { case SSL_AD_BAD_CERTIFICATE: return QUIC_TLS_BAD_CERTIFICATE; @@ -831,7 +843,7 @@ case SSL_AD_CERTIFICATE_REQUIRED: return QUIC_TLS_CERTIFICATE_REQUIRED; default: - return QUIC_HANDSHAKE_FAILED; + return std::nullopt; } }
diff --git a/quiche/quic/core/quic_error_codes.h b/quiche/quic/core/quic_error_codes.h index 19c806a..d37da04 100644 --- a/quiche/quic/core/quic_error_codes.h +++ b/quiche/quic/core/quic_error_codes.h
@@ -7,6 +7,7 @@ #include <cstdint> #include <limits> +#include <optional> #include <string> #include "quiche/quic/platform/api/quic_export.h" @@ -255,6 +256,20 @@ // Handshake failed due to invalid hostname in ClientHello. Only sent from // server. QUIC_HANDSHAKE_FAILED_INVALID_HOSTNAME = 216, + // Handshake failed because server is rejecting all connections. Only sent + // from server. + QUIC_HANDSHAKE_FAILED_REJECTING_ALL_CONNECTIONS = 217, + // Handshake failed because the connection failed validity checks in + // QuicDispatcher. Only sent from server. + QUIC_HANDSHAKE_FAILED_INVALID_CONNECTION = 218, + // Handshake failed because + // 1) There used to be a QuicConnection created for the connection ID. And + // 2) When the QuicConnection was destroyed, it did not have a termination + // packet so the QuicDispatcher synthesized one using this error code. + // Only sent from server. + QUIC_HANDSHAKE_FAILED_SYNTHETIC_CONNECTION_CLOSE = 219, + // Handshake failed because there is a CID collision. Only sent from server. + QUIC_HANDSHAKE_FAILED_CID_COLLISION = 220, // Handshake message contained out of order tags. QUIC_CRYPTO_TAGS_OUT_OF_ORDER = 29, // Handshake message contained too many entries. @@ -629,7 +644,7 @@ QUIC_CLIENT_LOST_NETWORK_ACCESS = 215, // No error. Used as bound while iterating. - QUIC_LAST_ERROR = 217, + QUIC_LAST_ERROR = 221, }; // QuicErrorCodes is encoded as four octets on-the-wire when doing Google QUIC, // or a varint62 when doing IETF QUIC. Ensure that its value does not exceed @@ -710,8 +725,10 @@ uint64_t ietf_application_code_; }; -// Convert TLS alert code to QuicErrorCode. -QUICHE_EXPORT QuicErrorCode TlsAlertToQuicErrorCode(uint8_t desc); +// Convert TLS alert code to QuicErrorCode. Returns nullopt if the alert code +// is not recognized. +QUICHE_EXPORT std::optional<QuicErrorCode> TlsAlertToQuicErrorCode( + uint8_t desc); // Returns the name of the QuicRstStreamErrorCode as a char* QUICHE_EXPORT const char* QuicRstStreamErrorCodeToString(
diff --git a/quiche/quic/core/tls_handshaker.cc b/quiche/quic/core/tls_handshaker.cc index 47eecfa..fcb6cae 100644 --- a/quiche/quic/core/tls_handshaker.cc +++ b/quiche/quic/core/tls_handshaker.cc
@@ -171,7 +171,8 @@ SSL_alert_desc_string_long(last_tls_alert_->desc), ". SSLErrorStack:", ssl_error_stack); QUIC_DLOG(ERROR) << error_details; - CloseConnection(TlsAlertToQuicErrorCode(last_tls_alert_->desc), + CloseConnection(TlsAlertToQuicErrorCode(last_tls_alert_->desc) + .value_or(QUIC_HANDSHAKE_FAILED), static_cast<QuicIetfTransportErrorCodes>( CRYPTO_ERROR_FIRST + last_tls_alert_->desc), error_details);