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());
diff --git a/quic/core/quic_data_reader.cc b/quic/core/quic_data_reader.cc
index fac462a..0a09a9f 100644
--- a/quic/core/quic_data_reader.cc
+++ b/quic/core/quic_data_reader.cc
@@ -165,6 +165,15 @@
return false;
}
+bool QuicDataReader::ReadStringPieceVarInt62(
+ quiche::QuicheStringPiece* result) {
+ uint64_t result_length;
+ if (!ReadVarInt62(&result_length)) {
+ return false;
+ }
+ return ReadStringPiece(result, result_length);
+}
+
bool QuicDataReader::ReadVarIntU32(uint32_t* result) {
uint64_t temp_uint64;
// TODO(fkastenholz): We should disambiguate read-errors from
diff --git a/quic/core/quic_data_reader.h b/quic/core/quic_data_reader.h
index 08366b9..6b5c4d0 100644
--- a/quic/core/quic_data_reader.h
+++ b/quic/core/quic_data_reader.h
@@ -77,6 +77,16 @@
// and that the integers in the range 0 ... (2^62)-1.
bool ReadVarInt62(uint64_t* result);
+ // Reads a string prefixed with a Variable Length integer length into the
+ // given output parameter.
+ //
+ // NOTE: Does not copy but rather references strings in the underlying buffer.
+ // This should be kept in mind when handling memory management!
+ //
+ // Forwards the internal iterator on success.
+ // Returns true on success, false otherwise.
+ bool ReadStringPieceVarInt62(quiche::QuicheStringPiece* result);
+
// Convenience method that reads a uint32_t.
// Attempts to read a varint into a uint32_t. using ReadVarInt62 and
// returns false if there is a read error or if the value is
diff --git a/quic/core/quic_data_writer_test.cc b/quic/core/quic_data_writer_test.cc
index d27b8b1..36038cf 100644
--- a/quic/core/quic_data_writer_test.cc
+++ b/quic/core/quic_data_writer_test.cc
@@ -1269,6 +1269,23 @@
sizeof(buffer));
}
+TEST_P(QuicDataWriterTest, StringPieceVarInt62) {
+ char inner_buffer[16] = {1, 2, 3, 4, 5, 6, 7, 8,
+ 9, 10, 11, 12, 13, 14, 15, 16};
+ quiche::QuicheStringPiece inner_payload_write(inner_buffer,
+ sizeof(inner_buffer));
+ char buffer[sizeof(inner_buffer) + sizeof(uint8_t)] = {};
+ QuicDataWriter writer(sizeof(buffer), buffer);
+ EXPECT_TRUE(writer.WriteStringPieceVarInt62(inner_payload_write));
+ EXPECT_EQ(0u, writer.remaining());
+ QuicDataReader reader(buffer, sizeof(buffer));
+ quiche::QuicheStringPiece inner_payload_read;
+ EXPECT_TRUE(reader.ReadStringPieceVarInt62(&inner_payload_read));
+ quiche::test::CompareCharArraysWithHexError(
+ "inner_payload", inner_payload_write.data(), inner_payload_write.length(),
+ inner_payload_read.data(), inner_payload_read.length());
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/quic_version_manager.cc b/quic/core/quic_version_manager.cc
index c5ccf62..2e32a14 100644
--- a/quic/core/quic_version_manager.cc
+++ b/quic/core/quic_version_manager.cc
@@ -17,7 +17,7 @@
ParsedQuicVersionVector supported_versions)
: enable_version_t099_(GetQuicReloadableFlag(quic_enable_version_t099)),
enable_version_draft_25_(
- GetQuicReloadableFlag(quic_enable_version_draft_25_v2)),
+ GetQuicReloadableFlag(quic_enable_version_draft_25_v3)),
disable_version_q050_(GetQuicReloadableFlag(quic_disable_version_q050)),
enable_version_t050_(GetQuicReloadableFlag(quic_enable_version_t050)),
disable_version_q049_(GetQuicReloadableFlag(quic_disable_version_q049)),
@@ -48,7 +48,7 @@
"Supported versions out of sync");
if (enable_version_t099_ != GetQuicReloadableFlag(quic_enable_version_t099) ||
enable_version_draft_25_ !=
- GetQuicReloadableFlag(quic_enable_version_draft_25_v2) ||
+ GetQuicReloadableFlag(quic_enable_version_draft_25_v3) ||
disable_version_q050_ !=
GetQuicReloadableFlag(quic_disable_version_q050) ||
enable_version_t050_ != GetQuicReloadableFlag(quic_enable_version_t050) ||
@@ -62,7 +62,7 @@
GetQuicReloadableFlag(quic_disable_version_q043)) {
enable_version_t099_ = GetQuicReloadableFlag(quic_enable_version_t099);
enable_version_draft_25_ =
- GetQuicReloadableFlag(quic_enable_version_draft_25_v2);
+ GetQuicReloadableFlag(quic_enable_version_draft_25_v3);
disable_version_q050_ = GetQuicReloadableFlag(quic_disable_version_q050);
enable_version_t050_ = GetQuicReloadableFlag(quic_enable_version_t050);
disable_version_q049_ = GetQuicReloadableFlag(quic_disable_version_q049);
diff --git a/quic/core/quic_version_manager.h b/quic/core/quic_version_manager.h
index c434dae..82a42f1 100644
--- a/quic/core/quic_version_manager.h
+++ b/quic/core/quic_version_manager.h
@@ -43,7 +43,7 @@
// Cached value of reloadable flags.
// quic_enable_version_t099 flag
bool enable_version_t099_;
- // quic_enable_version_draft_25_v2 flag
+ // quic_enable_version_draft_25_v3 flag
bool enable_version_draft_25_;
// quic_disable_version_q050 flag
bool disable_version_q050_;
diff --git a/quic/core/quic_version_manager_test.cc b/quic/core/quic_version_manager_test.cc
index e64bc3a..86c5729 100644
--- a/quic/core/quic_version_manager_test.cc
+++ b/quic/core/quic_version_manager_test.cc
@@ -19,7 +19,7 @@
static_assert(SupportedVersions().size() == 8u,
"Supported versions out of sync");
SetQuicReloadableFlag(quic_enable_version_t099, false);
- SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false);
+ SetQuicReloadableFlag(quic_enable_version_draft_25_v3, false);
SetQuicReloadableFlag(quic_enable_version_t050, false);
SetQuicReloadableFlag(quic_disable_version_q050, false);
SetQuicReloadableFlag(quic_disable_version_q049, false);
@@ -52,7 +52,7 @@
EXPECT_EQ(FilterSupportedVersions(AllSupportedVersions()),
manager.GetSupportedVersions());
- SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true);
+ SetQuicReloadableFlag(quic_enable_version_draft_25_v3, true);
expected_parsed_versions.push_back(
ParsedQuicVersion(PROTOCOL_TLS1_3, QUIC_VERSION_IETF_DRAFT_25));
EXPECT_EQ(expected_parsed_versions, manager.GetSupportedVersions());
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index e6efcc5..3458c99 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -153,6 +153,11 @@
return HasIetfQuicFrames() && handshake_protocol == PROTOCOL_TLS1_3;
}
+bool ParsedQuicVersion::HasVarIntTransportParams() const {
+ DCHECK(IsKnown());
+ return transport_version >= QUIC_VERSION_99;
+}
+
bool VersionHasLengthPrefixedConnectionIds(
QuicTransportVersion transport_version) {
DCHECK(transport_version != QUIC_VERSION_UNSUPPORTED);
@@ -293,7 +298,7 @@
}
} else if (version.transport_version == QUIC_VERSION_IETF_DRAFT_25) {
QUIC_BUG_IF(version.handshake_protocol != PROTOCOL_TLS1_3);
- if (GetQuicReloadableFlag(quic_enable_version_draft_25_v2)) {
+ if (GetQuicReloadableFlag(quic_enable_version_draft_25_v3)) {
filtered_versions.push_back(version);
}
} else if (version.transport_version == QUIC_VERSION_50) {
@@ -548,7 +553,7 @@
SetQuicReloadableFlag(quic_enable_version_t099, true);
} else if (parsed_version.transport_version == QUIC_VERSION_IETF_DRAFT_25) {
QUIC_BUG_IF(parsed_version.handshake_protocol != PROTOCOL_TLS1_3);
- SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true);
+ SetQuicReloadableFlag(quic_enable_version_draft_25_v3, true);
} else if (parsed_version.transport_version == QUIC_VERSION_50) {
if (parsed_version.handshake_protocol == PROTOCOL_QUIC_CRYPTO) {
SetQuicReloadableFlag(quic_disable_version_q050, false);
diff --git a/quic/core/quic_versions.h b/quic/core/quic_versions.h
index e2ab8d7..bc15354 100644
--- a/quic/core/quic_versions.h
+++ b/quic/core/quic_versions.h
@@ -125,7 +125,7 @@
QuicTransportVersion transport_version);
// IETF draft version most closely approximated by TLS + v99.
-enum : int { kQuicIetfDraftVersion = 26 };
+enum : int { kQuicIetfDraftVersion = 27 };
// The crypto handshake protocols that can be used with QUIC.
enum HandshakeProtocol {
@@ -293,6 +293,10 @@
// Returns true if this parsed version supports handshake done.
bool HasHandshakeDone() const;
+
+ // Returns true if this version uses variable-length integers when
+ // encoding transport parameter types and lengths.
+ bool HasVarIntTransportParams() const;
};
QUIC_EXPORT_PRIVATE ParsedQuicVersion UnsupportedQuicVersion();
diff --git a/quic/core/quic_versions_test.cc b/quic/core/quic_versions_test.cc
index 78f3f3b..9c2ed74 100644
--- a/quic/core/quic_versions_test.cc
+++ b/quic/core/quic_versions_test.cc
@@ -270,7 +270,7 @@
static_assert(SupportedVersions().size() == 8u,
"Supported versions out of sync");
SetQuicReloadableFlag(quic_enable_version_t099, true);
- SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true);
+ SetQuicReloadableFlag(quic_enable_version_draft_25_v3, true);
SetQuicReloadableFlag(quic_enable_version_t050, true);
SetQuicReloadableFlag(quic_disable_version_q050, false);
SetQuicReloadableFlag(quic_disable_version_q049, false);
@@ -305,7 +305,7 @@
static_assert(SupportedVersions().size() == 8u,
"Supported versions out of sync");
SetQuicReloadableFlag(quic_enable_version_t099, false);
- SetQuicReloadableFlag(quic_enable_version_draft_25_v2, true);
+ SetQuicReloadableFlag(quic_enable_version_draft_25_v3, true);
SetQuicReloadableFlag(quic_enable_version_t050, true);
SetQuicReloadableFlag(quic_disable_version_q050, false);
SetQuicReloadableFlag(quic_disable_version_q049, false);
@@ -337,7 +337,7 @@
static_assert(SupportedVersions().size() == 8u,
"Supported versions out of sync");
SetQuicReloadableFlag(quic_enable_version_t099, false);
- SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false);
+ SetQuicReloadableFlag(quic_enable_version_draft_25_v3, false);
SetQuicReloadableFlag(quic_enable_version_t050, false);
SetQuicReloadableFlag(quic_disable_version_q050, false);
SetQuicReloadableFlag(quic_disable_version_q049, false);
@@ -432,8 +432,8 @@
EXPECT_EQ("h3-Q050", AlpnForVersion(parsed_version_q050));
EXPECT_EQ("h3-T050", AlpnForVersion(parsed_version_t050));
EXPECT_EQ("h3-25", AlpnForVersion(parsed_version_draft_25));
- EXPECT_EQ("h3-26", AlpnForVersion(parsed_version_t099));
- static_assert(kQuicIetfDraftVersion == 26,
+ EXPECT_EQ("h3-27", AlpnForVersion(parsed_version_t099));
+ static_assert(kQuicIetfDraftVersion == 27,
"ALPN does not match draft version");
}
@@ -458,9 +458,9 @@
{
QuicFlagSaver flag_saver;
- SetQuicReloadableFlag(quic_enable_version_draft_25_v2, false);
+ SetQuicReloadableFlag(quic_enable_version_draft_25_v3, false);
QuicEnableVersion(parsed_version_draft_25);
- EXPECT_TRUE(GetQuicReloadableFlag(quic_enable_version_draft_25_v2));
+ EXPECT_TRUE(GetQuicReloadableFlag(quic_enable_version_draft_25_v3));
}
{
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index 6f0bc4d..4600986 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -167,11 +167,17 @@
const uint8_t* param_bytes;
size_t param_bytes_len;
SSL_get_peer_quic_transport_params(ssl(), ¶m_bytes, ¶m_bytes_len);
- if (param_bytes_len == 0 ||
- !ParseTransportParameters(session()->connection()->version(),
- Perspective::IS_SERVER, param_bytes,
- param_bytes_len, ¶ms)) {
- *error_details = "Unable to parse Transport Parameters";
+ if (param_bytes_len == 0) {
+ *error_details = "Server's transport parameters are missing";
+ return false;
+ }
+ std::string parse_error_details;
+ if (!ParseTransportParameters(
+ session()->connection()->version(), Perspective::IS_SERVER,
+ param_bytes, param_bytes_len, ¶ms, &parse_error_details)) {
+ DCHECK(!parse_error_details.empty());
+ *error_details =
+ "Unable to parse server's transport parameters: " + parse_error_details;
return false;
}
diff --git a/quic/core/tls_server_handshaker.cc b/quic/core/tls_server_handshaker.cc
index bb8a821..6831e69 100644
--- a/quic/core/tls_server_handshaker.cc
+++ b/quic/core/tls_server_handshaker.cc
@@ -225,11 +225,18 @@
size_t params_bytes_len;
SSL_get_peer_quic_transport_params(ssl(), &client_params_bytes,
¶ms_bytes_len);
- if (params_bytes_len == 0 ||
- !ParseTransportParameters(session()->connection()->version(),
+ if (params_bytes_len == 0) {
+ *error_details = "Client's transport parameters are missing";
+ return false;
+ }
+ std::string parse_error_details;
+ if (!ParseTransportParameters(session()->connection()->version(),
Perspective::IS_CLIENT, client_params_bytes,
- params_bytes_len, &client_params)) {
- *error_details = "Unable to parse Transport Parameters";
+ params_bytes_len, &client_params,
+ &parse_error_details)) {
+ DCHECK(!parse_error_details.empty());
+ *error_details =
+ "Unable to parse client's transport parameters: " + parse_error_details;
return false;
}
