Use three bits of Connection ID to indicate config. Connection IDs generated by LoadBalancerEncoder and QuicSiloIndexEncoder are never used in production, because we do not load any configs. Thus, the only part that needs to be flag-protected is the number of '1' bits added to the front of HASH_QUIC connection IDs (in CombinedConnectionIdGenerator). Protected by FLAGS_quic_restart_flag_quic_3bit_config_id. PiperOrigin-RevId: 554623294
diff --git a/quiche/quic/load_balancer/load_balancer_config.h b/quiche/quic/load_balancer/load_balancer_config.h index bc92d5f..39b7781 100644 --- a/quiche/quic/load_balancer/load_balancer_config.h +++ b/quiche/quic/load_balancer/load_balancer_config.h
@@ -11,7 +11,14 @@ namespace quic { -inline constexpr uint8_t kNumLoadBalancerConfigs = 3; +// The number of bits in the first byte used for the config ID +inline constexpr uint8_t kConfigIdBits = 3; +// The number of bits in the first byte used for the connection ID length, if +// the encoder uses this option. Otherwise, by spec it's random bits. +inline constexpr uint8_t kConnectionIdLengthBits = 8 - kConfigIdBits; +// One codepoint is reserved for unroutable connection IDs, so subtract one to +// find the maximum number of configs. +inline constexpr uint8_t kNumLoadBalancerConfigs = (1 << kConfigIdBits) - 1; inline constexpr uint8_t kLoadBalancerKeyLen = 16; // Regardless of key length, the AES block size is always 16 Bytes. inline constexpr uint8_t kLoadBalancerBlockSize = 16;
diff --git a/quiche/quic/load_balancer/load_balancer_config_test.cc b/quiche/quic/load_balancer/load_balancer_config_test.cc index bb3d367..bd4cbd2 100644 --- a/quiche/quic/load_balancer/load_balancer_config_test.cc +++ b/quiche/quic/load_balancer/load_balancer_config_test.cc
@@ -9,7 +9,6 @@ #include "absl/types/span.h" #include "quiche/quic/platform/api/quic_expect_bug.h" #include "quiche/quic/platform/api/quic_test.h" -#include "quiche/quic/test_tools/quic_test_utils.h" namespace quic { @@ -27,8 +26,8 @@ TEST_F(LoadBalancerConfigTest, InvalidParams) { // Bogus config_id. EXPECT_QUIC_BUG( - EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(3, 4, 10).has_value()), - "Invalid LoadBalancerConfig Config ID 3 Server ID Length 4 " + EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(7, 4, 10).has_value()), + "Invalid LoadBalancerConfig Config ID 7 Server ID Length 4 " "Nonce Length 10"); // Bad Server ID lengths. EXPECT_QUIC_BUG(EXPECT_FALSE(LoadBalancerConfig::Create( @@ -37,17 +36,17 @@ "Invalid LoadBalancerConfig Config ID 2 Server ID Length 0 " "Nonce Length 10"); EXPECT_QUIC_BUG( - EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(2, 16, 4).has_value()), - "Invalid LoadBalancerConfig Config ID 2 Server ID Length 16 " + EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(6, 16, 4).has_value()), + "Invalid LoadBalancerConfig Config ID 6 Server ID Length 16 " "Nonce Length 4"); // Bad Nonce lengths. EXPECT_QUIC_BUG( - EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(2, 4, 2).has_value()), - "Invalid LoadBalancerConfig Config ID 2 Server ID Length 4 " + EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(6, 4, 2).has_value()), + "Invalid LoadBalancerConfig Config ID 6 Server ID Length 4 " "Nonce Length 2"); EXPECT_QUIC_BUG( - EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(2, 1, 17).has_value()), - "Invalid LoadBalancerConfig Config ID 2 Server ID Length 1 " + EXPECT_FALSE(LoadBalancerConfig::CreateUnencrypted(6, 1, 17).has_value()), + "Invalid LoadBalancerConfig Config ID 6 Server ID Length 1 " "Nonce Length 17"); // Bad key lengths. EXPECT_QUIC_BUG(
diff --git a/quiche/quic/load_balancer/load_balancer_decoder.cc b/quiche/quic/load_balancer/load_balancer_decoder.cc index cdff314..a770c28 100644 --- a/quiche/quic/load_balancer/load_balancer_decoder.cc +++ b/quiche/quic/load_balancer/load_balancer_decoder.cc
@@ -4,6 +4,9 @@ #include "quiche/quic/load_balancer/load_balancer_decoder.h" +#include <cstdint> + +#include "quiche/quic/load_balancer/load_balancer_config.h" #include "quiche/quic/platform/api/quic_bug_tracker.h" namespace quic { @@ -80,7 +83,7 @@ absl::optional<uint8_t> LoadBalancerDecoder::GetConfigId( const uint8_t connection_id_first_byte) { - uint8_t codepoint = (connection_id_first_byte >> 6); + uint8_t codepoint = (connection_id_first_byte >> kConnectionIdLengthBits); if (codepoint < kNumLoadBalancerConfigs) { return codepoint; }
diff --git a/quiche/quic/load_balancer/load_balancer_decoder_test.cc b/quiche/quic/load_balancer/load_balancer_decoder_test.cc index b2bdfd3..eccc90f 100644 --- a/quiche/quic/load_balancer/load_balancer_decoder_test.cc +++ b/quiche/quic/load_balancer/load_balancer_decoder_test.cc
@@ -4,10 +4,13 @@ #include "quiche/quic/load_balancer/load_balancer_decoder.h" +#include <cstdint> + +#include "quiche/quic/core/quic_connection_id.h" +#include "quiche/quic/load_balancer/load_balancer_config.h" #include "quiche/quic/load_balancer/load_balancer_server_id.h" #include "quiche/quic/platform/api/quic_expect_bug.h" #include "quiche/quic/platform/api/quic_test.h" -#include "quiche/quic/test_tools/quic_test_utils.h" namespace quic { @@ -46,7 +49,7 @@ }, { *LoadBalancerConfig::CreateUnencrypted(1, 8, 5), - QuicConnectionId({0x4d, 0xed, 0x79, 0x3a, 0x51, 0xd4, 0x9b, 0x8f, + QuicConnectionId({0x2d, 0xed, 0x79, 0x3a, 0x51, 0xd4, 0x9b, 0x8f, 0x5f, 0xee, 0x15, 0xda, 0x27, 0xc4}), MakeServerId(kServerId, 8), }}; @@ -71,20 +74,20 @@ }, { *LoadBalancerConfig::Create(1, 10, 5, kKey), - QuicConnectionId({0x4f, 0xcd, 0x3f, 0x57, 0x2d, 0x4e, 0xef, 0xb0, + QuicConnectionId({0x2f, 0xcd, 0x3f, 0x57, 0x2d, 0x4e, 0xef, 0xb0, 0x46, 0xfd, 0xb5, 0x1d, 0x16, 0x4e, 0xfc, 0xcc}), MakeServerId(kServerId, 10), }, { *LoadBalancerConfig::Create(2, 8, 8, kKey), - QuicConnectionId({0x90, 0x4d, 0xd2, 0xd0, 0x5a, 0x7b, 0x0d, 0xe9, + QuicConnectionId({0x50, 0x4d, 0xd2, 0xd0, 0x5a, 0x7b, 0x0d, 0xe9, 0xb2, 0xb9, 0x90, 0x7a, 0xfb, 0x5e, 0xcf, 0x8c, 0xc3}), MakeServerId(kServerId, 8), }, { - *LoadBalancerConfig::Create(0, 9, 9, kKey), - QuicConnectionId({0x12, 0x12, 0x4d, 0x1e, 0xb8, 0xfb, 0xb2, 0x1e, + *LoadBalancerConfig::Create(3, 9, 9, kKey), + QuicConnectionId({0x72, 0x12, 0x4d, 0x1e, 0xb8, 0xfb, 0xb2, 0x1e, 0x4a, 0x49, 0x0c, 0xa5, 0x3c, 0xfe, 0x21, 0xd0, 0x4a, 0xe6, 0x3a}), MakeServerId(kServerId, 9), @@ -130,7 +133,7 @@ decoder.AddConfig(*LoadBalancerConfig::CreateUnencrypted(1, 3, 4))); EXPECT_FALSE(decoder .GetServerId(QuicConnectionId( - {0xc0, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07})) + {0xe0, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07})) .has_value()); } @@ -170,11 +173,11 @@ LoadBalancerDecoder decoder; decoder.AddConfig(*LoadBalancerConfig::CreateUnencrypted(2, 3, 4)); decoder.DeleteConfig(0); - EXPECT_QUIC_BUG(decoder.DeleteConfig(3), - "Decoder deleting config with invalid config_id 3"); + EXPECT_QUIC_BUG(decoder.DeleteConfig(7), + "Decoder deleting config with invalid config_id 7"); EXPECT_TRUE(decoder .GetServerId(QuicConnectionId( - {0x80, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07})) + {0x40, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07})) .has_value()); } @@ -184,7 +187,7 @@ decoder.DeleteConfig(2); EXPECT_FALSE(decoder .GetServerId(QuicConnectionId( - {0x80, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07})) + {0x40, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07})) .has_value()); } @@ -207,8 +210,9 @@ TEST_F(LoadBalancerDecoderTest, GetConfigId) { EXPECT_FALSE( LoadBalancerDecoder::GetConfigId(QuicConnectionId()).has_value()); - for (uint8_t i = 0; i < 3; i++) { - const QuicConnectionId connection_id({static_cast<unsigned char>(i << 6)}); + for (uint8_t i = 0; i < kNumLoadBalancerConfigs; i++) { + const QuicConnectionId connection_id( + {static_cast<unsigned char>(i << kConnectionIdLengthBits)}); auto config_id = LoadBalancerDecoder::GetConfigId(connection_id); EXPECT_EQ(config_id, LoadBalancerDecoder::GetConfigId(connection_id.data()[0])); @@ -216,7 +220,7 @@ EXPECT_EQ(*config_id, i); } EXPECT_FALSE( - LoadBalancerDecoder::GetConfigId(QuicConnectionId({0xc0})).has_value()); + LoadBalancerDecoder::GetConfigId(QuicConnectionId({0xe0})).has_value()); } TEST_F(LoadBalancerDecoderTest, GetConfig) {
diff --git a/quiche/quic/load_balancer/load_balancer_encoder.cc b/quiche/quic/load_balancer/load_balancer_encoder.cc index 9e71d6c..58cacbf 100644 --- a/quiche/quic/load_balancer/load_balancer_encoder.cc +++ b/quiche/quic/load_balancer/load_balancer_encoder.cc
@@ -4,6 +4,8 @@ #include "quiche/quic/load_balancer/load_balancer_encoder.h" +#include <cstdint> + #include "absl/numeric/int128.h" #include "quiche/quic/core/quic_connection_id.h" #include "quiche/quic/core/quic_data_reader.h" @@ -102,7 +104,7 @@ QuicConnectionId LoadBalancerEncoder::GenerateConnectionId() { uint8_t config_id = config_.has_value() ? config_->config_id() : kLoadBalancerUnroutableConfigId; - uint8_t shifted_config_id = config_id << 6; + uint8_t shifted_config_id = config_id << kConnectionIdLengthBits; uint8_t length = connection_id_lengths_[config_id]; if (config_.has_value() != server_id_.has_value()) { QUIC_BUG(quic_bug_435375038_04) @@ -188,15 +190,17 @@ if (len_self_encoded()) { return (first_byte &= kLoadBalancerLengthMask) + 1; } - return connection_id_lengths_[first_byte >> 6]; + return connection_id_lengths_[first_byte >> kConnectionIdLengthBits]; } QuicConnectionId LoadBalancerEncoder::MakeUnroutableConnectionId( uint8_t first_byte) { QuicConnectionId id; - id.set_length(connection_id_lengths_[kLoadBalancerUnroutableConfigId]); + uint8_t target_length = + connection_id_lengths_[kLoadBalancerUnroutableConfigId]; + id.set_length(target_length); id.mutable_data()[0] = first_byte; - random_.RandBytes(&id.mutable_data()[1], connection_id_lengths_[3] - 1); + random_.RandBytes(&id.mutable_data()[1], target_length - 1); return id; }
diff --git a/quiche/quic/load_balancer/load_balancer_encoder.h b/quiche/quic/load_balancer/load_balancer_encoder.h index ad83850..c780c5e 100644 --- a/quiche/quic/load_balancer/load_balancer_encoder.h +++ b/quiche/quic/load_balancer/load_balancer_encoder.h
@@ -20,16 +20,17 @@ inline constexpr uint8_t kLoadBalancerUnroutableLen = 8; // When the encoder is self-encoding the connection ID length, these are the // bits of the first byte that do so. -constexpr uint8_t kLoadBalancerLengthMask = 0x3f; +constexpr uint8_t kLoadBalancerLengthMask = (1 << kConnectionIdLengthBits) - 1; + // The bits of the connection ID first byte that encode the config ID. -constexpr uint8_t kLoadBalancerConfigIdMask = 0xc0; +constexpr uint8_t kLoadBalancerConfigIdMask = ~kLoadBalancerLengthMask; // The config ID that means the connection ID does not contain routing // information. constexpr uint8_t kLoadBalancerUnroutableConfigId = kNumLoadBalancerConfigs; // The bits of the connection ID first byte that correspond to a connection ID // that does not contain routing information. constexpr uint8_t kLoadBalancerUnroutablePrefix = - kLoadBalancerUnroutableConfigId << 6; + kLoadBalancerUnroutableConfigId << kConnectionIdLengthBits; // Interface which receives notifications when the current config is updated. class QUIC_EXPORT_PRIVATE LoadBalancerEncoderVisitorInterface { @@ -133,7 +134,8 @@ : random_(random), len_self_encoded_(len_self_encoded), visitor_(visitor) { - std::fill_n(connection_id_lengths_, 4, unroutable_connection_id_len); + std::fill_n(connection_id_lengths_, kNumLoadBalancerConfigs + 1, + unroutable_connection_id_len); } private:
diff --git a/quiche/quic/load_balancer/load_balancer_encoder_test.cc b/quiche/quic/load_balancer/load_balancer_encoder_test.cc index 88d09f4..d246758 100644 --- a/quiche/quic/load_balancer/load_balancer_encoder_test.cc +++ b/quiche/quic/load_balancer/load_balancer_encoder_test.cc
@@ -7,6 +7,7 @@ #include <cstdint> #include "absl/numeric/int128.h" +#include "quiche/quic/core/quic_connection_id.h" #include "quiche/quic/platform/api/quic_expect_bug.h" #include "quiche/quic/platform/api/quic_test.h" #include "quiche/quic/test_tools/quic_test_utils.h" @@ -169,8 +170,8 @@ }, { *LoadBalancerConfig::CreateUnencrypted(1, 8, 5), - QuicConnectionId({0x4d, 0xed, 0x79, 0x3a, 0x51, 0xd4, 0x9b, 0x8f, - 0x5f, 0xee, 0x15, 0xda, 0x27, 0xc4}), + QuicConnectionId({0x2d, 0xed, 0x79, 0x3a, 0x51, 0xd4, 0x9b, 0x8f, + 0x5f, 0x8e, 0x98, 0x53, 0xfe, 0x93}), MakeServerId(kServerId, 8), }, }; @@ -226,13 +227,13 @@ }, { *LoadBalancerConfig::Create(1, 10, 5, kKey), - QuicConnectionId({0x4f, 0xcd, 0x3f, 0x57, 0x2d, 0x4e, 0xef, 0xb0, + QuicConnectionId({0x2f, 0xcd, 0x3f, 0x57, 0x2d, 0x4e, 0xef, 0xb0, 0x46, 0xfd, 0xb5, 0x1d, 0x16, 0x4e, 0xfc, 0xcc}), MakeServerId(kServerId, 10), }, { *LoadBalancerConfig::Create(2, 8, 8, kKey), - QuicConnectionId({0x90, 0x4d, 0xd2, 0xd0, 0x5a, 0x7b, 0x0d, 0xe9, + QuicConnectionId({0x50, 0x4d, 0xd2, 0xd0, 0x5a, 0x7b, 0x0d, 0xe9, 0xb2, 0xb9, 0x90, 0x7a, 0xfb, 0x5e, 0xcf, 0x8c, 0xc3}), MakeServerId(kServerId, 8), @@ -281,9 +282,9 @@ ASSERT_TRUE(encoder.has_value()); EXPECT_EQ(encoder->num_nonces_left(), 0); auto connection_id = encoder->GenerateConnectionId(); - // The first byte is the config_id (0xc0) xored with (0x83 & 0x3f). + // The first byte is the config_id (0xe0) xored with (0x83 & 0x1f). // The remaining bytes are random, and therefore match kNonceHigh. - QuicConnectionId expected({0xc3, 0x5d, 0x52, 0xde, 0x4d, 0xe3, 0xe7, 0x21}); + QuicConnectionId expected({0xe3, 0x5d, 0x52, 0xde, 0x4d, 0xe3, 0xe7, 0x21}); EXPECT_EQ(expected, connection_id); } @@ -372,7 +373,7 @@ ASSERT_TRUE(encoder.has_value()); // The first byte is the config_id (0xc0) xored with (0x83 & 0x3f). // The remaining bytes are random, and therefore match kNonceHigh. - QuicConnectionId expected({0xc3, 0x5d, 0x52, 0xde, 0x4d, 0xe3, 0xe7, 0x21}); + QuicConnectionId expected({0xe3, 0x5d, 0x52, 0xde, 0x4d, 0xe3, 0xe7, 0x21}); EXPECT_EQ(*encoder->MaybeReplaceConnectionId(TestConnectionId(1), ParsedQuicVersion::RFCv1()), expected); @@ -393,7 +394,7 @@ ASSERT_TRUE(encoder.has_value()); // The first byte is the config_id (0xc0) xored with (0x83 & 0x3f). // The remaining bytes are random, and therefore match kNonceHigh. - QuicConnectionId expected({0xc3, 0x5d, 0x52, 0xde, 0x4d, 0xe3, 0xe7, 0x21}); + QuicConnectionId expected({0xe3, 0x5d, 0x52, 0xde, 0x4d, 0xe3, 0xe7, 0x21}); EXPECT_EQ(*encoder->GenerateNextConnectionId(TestConnectionId(1)), expected); } @@ -401,13 +402,13 @@ // The first byte literally encodes the length. auto len_encoder = LoadBalancerEncoder::Create(random_, nullptr, true); ASSERT_TRUE(len_encoder.has_value()); - EXPECT_EQ(len_encoder->ConnectionIdLength(0xc8), 9); + EXPECT_EQ(len_encoder->ConnectionIdLength(0xe8), 9); EXPECT_EQ(len_encoder->ConnectionIdLength(0x4a), 11); EXPECT_EQ(len_encoder->ConnectionIdLength(0x09), 10); // The length is not self-encoded anymore. auto encoder = LoadBalancerEncoder::Create(random_, nullptr, false); ASSERT_TRUE(encoder.has_value()); - EXPECT_EQ(encoder->ConnectionIdLength(0xc8), kQuicDefaultConnectionIdLength); + EXPECT_EQ(encoder->ConnectionIdLength(0xe8), kQuicDefaultConnectionIdLength); EXPECT_EQ(encoder->ConnectionIdLength(0x4a), kQuicDefaultConnectionIdLength); EXPECT_EQ(encoder->ConnectionIdLength(0x09), kQuicDefaultConnectionIdLength); // Add config ID 0, so that ID now returns a different length. @@ -420,7 +421,7 @@ ASSERT_TRUE(config0.has_value()); EXPECT_TRUE( encoder->UpdateConfig(*config0, MakeServerId(kServerId, server_id_len))); - EXPECT_EQ(encoder->ConnectionIdLength(0xc8), kQuicDefaultConnectionIdLength); + EXPECT_EQ(encoder->ConnectionIdLength(0xe8), kQuicDefaultConnectionIdLength); EXPECT_EQ(encoder->ConnectionIdLength(0x4a), kQuicDefaultConnectionIdLength); EXPECT_EQ(encoder->ConnectionIdLength(0x09), config_0_len); // Replace config ID 0 with 1. There are probably still packets with config @@ -434,13 +435,13 @@ // Old config length still there after replacement EXPECT_TRUE( encoder->UpdateConfig(*config1, MakeServerId(kServerId, server_id_len))); - EXPECT_EQ(encoder->ConnectionIdLength(0xc8), kQuicDefaultConnectionIdLength); - EXPECT_EQ(encoder->ConnectionIdLength(0x4a), config_1_len); + EXPECT_EQ(encoder->ConnectionIdLength(0xe8), kQuicDefaultConnectionIdLength); + EXPECT_EQ(encoder->ConnectionIdLength(0x2a), config_1_len); EXPECT_EQ(encoder->ConnectionIdLength(0x09), config_0_len); // Old config length still there after delete encoder->DeleteConfig(); - EXPECT_EQ(encoder->ConnectionIdLength(0xc8), kQuicDefaultConnectionIdLength); - EXPECT_EQ(encoder->ConnectionIdLength(0x4a), config_1_len); + EXPECT_EQ(encoder->ConnectionIdLength(0xe8), kQuicDefaultConnectionIdLength); + EXPECT_EQ(encoder->ConnectionIdLength(0x2a), config_1_len); EXPECT_EQ(encoder->ConnectionIdLength(0x09), config_0_len); }