Change the return type of QuicDispatcher::TryExtractChloOrBufferEarlyPacket from absl::optional<ParsedClientHello> to ExtractChloResult. The ExtractChloResult could contain a parsed CHLO, a TLS alert, or nothing. There is no behavior change because the caller of TryExtractChloOrBufferEarlyPacket does not use the TLS alert yet. cl/455683374 will use it and send a connection close in response to a TLS alert. PiperOrigin-RevId: 456862334
diff --git a/quiche/quic/core/quic_buffered_packet_store.cc b/quiche/quic/core/quic_buffered_packet_store.cc index 0269cdf..fae7ad1 100644 --- a/quiche/quic/core/quic_buffered_packet_store.cc +++ b/quiche/quic/core/quic_buffered_packet_store.cc
@@ -294,9 +294,10 @@ const QuicConnectionId& connection_id, const ParsedQuicVersion& version, const QuicReceivedPacket& packet, std::vector<std::string>* out_alpns, std::string* out_sni, bool* out_resumption_attempted, - bool* out_early_data_attempted) { + bool* out_early_data_attempted, absl::optional<uint8_t>* tls_alert) { QUICHE_DCHECK_NE(out_alpns, nullptr); QUICHE_DCHECK_NE(out_sni, nullptr); + QUICHE_DCHECK_NE(tls_alert, nullptr); QUICHE_DCHECK_EQ(version.handshake_protocol, PROTOCOL_TLS1_3); auto it = undecryptable_packets_.find(connection_id); if (it == undecryptable_packets_.end()) { @@ -306,6 +307,7 @@ } it->second.tls_chlo_extractor.IngestPacket(version, packet); if (!it->second.tls_chlo_extractor.HasParsedFullChlo()) { + *tls_alert = it->second.tls_chlo_extractor.tls_alert(); return false; } const TlsChloExtractor& tls_chlo_extractor = it->second.tls_chlo_extractor;
diff --git a/quiche/quic/core/quic_buffered_packet_store.h b/quiche/quic/core/quic_buffered_packet_store.h index f3310b4..95bb737 100644 --- a/quiche/quic/core/quic_buffered_packet_store.h +++ b/quiche/quic/core/quic_buffered_packet_store.h
@@ -119,13 +119,13 @@ // |out_resumption_attempted| is populated if the CHLO has the // 'pre_shared_key' TLS extension. |out_early_data_attempted| is populated if // the CHLO has the 'early_data' TLS extension. - bool IngestPacketForTlsChloExtraction(const QuicConnectionId& connection_id, - const ParsedQuicVersion& version, - const QuicReceivedPacket& packet, - std::vector<std::string>* out_alpns, - std::string* out_sni, - bool* out_resumption_attempted, - bool* out_early_data_attempted); + // When this returns false, and an unrecoverable error happened due to a TLS + // alert, |*tls_alert| will be set to the alert value. + bool IngestPacketForTlsChloExtraction( + const QuicConnectionId& connection_id, const ParsedQuicVersion& version, + const QuicReceivedPacket& packet, std::vector<std::string>* out_alpns, + std::string* out_sni, bool* out_resumption_attempted, + bool* out_early_data_attempted, absl::optional<uint8_t>* tls_alert); // Returns the list of buffered packets for |connection_id| and removes them // from the store. Returns an empty list if no early arrived packets for this
diff --git a/quiche/quic/core/quic_buffered_packet_store_test.cc b/quiche/quic/core/quic_buffered_packet_store_test.cc index 012ae6e..4944db7 100644 --- a/quiche/quic/core/quic_buffered_packet_store_test.cc +++ b/quiche/quic/core/quic_buffered_packet_store_test.cc
@@ -468,6 +468,7 @@ bool resumption_attempted = false; bool early_data_attempted = false; QuicConfig config; + absl::optional<uint8_t> tls_alert; EXPECT_FALSE(store_.HasBufferedPackets(connection_id)); store_.EnqueuePacket(connection_id, false, packet_, self_address_, @@ -477,7 +478,7 @@ // The packet in 'packet_' is not a TLS CHLO packet. EXPECT_FALSE(store_.IngestPacketForTlsChloExtraction( connection_id, valid_version_, packet_, &alpns, &sni, - &resumption_attempted, &early_data_attempted)); + &resumption_attempted, &early_data_attempted, &tls_alert)); store_.DiscardPackets(connection_id); @@ -498,10 +499,10 @@ EXPECT_TRUE(store_.HasBufferedPackets(connection_id)); EXPECT_FALSE(store_.IngestPacketForTlsChloExtraction( connection_id, valid_version_, *packets[0], &alpns, &sni, - &resumption_attempted, &early_data_attempted)); + &resumption_attempted, &early_data_attempted, &tls_alert)); EXPECT_TRUE(store_.IngestPacketForTlsChloExtraction( connection_id, valid_version_, *packets[1], &alpns, &sni, - &resumption_attempted, &early_data_attempted)); + &resumption_attempted, &early_data_attempted, &tls_alert)); EXPECT_THAT(alpns, ElementsAre(AlpnForVersion(valid_version_))); EXPECT_EQ(sni, TestHostname());
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc index 883a59d..81226f6 100644 --- a/quiche/quic/core/quic_dispatcher.cc +++ b/quiche/quic/core/quic_dispatcher.cc
@@ -717,8 +717,9 @@ QuicPacketFate fate = ValidityChecks(*packet_info); if (fate == kFateProcess) { - absl::optional<ParsedClientHello> parsed_chlo = + ExtractChloResult extract_chlo_result = TryExtractChloOrBufferEarlyPacket(*packet_info); + auto& parsed_chlo = extract_chlo_result.parsed_chlo; if (!parsed_chlo.has_value()) { // Client Hello incomplete. Packet has been buffered or (rarely) dropped. return; @@ -780,9 +781,10 @@ } } -absl::optional<ParsedClientHello> +QuicDispatcher::ExtractChloResult QuicDispatcher::TryExtractChloOrBufferEarlyPacket( const ReceivedPacketInfo& packet_info) { + ExtractChloResult result; if (packet_info.version.UsesTls()) { bool has_full_tls_chlo = false; std::string sni; @@ -795,7 +797,7 @@ has_full_tls_chlo = buffered_packets_.IngestPacketForTlsChloExtraction( packet_info.destination_connection_id, packet_info.version, packet_info.packet, &alpns, &sni, &resumption_attempted, - &early_data_attempted); + &early_data_attempted, &result.tls_alert); } else { // If we do not have a BufferedPacketList for this connection ID, // create a single-use one to check whether this packet contains a @@ -809,6 +811,8 @@ sni = tls_chlo_extractor.server_name(); resumption_attempted = tls_chlo_extractor.resumption_attempted(); early_data_attempted = tls_chlo_extractor.early_data_attempted(); + } else { + result.tls_alert = tls_chlo_extractor.tls_alert(); } } if (!has_full_tls_chlo) { @@ -816,10 +820,10 @@ // packet that arrived before the CHLO (due to loss or reordering), // or it could be a fragment of a multi-packet CHLO. BufferEarlyPacket(packet_info); - return absl::nullopt; + return result; } - ParsedClientHello parsed_chlo; + ParsedClientHello& parsed_chlo = result.parsed_chlo.emplace(); parsed_chlo.sni = std::move(sni); parsed_chlo.alpns = std::move(alpns); if (packet_info.retry_token.has_value()) { @@ -827,7 +831,7 @@ } parsed_chlo.resumption_attempted = resumption_attempted; parsed_chlo.early_data_attempted = early_data_attempted; - return parsed_chlo; + return result; } ChloAlpnSniExtractor alpn_extractor; @@ -838,7 +842,7 @@ packet_info.destination_connection_id.length())) { // Buffer non-CHLO packets. BufferEarlyPacket(packet_info); - return absl::nullopt; + return result; } // We only apply this check for versions that do not use the IETF @@ -851,16 +855,16 @@ QUIC_DVLOG(1) << "Dropping CHLO packet which is too short, length: " << packet_info.packet.length(); QUIC_CODE_COUNT(quic_drop_small_chlo_packets); - return absl::nullopt; + return result; } - ParsedClientHello parsed_chlo; + ParsedClientHello& parsed_chlo = result.parsed_chlo.emplace(); parsed_chlo.legacy_version_encapsulation_inner_packet = alpn_extractor.ConsumeLegacyVersionEncapsulationInnerPacket(); parsed_chlo.sni = alpn_extractor.ConsumeSni(); parsed_chlo.uaid = alpn_extractor.ConsumeUaid(); parsed_chlo.alpns = {alpn_extractor.ConsumeAlpn()}; - return parsed_chlo; + return result; } std::string QuicDispatcher::SelectAlpn(const std::vector<std::string>& alpns) {
diff --git a/quiche/quic/core/quic_dispatcher.h b/quiche/quic/core/quic_dispatcher.h index 878f6ac..e230739 100644 --- a/quiche/quic/core/quic_dispatcher.h +++ b/quiche/quic/core/quic_dispatcher.h
@@ -367,15 +367,26 @@ // ProcessValidatedPacketWithUnknownConnectionId. void ProcessHeader(ReceivedPacketInfo* packet_info); + struct ExtractChloResult { + // If set, a full client hello has been successfully parsed. + absl::optional<ParsedClientHello> parsed_chlo; + // If set, the TLS alert that will cause a connection close. + // Always empty for Google QUIC. + absl::optional<uint8_t> tls_alert; + }; + // Try to extract information(sni, alpns, ...) if the full Client Hello has // been parsed. // - // Return the parsed client hello if the full Client Hello has been - // successfully parsed. + // Returns the parsed client hello in ExtractChloResult.parsed_chlo, if the + // full Client Hello has been successfully parsed. // - // Otherwise return absl::nullopt and either buffer or (rarely) drop the - // packet. - absl::optional<ParsedClientHello> TryExtractChloOrBufferEarlyPacket( + // Returns the TLS alert in ExtractChloResult.tls_alert, if the extraction of + // Client Hello failed due to that alert. + // + // Otherwise returns a default-constructed ExtractChloResult and either buffer + // or (rarely) drop the packet. + ExtractChloResult TryExtractChloOrBufferEarlyPacket( const ReceivedPacketInfo& packet_info); // Deliver |packets| to |session| for further processing.
diff --git a/quiche/quic/core/tls_chlo_extractor.cc b/quiche/quic/core/tls_chlo_extractor.cc index 046f83d..09057f0 100644 --- a/quiche/quic/core/tls_chlo_extractor.cc +++ b/quiche/quic/core/tls_chlo_extractor.cc
@@ -267,6 +267,9 @@ HandleUnrecoverableError(absl::StrCat( "BoringSSL attempted to send alert ", static_cast<int>(tls_alert_value), " ", SSL_alert_desc_string_long(tls_alert_value))); + if (state_ == State::kUnrecoverableFailure) { + tls_alert_ = tls_alert_value; + } } // static
diff --git a/quiche/quic/core/tls_chlo_extractor.h b/quiche/quic/core/tls_chlo_extractor.h index b2cf50c..d08f56f 100644 --- a/quiche/quic/core/tls_chlo_extractor.h +++ b/quiche/quic/core/tls_chlo_extractor.h
@@ -62,6 +62,13 @@ state_ == State::kParsedFullMultiPacketChlo; } + // Returns the TLS alert that caused the unrecoverable error, if any. + absl::optional<uint8_t> tls_alert() const { + QUICHE_DCHECK(!tls_alert_.has_value() || + state_ == State::kUnrecoverableFailure); + return tls_alert_; + } + // Methods from QuicFramerVisitorInterface. void OnError(QuicFramer* /*framer*/) override {} bool OnProtocolVersionMismatch(ParsedQuicVersion version) override; @@ -249,6 +256,9 @@ // Whether early data is attempted from the CHLO, indicated by the // 'early_data' TLS extension. bool early_data_attempted_ = false; + // If set, contains the TLS alert that caused an unrecoverable error, which is + // an AlertDescription value defined in go/rfc/8446#appendix-B.2. + absl::optional<uint8_t> tls_alert_; }; // Convenience method to facilitate logging TlsChloExtractor::State.