Extend LoadBalancerEncoder to support ConnectionIdGeneratorInterface. PiperOrigin-RevId: 472464687
diff --git a/quiche/quic/load_balancer/load_balancer_encoder.cc b/quiche/quic/load_balancer/load_balancer_encoder.cc index bb9608b..6cc8e0e 100644 --- a/quiche/quic/load_balancer/load_balancer_encoder.cc +++ b/quiche/quic/load_balancer/load_balancer_encoder.cc
@@ -167,6 +167,23 @@ return id; } +absl::optional<QuicConnectionId> LoadBalancerEncoder::GenerateNextConnectionId( + [[maybe_unused]] const QuicConnectionId &original) { + // Do not allow new connection IDs if linkable. + return (IsEncoding() && !IsEncrypted()) ? absl::optional<QuicConnectionId>() + : GenerateConnectionId(); +} + +absl::optional<QuicConnectionId> LoadBalancerEncoder::MaybeReplaceConnectionId( + const QuicConnectionId &original, const ParsedQuicVersion &version) { + // Pre-IETF versions of QUIC can respond poorly to new connection IDs issued + // during the handshake. + return (!version.HasIetfQuicFrames() && + original.length() == CurrentConnectionIdLength()) + ? absl::optional<QuicConnectionId>() + : GenerateConnectionId(); +} + QuicConnectionId LoadBalancerEncoder::MakeUnroutableConnectionId( uint8_t first_byte) { QuicConnectionId id;
diff --git a/quiche/quic/load_balancer/load_balancer_encoder.h b/quiche/quic/load_balancer/load_balancer_encoder.h index b48bb19..589c347 100644 --- a/quiche/quic/load_balancer/load_balancer_encoder.h +++ b/quiche/quic/load_balancer/load_balancer_encoder.h
@@ -5,6 +5,7 @@ #ifndef QUICHE_QUIC_LOAD_BALANCER_LOAD_BALANCER_ENCODER_H_ #define QUICHE_QUIC_LOAD_BALANCER_LOAD_BALANCER_ENCODER_H_ +#include "quiche/quic/core/connection_id_generator.h" #include "quiche/quic/core/crypto/quic_random.h" #include "quiche/quic/load_balancer/load_balancer_config.h" #include "quiche/quic/load_balancer/load_balancer_server_id.h" @@ -50,7 +51,8 @@ // Manages QUIC-LB configurations to properly encode a given server ID in a // QUIC Connection ID. -class QUIC_EXPORT_PRIVATE LoadBalancerEncoder { +class QUIC_EXPORT_PRIVATE LoadBalancerEncoder + : public ConnectionIdGeneratorInterface { public: // Returns a newly created encoder with no active config, if // |unroutable_connection_id_length| is valid. |visitor| specifies an optional @@ -94,6 +96,13 @@ // length Connection ID. QuicConnectionId GenerateConnectionId(); + // Functions from ConnectionIdGeneratorInterface + absl::optional<QuicConnectionId> GenerateNextConnectionId( + const QuicConnectionId& original) override; + absl::optional<QuicConnectionId> MaybeReplaceConnectionId( + const QuicConnectionId& original, + const ParsedQuicVersion& version) override; + private: friend class test::LoadBalancerEncoderPeer; @@ -108,6 +117,10 @@ QuicConnectionId MakeUnroutableConnectionId(uint8_t first_byte); + uint8_t CurrentConnectionIdLength() const { + return IsEncoding() ? config_->total_len() : unroutable_connection_id_len_; + } + QuicRandom& random_; const bool len_self_encoded_; LoadBalancerEncoderVisitorInterface* const visitor_;
diff --git a/quiche/quic/load_balancer/load_balancer_encoder_test.cc b/quiche/quic/load_balancer/load_balancer_encoder_test.cc index f1df860..a331713 100644 --- a/quiche/quic/load_balancer/load_balancer_encoder_test.cc +++ b/quiche/quic/load_balancer/load_balancer_encoder_test.cc
@@ -130,10 +130,10 @@ TEST_F(LoadBalancerEncoderTest, BadServerIdLength) { auto encoder = LoadBalancerEncoder::Create(random_, nullptr, true); - EXPECT_TRUE(encoder.has_value()); + ASSERT_TRUE(encoder.has_value()); // Expects a 3 byte server ID and got 4. auto config = LoadBalancerConfig::CreateUnencrypted(1, 3, 4); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); EXPECT_QUIC_BUG( EXPECT_FALSE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 4))), "Server ID length 4 does not match configured value of 3"); @@ -143,10 +143,9 @@ TEST_F(LoadBalancerEncoderTest, FailToUpdateConfigWithSameId) { TestLoadBalancerEncoderVisitor visitor; auto encoder = LoadBalancerEncoder::Create(random_, &visitor, true); - EXPECT_TRUE(encoder.has_value()); - EXPECT_TRUE(encoder.has_value()); + ASSERT_TRUE(encoder.has_value()); auto config = LoadBalancerConfig::CreateUnencrypted(1, 3, 4); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); EXPECT_TRUE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 3))); EXPECT_EQ(visitor.num_adds(), 1u); EXPECT_QUIC_BUG( @@ -199,10 +198,10 @@ }; random_.AddNextValues(0, 0x75c2699c); auto encoder = LoadBalancerEncoder::Create(random_, nullptr, true, 8); - EXPECT_TRUE(encoder.has_value()); + ASSERT_TRUE(encoder.has_value()); auto config = LoadBalancerConfig::Create(config_id, server_id_len, nonce_len, absl::string_view(raw_key)); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); EXPECT_TRUE(encoder->UpdateConfig( *config, *LoadBalancerServerId::Create(raw_server_id))); EXPECT_TRUE(encoder->IsEncoding()); @@ -248,7 +247,7 @@ }; for (const auto &test : test_vectors) { auto encoder = LoadBalancerEncoder::Create(random_, nullptr, true, 8); - EXPECT_TRUE(encoder.has_value()); + ASSERT_TRUE(encoder.has_value()); random_.AddNextValues(kNonceHigh, kNonceLow); EXPECT_TRUE(encoder->UpdateConfig(test.config, test.server_id)); EXPECT_EQ(encoder->GenerateConnectionId(), test.connection_id); @@ -259,9 +258,9 @@ const uint8_t server_id_len = 3; TestLoadBalancerEncoderVisitor visitor; auto encoder = LoadBalancerEncoder::Create(random_, &visitor, true, 8); - EXPECT_TRUE(encoder.has_value()); + ASSERT_TRUE(encoder.has_value()); auto config = LoadBalancerConfig::Create(0, server_id_len, 4, kKey); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); EXPECT_TRUE( encoder->UpdateConfig(*config, MakeServerId(kServerId, server_id_len))); EXPECT_EQ(visitor.num_adds(), 1u); @@ -279,7 +278,7 @@ TEST_F(LoadBalancerEncoderTest, UnroutableConnectionId) { random_.AddNextValues(0x83, kNonceHigh); auto encoder = LoadBalancerEncoder::Create(random_, nullptr, false); - EXPECT_TRUE(encoder.has_value()); + 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). @@ -290,7 +289,7 @@ TEST_F(LoadBalancerEncoderTest, NonDefaultUnroutableConnectionIdLength) { auto encoder = LoadBalancerEncoder::Create(random_, nullptr, true, 9); - EXPECT_TRUE(encoder.has_value()); + ASSERT_TRUE(encoder.has_value()); QuicConnectionId connection_id = encoder->GenerateConnectionId(); EXPECT_EQ(connection_id.length(), 9); } @@ -298,14 +297,14 @@ TEST_F(LoadBalancerEncoderTest, DeleteConfigWhenNoConfigExists) { TestLoadBalancerEncoderVisitor visitor; auto encoder = LoadBalancerEncoder::Create(random_, &visitor, true); - EXPECT_TRUE(encoder.has_value()); + ASSERT_TRUE(encoder.has_value()); encoder->DeleteConfig(); EXPECT_EQ(visitor.num_deletes(), 0u); } TEST_F(LoadBalancerEncoderTest, AddConfig) { auto config = LoadBalancerConfig::CreateUnencrypted(0, 3, 4); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); TestLoadBalancerEncoderVisitor visitor; auto encoder = LoadBalancerEncoder::Create(random_, &visitor, true); EXPECT_TRUE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 3))); @@ -321,12 +320,12 @@ TEST_F(LoadBalancerEncoderTest, UpdateConfig) { auto config = LoadBalancerConfig::CreateUnencrypted(0, 3, 4); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); TestLoadBalancerEncoderVisitor visitor; auto encoder = LoadBalancerEncoder::Create(random_, &visitor, true); EXPECT_TRUE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 3))); config = LoadBalancerConfig::Create(1, 4, 4, kKey); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); EXPECT_TRUE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 4))); EXPECT_EQ(visitor.num_adds(), 2u); EXPECT_EQ(visitor.num_deletes(), 1u); @@ -336,7 +335,7 @@ TEST_F(LoadBalancerEncoderTest, DeleteConfig) { auto config = LoadBalancerConfig::CreateUnencrypted(0, 3, 4); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); TestLoadBalancerEncoderVisitor visitor; auto encoder = LoadBalancerEncoder::Create(random_, &visitor, true); EXPECT_TRUE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 3))); @@ -350,7 +349,7 @@ TEST_F(LoadBalancerEncoderTest, DeleteConfigNoVisitor) { auto config = LoadBalancerConfig::CreateUnencrypted(0, 3, 4); - EXPECT_TRUE(config.has_value()); + ASSERT_TRUE(config.has_value()); auto encoder = LoadBalancerEncoder::Create(random_, nullptr, true); EXPECT_TRUE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 3))); encoder->DeleteConfig(); @@ -359,6 +358,45 @@ EXPECT_EQ(encoder->num_nonces_left(), 0); } +TEST_F(LoadBalancerEncoderTest, MaybeReplaceConnectionIdReturnsNoChange) { + auto encoder = LoadBalancerEncoder::Create(random_, nullptr, false); + ASSERT_TRUE(encoder.has_value()); + EXPECT_EQ(encoder->MaybeReplaceConnectionId(TestConnectionId(1), + ParsedQuicVersion::Q050()), + absl::nullopt); +} + +TEST_F(LoadBalancerEncoderTest, MaybeReplaceConnectionIdReturnsChange) { + random_.AddNextValues(0x83, kNonceHigh); + auto encoder = LoadBalancerEncoder::Create(random_, nullptr, false); + 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}); + EXPECT_EQ(*encoder->MaybeReplaceConnectionId(TestConnectionId(1), + ParsedQuicVersion::RFCv1()), + expected); +} + +TEST_F(LoadBalancerEncoderTest, GenerateNextConnectionIdReturnsNoChange) { + auto config = LoadBalancerConfig::CreateUnencrypted(0, 3, 4); + ASSERT_TRUE(config.has_value()); + auto encoder = LoadBalancerEncoder::Create(random_, nullptr, true); + EXPECT_TRUE(encoder->UpdateConfig(*config, MakeServerId(kServerId, 3))); + EXPECT_EQ(encoder->GenerateNextConnectionId(TestConnectionId(1)), + absl::nullopt); +} + +TEST_F(LoadBalancerEncoderTest, GenerateNextConnectionIdReturnsChange) { + random_.AddNextValues(0x83, kNonceHigh); + auto encoder = LoadBalancerEncoder::Create(random_, nullptr, false); + 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}); + EXPECT_EQ(*encoder->GenerateNextConnectionId(TestConnectionId(1)), expected); +} + } // namespace } // namespace test