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.
