Add support for draft-27 This CL updates v99 from draft 26 to draft 27. The main change is a rewrite of how transport parameters are read and written. This CL also replaces quic_enable_version_draft_25_v2 with quic_enable_version_draft_25_v3 to ensure that we only enable draft25 with the new code. gfe-relnote: refactor transport parameter encoding code, protected by disabled flag quic_enable_version_draft_25_v3 PiperOrigin-RevId: 298427763 Change-Id: I2417b25584d914d38a55155ffddbac6e073fcd80
diff --git a/quic/core/crypto/crypto_utils.cc b/quic/core/crypto/crypto_utils.cc index 0215d7f..8fcfbdd 100644 --- a/quic/core/crypto/crypto_utils.cc +++ b/quic/core/crypto/crypto_utils.cc
@@ -115,7 +115,7 @@ namespace { -static_assert(kQuicIetfDraftVersion == 26, "Salts do not match draft version"); +static_assert(kQuicIetfDraftVersion == 27, "Salts do not match draft version"); // Salt from https://tools.ietf.org/html/draft-ietf-quic-tls-25#section-5.2 const uint8_t kDraft25InitialSalt[] = {0xc3, 0xee, 0xf7, 0x12, 0xc7, 0x2e, 0xbb, 0x5a, 0x11, 0xa7, 0xd2, 0x43, 0x2b, 0xb4, @@ -183,7 +183,7 @@ // Retry Integrity Protection Keys and Nonces. // https://tools.ietf.org/html/draft-ietf-quic-tls-25#section-5.8 -static_assert(kQuicIetfDraftVersion == 26, "Keys do not match draft version"); +static_assert(kQuicIetfDraftVersion == 27, "Keys do not match draft version"); const uint8_t kDraft25RetryIntegrityKey[] = {0x4d, 0x32, 0xec, 0xdb, 0x2a, 0x21, 0x33, 0xc8, 0x41, 0xe4, 0x04, 0x3d, 0xf2, 0x7d, 0x44, 0x30};
diff --git a/quic/core/crypto/transport_parameters.cc b/quic/core/crypto/transport_parameters.cc index a7df751..0f184d8 100644 --- a/quic/core/crypto/transport_parameters.cc +++ b/quic/core/crypto/transport_parameters.cc
@@ -9,7 +9,6 @@ #include <forward_list> #include <utility> -#include "third_party/boringssl/src/include/openssl/bytestring.h" #include "net/third_party/quiche/src/quic/core/crypto/crypto_framer.h" #include "net/third_party/quiche/src/quic/core/quic_connection_id.h" #include "net/third_party/quiche/src/quic/core/quic_data_reader.h" @@ -18,6 +17,7 @@ #include "net/third_party/quiche/src/quic/core/quic_utils.h" #include "net/third_party/quiche/src/quic/core/quic_versions.h" #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" +#include "net/third_party/quiche/src/common/platform/api/quiche_str_cat.h" #include "net/third_party/quiche/src/common/platform/api/quiche_string_piece.h" #include "net/third_party/quiche/src/common/platform/api/quiche_text_utils.h" @@ -28,7 +28,7 @@ // When parameters are encoded, one of these enum values is used to indicate // which parameter is encoded. The supported draft version is noted in // transport_parameters.h. -enum TransportParameters::TransportParameterId : uint16_t { +enum TransportParameters::TransportParameterId : uint64_t { kOriginalConnectionId = 0, kIdleTimeout = 1, kStatelessResetToken = 2, @@ -107,6 +107,89 @@ return "Unknown(" + quiche::QuicheTextUtils::Uint64ToString(param_id) + ")"; } +bool WriteTransportParameterId( + QuicDataWriter* writer, + TransportParameters::TransportParameterId param_id, + ParsedQuicVersion version) { + if (version.HasVarIntTransportParams()) { + if (!writer->WriteVarInt62(param_id)) { + QUIC_BUG << "Failed to write param_id for " + << TransportParameterIdToString(param_id); + return false; + } + } else { + if (param_id > std::numeric_limits<uint16_t>::max()) { + QUIC_BUG << "Cannot serialize transport parameter " + << TransportParameterIdToString(param_id) << " with version " + << version; + return false; + } + if (!writer->WriteUInt16(param_id)) { + QUIC_BUG << "Failed to write param_id16 for " + << TransportParameterIdToString(param_id); + return false; + } + } + return true; +} + +bool WriteTransportParameterLength(QuicDataWriter* writer, + uint64_t length, + ParsedQuicVersion version) { + if (version.HasVarIntTransportParams()) { + return writer->WriteVarInt62(length); + } + if (length > std::numeric_limits<uint16_t>::max()) { + QUIC_BUG << "Cannot serialize transport parameter length " << length + << " with version " << version; + return false; + } + return writer->WriteUInt16(length); +} + +bool WriteTransportParameterStringPiece(QuicDataWriter* writer, + quiche::QuicheStringPiece value, + ParsedQuicVersion version) { + if (version.HasVarIntTransportParams()) { + return writer->WriteStringPieceVarInt62(value); + } + return writer->WriteStringPiece16(value); +} + +bool ReadTransportParameterId( + QuicDataReader* reader, + ParsedQuicVersion version, + TransportParameters::TransportParameterId* out_param_id) { + if (version.HasVarIntTransportParams()) { + uint64_t param_id64; + if (!reader->ReadVarInt62(¶m_id64)) { + QUIC_BUG << "Failed to read param_id"; + return false; + } + *out_param_id = + static_cast<TransportParameters::TransportParameterId>(param_id64); + } else { + uint16_t param_id16; + if (!reader->ReadUInt16(¶m_id16)) { + QUIC_BUG << "Failed to read param_id16"; + return false; + } + *out_param_id = + static_cast<TransportParameters::TransportParameterId>(param_id16); + } + return true; +} + +bool ReadTransportParameterLengthAndValue( + QuicDataReader* reader, + ParsedQuicVersion version, + quiche::QuicheStringPiece* out_value) { + if (version.HasVarIntTransportParams()) { + return reader->ReadStringPieceVarInt62(out_value); + } + return reader->ReadStringPiece16(out_value); +} + } // namespace TransportParameters::IntegerParameter::IntegerParameter( @@ -119,7 +202,7 @@ default_value_(default_value), min_value_(min_value), max_value_(max_value), - has_been_read_from_cbs_(false) { + has_been_read_(false) { DCHECK_LE(min_value, default_value); DCHECK_LE(default_value, max_value); DCHECK_LE(max_value, kVarInt62MaxValue); @@ -145,47 +228,56 @@ return min_value_ <= value_ && value_ <= max_value_; } -bool TransportParameters::IntegerParameter::WriteToCbb(CBB* parent_cbb) const { +bool TransportParameters::IntegerParameter::Write( + QuicDataWriter* writer, + ParsedQuicVersion version) const { DCHECK(IsValid()); if (value_ == default_value_) { // Do not write if the value is default. return true; } - uint8_t encoded_data[sizeof(uint64_t)] = {}; - QuicDataWriter writer(sizeof(encoded_data), - reinterpret_cast<char*>(encoded_data)); - writer.WriteVarInt62(value_); - const uint16_t value_length = writer.length(); - DCHECK_LE(value_length, sizeof(encoded_data)); - const bool ok = CBB_add_u16(parent_cbb, param_id_) && - CBB_add_u16(parent_cbb, value_length) && - CBB_add_bytes(parent_cbb, encoded_data, value_length); - QUIC_BUG_IF(!ok) << "Failed to write " << this; - return ok; + if (!WriteTransportParameterId(writer, param_id_, version)) { + QUIC_BUG << "Failed to write param_id for " << *this; + return false; + } + const QuicVariableLengthIntegerLength value_length = + QuicDataWriter::GetVarInt62Len(value_); + if (version.HasVarIntTransportParams()) { + if (!writer->WriteVarInt62(value_length)) { + QUIC_BUG << "Failed to write value_length for " << *this; + return false; + } + } else { + if (!writer->WriteUInt16(value_length)) { + QUIC_BUG << "Failed to write value_length16 for " << *this; + return false; + } + } + if (!writer->WriteVarInt62(value_, value_length)) { + QUIC_BUG << "Failed to write value for " << *this; + return false; + } + return true; } -bool TransportParameters::IntegerParameter::ReadFromCbs(CBS* const value_cbs) { - if (has_been_read_from_cbs_) { - QUIC_DLOG(ERROR) << "Received a second " - << TransportParameterIdToString(param_id_); +bool TransportParameters::IntegerParameter::Read(QuicDataReader* reader, + std::string* error_details) { + if (has_been_read_) { + *error_details = + "Received a second " + TransportParameterIdToString(param_id_); return false; } - has_been_read_from_cbs_ = true; - QuicDataReader reader(reinterpret_cast<const char*>(CBS_data(value_cbs)), - CBS_len(value_cbs)); - QuicVariableLengthIntegerLength value_length = reader.PeekVarInt62Length(); - if (value_length == 0 || !reader.ReadVarInt62(&value_)) { - QUIC_DLOG(ERROR) << "Failed to parse value for " - << TransportParameterIdToString(param_id_); + has_been_read_ = true; + + if (!reader->ReadVarInt62(&value_)) { + *error_details = + "Failed to parse value for " + TransportParameterIdToString(param_id_); return false; } - if (!reader.IsDoneReading()) { - QUIC_DLOG(ERROR) << "Received unexpected " << reader.BytesRemaining() - << " bytes after parsing " << this; - return false; - } - if (!CBS_skip(value_cbs, value_length)) { - QUIC_BUG << "Failed to advance CBS past value for " << this; + if (!reader->IsDoneReading()) { + *error_details = + quiche::QuicheStrCat("Received unexpected ", reader->BytesRemaining(), + " bytes after parsing ", this->ToString(false)); return false; } return true; @@ -325,39 +417,40 @@ // ParseTransportParameters. {} -bool TransportParameters::AreValid() const { +bool TransportParameters::AreValid(std::string* error_details) const { DCHECK(perspective == Perspective::IS_CLIENT || perspective == Perspective::IS_SERVER); if (perspective == Perspective::IS_CLIENT && !stateless_reset_token.empty()) { - QUIC_DLOG(ERROR) << "Client cannot send stateless reset token"; + *error_details = "Client cannot send stateless reset token"; return false; } if (perspective == Perspective::IS_CLIENT && !original_connection_id.IsEmpty()) { - QUIC_DLOG(ERROR) << "Client cannot send original connection ID"; + *error_details = "Client cannot send original connection ID"; return false; } if (!stateless_reset_token.empty() && stateless_reset_token.size() != kStatelessResetTokenLength) { - QUIC_DLOG(ERROR) << "Stateless reset token has bad length " - << stateless_reset_token.size(); + *error_details = quiche::QuicheStrCat( + "Stateless reset token has bad length ", stateless_reset_token.size()); return false; } if (perspective == Perspective::IS_CLIENT && preferred_address) { - QUIC_DLOG(ERROR) << "Client cannot send preferred address"; + *error_details = "Client cannot send preferred address"; return false; } if (preferred_address && preferred_address->stateless_reset_token.size() != kStatelessResetTokenLength) { - QUIC_DLOG(ERROR) - << "Preferred address stateless reset token has bad length " - << preferred_address->stateless_reset_token.size(); + *error_details = quiche::QuicheStrCat( + "Preferred address stateless reset token has bad length ", + preferred_address->stateless_reset_token.size()); return false; } if (preferred_address && (!preferred_address->ipv4_socket_address.host().IsIPv4() || !preferred_address->ipv6_socket_address.host().IsIPv6())) { QUIC_BUG << "Preferred address family failure"; + *error_details = "Internal preferred address family failure"; return false; } const bool ok = @@ -370,104 +463,112 @@ ack_delay_exponent.IsValid() && max_ack_delay.IsValid() && active_connection_id_limit.IsValid() && max_datagram_frame_size.IsValid(); if (!ok) { - QUIC_DLOG(ERROR) << "Invalid transport parameters " << *this; + *error_details = "Invalid transport parameters " + this->ToString(); } return ok; } TransportParameters::~TransportParameters() = default; -bool SerializeTransportParameters(ParsedQuicVersion /*version*/, +bool SerializeTransportParameters(ParsedQuicVersion version, const TransportParameters& in, std::vector<uint8_t>* out) { - if (!in.AreValid()) { - QUIC_DLOG(ERROR) << "Not serializing invalid transport parameters " << in; + std::string error_details; + if (!in.AreValid(&error_details)) { + QUIC_BUG << "Not serializing invalid transport parameters: " + << error_details; return false; } if (in.version == 0 || (in.perspective == Perspective::IS_SERVER && in.supported_versions.empty())) { - QUIC_DLOG(ERROR) << "Refusing to serialize without versions"; + QUIC_BUG << "Refusing to serialize without versions"; return false; } - bssl::ScopedCBB cbb; // Empirically transport parameters generally fit within 128 bytes. - // The CBB will grow to fit larger serializations if required. - if (!CBB_init(cbb.get(), 128)) { - QUIC_BUG << "Failed to initialize CBB for " << in; - return false; - } + // For now we hope this will be enough. + // TODO(dschinazi) make this grow if needed. + static const size_t kMaxTransportParametersLength = 4096; + out->resize(kMaxTransportParametersLength); + QuicDataWriter writer(out->size(), reinterpret_cast<char*>(out->data())); - CBB params; - // Add length of the transport parameters list. - if (!CBB_add_u16_length_prefixed(cbb.get(), ¶ms)) { - QUIC_BUG << "Failed to write parameter length for " << in; - return false; + if (!version.HasVarIntTransportParams()) { + // Versions that do not use variable integer transport parameters carry + // a 16-bit length of the remaining transport parameters. We write 0 here + // to reserve 16 bits, and we fill it in at the end of this function. + // TODO(b/150465921) add support for doing this in QuicDataWriter. + if (!writer.WriteUInt16(0)) { + QUIC_BUG << "Failed to write transport parameter fake length prefix for " + << in; + return false; + } } // original_connection_id - CBB original_connection_id_param; if (!in.original_connection_id.IsEmpty()) { DCHECK_EQ(Perspective::IS_SERVER, in.perspective); - if (!CBB_add_u16(¶ms, TransportParameters::kOriginalConnectionId) || - !CBB_add_u16_length_prefixed(¶ms, &original_connection_id_param) || - !CBB_add_bytes( - &original_connection_id_param, - reinterpret_cast<const uint8_t*>(in.original_connection_id.data()), - in.original_connection_id.length())) { + if (!WriteTransportParameterId( + &writer, TransportParameters::kOriginalConnectionId, version) || + !WriteTransportParameterStringPiece( + &writer, + quiche::QuicheStringPiece(in.original_connection_id.data(), + in.original_connection_id.length()), + version)) { QUIC_BUG << "Failed to write original_connection_id " << in.original_connection_id << " for " << in; return false; } } - if (!in.idle_timeout_milliseconds.WriteToCbb(¶ms)) { + if (!in.idle_timeout_milliseconds.Write(&writer, version)) { QUIC_BUG << "Failed to write idle_timeout for " << in; return false; } // stateless_reset_token - CBB stateless_reset_token_param; if (!in.stateless_reset_token.empty()) { DCHECK_EQ(kStatelessResetTokenLength, in.stateless_reset_token.size()); DCHECK_EQ(Perspective::IS_SERVER, in.perspective); - if (!CBB_add_u16(¶ms, TransportParameters::kStatelessResetToken) || - !CBB_add_u16_length_prefixed(¶ms, &stateless_reset_token_param) || - !CBB_add_bytes(&stateless_reset_token_param, - in.stateless_reset_token.data(), - in.stateless_reset_token.size())) { + if (!WriteTransportParameterId( + &writer, TransportParameters::kStatelessResetToken, version) || + !WriteTransportParameterStringPiece( + &writer, + quiche::QuicheStringPiece( + reinterpret_cast<const char*>(in.stateless_reset_token.data()), + in.stateless_reset_token.size()), + version)) { QUIC_BUG << "Failed to write stateless_reset_token of length " << in.stateless_reset_token.size() << " for " << in; return false; } } - if (!in.max_packet_size.WriteToCbb(¶ms) || - !in.initial_max_data.WriteToCbb(¶ms) || - !in.initial_max_stream_data_bidi_local.WriteToCbb(¶ms) || - !in.initial_max_stream_data_bidi_remote.WriteToCbb(¶ms) || - !in.initial_max_stream_data_uni.WriteToCbb(¶ms) || - !in.initial_max_streams_bidi.WriteToCbb(¶ms) || - !in.initial_max_streams_uni.WriteToCbb(¶ms) || - !in.ack_delay_exponent.WriteToCbb(¶ms) || - !in.max_ack_delay.WriteToCbb(¶ms) || - !in.active_connection_id_limit.WriteToCbb(¶ms) || - !in.max_datagram_frame_size.WriteToCbb(¶ms)) { + if (!in.max_packet_size.Write(&writer, version) || + !in.initial_max_data.Write(&writer, version) || + !in.initial_max_stream_data_bidi_local.Write(&writer, version) || + !in.initial_max_stream_data_bidi_remote.Write(&writer, version) || + !in.initial_max_stream_data_uni.Write(&writer, version) || + !in.initial_max_streams_bidi.Write(&writer, version) || + !in.initial_max_streams_uni.Write(&writer, version) || + !in.ack_delay_exponent.Write(&writer, version) || + !in.max_ack_delay.Write(&writer, version) || + !in.active_connection_id_limit.Write(&writer, version) || + !in.max_datagram_frame_size.Write(&writer, version)) { QUIC_BUG << "Failed to write integers for " << in; return false; } // disable_migration if (in.disable_migration) { - if (!CBB_add_u16(¶ms, TransportParameters::kDisableMigration) || - !CBB_add_u16(¶ms, 0u)) { // 0 is the length of this parameter. + if (!WriteTransportParameterId( + &writer, TransportParameters::kDisableMigration, version) || + !WriteTransportParameterLength(&writer, /*length=*/0, version)) { QUIC_BUG << "Failed to write disable_migration for " << in; return false; } } // preferred_address - CBB preferred_address_params, preferred_address_connection_id_param; if (in.preferred_address) { std::string v4_address_bytes = in.preferred_address->ipv4_socket_address.host().ToPackedString(); @@ -479,45 +580,39 @@ QUIC_BUG << "Bad lengths " << *in.preferred_address; return false; } - if (!CBB_add_u16(¶ms, TransportParameters::kPreferredAddress) || - !CBB_add_u16_length_prefixed(¶ms, &preferred_address_params) || - !CBB_add_bytes( - &preferred_address_params, - reinterpret_cast<const uint8_t*>(v4_address_bytes.data()), - v4_address_bytes.length()) || - !CBB_add_u16(&preferred_address_params, - in.preferred_address->ipv4_socket_address.port()) || - !CBB_add_bytes( - &preferred_address_params, - reinterpret_cast<const uint8_t*>(v6_address_bytes.data()), - v6_address_bytes.length()) || - !CBB_add_u16(&preferred_address_params, - in.preferred_address->ipv6_socket_address.port()) || - !CBB_add_u8_length_prefixed(&preferred_address_params, - &preferred_address_connection_id_param) || - !CBB_add_bytes(&preferred_address_connection_id_param, - reinterpret_cast<const uint8_t*>( - in.preferred_address->connection_id.data()), - in.preferred_address->connection_id.length()) || - !CBB_add_bytes(&preferred_address_params, - in.preferred_address->stateless_reset_token.data(), - in.preferred_address->stateless_reset_token.size())) { + const uint64_t preferred_address_length = + v4_address_bytes.length() + /* IPv4 port */ sizeof(uint16_t) + + v6_address_bytes.length() + /* IPv6 port */ sizeof(uint16_t) + + /* connection ID length byte */ sizeof(uint8_t) + + in.preferred_address->connection_id.length() + + in.preferred_address->stateless_reset_token.size(); + if (!WriteTransportParameterId( + &writer, TransportParameters::kPreferredAddress, version) || + !WriteTransportParameterLength(&writer, preferred_address_length, + version) || + !writer.WriteStringPiece(v4_address_bytes) || + !writer.WriteUInt16(in.preferred_address->ipv4_socket_address.port()) || + !writer.WriteStringPiece(v6_address_bytes) || + !writer.WriteUInt16(in.preferred_address->ipv6_socket_address.port()) || + !writer.WriteUInt8(in.preferred_address->connection_id.length()) || + !writer.WriteBytes(in.preferred_address->connection_id.data(), + in.preferred_address->connection_id.length()) || + !writer.WriteBytes( + in.preferred_address->stateless_reset_token.data(), + in.preferred_address->stateless_reset_token.size())) { QUIC_BUG << "Failed to write preferred_address for " << in; return false; } } // Google-specific non-standard parameter. - CBB google_quic_params; if (in.google_quic_params) { const QuicData& serialized_google_quic_params = in.google_quic_params->GetSerialized(); - if (!CBB_add_u16(¶ms, TransportParameters::kGoogleQuicParam) || - !CBB_add_u16_length_prefixed(¶ms, &google_quic_params) || - !CBB_add_bytes(&google_quic_params, - reinterpret_cast<const uint8_t*>( - serialized_google_quic_params.data()), - serialized_google_quic_params.length())) { + if (!WriteTransportParameterId( + &writer, TransportParameters::kGoogleQuicParam, version) || + !WriteTransportParameterStringPiece( + &writer, serialized_google_quic_params.AsStringPiece(), version)) { QUIC_BUG << "Failed to write Google params of length " << serialized_google_quic_params.length() << " for " << in; return false; @@ -525,51 +620,64 @@ } // Google-specific version extension. - CBB google_version_params; - if (!CBB_add_u16(¶ms, TransportParameters::kGoogleQuicVersion) || - !CBB_add_u16_length_prefixed(¶ms, &google_version_params) || - !CBB_add_u32(&google_version_params, in.version)) { + static_assert(sizeof(QuicVersionLabel) == sizeof(uint32_t), "bad length"); + uint64_t google_version_length = sizeof(in.version); + if (in.perspective == Perspective::IS_SERVER) { + google_version_length += + /* versions length */ sizeof(uint8_t) + + sizeof(QuicVersionLabel) * in.supported_versions.size(); + } + if (!WriteTransportParameterId( + &writer, TransportParameters::kGoogleQuicVersion, version) || + !WriteTransportParameterLength(&writer, google_version_length, version) || + !writer.WriteUInt32(in.version)) { QUIC_BUG << "Failed to write Google version extension for " << in; return false; } - CBB versions; if (in.perspective == Perspective::IS_SERVER) { - if (!CBB_add_u8_length_prefixed(&google_version_params, &versions)) { + if (!writer.WriteUInt8(sizeof(QuicVersionLabel) * + in.supported_versions.size())) { QUIC_BUG << "Failed to write versions length for " << in; return false; } - for (QuicVersionLabel version : in.supported_versions) { - if (!CBB_add_u32(&versions, version)) { + for (QuicVersionLabel version_label : in.supported_versions) { + if (!writer.WriteUInt32(version_label)) { QUIC_BUG << "Failed to write supported version for " << in; return false; } } } - auto custom_parameters = std::make_unique<CBB[]>(in.custom_parameters.size()); - int i = 0; for (const auto& kv : in.custom_parameters) { - CBB* custom_parameter = &custom_parameters[i++]; QUIC_BUG_IF(kv.first < 0xff00) << "custom_parameters should not be used " "for non-private use parameters"; - if (!CBB_add_u16(¶ms, kv.first) || - !CBB_add_u16_length_prefixed(¶ms, custom_parameter) || - !CBB_add_bytes(custom_parameter, - reinterpret_cast<const uint8_t*>(kv.second.data()), - kv.second.size())) { - QUIC_BUG << "Failed to write custom parameter " - << static_cast<int>(kv.first); + if (!WriteTransportParameterId(&writer, kv.first, version) || + !WriteTransportParameterStringPiece(&writer, kv.second, version)) { + QUIC_BUG << "Failed to write custom parameter " << kv.first; return false; } } - if (!CBB_flush(cbb.get())) { - QUIC_BUG << "Failed to flush CBB for " << in; - return false; + if (!version.HasVarIntTransportParams()) { + // Fill in the length prefix at the start of the transport parameters. + if (writer.length() < sizeof(uint16_t) || + writer.length() - sizeof(uint16_t) > + std::numeric_limits<uint16_t>::max()) { + QUIC_BUG << "Cannot write length " << writer.length() << " for " << in; + return false; + } + const uint16_t length_prefix = writer.length() - sizeof(uint16_t); + QuicDataWriter length_prefix_writer(out->size(), + reinterpret_cast<char*>(out->data())); + if (!length_prefix_writer.WriteUInt16(length_prefix)) { + QUIC_BUG << "Failed to write length prefix for " << in; + return false; + } } - out->resize(CBB_len(cbb.get())); - memcpy(out->data(), CBB_data(cbb.get()), CBB_len(cbb.get())); - QUIC_DLOG(INFO) << "Serialized " << in << " as " << CBB_len(cbb.get()) + + out->resize(writer.length()); + + QUIC_DLOG(INFO) << "Serialized " << in << " as " << writer.length() << " bytes"; return true; } @@ -578,198 +686,192 @@ Perspective perspective, const uint8_t* in, size_t in_len, - TransportParameters* out) { + TransportParameters* out, + std::string* error_details) { out->perspective = perspective; - CBS cbs; - CBS_init(&cbs, in, in_len); + QuicDataReader reader(reinterpret_cast<const char*>(in), in_len); - CBS params; - if (!CBS_get_u16_length_prefixed(&cbs, ¶ms)) { - QUIC_DLOG(ERROR) << "Failed to parse the number of transport parameters"; - return false; + if (!version.HasVarIntTransportParams()) { + uint16_t full_length; + if (!reader.ReadUInt16(&full_length)) { + *error_details = "Failed to parse the transport parameter full length"; + return false; + } + if (full_length != reader.BytesRemaining()) { + *error_details = + quiche::QuicheStrCat("Invalid transport parameter full length ", + full_length, " != ", reader.BytesRemaining()); + return false; + } } - while (CBS_len(¶ms) > 0) { + while (!reader.IsDoneReading()) { TransportParameters::TransportParameterId param_id; - CBS value; - static_assert(sizeof(param_id) == sizeof(uint16_t), "bad size"); - if (!CBS_get_u16(¶ms, reinterpret_cast<uint16_t*>(¶m_id))) { - QUIC_DLOG(ERROR) << "Failed to parse transport parameter ID"; + if (!ReadTransportParameterId(&reader, version, ¶m_id)) { + *error_details = "Failed to parse transport parameter ID"; return false; } - if (!CBS_get_u16_length_prefixed(¶ms, &value)) { - QUIC_DLOG(ERROR) << "Failed to parse length of transport parameter " - << TransportParameterIdToString(param_id); + quiche::QuicheStringPiece value; + if (!ReadTransportParameterLengthAndValue(&reader, version, &value)) { + *error_details = + "Failed to read length and value of transport parameter " + + TransportParameterIdToString(param_id); return false; } + QuicDataReader value_reader(value); bool parse_success = true; switch (param_id) { case TransportParameters::kOriginalConnectionId: { if (!out->original_connection_id.IsEmpty()) { - QUIC_DLOG(ERROR) << "Received a second original connection ID"; + *error_details = "Received a second original connection ID"; return false; } - const size_t connection_id_length = CBS_len(&value); + const size_t connection_id_length = value_reader.BytesRemaining(); if (!QuicUtils::IsConnectionIdLengthValidForVersion( connection_id_length, version.transport_version)) { - QUIC_DLOG(ERROR) << "Received original connection ID of " - << "invalid length " << connection_id_length; + *error_details = quiche::QuicheStrCat( + "Received original connection ID of invalid length ", + connection_id_length); return false; } - out->original_connection_id.set_length( - static_cast<uint8_t>(connection_id_length)); - if (out->original_connection_id.length() != 0) { - memcpy(out->original_connection_id.mutable_data(), CBS_data(&value), - out->original_connection_id.length()); + if (!value_reader.ReadConnectionId(&out->original_connection_id, + connection_id_length)) { + *error_details = "Failed to read original connection ID"; + return false; } } break; case TransportParameters::kIdleTimeout: - parse_success = out->idle_timeout_milliseconds.ReadFromCbs(&value); + parse_success = + out->idle_timeout_milliseconds.Read(&value_reader, error_details); break; - case TransportParameters::kStatelessResetToken: + case TransportParameters::kStatelessResetToken: { if (!out->stateless_reset_token.empty()) { - QUIC_DLOG(ERROR) << "Received a second stateless reset token"; + *error_details = "Received a second stateless reset token"; return false; } - if (CBS_len(&value) != kStatelessResetTokenLength) { - QUIC_DLOG(ERROR) << "Received stateless reset token of " - << "invalid length " << CBS_len(&value); + quiche::QuicheStringPiece stateless_reset_token = + value_reader.ReadRemainingPayload(); + if (stateless_reset_token.length() != kStatelessResetTokenLength) { + *error_details = quiche::QuicheStrCat( + "Received stateless reset token of invalid length ", + stateless_reset_token.length()); return false; } - out->stateless_reset_token.assign(CBS_data(&value), - CBS_data(&value) + CBS_len(&value)); - break; + out->stateless_reset_token.assign( + stateless_reset_token.data(), + stateless_reset_token.data() + stateless_reset_token.length()); + } break; case TransportParameters::kMaxPacketSize: - parse_success = out->max_packet_size.ReadFromCbs(&value); + parse_success = out->max_packet_size.Read(&value_reader, error_details); break; case TransportParameters::kInitialMaxData: - parse_success = out->initial_max_data.ReadFromCbs(&value); + parse_success = + out->initial_max_data.Read(&value_reader, error_details); break; case TransportParameters::kInitialMaxStreamDataBidiLocal: - parse_success = - out->initial_max_stream_data_bidi_local.ReadFromCbs(&value); + parse_success = out->initial_max_stream_data_bidi_local.Read( + &value_reader, error_details); break; case TransportParameters::kInitialMaxStreamDataBidiRemote: - parse_success = - out->initial_max_stream_data_bidi_remote.ReadFromCbs(&value); + parse_success = out->initial_max_stream_data_bidi_remote.Read( + &value_reader, error_details); break; case TransportParameters::kInitialMaxStreamDataUni: - parse_success = out->initial_max_stream_data_uni.ReadFromCbs(&value); + parse_success = + out->initial_max_stream_data_uni.Read(&value_reader, error_details); break; case TransportParameters::kInitialMaxStreamsBidi: - parse_success = out->initial_max_streams_bidi.ReadFromCbs(&value); + parse_success = + out->initial_max_streams_bidi.Read(&value_reader, error_details); break; case TransportParameters::kInitialMaxStreamsUni: - parse_success = out->initial_max_streams_uni.ReadFromCbs(&value); + parse_success = + out->initial_max_streams_uni.Read(&value_reader, error_details); break; case TransportParameters::kAckDelayExponent: - parse_success = out->ack_delay_exponent.ReadFromCbs(&value); + parse_success = + out->ack_delay_exponent.Read(&value_reader, error_details); break; case TransportParameters::kMaxAckDelay: - parse_success = out->max_ack_delay.ReadFromCbs(&value); + parse_success = out->max_ack_delay.Read(&value_reader, error_details); break; case TransportParameters::kDisableMigration: if (out->disable_migration) { - QUIC_DLOG(ERROR) << "Received a second disable migration"; - return false; - } - if (CBS_len(&value) != 0) { - QUIC_DLOG(ERROR) << "Received disable migration of invalid length " - << CBS_len(&value); + *error_details = "Received a second disable migration"; return false; } out->disable_migration = true; break; case TransportParameters::kPreferredAddress: { + TransportParameters::PreferredAddress preferred_address; uint16_t ipv4_port, ipv6_port; in_addr ipv4_address; in6_addr ipv6_address; - if (!CBS_copy_bytes(&value, reinterpret_cast<uint8_t*>(&ipv4_address), - sizeof(ipv4_address)) || - !CBS_get_u16(&value, &ipv4_port) || - !CBS_copy_bytes(&value, reinterpret_cast<uint8_t*>(&ipv6_address), - sizeof(ipv6_address)) || - !CBS_get_u16(&value, &ipv6_port)) { - QUIC_DLOG(ERROR) << "Failed to parse preferred address IPs and ports"; + preferred_address.stateless_reset_token.resize( + kStatelessResetTokenLength); + if (!value_reader.ReadBytes(&ipv4_address, sizeof(ipv4_address)) || + !value_reader.ReadUInt16(&ipv4_port) || + !value_reader.ReadBytes(&ipv6_address, sizeof(ipv6_address)) || + !value_reader.ReadUInt16(&ipv6_port) || + !value_reader.ReadLengthPrefixedConnectionId( + &preferred_address.connection_id) || + !value_reader.ReadBytes(&preferred_address.stateless_reset_token[0], + kStatelessResetTokenLength)) { + *error_details = "Failed to read preferred address"; return false; } - TransportParameters::PreferredAddress preferred_address; preferred_address.ipv4_socket_address = QuicSocketAddress(QuicIpAddress(ipv4_address), ipv4_port); preferred_address.ipv6_socket_address = QuicSocketAddress(QuicIpAddress(ipv6_address), ipv6_port); if (!preferred_address.ipv4_socket_address.host().IsIPv4() || !preferred_address.ipv6_socket_address.host().IsIPv6()) { - QUIC_DLOG(ERROR) << "Received preferred addresses of bad families " - << preferred_address; + *error_details = "Received preferred addresses of bad families " + + preferred_address.ToString(); return false; } - CBS connection_id_cbs; - if (!CBS_get_u8_length_prefixed(&value, &connection_id_cbs)) { - QUIC_DLOG(ERROR) - << "Failed to parse length of preferred address connection ID"; + if (!QuicUtils::IsConnectionIdValidForVersion( + preferred_address.connection_id, version.transport_version)) { + *error_details = "Received invalid preferred address connection ID " + + preferred_address.ToString(); return false; } - const size_t connection_id_length = CBS_len(&connection_id_cbs); - if (!QuicUtils::IsConnectionIdLengthValidForVersion( - connection_id_length, version.transport_version)) { - QUIC_DLOG(ERROR) << "Received preferred address connection ID of " - << "invalid length " << connection_id_length; - return false; - } - preferred_address.connection_id.set_length( - static_cast<uint8_t>(connection_id_length)); - if (preferred_address.connection_id.length() > 0 && - !CBS_copy_bytes(&connection_id_cbs, - reinterpret_cast<uint8_t*>( - preferred_address.connection_id.mutable_data()), - preferred_address.connection_id.length())) { - QUIC_DLOG(ERROR) << "Failed to read preferred address connection ID"; - return false; - } - if (CBS_len(&value) != kStatelessResetTokenLength) { - QUIC_DLOG(ERROR) << "Received preferred address with " - << "invalid remaining length " << CBS_len(&value); - return false; - } - preferred_address.stateless_reset_token.assign( - CBS_data(&value), CBS_data(&value) + CBS_len(&value)); out->preferred_address = std::make_unique<TransportParameters::PreferredAddress>( preferred_address); } break; case TransportParameters::kActiveConnectionIdLimit: - parse_success = out->active_connection_id_limit.ReadFromCbs(&value); + parse_success = + out->active_connection_id_limit.Read(&value_reader, error_details); break; case TransportParameters::kMaxDatagramFrameSize: - parse_success = out->max_datagram_frame_size.ReadFromCbs(&value); + parse_success = + out->max_datagram_frame_size.Read(&value_reader, error_details); break; case TransportParameters::kGoogleQuicParam: { if (out->google_quic_params) { - QUIC_DLOG(ERROR) << "Received a second Google parameter"; + *error_details = "Received a second Google parameter"; return false; } - quiche::QuicheStringPiece serialized_params( - reinterpret_cast<const char*>(CBS_data(&value)), CBS_len(&value)); - out->google_quic_params = CryptoFramer::ParseMessage(serialized_params); + out->google_quic_params = + CryptoFramer::ParseMessage(value_reader.ReadRemainingPayload()); } break; case TransportParameters::kGoogleQuicVersion: { - if (!CBS_get_u32(&value, &out->version)) { - QUIC_DLOG(ERROR) << "Failed to parse Google version extension"; + if (!value_reader.ReadUInt32(&out->version)) { + *error_details = "Failed to read Google version extension version"; return false; } if (perspective == Perspective::IS_SERVER) { - CBS versions; - if (!CBS_get_u8_length_prefixed(&value, &versions) || - CBS_len(&versions) % 4 != 0) { - QUIC_DLOG(ERROR) - << "Failed to parse Google supported versions length"; + uint8_t versions_length; + if (!value_reader.ReadUInt8(&versions_length)) { + *error_details = "Failed to parse Google supported versions length"; return false; } - while (CBS_len(&versions) > 0) { + const uint8_t num_versions = versions_length / sizeof(uint32_t); + for (uint8_t i = 0; i < num_versions; ++i) { QuicVersionLabel version; - if (!CBS_get_u32(&versions, &version)) { - QUIC_DLOG(ERROR) << "Failed to parse Google supported version"; + if (!value_reader.ReadUInt32(&version)) { + *error_details = "Failed to parse Google supported version"; return false; } out->supported_versions.push_back(version); @@ -777,24 +879,37 @@ } } break; default: - out->custom_parameters[param_id] = std::string( - reinterpret_cast<const char*>(CBS_data(&value)), CBS_len(&value)); + if (out->custom_parameters.find(param_id) != + out->custom_parameters.end()) { + *error_details = "Received a second unknown parameter" + + TransportParameterIdToString(param_id); + return false; + } + out->custom_parameters[param_id] = + std::string(value_reader.ReadRemainingPayload()); break; } if (!parse_success) { + DCHECK(!error_details->empty()); + return false; + } + if (!value_reader.IsDoneReading()) { + *error_details = quiche::QuicheStrCat( + "Received unexpected ", value_reader.BytesRemaining(), + " bytes after parsing ", TransportParameterIdToString(param_id)); return false; } } - const bool ok = out->AreValid(); - if (ok) { - QUIC_DLOG(INFO) << "Parsed transport parameters " << *out << " from " - << in_len << " bytes"; - } else { - QUIC_DLOG(ERROR) << "Transport parameter validity check failed " << *out - << " from " << in_len << " bytes"; + if (!out->AreValid(error_details)) { + DCHECK(!error_details->empty()); + return false; } - return ok; + + QUIC_DLOG(INFO) << "Parsed transport parameters " << *out << " from " + << in_len << " bytes"; + + return true; } } // namespace quic
diff --git a/quic/core/crypto/transport_parameters.h b/quic/core/crypto/transport_parameters.h index 07b0132..831caec 100644 --- a/quic/core/crypto/transport_parameters.h +++ b/quic/core/crypto/transport_parameters.h
@@ -8,9 +8,9 @@ #include <memory> #include <vector> -#include "third_party/boringssl/src/include/openssl/bytestring.h" #include "net/third_party/quiche/src/quic/core/crypto/crypto_handshake_message.h" #include "net/third_party/quiche/src/quic/core/quic_connection_id.h" +#include "net/third_party/quiche/src/quic/core/quic_data_reader.h" #include "net/third_party/quiche/src/quic/core/quic_data_writer.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_versions.h" @@ -25,7 +25,7 @@ // This struct currently uses the values from draft 20. struct QUIC_EXPORT_PRIVATE TransportParameters { // The identifier used to differentiate transport parameters. - enum TransportParameterId : uint16_t; + enum TransportParameterId : uint64_t; // A map used to specify custom parameters. using ParameterMap = QuicUnorderedMap<TransportParameterId, std::string>; // Represents an individual QUIC transport parameter that only encodes a @@ -46,10 +46,12 @@ // Writes to a crypto byte buffer, used during serialization. Does not write // anything if the value is equal to the parameter's default value. // Returns whether the write was successful. - bool WriteToCbb(CBB* parent_cbb) const; + bool Write(QuicDataWriter* writer, ParsedQuicVersion version) const; // Reads from a crypto byte string, used during parsing. // Returns whether the read was successful. - bool ReadFromCbs(CBS* const value_cbs); + // On failure, this method will write a human-readable error message to + // |error_details|. + bool Read(QuicDataReader* reader, std::string* error_details); // operator<< allows easily logging integer transport parameters. friend QUIC_EXPORT_PRIVATE std::ostream& operator<<( std::ostream& os, @@ -79,7 +81,7 @@ // Maximum value of this transport parameter, as per IETF specification. const uint64_t max_value_; // Ensures this parameter is not parsed twice in the same message. - bool has_been_read_from_cbs_; + bool has_been_read_; }; // Represents the preferred_address transport parameter that a server can @@ -179,8 +181,9 @@ std::unique_ptr<CryptoHandshakeMessage> google_quic_params; // Validates whether transport parameters are valid according to - // the specification. - bool AreValid() const; + // the specification. If the transport parameters are not valid, this method + // will write a human-readable error message to |error_details|. + bool AreValid(std::string* error_details) const; // Custom parameters that may be specific to application protocol. ParameterMap custom_parameters; @@ -204,12 +207,15 @@ // parsed parameters into |*out|. Input is read from |in| for |in_len| bytes. // |perspective| indicates whether the input came from a client or a server. // This method returns true if the input was successfully parsed. +// On failure, this method will write a human-readable error message to +// |error_details|. // TODO(nharper): Write fuzz tests for this method. QUIC_EXPORT_PRIVATE bool ParseTransportParameters(ParsedQuicVersion version, Perspective perspective, const uint8_t* in, size_t in_len, - TransportParameters* out); + TransportParameters* out, + std::string* error_details); } // namespace quic
diff --git a/quic/core/crypto/transport_parameters_test.cc b/quic/core/crypto/transport_parameters_test.cc index ed7a63c..bbcba74 100644 --- a/quic/core/crypto/transport_parameters_test.cc +++ b/quic/core/crypto/transport_parameters_test.cc
@@ -7,8 +7,8 @@ #include <cstring> #include <utility> -#include "third_party/boringssl/src/include/openssl/mem.h" #include "net/third_party/quiche/src/quic/core/quic_versions.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" #include "net/third_party/quiche/src/quic/platform/api/quic_ip_address.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" @@ -22,7 +22,6 @@ using testing::Pair; using testing::UnorderedElementsAre; -const ParsedQuicVersion kVersion(PROTOCOL_TLS1_3, QUIC_VERSION_99); const QuicVersionLabel kFakeVersionLabel = 0x01234567; const QuicVersionLabel kFakeVersionLabel2 = 0x89ABCDEF; const uint64_t kFakeIdleTimeoutMilliseconds = 12012; @@ -102,11 +101,31 @@ preferred_address); } +std::vector<ParsedQuicVersion> AllSupportedTlsVersions() { + std::vector<ParsedQuicVersion> tls_versions; + for (const ParsedQuicVersion& version : AllSupportedVersions()) { + if (version.handshake_protocol == PROTOCOL_TLS1_3) { + tls_versions.push_back(version); + } + } + return tls_versions; +} + } // namespace -class TransportParametersTest : public QuicTest {}; +class TransportParametersTest : public QuicTestWithParam<ParsedQuicVersion> { + protected: + TransportParametersTest() : version_(GetParam()) {} -TEST_F(TransportParametersTest, RoundTripClient) { + ParsedQuicVersion version_; +}; + +INSTANTIATE_TEST_SUITE_P(TransportParametersTests, + TransportParametersTest, + ::testing::ValuesIn(AllSupportedTlsVersions()), + ::testing::PrintToStringParamName()); + +TEST_P(TransportParametersTest, RoundTripClient) { TransportParameters orig_params; orig_params.perspective = Perspective::IS_CLIENT; orig_params.version = kFakeVersionLabel; @@ -130,13 +149,15 @@ orig_params.custom_parameters[kCustomParameter2] = kCustomParameter2Value; std::vector<uint8_t> serialized; - ASSERT_TRUE(SerializeTransportParameters(kVersion, orig_params, &serialized)); + ASSERT_TRUE(SerializeTransportParameters(version_, orig_params, &serialized)); TransportParameters new_params; - ASSERT_TRUE(ParseTransportParameters(kVersion, Perspective::IS_CLIENT, + std::string error_details; + ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_CLIENT, serialized.data(), serialized.size(), - &new_params)); - + &new_params, &error_details)) + << error_details; + EXPECT_TRUE(error_details.empty()); EXPECT_EQ(Perspective::IS_CLIENT, new_params.perspective); EXPECT_EQ(kFakeVersionLabel, new_params.version); EXPECT_TRUE(new_params.supported_versions.empty()); @@ -167,7 +188,7 @@ Pair(kCustomParameter2, kCustomParameter2Value))); } -TEST_F(TransportParametersTest, RoundTripServer) { +TEST_P(TransportParametersTest, RoundTripServer) { TransportParameters orig_params; orig_params.perspective = Perspective::IS_SERVER; orig_params.version = kFakeVersionLabel; @@ -194,13 +215,15 @@ kFakeActiveConnectionIdLimit); std::vector<uint8_t> serialized; - ASSERT_TRUE(SerializeTransportParameters(kVersion, orig_params, &serialized)); + ASSERT_TRUE(SerializeTransportParameters(version_, orig_params, &serialized)); TransportParameters new_params; - ASSERT_TRUE(ParseTransportParameters(kVersion, Perspective::IS_SERVER, + std::string error_details; + ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_SERVER, serialized.data(), serialized.size(), - &new_params)); - + &new_params, &error_details)) + << error_details; + EXPECT_TRUE(error_details.empty()); EXPECT_EQ(Perspective::IS_SERVER, new_params.perspective); EXPECT_EQ(kFakeVersionLabel, new_params.version); EXPECT_EQ(2u, new_params.supported_versions.size()); @@ -240,50 +263,76 @@ EXPECT_EQ(0u, new_params.custom_parameters.size()); } -TEST_F(TransportParametersTest, IsValid) { +TEST_P(TransportParametersTest, AreValid) { { TransportParameters params; + std::string error_details; params.perspective = Perspective::IS_CLIENT; - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); } { TransportParameters params; + std::string error_details; params.perspective = Perspective::IS_CLIENT; - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.idle_timeout_milliseconds.set_value(kFakeIdleTimeoutMilliseconds); - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.idle_timeout_milliseconds.set_value(601000); - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); } { TransportParameters params; + std::string error_details; params.perspective = Perspective::IS_CLIENT; - EXPECT_TRUE(params.AreValid()); - params.max_packet_size.set_value(0); - EXPECT_FALSE(params.AreValid()); - params.max_packet_size.set_value(1199); - EXPECT_FALSE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.max_packet_size.set_value(1200); - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.max_packet_size.set_value(65535); - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.max_packet_size.set_value(9999999); - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); + params.max_packet_size.set_value(0); + error_details = ""; + EXPECT_FALSE(params.AreValid(&error_details)); + EXPECT_EQ( + error_details, + "Invalid transport parameters [Client max_packet_size 0 (Invalid)]"); + params.max_packet_size.set_value(1199); + error_details = ""; + EXPECT_FALSE(params.AreValid(&error_details)); + EXPECT_EQ( + error_details, + "Invalid transport parameters [Client max_packet_size 1199 (Invalid)]"); } { TransportParameters params; + std::string error_details; params.perspective = Perspective::IS_CLIENT; - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.ack_delay_exponent.set_value(0); - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.ack_delay_exponent.set_value(20); - EXPECT_TRUE(params.AreValid()); + EXPECT_TRUE(params.AreValid(&error_details)); + EXPECT_TRUE(error_details.empty()); params.ack_delay_exponent.set_value(21); - EXPECT_FALSE(params.AreValid()); + EXPECT_FALSE(params.AreValid(&error_details)); + EXPECT_EQ(error_details, + "Invalid transport parameters [Client ack_delay_exponent 21 " + "(Invalid)]"); } } -TEST_F(TransportParametersTest, NoClientParamsWithStatelessResetToken) { +TEST_P(TransportParametersTest, NoClientParamsWithStatelessResetToken) { TransportParameters orig_params; orig_params.perspective = Perspective::IS_CLIENT; orig_params.version = kFakeVersionLabel; @@ -292,12 +341,17 @@ orig_params.max_packet_size.set_value(kFakeMaxPacketSize); std::vector<uint8_t> out; - EXPECT_FALSE(SerializeTransportParameters(kVersion, orig_params, &out)); + bool ok; + EXPECT_QUIC_BUG( + ok = SerializeTransportParameters(version_, orig_params, &out), + "Not serializing invalid transport parameters: Client cannot send " + "stateless reset token"); + EXPECT_FALSE(ok); } -TEST_F(TransportParametersTest, ParseClientParams) { +TEST_P(TransportParametersTest, ParseClientParams) { // clang-format off - const uint8_t kClientParams[] = { + const uint8_t kClientParamsOld[] = { 0x00, 0x49, // length of the parameters array that follows // idle_timeout 0x00, 0x01, // parameter id @@ -351,13 +405,74 @@ 0x00, 0x04, // length 0x01, 0x23, 0x45, 0x67, // initial version }; + const uint8_t kClientParams[] = { + // idle_timeout + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + // max_packet_size + 0x03, // parameter id + 0x02, // length + 0x63, 0x29, // value + // initial_max_data + 0x04, // parameter id + 0x02, // length + 0x40, 0x65, // value + // initial_max_stream_data_bidi_local + 0x05, // parameter id + 0x02, // length + 0x47, 0xD1, // value + // initial_max_stream_data_bidi_remote + 0x06, // parameter id + 0x02, // length + 0x47, 0xD2, // value + // initial_max_stream_data_uni + 0x07, // parameter id + 0x02, // length + 0x4B, 0xB8, // value + // initial_max_streams_bidi + 0x08, // parameter id + 0x01, // length + 0x15, // value + // initial_max_streams_uni + 0x09, // parameter id + 0x01, // length + 0x16, // value + // ack_delay_exponent + 0x0a, // parameter id + 0x01, // length + 0x0a, // value + // max_ack_delay + 0x0b, // parameter id + 0x01, // length + 0x33, // value + // disable_migration + 0x0c, // parameter id + 0x00, // length + // active_connection_id_limit + 0x0e, // parameter id + 0x01, // length + 0x34, // value + // Google version extension + 0x80, 0x00, 0x47, 0x52, // parameter id + 0x04, // length + 0x01, 0x23, 0x45, 0x67, // initial version + }; // clang-format on - + const uint8_t* client_params = + reinterpret_cast<const uint8_t*>(kClientParams); + size_t client_params_length = QUICHE_ARRAYSIZE(kClientParams); + if (!version_.HasVarIntTransportParams()) { + client_params = reinterpret_cast<const uint8_t*>(kClientParamsOld); + client_params_length = QUICHE_ARRAYSIZE(kClientParamsOld); + } TransportParameters new_params; - ASSERT_TRUE( - ParseTransportParameters(kVersion, Perspective::IS_CLIENT, kClientParams, - QUICHE_ARRAYSIZE(kClientParams), &new_params)); - + std::string error_details; + ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_CLIENT, + client_params, client_params_length, + &new_params, &error_details)) + << error_details; + EXPECT_TRUE(error_details.empty()); EXPECT_EQ(Perspective::IS_CLIENT, new_params.perspective); EXPECT_EQ(kFakeVersionLabel, new_params.version); EXPECT_TRUE(new_params.supported_versions.empty()); @@ -384,19 +499,18 @@ new_params.active_connection_id_limit.value()); } -TEST_F(TransportParametersTest, ParseClientParamsFailsWithStatelessResetToken) { - TransportParameters out_params; - +TEST_P(TransportParametersTest, + ParseClientParamsFailsWithFullStatelessResetToken) { // clang-format off - const uint8_t kClientParamsWithFullToken[] = { + const uint8_t kClientParamsWithFullTokenOld[] = { 0x00, 0x26, // length parameters array that follows // idle_timeout 0x00, 0x01, // parameter id 0x00, 0x02, // length 0x6e, 0xec, // value // stateless_reset_token - 0x00, 0x02, - 0x00, 0x10, + 0x00, 0x02, // parameter id + 0x00, 0x10, // length 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, 0x98, 0x99, 0x9A, 0x9B, 0x9C, 0x9D, 0x9E, 0x9F, // max_packet_size @@ -408,21 +522,53 @@ 0x00, 0x02, // length 0x40, 0x65, // value }; + const uint8_t kClientParamsWithFullToken[] = { + // idle_timeout + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + // stateless_reset_token + 0x02, // parameter id + 0x10, // length + 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, + 0x98, 0x99, 0x9A, 0x9B, 0x9C, 0x9D, 0x9E, 0x9F, + // max_packet_size + 0x03, // parameter id + 0x02, // length + 0x63, 0x29, // value + // initial_max_data + 0x04, // parameter id + 0x02, // length + 0x40, 0x65, // value + }; // clang-format on + const uint8_t* client_params = + reinterpret_cast<const uint8_t*>(kClientParamsWithFullToken); + size_t client_params_length = QUICHE_ARRAYSIZE(kClientParamsWithFullToken); + if (!version_.HasVarIntTransportParams()) { + client_params = + reinterpret_cast<const uint8_t*>(kClientParamsWithFullTokenOld); + client_params_length = QUICHE_ARRAYSIZE(kClientParamsWithFullTokenOld); + } + TransportParameters out_params; + std::string error_details; + EXPECT_FALSE(ParseTransportParameters(version_, Perspective::IS_CLIENT, + client_params, client_params_length, + &out_params, &error_details)); + EXPECT_EQ(error_details, "Client cannot send stateless reset token"); +} - EXPECT_FALSE(ParseTransportParameters( - kVersion, Perspective::IS_CLIENT, kClientParamsWithFullToken, - QUICHE_ARRAYSIZE(kClientParamsWithFullToken), &out_params)); - +TEST_P(TransportParametersTest, + ParseClientParamsFailsWithEmptyStatelessResetToken) { // clang-format off - const uint8_t kClientParamsWithEmptyToken[] = { + const uint8_t kClientParamsWithEmptyTokenOld[] = { 0x00, 0x16, // length parameters array that follows // idle_timeout 0x00, 0x01, // parameter id 0x00, 0x02, // length 0x6e, 0xec, // value // stateless_reset_token - 0x00, 0x02, + 0x00, 0x02, // parameter id 0x00, 0x00, // max_packet_size 0x00, 0x03, // parameter id @@ -433,24 +579,49 @@ 0x00, 0x02, // length 0x40, 0x65, // value }; + const uint8_t kClientParamsWithEmptyToken[] = { + // idle_timeout + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + // stateless_reset_token + 0x02, // parameter id + 0x00, // length + // max_packet_size + 0x03, // parameter id + 0x02, // length + 0x63, 0x29, // value + // initial_max_data + 0x04, // parameter id + 0x02, // length + 0x40, 0x65, // value + }; // clang-format on - - EXPECT_FALSE(ParseTransportParameters( - kVersion, Perspective::IS_CLIENT, kClientParamsWithEmptyToken, - QUICHE_ARRAYSIZE(kClientParamsWithEmptyToken), &out_params)); + const uint8_t* client_params = + reinterpret_cast<const uint8_t*>(kClientParamsWithEmptyToken); + size_t client_params_length = QUICHE_ARRAYSIZE(kClientParamsWithEmptyToken); + if (!version_.HasVarIntTransportParams()) { + client_params = + reinterpret_cast<const uint8_t*>(kClientParamsWithEmptyTokenOld); + client_params_length = QUICHE_ARRAYSIZE(kClientParamsWithEmptyTokenOld); + } + TransportParameters out_params; + std::string error_details; + EXPECT_FALSE(ParseTransportParameters(version_, Perspective::IS_CLIENT, + client_params, client_params_length, + &out_params, &error_details)); + EXPECT_EQ(error_details, + "Received stateless reset token of invalid length 0"); } -TEST_F(TransportParametersTest, ParseClientParametersRepeated) { +TEST_P(TransportParametersTest, ParseClientParametersRepeated) { // clang-format off - const uint8_t kClientParamsRepeated[] = { - 0x00, 0x16, // length parameters array that follows + const uint8_t kClientParamsRepeatedOld[] = { + 0x00, 0x12, // length parameters array that follows // idle_timeout 0x00, 0x01, // parameter id 0x00, 0x02, // length 0x6e, 0xec, // value - // stateless_reset_token - 0x00, 0x02, - 0x00, 0x00, // max_packet_size 0x00, 0x03, // parameter id 0x00, 0x02, // length @@ -460,16 +631,39 @@ 0x00, 0x02, // length 0x6e, 0xec, // value }; + const uint8_t kClientParamsRepeated[] = { + // idle_timeout + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + // max_packet_size + 0x03, // parameter id + 0x02, // length + 0x63, 0x29, // value + // idle_timeout (repeated) + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + }; // clang-format on + const uint8_t* client_params = + reinterpret_cast<const uint8_t*>(kClientParamsRepeated); + size_t client_params_length = QUICHE_ARRAYSIZE(kClientParamsRepeated); + if (!version_.HasVarIntTransportParams()) { + client_params = reinterpret_cast<const uint8_t*>(kClientParamsRepeatedOld); + client_params_length = QUICHE_ARRAYSIZE(kClientParamsRepeatedOld); + } TransportParameters out_params; - EXPECT_FALSE(ParseTransportParameters( - kVersion, Perspective::IS_CLIENT, kClientParamsRepeated, - QUICHE_ARRAYSIZE(kClientParamsRepeated), &out_params)); + std::string error_details; + EXPECT_FALSE(ParseTransportParameters(version_, Perspective::IS_CLIENT, + client_params, client_params_length, + &out_params, &error_details)); + EXPECT_EQ(error_details, "Received a second idle_timeout"); } -TEST_F(TransportParametersTest, ParseServerParams) { +TEST_P(TransportParametersTest, ParseServerParams) { // clang-format off - const uint8_t kServerParams[] = { + const uint8_t kServerParamsOld[] = { 0x00, 0xa7, // length of parameters array that follows // original_connection_id 0x00, 0x00, // parameter id @@ -480,8 +674,8 @@ 0x00, 0x02, // length 0x6e, 0xec, // value // stateless_reset_token - 0x00, 0x02, - 0x00, 0x10, + 0x00, 0x02, // parameter id + 0x00, 0x10, // length 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, 0x98, 0x99, 0x9A, 0x9B, 0x9C, 0x9D, 0x9E, 0x9F, // max_packet_size @@ -547,13 +741,98 @@ 0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, }; + const uint8_t kServerParams[] = { + // original_connection_id + 0x00, // parameter id + 0x08, // length + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x13, 0x37, + // idle_timeout + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + // stateless_reset_token + 0x02, // parameter id + 0x10, // length + 0x90, 0x91, 0x92, 0x93, 0x94, 0x95, 0x96, 0x97, + 0x98, 0x99, 0x9A, 0x9B, 0x9C, 0x9D, 0x9E, 0x9F, + // max_packet_size + 0x03, // parameter id + 0x02, // length + 0x63, 0x29, // value + // initial_max_data + 0x04, // parameter id + 0x02, // length + 0x40, 0x65, // value + // initial_max_stream_data_bidi_local + 0x05, // parameter id + 0x02, // length + 0x47, 0xD1, // value + // initial_max_stream_data_bidi_remote + 0x06, // parameter id + 0x02, // length + 0x47, 0xD2, // value + // initial_max_stream_data_uni + 0x07, // parameter id + 0x02, // length + 0x4B, 0xB8, // value + // initial_max_streams_bidi + 0x08, // parameter id + 0x01, // length + 0x15, // value + // initial_max_streams_uni + 0x09, // parameter id + 0x01, // length + 0x16, // value + // ack_delay_exponent + 0x0a, // parameter id + 0x01, // length + 0x0a, // value + // max_ack_delay + 0x0b, // parameter id + 0x01, // length + 0x33, // value + // disable_migration + 0x0c, // parameter id + 0x00, // length + // preferred_address + 0x0d, // parameter id + 0x31, // length + 0x41, 0x42, 0x43, 0x44, // IPv4 address + 0x48, 0x84, // IPv4 port + 0x60, 0x61, 0x62, 0x63, 0x64, 0x65, 0x66, 0x67, // IPv6 address + 0x68, 0x69, 0x6a, 0x6b, 0x6c, 0x6d, 0x6e, 0x6f, + 0x63, 0x36, // IPv6 port + 0x08, // connection ID length + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xBE, 0xEF, // connection ID + 0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87, // stateless reset token + 0x88, 0x89, 0x8A, 0x8B, 0x8C, 0x8D, 0x8E, 0x8F, + // active_connection_id_limit + 0x0e, // parameter id + 0x01, // length + 0x34, // value + // Google version extension + 0x80, 0x00, 0x47, 0x52, // parameter id + 0x0d, // length + 0x01, 0x23, 0x45, 0x67, // negotiated_version + 0x08, // length of supported versions array + 0x01, 0x23, 0x45, 0x67, + 0x89, 0xab, 0xcd, 0xef, + }; // clang-format on - + const uint8_t* server_params = + reinterpret_cast<const uint8_t*>(kServerParams); + size_t server_params_length = QUICHE_ARRAYSIZE(kServerParams); + if (!version_.HasVarIntTransportParams()) { + server_params = reinterpret_cast<const uint8_t*>(kServerParamsOld); + server_params_length = QUICHE_ARRAYSIZE(kServerParamsOld); + } TransportParameters new_params; - ASSERT_TRUE( - ParseTransportParameters(kVersion, Perspective::IS_SERVER, kServerParams, - QUICHE_ARRAYSIZE(kServerParams), &new_params)); - + std::string error_details; + ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_SERVER, + server_params, server_params_length, + &new_params, &error_details)) + << error_details; + EXPECT_TRUE(error_details.empty()); EXPECT_EQ(Perspective::IS_SERVER, new_params.perspective); EXPECT_EQ(kFakeVersionLabel, new_params.version); EXPECT_EQ(2u, new_params.supported_versions.size()); @@ -592,9 +871,9 @@ new_params.active_connection_id_limit.value()); } -TEST_F(TransportParametersTest, ParseServerParametersRepeated) { +TEST_P(TransportParametersTest, ParseServerParametersRepeated) { // clang-format off - const uint8_t kServerParamsRepeated[] = { + const uint8_t kServerParamsRepeatedOld[] = { 0x00, 0x2c, // length of parameters array that follows // original_connection_id 0x00, 0x00, // parameter id @@ -605,8 +884,8 @@ 0x00, 0x02, // length 0x6e, 0xec, // value // stateless_reset_token - 0x00, 0x02, - 0x00, 0x10, + 0x00, 0x02, // parameter id + 0x00, 0x10, // length 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, // idle_timeout (repeated) @@ -614,15 +893,42 @@ 0x00, 0x02, // length 0x6e, 0xec, // value }; + const uint8_t kServerParamsRepeated[] = { + // original_connection_id + 0x00, // parameter id + 0x08, // length + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x13, 0x37, + // idle_timeout + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + // stateless_reset_token + 0x02, // parameter id + 0x10, // length + 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, + 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, + // idle_timeout (repeated) + 0x01, // parameter id + 0x02, // length + 0x6e, 0xec, // value + }; // clang-format on - + const uint8_t* server_params = + reinterpret_cast<const uint8_t*>(kServerParamsRepeated); + size_t server_params_length = QUICHE_ARRAYSIZE(kServerParamsRepeated); + if (!version_.HasVarIntTransportParams()) { + server_params = reinterpret_cast<const uint8_t*>(kServerParamsRepeatedOld); + server_params_length = QUICHE_ARRAYSIZE(kServerParamsRepeatedOld); + } TransportParameters out_params; - EXPECT_FALSE(ParseTransportParameters( - kVersion, Perspective::IS_SERVER, kServerParamsRepeated, - QUICHE_ARRAYSIZE(kServerParamsRepeated), &out_params)); + std::string error_details; + EXPECT_FALSE(ParseTransportParameters(version_, Perspective::IS_SERVER, + server_params, server_params_length, + &out_params, &error_details)); + EXPECT_EQ(error_details, "Received a second idle_timeout"); } -TEST_F(TransportParametersTest, CryptoHandshakeMessageRoundtrip) { +TEST_P(TransportParametersTest, CryptoHandshakeMessageRoundtrip) { TransportParameters orig_params; orig_params.perspective = Perspective::IS_CLIENT; orig_params.version = kFakeVersionLabel; @@ -635,13 +941,15 @@ orig_params.google_quic_params->SetValue(1337, kTestValue); std::vector<uint8_t> serialized; - ASSERT_TRUE(SerializeTransportParameters(kVersion, orig_params, &serialized)); + ASSERT_TRUE(SerializeTransportParameters(version_, orig_params, &serialized)); TransportParameters new_params; - ASSERT_TRUE(ParseTransportParameters(kVersion, Perspective::IS_CLIENT, + std::string error_details; + ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_CLIENT, serialized.data(), serialized.size(), - &new_params)); - + &new_params, &error_details)) + << error_details; + EXPECT_TRUE(error_details.empty()); ASSERT_NE(new_params.google_quic_params.get(), nullptr); EXPECT_EQ(new_params.google_quic_params->tag(), orig_params.google_quic_params->tag());