Refactor handling of supported versions
This CL introduces ParsedQuicVersionIsValid and refactors AllSupportedVersions to use it. It slightly changes how ParseQuicVersionLabel ParseQuicVersionString work, but only in the parsing of invalid versions such as T043. This does not change the behavior of the GFE in practice because packets of these versions would either way be time-waited because T043 cannot be in the CurrentSupportedVersions list.
This CL also has a minor fix to QuicFramer to ensure that, when processing short headers, the decision to fill packet_number_length is based on the framer's version as opposed to the empty version in the short header. HasHeaderProtection returns false for the empty version meaning that packet_number_length would always be filled in where the code is changed by this CL. However, in versions that do support header protection, packet_number_length is filled in by RemoveHeaderProtection <http://shortn/_Pfz02eXtt7>, which is gated on the version from the framer <http://shortn/_HHqgFPW8lP>. Therefore this change does not change behavior.
gfe-relnote: refactor, no behavior change, not flag-protected
PiperOrigin-RevId: 289535913
Change-Id: I864a9f65c6dc23ea1665f49893476a44e3db1493
diff --git a/quic/core/quic_crypto_server_stream_test.cc b/quic/core/quic_crypto_server_stream_test.cc
index 741a8aa..78f2f52 100644
--- a/quic/core/quic_crypto_server_stream_test.cc
+++ b/quic/core/quic_crypto_server_stream_test.cc
@@ -207,10 +207,11 @@
TEST_P(QuicCryptoServerStreamTest, ConnectedAfterTlsHandshake) {
client_options_.only_tls_versions = true;
supported_versions_.clear();
- for (QuicTransportVersion transport_version :
- AllSupportedTransportVersions()) {
- supported_versions_.push_back(
- ParsedQuicVersion(PROTOCOL_TLS1_3, transport_version));
+ for (ParsedQuicVersion version : AllSupportedVersions()) {
+ if (version.handshake_protocol != PROTOCOL_TLS1_3) {
+ continue;
+ }
+ supported_versions_.push_back(version);
}
Initialize();
CompleteCryptoHandshake();
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index fba4287..11d3aa6 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -2475,7 +2475,7 @@
set_detailed_error("Fixed bit is 0 in short header.");
return false;
}
- if (!header->version.HasHeaderProtection()) {
+ if (!version_.HasHeaderProtection()) {
header->packet_number_length = GetShortHeaderPacketNumberLength(type);
}
QUIC_DVLOG(1) << "packet_number_length = " << header->packet_number_length;
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 372d5d0..536797f 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -15,6 +15,7 @@
#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_arraysize.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_endian.h"
+#include "net/third_party/quiche/src/common/platform/api/quiche_str_cat.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_text_utils.h"
namespace quic {
@@ -40,6 +41,13 @@
} // namespace
+bool ParsedQuicVersion::IsKnown() const {
+ DCHECK(ParsedQuicVersionIsValid(handshake_protocol, transport_version))
+ << QuicVersionToString(transport_version) << " "
+ << HandshakeProtocolToString(handshake_protocol);
+ return transport_version != QUIC_VERSION_UNSUPPORTED;
+}
+
bool ParsedQuicVersion::KnowsWhichDecrypterToUse() const {
return transport_version > QUIC_VERSION_46 ||
handshake_protocol == PROTOCOL_TLS1_3;
@@ -149,14 +157,9 @@
}
ParsedQuicVersion ParseQuicVersionLabel(QuicVersionLabel version_label) {
- std::vector<HandshakeProtocol> protocols = {PROTOCOL_QUIC_CRYPTO,
- PROTOCOL_TLS1_3};
- for (QuicTransportVersion version : kSupportedTransportVersions) {
- for (HandshakeProtocol handshake : protocols) {
- if (version_label ==
- CreateQuicVersionLabel(ParsedQuicVersion(handshake, version))) {
- return ParsedQuicVersion(handshake, version);
- }
+ for (const ParsedQuicVersion& version : AllSupportedVersions()) {
+ if (version_label == CreateQuicVersionLabel(version)) {
+ return version;
}
}
// Reading from the client so this should not be considered an ERROR.
@@ -177,13 +180,9 @@
PROTOCOL_QUIC_CRYPTO,
static_cast<QuicTransportVersion>(quic_version_number));
}
- for (QuicTransportVersion version : kSupportedTransportVersions) {
- for (HandshakeProtocol handshake : kSupportedHandshakeProtocols) {
- const ParsedQuicVersion parsed_version =
- ParsedQuicVersion(handshake, version);
- if (version_string == ParsedQuicVersionToString(parsed_version)) {
- return parsed_version;
- }
+ for (const ParsedQuicVersion& version : AllSupportedVersions()) {
+ if (version_string == ParsedQuicVersionToString(version)) {
+ return version;
}
}
// Still recognize T099 even if flag quic_ietf_draft_version has been changed.
@@ -205,18 +204,30 @@
return supported_versions;
}
+bool ParsedQuicVersionIsValid(HandshakeProtocol handshake_protocol,
+ QuicTransportVersion transport_version) {
+ switch (handshake_protocol) {
+ case PROTOCOL_UNSUPPORTED:
+ return transport_version == QUIC_VERSION_UNSUPPORTED;
+ case PROTOCOL_QUIC_CRYPTO:
+ return transport_version != QUIC_VERSION_UNSUPPORTED;
+ case PROTOCOL_TLS1_3:
+ // The TLS handshake is only deployable if CRYPTO frames are also used.
+ // We explicitly removed support for T048 and T049 to reduce test load.
+ return QuicVersionUsesCryptoFrames(transport_version) &&
+ transport_version > QUIC_VERSION_49;
+ }
+ return false;
+}
+
ParsedQuicVersionVector AllSupportedVersions() {
ParsedQuicVersionVector supported_versions;
- for (HandshakeProtocol protocol : kSupportedHandshakeProtocols) {
- for (QuicTransportVersion version : kSupportedTransportVersions) {
- if (protocol == PROTOCOL_TLS1_3 &&
- (!QuicVersionUsesCryptoFrames(version) ||
- version <= QUIC_VERSION_49)) {
- // The TLS handshake is only deployable if CRYPTO frames are also used.
- // We explicitly removed support for T048 and T049 to reduce test load.
- continue;
+ for (HandshakeProtocol handshake_protocol : kSupportedHandshakeProtocols) {
+ for (QuicTransportVersion transport_version : kSupportedTransportVersions) {
+ if (ParsedQuicVersionIsValid(handshake_protocol, transport_version)) {
+ supported_versions.push_back(
+ ParsedQuicVersion(handshake_protocol, transport_version));
}
- supported_versions.push_back(ParsedQuicVersion(protocol, version));
}
}
return supported_versions;
@@ -363,9 +374,21 @@
RETURN_STRING_LITERAL(QUIC_VERSION_49);
RETURN_STRING_LITERAL(QUIC_VERSION_50);
RETURN_STRING_LITERAL(QUIC_VERSION_99);
- default:
- return "QUIC_VERSION_UNSUPPORTED";
+ RETURN_STRING_LITERAL(QUIC_VERSION_UNSUPPORTED);
+ RETURN_STRING_LITERAL(QUIC_VERSION_RESERVED_FOR_NEGOTIATION);
}
+ return quiche::QuicheStrCat("QUIC_VERSION_UNKNOWN(",
+ static_cast<int>(transport_version), ")");
+}
+
+std::string HandshakeProtocolToString(HandshakeProtocol handshake_protocol) {
+ switch (handshake_protocol) {
+ RETURN_STRING_LITERAL(PROTOCOL_UNSUPPORTED);
+ RETURN_STRING_LITERAL(PROTOCOL_QUIC_CRYPTO);
+ RETURN_STRING_LITERAL(PROTOCOL_TLS1_3);
+ }
+ return quiche::QuicheStrCat("PROTOCOL_UNKNOWN(",
+ static_cast<int>(handshake_protocol), ")");
}
std::string ParsedQuicVersionToString(ParsedQuicVersion version) {
diff --git a/quic/core/quic_versions.h b/quic/core/quic_versions.h
index 5b09ad6..66553cd 100644
--- a/quic/core/quic_versions.h
+++ b/quic/core/quic_versions.h
@@ -118,6 +118,11 @@
QUIC_VERSION_RESERVED_FOR_NEGOTIATION = 999,
};
+// Helper function which translates from a QuicTransportVersion to a string.
+// Returns strings corresponding to enum names (e.g. QUIC_VERSION_6).
+QUIC_EXPORT_PRIVATE std::string QuicVersionToString(
+ QuicTransportVersion transport_version);
+
// IETF draft version most closely approximated by TLS + v99.
static const int kQuicIetfDraftVersion = 24;
@@ -128,6 +133,18 @@
PROTOCOL_TLS1_3,
};
+// Helper function which translates from a HandshakeProtocol to a string.
+QUIC_EXPORT_PRIVATE std::string HandshakeProtocolToString(
+ HandshakeProtocol handshake_protocol);
+
+// Returns whether this combination of handshake protocol and transport
+// version is allowed. For example, {PROTOCOL_TLS1_3, QUIC_VERSION_43} is NOT
+// allowed as TLS requires crypto frames which v43 does not support. Note that
+// UnsupportedQuicVersion is a valid version.
+QUIC_EXPORT_PRIVATE bool ParsedQuicVersionIsValid(
+ HandshakeProtocol handshake_protocol,
+ QuicTransportVersion transport_version);
+
// A parsed QUIC version label which determines that handshake protocol
// and the transport version.
struct QUIC_EXPORT_PRIVATE ParsedQuicVersion {
@@ -161,6 +178,12 @@
transport_version != other.transport_version;
}
+ // Returns whether our codebase understands this version. This should only be
+ // called on valid versions, see ParsedQuicVersionIsValid. Assuming the
+ // version is valid, IsKnown returns whether the version is not
+ // UnsupportedQuicVersion.
+ bool IsKnown() const;
+
bool KnowsWhichDecrypterToUse() const;
// Returns whether this version uses keys derived from the Connection ID for
@@ -321,11 +344,6 @@
QUIC_EXPORT_PRIVATE HandshakeProtocol
QuicVersionLabelToHandshakeProtocol(QuicVersionLabel version_label);
-// Helper function which translates from a QuicTransportVersion to a string.
-// Returns strings corresponding to enum names (e.g. QUIC_VERSION_6).
-QUIC_EXPORT_PRIVATE std::string QuicVersionToString(
- QuicTransportVersion transport_version);
-
// Helper function which translates from a ParsedQuicVersion to a string.
// Returns strings corresponding to the on-the-wire tag.
QUIC_EXPORT_PRIVATE std::string ParsedQuicVersionToString(
diff --git a/quic/core/quic_versions_test.cc b/quic/core/quic_versions_test.cc
index 0c3ddec..81f9940 100644
--- a/quic/core/quic_versions_test.cc
+++ b/quic/core/quic_versions_test.cc
@@ -48,8 +48,24 @@
}
TEST_F(QuicVersionsTest, QuicVersionToQuicVersionLabelUnsupported) {
- EXPECT_QUIC_BUG(QuicVersionToQuicVersionLabel(QUIC_VERSION_UNSUPPORTED),
- "Unsupported QuicTransportVersion: 0");
+ EXPECT_QUIC_BUG(CreateQuicVersionLabel(UnsupportedQuicVersion()),
+ "Invalid HandshakeProtocol: 0");
+}
+
+TEST_F(QuicVersionsTest, KnownAndValid) {
+ for (const ParsedQuicVersion& version : AllSupportedVersions()) {
+ EXPECT_TRUE(version.IsKnown());
+ EXPECT_TRUE(ParsedQuicVersionIsValid(version.handshake_protocol,
+ version.transport_version));
+ }
+ ParsedQuicVersion unsupported = UnsupportedQuicVersion();
+ EXPECT_FALSE(unsupported.IsKnown());
+ EXPECT_TRUE(ParsedQuicVersionIsValid(unsupported.handshake_protocol,
+ unsupported.transport_version));
+ ParsedQuicVersion reserved = QuicVersionReservedForNegotiation();
+ EXPECT_TRUE(reserved.IsKnown());
+ EXPECT_TRUE(ParsedQuicVersionIsValid(reserved.handshake_protocol,
+ reserved.transport_version));
}
TEST_F(QuicVersionsTest, QuicVersionLabelToQuicTransportVersion) {
@@ -99,15 +115,18 @@
EXPECT_QUIC_LOG_CALL(log).Times(0);
log.StartCapturingLogs();
- for (size_t i = 0; i < QUICHE_ARRAYSIZE(kSupportedTransportVersions); ++i) {
+ for (const ParsedQuicVersion& version : AllSupportedVersions()) {
+ if (version.handshake_protocol != PROTOCOL_QUIC_CRYPTO) {
+ continue;
+ }
QuicVersionLabel version_label =
- QuicVersionToQuicVersionLabel(kSupportedTransportVersions[i]);
+ QuicVersionToQuicVersionLabel(version.transport_version);
EXPECT_EQ(PROTOCOL_QUIC_CRYPTO,
QuicVersionLabelToHandshakeProtocol(version_label));
}
// Test a TLS version:
- QuicTag tls_tag = MakeQuicTag('3', '4', '0', 'T');
+ QuicTag tls_tag = MakeQuicTag('0', '5', '0', 'T');
EXPECT_EQ(PROTOCOL_TLS1_3, QuicVersionLabelToHandshakeProtocol(tls_tag));
}
@@ -120,14 +139,6 @@
ParseQuicVersionLabel(MakeVersionLabel('Q', '0', '4', '8')));
EXPECT_EQ(ParsedQuicVersion(PROTOCOL_QUIC_CRYPTO, QUIC_VERSION_50),
ParseQuicVersionLabel(MakeVersionLabel('Q', '0', '5', '0')));
-
- // Test TLS versions:
- EXPECT_EQ(ParsedQuicVersion(PROTOCOL_TLS1_3, QUIC_VERSION_43),
- ParseQuicVersionLabel(MakeVersionLabel('T', '0', '4', '3')));
- EXPECT_EQ(ParsedQuicVersion(PROTOCOL_TLS1_3, QUIC_VERSION_46),
- ParseQuicVersionLabel(MakeVersionLabel('T', '0', '4', '6')));
- EXPECT_EQ(ParsedQuicVersion(PROTOCOL_TLS1_3, QUIC_VERSION_48),
- ParseQuicVersionLabel(MakeVersionLabel('T', '0', '4', '8')));
EXPECT_EQ(ParsedQuicVersion(PROTOCOL_TLS1_3, QUIC_VERSION_50),
ParseQuicVersionLabel(MakeVersionLabel('T', '0', '5', '0')));
}
@@ -245,11 +256,8 @@
// Make sure that all supported versions are present in
// ParsedQuicVersionToString.
- for (QuicTransportVersion transport_version : kSupportedTransportVersions) {
- for (HandshakeProtocol protocol : kSupportedHandshakeProtocols) {
- EXPECT_NE("0", ParsedQuicVersionToString(
- ParsedQuicVersion(protocol, transport_version)));
- }
+ for (const ParsedQuicVersion& version : AllSupportedVersions()) {
+ EXPECT_NE("0", ParsedQuicVersionToString(version));
}
}
diff --git a/quic/test_tools/crypto_test_utils.cc b/quic/test_tools/crypto_test_utils.cc
index a28f85c..0e8289c 100644
--- a/quic/test_tools/crypto_test_utils.cc
+++ b/quic/test_tools/crypto_test_utils.cc
@@ -273,10 +273,11 @@
ParsedQuicVersionVector supported_versions = AllSupportedVersions();
if (options.only_tls_versions) {
supported_versions.clear();
- for (QuicTransportVersion transport_version :
- AllSupportedTransportVersions()) {
- supported_versions.push_back(
- ParsedQuicVersion(PROTOCOL_TLS1_3, transport_version));
+ for (ParsedQuicVersion version : AllSupportedVersions()) {
+ if (version.handshake_protocol != PROTOCOL_TLS1_3) {
+ continue;
+ }
+ supported_versions.push_back(version);
}
}
PacketSavingConnection* client_conn = new PacketSavingConnection(
