Apply the amplification limit for all packets, not just handshake packets in ietf quic. also fixes an issue where shouldgeneratepacket doesn't check if the connection is connected. protected by gfe2_reloadable_flag_quic_move_amplification_limit. Because of lack of cert compression, set anti_amplification_factor to 5. This change is from pending cl/313200305. PiperOrigin-RevId: 313595976 Change-Id: Ia67e8c8faa6c17ebc682318047dc8996df55715f
diff --git a/quic/core/http/quic_server_session_base_test.cc b/quic/core/http/quic_server_session_base_test.cc index c482856..291884d 100644 --- a/quic/core/http/quic_server_session_base_test.cc +++ b/quic/core/http/quic_server_session_base_test.cc
@@ -157,6 +157,9 @@ QuicConfigPeer::SetReceivedInitialSessionFlowControlWindow( session_->config(), kMinimumFlowControlSendWindow); session_->OnConfigNegotiated(); + if (connection_->version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(connection_); + } } QuicStreamId GetNthClientInitiatedBidirectionalId(int n) {
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 8b78d7f..9e1280a 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -197,6 +197,9 @@ this->connection()->SetEncrypter( ENCRYPTION_FORWARD_SECURE, std::make_unique<NullEncrypter>(connection->perspective())); + if (this->connection()->version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(this->connection()); + } } ~TestSession() override { DeleteConnection(); }
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc index bafb0f2..41c204f 100644 --- a/quic/core/http/quic_spdy_stream_test.cc +++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -324,6 +324,9 @@ &helper_, &alarm_factory_, perspective, SupportedVersions(GetParam())); session_ = std::make_unique<StrictMock<TestSession>>(connection_); session_->Initialize(); + if (connection_->version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(connection_); + } connection_->AdvanceTime(QuicTime::Delta::FromSeconds(1)); ON_CALL(*session_, WritevData(_, _, _, _, _, _)) .WillByDefault(
diff --git a/quic/core/qpack/qpack_send_stream_test.cc b/quic/core/qpack/qpack_send_stream_test.cc index c77a621..63e0f12 100644 --- a/quic/core/qpack/qpack_send_stream_test.cc +++ b/quic/core/qpack/qpack_send_stream_test.cc
@@ -7,6 +7,7 @@ #include "net/third_party/quiche/src/quic/core/http/http_constants.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/test_tools/quic_config_peer.h" +#include "net/third_party/quiche/src/quic/test_tools/quic_connection_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_spdy_session_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" #include "net/third_party/quiche/src/common/platform/api/quiche_str_cat.h" @@ -66,6 +67,9 @@ SupportedVersions(GetParam().version))), session_(connection_) { session_.Initialize(); + if (connection_->version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(connection_); + } QuicConfigPeer::SetReceivedInitialSessionFlowControlWindow( session_.config(), kMinimumFlowControlSendWindow); QuicConfigPeer::SetReceivedInitialMaxStreamDataBytesUnidirectional(
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index d6c75bf..1ed056d 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2206,9 +2206,13 @@ bool QuicConnection::ShouldGeneratePacket( HasRetransmittableData retransmittable, IsHandshake handshake) { + DCHECK(handshake != IS_HANDSHAKE || + QuicVersionUsesCryptoFrames(transport_version())) + << ENDPOINT + << "Handshake in STREAM frames should not check ShouldGeneratePacket"; // We should serialize handshake packets immediately to ensure that they // end up sent at the right encryption level. - if (handshake == IS_HANDSHAKE) { + if (!move_amplification_limit_ && handshake == IS_HANDSHAKE) { if (LimitedByAmplificationFactor()) { // Server is constrained by the amplification restriction. QUIC_DVLOG(1) << ENDPOINT << "Constrained by amplification restriction"; @@ -2253,6 +2257,14 @@ return false; } + if (move_amplification_limit_ && LimitedByAmplificationFactor()) { + // Server is constrained by the amplification restriction. + QUIC_RELOADABLE_FLAG_COUNT(quic_move_amplification_limit); + QUIC_CODE_COUNT(quic_throttled_by_amplification_limit); + QUIC_DVLOG(1) << ENDPOINT << "Constrained by amplification restriction"; + return false; + } + if (sent_packet_manager_.pending_timer_transmission_count() > 0) { // Force sending the retransmissions for HANDSHAKE, TLP, RTO, PROBING cases. return true; @@ -4033,8 +4045,12 @@ const bool flushed = packet_creator_.FlushAckFrame(frames); if (!flushed) { // Connection is write blocked. - QUIC_BUG_IF(!writer_->IsWriteBlocked()) - << "Writer not blocked, but ACK not flushed for packet space:" << i; + QUIC_BUG_IF( + !writer_->IsWriteBlocked() && + (!move_amplification_limit_ || !LimitedByAmplificationFactor())) + << "Writer not blocked and not throttled by amplification factor, " + "but ACK not flushed for packet space:" + << i; break; } ResetAckStates(); @@ -4215,7 +4231,7 @@ bool QuicConnection::LimitedByAmplificationFactor() const { return EnforceAntiAmplificationLimit() && bytes_sent_before_address_validation_ >= - GetQuicFlag(FLAGS_quic_anti_amplification_factor) * + anti_amplification_factor_ * bytes_received_before_address_validation_; }
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 420f8c8..589568a 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -979,6 +979,10 @@ void OnTransportParametersReceived( const TransportParameters& transport_parameters) const; + size_t anti_amplification_factor() const { + return anti_amplification_factor_; + } + protected: // Calls cancel() on all the alarms owned by this connection. void CancelAllAlarms(); @@ -1669,6 +1673,19 @@ const bool update_ack_alarm_in_send_all_pending_acks_ = GetQuicReloadableFlag(quic_update_ack_alarm_in_send_all_pending_acks); + + const bool move_amplification_limit_ = + GetQuicReloadableFlag(quic_move_amplification_limit); + + // TODO(fayang): Change the default value of quic_anti_amplification_factor to + // 5 when deprecating quic_move_amplification_limit. + // TODO(b/153892665): Change the default value of + // quic_anti_amplification_factor back to 3 when cert compression is + // supported. + const size_t anti_amplification_factor_ = + move_amplification_limit_ + ? 5 + : GetQuicFlag(FLAGS_quic_anti_amplification_factor); }; } // namespace quic
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index a9e25f0..ff29c26 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -1752,6 +1752,9 @@ void MtuDiscoveryTestInit() { set_perspective(Perspective::IS_SERVER); QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false); + if (version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(&connection_); + } connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); peer_creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); // QuicFramer::GetMaxPlaintextSize uses the smallest max plaintext size @@ -1999,6 +2002,9 @@ set_perspective(Perspective::IS_SERVER); QuicPacketCreatorPeer::SetSendVersionInPacket(creator_, false); EXPECT_EQ(Perspective::IS_SERVER, connection_.perspective()); + if (version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(&connection_); + } // Clear direct_peer_address. QuicConnectionPeer::SetDirectPeerAddress(&connection_, QuicSocketAddress()); @@ -7074,12 +7080,24 @@ ProcessPacket(1); BlockOnNextWrite(); writer_->set_is_write_blocked_data_buffered(true); + if (GetQuicReloadableFlag(quic_move_amplification_limit) && + QuicVersionUsesCryptoFrames(connection_.transport_version())) { + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); + } else { + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2); + } connection_.SendCryptoDataWithString("foo", 0); EXPECT_TRUE(writer_->IsWriteBlocked()); EXPECT_FALSE(connection_.HasQueuedData()); connection_.SendCryptoDataWithString("bar", 3); EXPECT_TRUE(writer_->IsWriteBlocked()); - EXPECT_TRUE(connection_.HasQueuedData()); + if (GetQuicReloadableFlag(quic_move_amplification_limit) && + QuicVersionUsesCryptoFrames(connection_.transport_version())) { + // CRYPTO frames are not flushed when writer is blocked. + EXPECT_FALSE(connection_.HasQueuedData()); + } else { + EXPECT_TRUE(connection_.HasQueuedData()); + } } TEST_P(QuicConnectionTest, BundleAckForSecondCHLO) { @@ -10045,13 +10063,17 @@ // Verify no data can be sent at the beginning because bytes received is 0. EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); connection_.SendCryptoDataWithString("foo", 0); + if (GetQuicReloadableFlag(quic_move_amplification_limit)) { + EXPECT_FALSE(connection_.CanWrite(HAS_RETRANSMITTABLE_DATA)); + EXPECT_FALSE(connection_.CanWrite(NO_RETRANSMITTABLE_DATA)); + } EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet()); // Receives packet 1. ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL); const size_t anti_amplification_factor = - GetQuicFlag(FLAGS_quic_anti_amplification_factor); + connection_.anti_amplification_factor(); // Verify now packets can be sent. for (size_t i = 0; i < anti_amplification_factor; ++i) { EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); @@ -10086,6 +10108,48 @@ } } +TEST_P(QuicConnectionTest, AckPendingWithAmplificationLimited) { + if (!connection_.version().SupportsAntiAmplificationLimit() || + !GetQuicReloadableFlag(quic_move_amplification_limit)) { + return; + } + EXPECT_CALL(visitor_, OnCryptoFrame(_)).Times(AnyNumber()); + EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(AnyNumber()); + set_perspective(Perspective::IS_SERVER); + use_tagging_decrypter(); + connection_.SetEncrypter(ENCRYPTION_INITIAL, + std::make_unique<TaggingEncrypter>(0x01)); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_INITIAL); + // Receives packet 1. + ProcessCryptoPacketAtLevel(1, ENCRYPTION_INITIAL); + connection_.SetEncrypter(ENCRYPTION_HANDSHAKE, + std::make_unique<TaggingEncrypter>(0x02)); + connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE); + EXPECT_TRUE(connection_.GetAckAlarm()->IsSet()); + // Send response in different encryption level and cause amplification factor + // throttled. + size_t i = 0; + while (connection_.CanWrite(HAS_RETRANSMITTABLE_DATA)) { + connection_.SendCryptoDataWithString(std::string(1024, 'a'), i * 1024, + ENCRYPTION_HANDSHAKE); + ++i; + } + // Verify ACK is still pending. + EXPECT_TRUE(connection_.GetAckAlarm()->IsSet()); + + // Fire ACK alarm and verify ACK cannot be sent due to amplification factor. + clock_.AdvanceTime(connection_.GetAckAlarm()->deadline() - clock_.Now()); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); + connection_.GetAckAlarm()->Fire(); + // Verify ACK alarm is cancelled. + EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); + + // Receives packet 2 and verify ACK gets flushed. + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); + ProcessCryptoPacketAtLevel(2, ENCRYPTION_INITIAL); + EXPECT_FALSE(writer_->ack_frames().empty()); +} + TEST_P(QuicConnectionTest, ConnectionCloseFrameType) { if (!VersionHasIetfQuicFrames(version().transport_version)) { // Test relevent only for IETF QUIC.
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 9d5e977..ddebac2 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -183,6 +183,9 @@ this->connection()->SetEncrypter( ENCRYPTION_FORWARD_SECURE, std::make_unique<NullEncrypter>(connection->perspective())); + if (this->connection()->version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(this->connection()); + } } ~TestSession() override { DeleteConnection(); }
diff --git a/quic/quartc/quartc_packet_writer.h b/quic/quartc/quartc_packet_writer.h index 8c89e23..a59344c 100644 --- a/quic/quartc/quartc_packet_writer.h +++ b/quic/quartc/quartc_packet_writer.h
@@ -106,7 +106,7 @@ QuicByteCount max_packet_size_; // Whether packets can be written. - bool writable_ = false; + bool writable_ = true; }; } // namespace quic
diff --git a/quic/test_tools/simulator/quic_endpoint.cc b/quic/test_tools/simulator/quic_endpoint.cc index 5f8ee56..7061898 100644 --- a/quic/test_tools/simulator/quic_endpoint.cc +++ b/quic/test_tools/simulator/quic_endpoint.cc
@@ -55,6 +55,7 @@ // Skip version negotiation. test::QuicConnectionPeer::SetNegotiatedVersion(connection_.get()); } + test::QuicConnectionPeer::SetAddressValidated(connection_.get()); connection_->SetDataProducer(&producer_); connection_->SetSessionNotifier(this); notifier_ = std::make_unique<test::SimpleSessionNotifier>(connection_.get());
diff --git a/quic/tools/quic_simple_server_stream_test.cc b/quic/tools/quic_simple_server_stream_test.cc index 3dfaa1e..5c84084 100644 --- a/quic/tools/quic_simple_server_stream_test.cc +++ b/quic/tools/quic_simple_server_stream_test.cc
@@ -18,6 +18,7 @@ #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" #include "net/third_party/quiche/src/quic/test_tools/crypto_test_utils.h" #include "net/third_party/quiche/src/quic/test_tools/quic_config_peer.h" +#include "net/third_party/quiche/src/quic/test_tools/quic_connection_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_session_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_spdy_session_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_stream_peer.h" @@ -239,6 +240,9 @@ session_.config()->SetInitialSessionFlowControlWindowToSend( kInitialSessionFlowControlWindowForTest); session_.Initialize(); + if (connection_->version().SupportsAntiAmplificationLimit()) { + QuicConnectionPeer::SetAddressValidated(connection_); + } stream_ = new StrictMock<TestStream>( GetNthClientInitiatedBidirectionalStreamId( connection_->transport_version(), 0),