Plumb original_connection_id transport parameter
This transport parameter was already parsed in TransportParameters, but was not acted on. This CL plumbs the received value through QuicConfig to QuicConnection and fails the handshake if the value is not the expected one. This CL also improves debugging in various tests.
Client-only change
PiperOrigin-RevId: 310221632
Change-Id: I75b50926034f1ee5d913c7579e4f3a5e15e97b4a
diff --git a/quic/core/crypto/transport_parameters.cc b/quic/core/crypto/transport_parameters.cc
index 6c3f3c5..75f55ca 100644
--- a/quic/core/crypto/transport_parameters.cc
+++ b/quic/core/crypto/transport_parameters.cc
@@ -387,9 +387,9 @@
rv += " supported_versions " +
QuicVersionLabelVectorToString(supported_versions);
}
- if (!original_connection_id.IsEmpty()) {
+ if (original_connection_id.has_value()) {
rv += " " + TransportParameterIdToString(kOriginalConnectionId) + " " +
- original_connection_id.ToString();
+ original_connection_id.value().ToString();
}
rv += idle_timeout_milliseconds.ToString(/*for_use_in_list=*/true);
if (!stateless_reset_token.empty()) {
@@ -429,7 +429,6 @@
TransportParameters::TransportParameters()
: version(0),
- original_connection_id(EmptyQuicConnectionId()),
idle_timeout_milliseconds(kIdleTimeout),
max_packet_size(kMaxPacketSize,
kDefaultMaxPacketSizeTransportParam,
@@ -550,7 +549,7 @@
return false;
}
if (perspective == Perspective::IS_CLIENT &&
- !original_connection_id.IsEmpty()) {
+ original_connection_id.has_value()) {
*error_details = "Client cannot send original connection ID";
return false;
}
@@ -638,17 +637,18 @@
}
// original_connection_id
- if (!in.original_connection_id.IsEmpty()) {
+ if (in.original_connection_id.has_value()) {
DCHECK_EQ(Perspective::IS_SERVER, in.perspective);
+ QuicConnectionId original_connection_id = in.original_connection_id.value();
if (!WriteTransportParameterId(
&writer, TransportParameters::kOriginalConnectionId, version) ||
!WriteTransportParameterStringPiece(
&writer,
- quiche::QuicheStringPiece(in.original_connection_id.data(),
- in.original_connection_id.length()),
+ quiche::QuicheStringPiece(original_connection_id.data(),
+ original_connection_id.length()),
version)) {
QUIC_BUG << "Failed to write original_connection_id "
- << in.original_connection_id << " for " << in;
+ << in.original_connection_id.value() << " for " << in;
return false;
}
}
@@ -900,7 +900,7 @@
bool parse_success = true;
switch (param_id) {
case TransportParameters::kOriginalConnectionId: {
- if (!out->original_connection_id.IsEmpty()) {
+ if (out->original_connection_id.has_value()) {
*error_details = "Received a second original connection ID";
return false;
}
@@ -912,11 +912,13 @@
connection_id_length);
return false;
}
- if (!value_reader.ReadConnectionId(&out->original_connection_id,
+ QuicConnectionId original_connection_id;
+ if (!value_reader.ReadConnectionId(&original_connection_id,
connection_id_length)) {
*error_details = "Failed to read original connection ID";
return false;
}
+ out->original_connection_id = original_connection_id;
} break;
case TransportParameters::kIdleTimeout:
parse_success =
diff --git a/quic/core/crypto/transport_parameters.h b/quic/core/crypto/transport_parameters.h
index 28dcc24..d4dd82f 100644
--- a/quic/core/crypto/transport_parameters.h
+++ b/quic/core/crypto/transport_parameters.h
@@ -15,6 +15,7 @@
#include "net/third_party/quiche/src/quic/core/quic_types.h"
#include "net/third_party/quiche/src/quic/core/quic_versions.h"
#include "net/third_party/quiche/src/quic/platform/api/quic_containers.h"
+#include "net/third_party/quiche/src/common/platform/api/quiche_optional.h"
#include "net/third_party/quiche/src/common/platform/api/quiche_string_piece.h"
namespace quic {
@@ -131,7 +132,7 @@
// The value of the Destination Connection ID field from the first
// Initial packet sent by the client.
- QuicConnectionId original_connection_id;
+ quiche::QuicheOptional<QuicConnectionId> original_connection_id;
// Idle timeout expressed in milliseconds.
IntegerParameter idle_timeout_milliseconds;
diff --git a/quic/core/crypto/transport_parameters_test.cc b/quic/core/crypto/transport_parameters_test.cc
index bfaeb1f..72654a7 100644
--- a/quic/core/crypto/transport_parameters_test.cc
+++ b/quic/core/crypto/transport_parameters_test.cc
@@ -7,6 +7,7 @@
#include <cstring>
#include <utility>
+#include "net/third_party/quiche/src/quic/core/quic_connection_id.h"
#include "net/third_party/quiche/src/quic/core/quic_types.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"
@@ -533,7 +534,7 @@
EXPECT_EQ(Perspective::IS_CLIENT, new_params.perspective);
EXPECT_EQ(kFakeVersionLabel, new_params.version);
EXPECT_TRUE(new_params.supported_versions.empty());
- EXPECT_EQ(EmptyQuicConnectionId(), new_params.original_connection_id);
+ EXPECT_FALSE(new_params.original_connection_id.has_value());
EXPECT_EQ(kFakeIdleTimeoutMilliseconds,
new_params.idle_timeout_milliseconds.value());
EXPECT_TRUE(new_params.stateless_reset_token.empty());
@@ -895,8 +896,9 @@
EXPECT_EQ(2u, new_params.supported_versions.size());
EXPECT_EQ(kFakeVersionLabel, new_params.supported_versions[0]);
EXPECT_EQ(kFakeVersionLabel2, new_params.supported_versions[1]);
+ ASSERT_TRUE(new_params.original_connection_id.has_value());
EXPECT_EQ(CreateFakeOriginalConnectionId(),
- new_params.original_connection_id);
+ new_params.original_connection_id.value());
EXPECT_EQ(kFakeIdleTimeoutMilliseconds,
new_params.idle_timeout_milliseconds.value());
EXPECT_EQ(CreateFakeStatelessResetToken(), new_params.stateless_reset_token);
@@ -985,6 +987,59 @@
EXPECT_EQ(error_details, "Received a second idle_timeout");
}
+TEST_P(TransportParametersTest,
+ ParseServerParametersEmptyOriginalConnectionId) {
+ // clang-format off
+ const uint8_t kServerParamsEmptyOriginalConnectionIdOld[] = {
+ 0x00, 0x1e, // length of parameters array that follows
+ // original_connection_id
+ 0x00, 0x00, // parameter id
+ 0x00, 0x00, // length
+ // idle_timeout
+ 0x00, 0x01, // parameter id
+ 0x00, 0x02, // length
+ 0x6e, 0xec, // value
+ // stateless_reset_token
+ 0x00, 0x02, // parameter id
+ 0x00, 0x10, // length
+ 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
+ 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10,
+ };
+ const uint8_t kServerParamsEmptyOriginalConnectionId[] = {
+ // original_connection_id
+ 0x00, // parameter id
+ 0x00, // length
+ // 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,
+ };
+ // clang-format on
+ const uint8_t* server_params =
+ reinterpret_cast<const uint8_t*>(kServerParamsEmptyOriginalConnectionId);
+ size_t server_params_length =
+ QUICHE_ARRAYSIZE(kServerParamsEmptyOriginalConnectionId);
+ if (!version_.HasVarIntTransportParams()) {
+ server_params = reinterpret_cast<const uint8_t*>(
+ kServerParamsEmptyOriginalConnectionIdOld);
+ server_params_length =
+ QUICHE_ARRAYSIZE(kServerParamsEmptyOriginalConnectionIdOld);
+ }
+ TransportParameters out_params;
+ std::string error_details;
+ ASSERT_TRUE(ParseTransportParameters(version_, Perspective::IS_SERVER,
+ server_params, server_params_length,
+ &out_params, &error_details))
+ << error_details;
+ ASSERT_TRUE(out_params.original_connection_id.has_value());
+ EXPECT_EQ(out_params.original_connection_id.value(), EmptyQuicConnectionId());
+}
+
TEST_P(TransportParametersTest, CryptoHandshakeMessageRoundtrip) {
TransportParameters orig_params;
orig_params.perspective = Perspective::IS_CLIENT;
