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),
