Send correct QUIC CRYPTO_ERROR connection error for TLS errors Protected by FLAGS_quic_reloadable_flag_quic_send_tls_crypto_error_code. PiperOrigin-RevId: 351885444 Change-Id: I5388eaa8632342f17a0212c68ab587ea0d04ee79
diff --git a/quic/core/frames/quic_connection_close_frame.cc b/quic/core/frames/quic_connection_close_frame.cc index ccc352a..0eee910 100644 --- a/quic/core/frames/quic_connection_close_frame.cc +++ b/quic/core/frames/quic_connection_close_frame.cc
@@ -14,6 +14,7 @@ QuicConnectionCloseFrame::QuicConnectionCloseFrame( QuicTransportVersion transport_version, QuicErrorCode error_code, + QuicIetfTransportErrorCodes ietf_error, std::string error_phrase, uint64_t frame_type) : quic_error_code(error_code), error_details(error_phrase) { @@ -25,7 +26,11 @@ } QuicErrorCodeToIetfMapping mapping = QuicErrorCodeToTransportErrorCode(error_code); - wire_error_code = mapping.error_code; + if (ietf_error != NO_IETF_QUIC_ERROR) { + wire_error_code = ietf_error; + } else { + wire_error_code = mapping.error_code; + } if (mapping.is_transport_close) { // Maps to a transport close close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
diff --git a/quic/core/frames/quic_connection_close_frame.h b/quic/core/frames/quic_connection_close_frame.h index 3d5df24..66a90ad 100644 --- a/quic/core/frames/quic_connection_close_frame.h +++ b/quic/core/frames/quic_connection_close_frame.h
@@ -22,8 +22,12 @@ // and the mapping of error_code. THIS IS THE PREFERRED C'TOR // TO USE IF YOU NEED TO CREATE A CONNECTION-CLOSE-FRAME AND // HAVE IT BE CORRECT FOR THE VERSION AND CODE MAPPINGS. + // |ietf_error| may optionally be be used to directly specify the wire + // error code. Otherwise if |ietf_error| is NO_IETF_QUIC_ERROR, the + // QuicErrorCodeToTransportErrorCode mapping of |error_code| will be used. QuicConnectionCloseFrame(QuicTransportVersion transport_version, QuicErrorCode error_code, + QuicIetfTransportErrorCodes ietf_error, std::string error_phrase, uint64_t transport_close_frame_type);
diff --git a/quic/core/http/quic_receive_control_stream_test.cc b/quic/core/http/quic_receive_control_stream_test.cc index 0d1fd6f..fdd74b0 100644 --- a/quic/core/http/quic_receive_control_stream_test.cc +++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -209,7 +209,7 @@ "Settings frames are received twice.", _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(session_, OnConnectionClosed(_, _)); // Receive second SETTINGS frame. @@ -264,7 +264,7 @@ "PRIORITY_UPDATE frame received before SETTINGS.", _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(session_, OnConnectionClosed(_, _)); receive_control_stream_->OnStreamFrame(data); @@ -315,7 +315,7 @@ CloseConnection(QUIC_HTTP_FRAME_UNEXPECTED_ON_CONTROL_STREAM, _, _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(session_, OnConnectionClosed(_, _)); receive_control_stream_->OnStreamFrame(frame); } @@ -388,7 +388,7 @@ "CANCEL_PUSH frame received before SETTINGS.", _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(session_, OnConnectionClosed(_, _)); receive_control_stream_->OnStreamFrame( @@ -409,7 +409,7 @@ _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(session_, OnConnectionClosed(_, _)); receive_control_stream_->OnStreamFrame( @@ -447,7 +447,7 @@ "ACCEPT_CH frame received on control stream", _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(session_, OnConnectionClosed(_, _)); } } else { @@ -471,7 +471,7 @@ "Unknown frame received before SETTINGS.", _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(session_, OnConnectionClosed(_, _)); receive_control_stream_->OnStreamFrame(
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 568c891..767a881 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1915,7 +1915,7 @@ EXPECT_CALL(*connection_, CloseConnection(_, _, _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); } session_.OnStreamFrame(data1); } @@ -3194,7 +3194,7 @@ EXPECT_CALL(*connection_, CloseConnection(QUIC_NO_ERROR, _, _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(QUIC_NO_ERROR, _)) + EXPECT_CALL(*connection_, SendConnectionClosePacket(QUIC_NO_ERROR, _, _)) .WillOnce(Invoke(connection_, &MockQuicConnection::ReallySendConnectionClosePacket)); connection_->CloseConnection(
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index 0aeba50..d7aa6e7 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -679,7 +679,7 @@ connection_->ReallyCloseConnection(error, error_details, connection_close_behavior); }))); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(*session_, OnConnectionClosed(_, _)) .WillOnce(Invoke([this](const QuicConnectionCloseFrame& frame, ConnectionCloseSource source) { @@ -2110,7 +2110,7 @@ connection_->ReallyCloseConnection(error, error_details, connection_close_behavior); }))); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(*session_, OnConnectionClosed(_, _)) .WillOnce(Invoke([this](const QuicConnectionCloseFrame& frame, ConnectionCloseSource source) { @@ -2151,7 +2151,7 @@ connection_->ReallyCloseConnection(error, error_details, connection_close_behavior); }))); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(*session_, OnConnectionClosed(_, _)) .WillOnce(Invoke([this](const QuicConnectionCloseFrame& frame, ConnectionCloseSource source) { @@ -2959,7 +2959,7 @@ CloseConnection(QUIC_HTTP_FRAME_UNEXPECTED_ON_SPDY_STREAM, _, _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); EXPECT_CALL(*session_, OnConnectionClosed(_, _)); stream_->OnStreamFrame(QuicStreamFrame(stream_->id(), /* fin = */ false,
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index d4a884e..ac8b22d 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -848,8 +848,8 @@ } QUIC_DLOG(INFO) << ENDPOINT << error_details; QUIC_CODE_COUNT(quic_tear_down_local_connection_on_public_reset); - TearDownLocalConnectionState(QUIC_PUBLIC_RESET, error_details, - ConnectionCloseSource::FROM_PEER); + TearDownLocalConnectionState(QUIC_PUBLIC_RESET, NO_IETF_QUIC_ERROR, + error_details, ConnectionCloseSource::FROM_PEER); } bool QuicConnection::OnProtocolVersionMismatch( @@ -1982,8 +1982,8 @@ const std::string error_details = "Received stateless reset."; QUIC_CODE_COUNT(quic_tear_down_local_connection_on_stateless_reset); - TearDownLocalConnectionState(QUIC_PUBLIC_RESET, error_details, - ConnectionCloseSource::FROM_PEER); + TearDownLocalConnectionState(QUIC_PUBLIC_RESET, NO_IETF_QUIC_ERROR, + error_details, ConnectionCloseSource::FROM_PEER); } void QuicConnection::OnKeyUpdate(KeyUpdateReason reason) { @@ -3909,6 +3909,15 @@ void QuicConnection::CloseConnection( QuicErrorCode error, + const std::string& details, + ConnectionCloseBehavior connection_close_behavior) { + CloseConnection(error, NO_IETF_QUIC_ERROR, details, + connection_close_behavior); +} + +void QuicConnection::CloseConnection( + QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, const std::string& error_details, ConnectionCloseBehavior connection_close_behavior) { DCHECK(!error_details.empty()); @@ -3917,20 +3926,29 @@ return; } - QUIC_DLOG(INFO) << ENDPOINT << "Closing connection: " << connection_id() - << ", with error: " << QuicErrorCodeToString(error) << " (" - << error << "), and details: " << error_details; - - if (connection_close_behavior != ConnectionCloseBehavior::SILENT_CLOSE) { - SendConnectionClosePacket(error, error_details); + if (ietf_error != NO_IETF_QUIC_ERROR) { + QUIC_DLOG(INFO) << ENDPOINT << "Closing connection: " << connection_id() + << ", with wire error: " << ietf_error + << ", error: " << QuicErrorCodeToString(error) + << ", and details: " << error_details; + } else { + QUIC_DLOG(INFO) << ENDPOINT << "Closing connection: " << connection_id() + << ", with error: " << QuicErrorCodeToString(error) << " (" + << error << "), and details: " << error_details; } - TearDownLocalConnectionState(error, error_details, + if (connection_close_behavior != ConnectionCloseBehavior::SILENT_CLOSE) { + SendConnectionClosePacket(error, ietf_error, error_details); + } + + TearDownLocalConnectionState(error, ietf_error, error_details, ConnectionCloseSource::FROM_SELF); } -void QuicConnection::SendConnectionClosePacket(QuicErrorCode error, - const std::string& details) { +void QuicConnection::SendConnectionClosePacket( + QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) { // Always use the current path to send CONNECTION_CLOSE. QuicPacketCreator::ScopedPeerAddressContext context(&packet_creator_, peer_address()); @@ -3961,7 +3979,8 @@ } QuicConnectionCloseFrame* frame; - frame = new QuicConnectionCloseFrame(transport_version(), error, details, + frame = new QuicConnectionCloseFrame(transport_version(), error, ietf_error, + details, framer_.current_received_frame_type()); packet_creator_.ConsumeRetransmittableControlFrame(QuicFrame(frame)); packet_creator_.FlushCurrentPacket(); @@ -4014,9 +4033,9 @@ visitor_->BeforeConnectionCloseSent(); } - auto* frame = - new QuicConnectionCloseFrame(transport_version(), error, details, - framer_.current_received_frame_type()); + auto* frame = new QuicConnectionCloseFrame( + transport_version(), error, ietf_error, details, + framer_.current_received_frame_type()); packet_creator_.ConsumeRetransmittableControlFrame(QuicFrame(frame)); packet_creator_.FlushCurrentPacket(); } @@ -4033,9 +4052,11 @@ void QuicConnection::TearDownLocalConnectionState( QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, const std::string& error_details, ConnectionCloseSource source) { - QuicConnectionCloseFrame frame(transport_version(), error, error_details, + QuicConnectionCloseFrame frame(transport_version(), error, ietf_error, + error_details, framer_.current_received_frame_type()); return TearDownLocalConnectionState(frame, source); }
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 600e882..b96952e 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -529,6 +529,13 @@ QuicErrorCode error, const std::string& details, ConnectionCloseBehavior connection_close_behavior); + // Closes the connection, specifying the wire error code |ietf_error| + // explicitly. + virtual void CloseConnection( + QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details, + ConnectionCloseBehavior connection_close_behavior); QuicConnectionStats& mutable_stats() { return stats_; } @@ -1229,7 +1236,11 @@ // Sends a connection close packet to the peer and includes an ACK if the ACK // is not empty, the |error| is not PACKET_WRITE_ERROR, and it fits. + // |ietf_error| may optionally be be used to directly specify the wire + // error code. Otherwise if |ietf_error| is NO_IETF_QUIC_ERROR, the + // QuicErrorCodeToTransportErrorCode mapping of |error| will be used. virtual void SendConnectionClosePacket(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, const std::string& details); // Returns true if the packet should be discarded and not sent. @@ -1331,7 +1342,11 @@ // Does not send a connection close frame to the peer. It should only be // called by CloseConnection or OnConnectionCloseFrame, OnPublicResetPacket, // and OnAuthenticatedIetfStatelessResetPacket. + // |ietf_error| may optionally be be used to directly specify the wire + // error code. Otherwise if |ietf_error| is NO_IETF_QUIC_ERROR, the + // QuicErrorCodeToTransportErrorCode mapping of |error| will be used. void TearDownLocalConnectionState(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, const std::string& details, ConnectionCloseSource source); void TearDownLocalConnectionState(const QuicConnectionCloseFrame& frame,
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 32cbf8d..8ad4214 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -1211,7 +1211,7 @@ QuicErrorCode kQuicErrorCode = QUIC_PEER_GOING_AWAY; QuicConnectionCloseFrame qccf(peer_framer_.transport_version(), - kQuicErrorCode, "", + kQuicErrorCode, NO_IETF_QUIC_ERROR, "", /*transport_close_frame_type=*/0); QuicFrames frames; frames.push_back(QuicFrame(&qccf)); @@ -6776,7 +6776,7 @@ // close. If doing IETF QUIC then set fields appropriately for CC/T or CC/A, // depending on the mapping. QuicConnectionCloseFrame qccf(peer_framer_.transport_version(), - kQuicErrorCode, "", + kQuicErrorCode, NO_IETF_QUIC_ERROR, "", /*transport_close_frame_type=*/0); QuicFrames frames; frames.push_back(QuicFrame(frame1_)); @@ -9028,7 +9028,7 @@ const QuicErrorCode kErrorCode = QUIC_INTERNAL_ERROR; std::unique_ptr<QuicConnectionCloseFrame> connection_close_frame( new QuicConnectionCloseFrame(connection_.transport_version(), kErrorCode, - "", + NO_IETF_QUIC_ERROR, "", /*transport_close_frame_type=*/0)); // Received 2 packets.
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc index a4b4099..8a1d854 100644 --- a/quic/core/quic_dispatcher.cc +++ b/quic/core/quic_dispatcher.cc
@@ -170,9 +170,10 @@ private: void SerializeConnectionClosePacket(QuicErrorCode error_code, const std::string& error_details) { - QuicConnectionCloseFrame* frame = new QuicConnectionCloseFrame( - framer_.transport_version(), error_code, error_details, - /*transport_close_frame_type=*/0); + QuicConnectionCloseFrame* frame = + new QuicConnectionCloseFrame(framer_.transport_version(), error_code, + NO_IETF_QUIC_ERROR, error_details, + /*transport_close_frame_type=*/0); if (!creator_.AddFrame(QuicFrame(frame), NOT_RETRANSMISSION)) { QUIC_BUG << "Unable to add frame to an empty packet";
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc index cbc6590..8363dc1 100644 --- a/quic/core/quic_error_codes.cc +++ b/quic/core/quic_error_codes.cc
@@ -259,6 +259,14 @@ RETURN_STRING_LITERAL(QUIC_AEAD_LIMIT_REACHED); RETURN_STRING_LITERAL(QUIC_MAX_AGE_TIMEOUT); RETURN_STRING_LITERAL(QUIC_INVALID_PRIORITY_UPDATE); + RETURN_STRING_LITERAL(QUIC_TLS_BAD_CERTIFICATE); + RETURN_STRING_LITERAL(QUIC_TLS_UNSUPPORTED_CERTIFICATE); + RETURN_STRING_LITERAL(QUIC_TLS_CERTIFICATE_REVOKED); + RETURN_STRING_LITERAL(QUIC_TLS_CERTIFICATE_EXPIRED); + RETURN_STRING_LITERAL(QUIC_TLS_CERTIFICATE_UNKNOWN); + RETURN_STRING_LITERAL(QUIC_TLS_INTERNAL_ERROR); + RETURN_STRING_LITERAL(QUIC_TLS_UNRECOGNIZED_NAME); + RETURN_STRING_LITERAL(QUIC_TLS_CERTIFICATE_REQUIRED); RETURN_STRING_LITERAL(QUIC_LAST_ERROR); // Intentionally have no default case, so we'll break the build @@ -726,6 +734,31 @@ case QUIC_INVALID_PRIORITY_UPDATE: return {false, static_cast<uint64_t>( QuicHttp3ErrorCode::GENERAL_PROTOCOL_ERROR)}; + case QUIC_TLS_BAD_CERTIFICATE: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_BAD_CERTIFICATE)}; + case QUIC_TLS_UNSUPPORTED_CERTIFICATE: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_UNSUPPORTED_CERTIFICATE)}; + case QUIC_TLS_CERTIFICATE_REVOKED: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_CERTIFICATE_REVOKED)}; + case QUIC_TLS_CERTIFICATE_EXPIRED: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_CERTIFICATE_EXPIRED)}; + case QUIC_TLS_CERTIFICATE_UNKNOWN: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_CERTIFICATE_UNKNOWN)}; + case QUIC_TLS_INTERNAL_ERROR: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_INTERNAL_ERROR)}; + case QUIC_TLS_UNRECOGNIZED_NAME: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_UNRECOGNIZED_NAME)}; + case QUIC_TLS_CERTIFICATE_REQUIRED: + return {true, static_cast<uint64_t>(CRYPTO_ERROR_FIRST + + SSL_AD_CERTIFICATE_REQUIRED)}; + case QUIC_LAST_ERROR: return {false, static_cast<uint64_t>(QUIC_LAST_ERROR)}; } @@ -733,6 +766,29 @@ return {true, static_cast<uint64_t>(INTERNAL_ERROR)}; } +QuicErrorCode TlsAlertToQuicErrorCode(uint8_t desc) { + switch (desc) { + case SSL_AD_BAD_CERTIFICATE: + return QUIC_TLS_BAD_CERTIFICATE; + case SSL_AD_UNSUPPORTED_CERTIFICATE: + return QUIC_TLS_UNSUPPORTED_CERTIFICATE; + case SSL_AD_CERTIFICATE_REVOKED: + return QUIC_TLS_CERTIFICATE_REVOKED; + case SSL_AD_CERTIFICATE_EXPIRED: + return QUIC_TLS_CERTIFICATE_EXPIRED; + case SSL_AD_CERTIFICATE_UNKNOWN: + return QUIC_TLS_CERTIFICATE_UNKNOWN; + case SSL_AD_INTERNAL_ERROR: + return QUIC_TLS_INTERNAL_ERROR; + case SSL_AD_UNRECOGNIZED_NAME: + return QUIC_TLS_UNRECOGNIZED_NAME; + case SSL_AD_CERTIFICATE_REQUIRED: + return QUIC_TLS_CERTIFICATE_REQUIRED; + default: + return QUIC_HANDSHAKE_FAILED; + } +} + // Convert a QuicRstStreamErrorCode to an application error code to be used in // an IETF QUIC RESET_STREAM frame uint64_t RstStreamErrorCodeToIetfResetStreamErrorCode(
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h index b8e4748..885e088 100644 --- a/quic/core/quic_error_codes.h +++ b/quic/core/quic_error_codes.h
@@ -570,8 +570,21 @@ // Received PRIORITY_UPDATE frame with invalid payload. QUIC_INVALID_PRIORITY_UPDATE = 193, + // Maps to specific errors from the CRYPTO_ERROR range from + // https://quicwg.org/base-drafts/draft-ietf-quic-transport.html#name-transport-error-codes + // This attempts to choose a subset of the most interesting errors rather + // than mapping every possible CRYPTO_ERROR code. + QUIC_TLS_BAD_CERTIFICATE = 195, + QUIC_TLS_UNSUPPORTED_CERTIFICATE = 196, + QUIC_TLS_CERTIFICATE_REVOKED = 197, + QUIC_TLS_CERTIFICATE_EXPIRED = 198, + QUIC_TLS_CERTIFICATE_UNKNOWN = 199, + QUIC_TLS_INTERNAL_ERROR = 200, + QUIC_TLS_UNRECOGNIZED_NAME = 201, + QUIC_TLS_CERTIFICATE_REQUIRED = 202, + // No error. Used as bound while iterating. - QUIC_LAST_ERROR = 195, + QUIC_LAST_ERROR = 203, }; // 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 @@ -580,6 +593,9 @@ static_cast<uint64_t>(std::numeric_limits<uint32_t>::max()), "QuicErrorCode exceeds four octets"); +// Convert TLS alert code to QuicErrorCode. +QUIC_EXPORT_PRIVATE QuicErrorCode TlsAlertToQuicErrorCode(uint8_t desc); + // Returns the name of the QuicRstStreamErrorCode as a char* QUIC_EXPORT_PRIVATE const char* QuicRstStreamErrorCodeToString( QuicRstStreamErrorCode error);
diff --git a/quic/core/quic_error_codes_test.cc b/quic/core/quic_error_codes_test.cc index 6bf63d0..f72c150 100644 --- a/quic/core/quic_error_codes_test.cc +++ b/quic/core/quic_error_codes_test.cc
@@ -78,7 +78,17 @@ if (ietf_error_code.is_transport_close) { QuicIetfTransportErrorCodes transport_error_code = static_cast<QuicIetfTransportErrorCodes>(ietf_error_code.error_code); - bool is_valid_transport_error_code = transport_error_code <= 0x0f; + bool is_transport_crypto_error_code = + transport_error_code >= 0x100 && transport_error_code <= 0x1ff; + if (is_transport_crypto_error_code) { + // Ensure that every QuicErrorCode that maps to a CRYPTO_ERROR code has + // a corresponding reverse mapping in TlsAlertToQuicErrorCode: + EXPECT_EQ( + internal_error_code, + TlsAlertToQuicErrorCode(transport_error_code - CRYPTO_ERROR_FIRST)); + } + bool is_valid_transport_error_code = + transport_error_code <= 0x0f || is_transport_crypto_error_code; EXPECT_TRUE(is_valid_transport_error_code) << internal_error_code_string; } else { // Non-transport errors are application errors, either HTTP/3 or QPACK.
diff --git a/quic/core/quic_flags_list.h b/quic/core/quic_flags_list.h index 8461aa7..b74e936 100644 --- a/quic/core/quic_flags_list.h +++ b/quic/core/quic_flags_list.h
@@ -56,6 +56,7 @@ QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_goaway_with_connection_close, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_path_response, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_timestamps, false) +QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_tls_crypto_error_code, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_send_version_negotiation_for_short_connection_ids, true) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_single_ack_in_packet2, false) QUIC_FLAG(FLAGS_quic_reloadable_flag_quic_split_up_send_rst_2, true)
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc index 803a06b..218d9a9 100644 --- a/quic/core/quic_framer_test.cc +++ b/quic/core/quic_framer_test.cc
@@ -7820,8 +7820,9 @@ header.version_flag = false; header.packet_number = kPacketNumber; - QuicConnectionCloseFrame close_frame( - framer_.transport_version(), QUIC_INTERNAL_ERROR, "because I can", 0x05); + QuicConnectionCloseFrame close_frame(framer_.transport_version(), + QUIC_INTERNAL_ERROR, NO_IETF_QUIC_ERROR, + "because I can", 0x05); QuicFrames frames = {QuicFrame(&close_frame)}; // clang-format off @@ -7921,7 +7922,7 @@ static_cast<QuicErrorCode>( VersionHasIetfQuicFrames(framer_.transport_version()) ? 0x01 : 0x05060708), - "because I can", 0x05); + NO_IETF_QUIC_ERROR, "because I can", 0x05); // Set this so that it is "there" for both Google QUIC and IETF QUIC // framing. It better not show up for Google QUIC! close_frame.quic_error_code = static_cast<QuicErrorCode>(0x4567); @@ -8023,7 +8024,7 @@ header.packet_number = kPacketNumber; QuicConnectionCloseFrame close_frame(framer_.transport_version(), - QUIC_INTERNAL_ERROR, + QUIC_INTERNAL_ERROR, NO_IETF_QUIC_ERROR, std::string(2048, 'A'), 0x05); QuicFrames frames = {QuicFrame(&close_frame)}; @@ -11463,9 +11464,10 @@ framer_.transport_version(), QuicFrame(&rst_stream))); std::string error_detail(2048, 'e'); - QuicConnectionCloseFrame connection_close( - framer_.transport_version(), QUIC_NETWORK_IDLE_TIMEOUT, error_detail, - /*transport_close_frame_type=*/0); + QuicConnectionCloseFrame connection_close(framer_.transport_version(), + QUIC_NETWORK_IDLE_TIMEOUT, + NO_IETF_QUIC_ERROR, error_detail, + /*transport_close_frame_type=*/0); EXPECT_EQ(QuicFramer::GetConnectionCloseFrameSize(framer_.transport_version(), connection_close),
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc index 287a41d..da0c6b2 100644 --- a/quic/core/quic_packet_creator_test.cc +++ b/quic/core/quic_packet_creator_test.cc
@@ -340,7 +340,7 @@ TEST_P(QuicPacketCreatorTest, SerializeConnectionClose) { QuicConnectionCloseFrame* frame = new QuicConnectionCloseFrame( - creator_.transport_version(), QUIC_NO_ERROR, "error", + creator_.transport_version(), QUIC_NO_ERROR, NO_IETF_QUIC_ERROR, "error", /*transport_close_frame_type=*/0); QuicFrames frames; @@ -3554,7 +3554,8 @@ const QuicErrorCode kQuicErrorCode = QUIC_PACKET_WRITE_ERROR; QuicConnectionCloseFrame* frame = new QuicConnectionCloseFrame( - framer_.transport_version(), kQuicErrorCode, std::string(error_details), + framer_.transport_version(), kQuicErrorCode, NO_IETF_QUIC_ERROR, + std::string(error_details), /*transport_close_frame_type=*/0); creator_.ConsumeRetransmittableControlFrame(QuicFrame(frame), /*bundle_ack=*/false);
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 0c17d6c..26c6ea7 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -976,6 +976,14 @@ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); } +void QuicSession::OnStreamError(QuicErrorCode error_code, + QuicIetfTransportErrorCodes ietf_error, + std::string error_details) { + connection_->CloseConnection( + error_code, ietf_error, error_details, + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); +} + void QuicSession::SendMaxStreams(QuicStreamCount stream_count, bool unidirectional) { if (!is_configured_) {
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 3eb2867..f3662f0 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -303,6 +303,9 @@ // Implement StreamDelegateInterface. void OnStreamError(QuicErrorCode error_code, std::string error_details) override; + void OnStreamError(QuicErrorCode error_code, + QuicIetfTransportErrorCodes ietf_error, + std::string error_details) override; // Sets priority in the write blocked list. void RegisterStreamPriority( QuicStreamId id,
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index dbe9624..3288047 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -2174,7 +2174,7 @@ EXPECT_CALL(*connection_, CloseConnection(_, _, _)) .WillOnce( Invoke(connection_, &MockQuicConnection::ReallyCloseConnection)); - EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _)); + EXPECT_CALL(*connection_, SendConnectionClosePacket(_, _, _)); connection_->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); session_.OnConfigNegotiated();
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc index 1ce96a5..5aa7150 100644 --- a/quic/core/quic_stream.cc +++ b/quic/core/quic_stream.cc
@@ -154,6 +154,12 @@ stream_delegate_->OnStreamError(error, details); } +void PendingStream::OnUnrecoverableError(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) { + stream_delegate_->OnStreamError(error, ietf_error, details); +} + QuicStreamId PendingStream::id() const { return id_; } @@ -621,6 +627,12 @@ stream_delegate_->OnStreamError(error, details); } +void QuicStream::OnUnrecoverableError(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) { + stream_delegate_->OnStreamError(error, ietf_error, details); +} + const spdy::SpdyStreamPrecedence& QuicStream::precedence() const { return precedence_; }
diff --git a/quic/core/quic_stream.h b/quic/core/quic_stream.h index d13ea7e..83dd375 100644 --- a/quic/core/quic_stream.h +++ b/quic/core/quic_stream.h
@@ -61,6 +61,9 @@ void Reset(QuicRstStreamErrorCode error) override; void OnUnrecoverableError(QuicErrorCode error, const std::string& details) override; + void OnUnrecoverableError(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) override; QuicStreamId id() const override; ParsedQuicVersion version() const override; @@ -163,6 +166,9 @@ // this end. void OnUnrecoverableError(QuicErrorCode error, const std::string& details) override; + void OnUnrecoverableError(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) override; // Called by the session when a (potentially duplicate) stream frame has been // received for this stream.
diff --git a/quic/core/quic_stream_sequencer.h b/quic/core/quic_stream_sequencer.h index e5530a6..b7ea49d 100644 --- a/quic/core/quic_stream_sequencer.h +++ b/quic/core/quic_stream_sequencer.h
@@ -42,6 +42,11 @@ // being closed. virtual void OnUnrecoverableError(QuicErrorCode error, const std::string& details) = 0; + // Called when an error has occurred which should result in the connection + // being closed, specifying the wire error code |ietf_error| explicitly. + virtual void OnUnrecoverableError(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) = 0; // Returns the stream id of this stream. virtual QuicStreamId id() const = 0;
diff --git a/quic/core/quic_stream_sequencer_test.cc b/quic/core/quic_stream_sequencer_test.cc index e30cee0..e8fe6f3 100644 --- a/quic/core/quic_stream_sequencer_test.cc +++ b/quic/core/quic_stream_sequencer_test.cc
@@ -37,6 +37,12 @@ OnUnrecoverableError, (QuicErrorCode error, const std::string& details), (override)); + MOCK_METHOD(void, + OnUnrecoverableError, + (QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details), + (override)); MOCK_METHOD(void, Reset, (QuicRstStreamErrorCode error), (override)); MOCK_METHOD(void, AddBytesConsumed, (QuicByteCount bytes), (override));
diff --git a/quic/core/stream_delegate_interface.h b/quic/core/stream_delegate_interface.h index f29e15c..38b8e53 100644 --- a/quic/core/stream_delegate_interface.h +++ b/quic/core/stream_delegate_interface.h
@@ -23,6 +23,11 @@ // Called when the stream has encountered errors that it can't handle. virtual void OnStreamError(QuicErrorCode error_code, std::string error_details) = 0; + // Called when the stream has encountered errors that it can't handle, + // specifying the wire error code |ietf_error| explicitly. + virtual void OnStreamError(QuicErrorCode error_code, + QuicIetfTransportErrorCodes ietf_error, + std::string error_details) = 0; // Called when the stream needs to write data. If |level| is present, the data // will be written at the specified |level|. The data will be written // at specified transmission |type|.
diff --git a/quic/core/tls_chlo_extractor.cc b/quic/core/tls_chlo_extractor.cc index 505ad28..89aaea8 100644 --- a/quic/core/tls_chlo_extractor.cc +++ b/quic/core/tls_chlo_extractor.cc
@@ -142,6 +142,15 @@ "Crypto stream error ", QuicErrorCodeToString(error), ": ", details)); } +void TlsChloExtractor::OnUnrecoverableError( + QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) { + HandleUnrecoverableError(absl::StrCat( + "Crypto stream error ", QuicErrorCodeToString(error), "(", + QuicIetfTransportErrorCodeString(ietf_error), "): ", details)); +} + // This is called by the framer if it sees a CRYPTO frame during parsing. bool TlsChloExtractor::OnCryptoFrame(const QuicCryptoFrame& frame) { if (frame.level != ENCRYPTION_INITIAL) {
diff --git a/quic/core/tls_chlo_extractor.h b/quic/core/tls_chlo_extractor.h index a95f52e..f7392a5 100644 --- a/quic/core/tls_chlo_extractor.h +++ b/quic/core/tls_chlo_extractor.h
@@ -178,6 +178,9 @@ void Reset(QuicRstStreamErrorCode /*error*/) override {} void OnUnrecoverableError(QuicErrorCode error, const std::string& details) override; + void OnUnrecoverableError(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details) override; QuicStreamId id() const override { return 0; } ParsedQuicVersion version() const override { return framer_->version(); }
diff --git a/quic/core/tls_client_handshaker_test.cc b/quic/core/tls_client_handshaker_test.cc index 2c1a14a..19fdab5 100644 --- a/quic/core/tls_client_handshaker_test.cc +++ b/quic/core/tls_client_handshaker_test.cc
@@ -280,7 +280,11 @@ TEST_P(TlsClientHandshakerTest, ConnectionClosedOnTlsError) { // Have client send ClientHello. stream()->CryptoConnect(); - EXPECT_CALL(*connection_, CloseConnection(QUIC_HANDSHAKE_FAILED, _, _)); + if (GetQuicReloadableFlag(quic_send_tls_crypto_error_code)) { + EXPECT_CALL(*connection_, CloseConnection(QUIC_HANDSHAKE_FAILED, _, _, _)); + } else { + EXPECT_CALL(*connection_, CloseConnection(QUIC_HANDSHAKE_FAILED, _, _)); + } // Send a zero-length ServerHello from server to client. char bogus_handshake_message[] = { @@ -562,11 +566,23 @@ .WillOnce([kTestAlpn](const std::vector<absl::string_view>& alpns) { return std::find(alpns.cbegin(), alpns.cend(), kTestAlpn); }); - EXPECT_CALL(*server_connection_, - CloseConnection(QUIC_HANDSHAKE_FAILED, - "TLS handshake failure (ENCRYPTION_INITIAL) 120: " - "no application protocol", - _)); + if (GetQuicReloadableFlag(quic_send_tls_crypto_error_code)) { + EXPECT_CALL( + *server_connection_, + CloseConnection( + QUIC_HANDSHAKE_FAILED, + static_cast<QuicIetfTransportErrorCodes>(CRYPTO_ERROR_FIRST + 120), + "TLS handshake failure (ENCRYPTION_INITIAL) 120: " + "no application protocol", + _)); + } else { + EXPECT_CALL( + *server_connection_, + CloseConnection(QUIC_HANDSHAKE_FAILED, + "TLS handshake failure (ENCRYPTION_INITIAL) 120: " + "no application protocol", + _)); + } stream()->CryptoConnect(); crypto_test_utils::AdvanceHandshake(connection_, stream(), 0,
diff --git a/quic/core/tls_handshaker.cc b/quic/core/tls_handshaker.cc index 9d787e6..d0079c5 100644 --- a/quic/core/tls_handshaker.cc +++ b/quic/core/tls_handshaker.cc
@@ -119,6 +119,14 @@ is_connection_closed_ = true; } +void TlsHandshaker::CloseConnection(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& reason_phrase) { + DCHECK(!reason_phrase.empty()); + stream()->OnUnrecoverableError(error, ietf_error, reason_phrase); + is_connection_closed_ = true; +} + void TlsHandshaker::OnConnectionClosed(QuicErrorCode /*error*/, ConnectionCloseSource /*source*/) { is_connection_closed_ = true; @@ -290,17 +298,19 @@ void TlsHandshaker::FlushFlight() {} void TlsHandshaker::SendAlert(EncryptionLevel level, uint8_t desc) { - // TODO(b/151676147): Alerts should be sent on the wire as a varint QUIC error - // code computed to be 0x100 | desc (draft-ietf-quic-tls-27, section 4.9). - // This puts it in the range reserved for CRYPTO_ERROR - // (draft-ietf-quic-transport-27, section 20). However, according to - // quic_error_codes.h, this QUIC implementation only sends 1-byte error codes - // right now. std::string error_details = absl::StrCat( "TLS handshake failure (", EncryptionLevelToString(level), ") ", static_cast<int>(desc), ": ", SSL_alert_desc_string_long(desc)); QUIC_DLOG(ERROR) << error_details; - CloseConnection(QUIC_HANDSHAKE_FAILED, error_details); + if (GetQuicReloadableFlag(quic_send_tls_crypto_error_code)) { + QUIC_RELOADABLE_FLAG_COUNT(quic_send_tls_crypto_error_code); + CloseConnection( + TlsAlertToQuicErrorCode(desc), + static_cast<QuicIetfTransportErrorCodes>(CRYPTO_ERROR_FIRST + desc), + error_details); + } else { + CloseConnection(QUIC_HANDSHAKE_FAILED, error_details); + } } } // namespace quic
diff --git a/quic/core/tls_handshaker.h b/quic/core/tls_handshaker.h index af25ed5..4f8bf47 100644 --- a/quic/core/tls_handshaker.h +++ b/quic/core/tls_handshaker.h
@@ -59,6 +59,11 @@ void AdvanceHandshake(); void CloseConnection(QuicErrorCode error, const std::string& reason_phrase); + // Closes the connection, specifying the wire error code |ietf_error| + // explicitly. + void CloseConnection(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& reason_phrase); void OnConnectionClosed(QuicErrorCode error, ConnectionCloseSource source);
diff --git a/quic/core/tls_server_handshaker_test.cc b/quic/core/tls_server_handshaker_test.cc index c0b1337..e2cb745 100644 --- a/quic/core/tls_server_handshaker_test.cc +++ b/quic/core/tls_server_handshaker_test.cc
@@ -537,8 +537,13 @@ } TEST_P(TlsServerHandshakerTest, ConnectionClosedOnTlsError) { - EXPECT_CALL(*server_connection_, - CloseConnection(QUIC_HANDSHAKE_FAILED, _, _)); + if (GetQuicReloadableFlag(quic_send_tls_crypto_error_code)) { + EXPECT_CALL(*server_connection_, + CloseConnection(QUIC_HANDSHAKE_FAILED, _, _, _)); + } else { + EXPECT_CALL(*server_connection_, + CloseConnection(QUIC_HANDSHAKE_FAILED, _, _)); + } // Send a zero-length ClientHello from client to server. char bogus_handshake_message[] = { @@ -558,11 +563,23 @@ const std::string kTestBadClientAlpn = "bad-client-alpn"; EXPECT_CALL(*client_session_, GetAlpnsToOffer()) .WillOnce(Return(std::vector<std::string>({kTestBadClientAlpn}))); - EXPECT_CALL(*server_connection_, - CloseConnection(QUIC_HANDSHAKE_FAILED, - "TLS handshake failure (ENCRYPTION_INITIAL) 120: " - "no application protocol", - _)); + if (GetQuicReloadableFlag(quic_send_tls_crypto_error_code)) { + EXPECT_CALL( + *server_connection_, + CloseConnection( + QUIC_HANDSHAKE_FAILED, + static_cast<QuicIetfTransportErrorCodes>(CRYPTO_ERROR_FIRST + 120), + "TLS handshake failure (ENCRYPTION_INITIAL) 120: " + "no application protocol", + _)); + } else { + EXPECT_CALL( + *server_connection_, + CloseConnection(QUIC_HANDSHAKE_FAILED, + "TLS handshake failure (ENCRYPTION_INITIAL) 120: " + "no application protocol", + _)); + } AdvanceHandshakeWithFakeClient();
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc index c446da3..26df816 100644 --- a/quic/test_tools/quic_connection_peer.cc +++ b/quic/test_tools/quic_connection_peer.cc
@@ -304,10 +304,12 @@ } // static -void QuicConnectionPeer::SendConnectionClosePacket(QuicConnection* connection, - QuicErrorCode error, - const std::string& details) { - connection->SendConnectionClosePacket(error, details); +void QuicConnectionPeer::SendConnectionClosePacket( + QuicConnection* connection, + QuicIetfTransportErrorCodes ietf_error, + QuicErrorCode error, + const std::string& details) { + connection->SendConnectionClosePacket(error, ietf_error, details); } // static
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h index ad8a5ce..ce72e6e 100644 --- a/quic/test_tools/quic_connection_peer.h +++ b/quic/test_tools/quic_connection_peer.h
@@ -130,6 +130,7 @@ static void SetAddressValidated(QuicConnection* connection); static void SendConnectionClosePacket(QuicConnection* connection, + QuicIetfTransportErrorCodes ietf_error, QuicErrorCode error, const std::string& details);
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index af8a33d..8231c62 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -697,8 +697,17 @@ ConnectionCloseBehavior connection_close_behavior), (override)); MOCK_METHOD(void, + CloseConnection, + (QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details, + ConnectionCloseBehavior connection_close_behavior), + (override)); + MOCK_METHOD(void, SendConnectionClosePacket, - (QuicErrorCode error, const std::string& details), + (QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, + const std::string& details), (override)); MOCK_METHOD(void, OnCanWrite, (), (override)); MOCK_METHOD(void, @@ -749,12 +758,17 @@ QuicErrorCode error, const std::string& details, ConnectionCloseBehavior connection_close_behavior) { - QuicConnection::CloseConnection(error, details, connection_close_behavior); + // Call the 4-param method directly instead of the 3-param method, so that + // it doesn't invoke the virtual 4-param method causing the mock 4-param + // method to trigger. + QuicConnection::CloseConnection(error, NO_IETF_QUIC_ERROR, details, + connection_close_behavior); } void ReallySendConnectionClosePacket(QuicErrorCode error, + QuicIetfTransportErrorCodes ietf_error, const std::string& details) { - QuicConnection::SendConnectionClosePacket(error, details); + QuicConnection::SendConnectionClosePacket(error, ietf_error, details); } void ReallyProcessUdpPacket(const QuicSocketAddress& self_address,