Enforce limits on sending ECN. QuicConnection only permits ECN via a state variable accessible only by QuicConnectionPeer. The send limits will be relaxed as new code meets the requirements. There is no code that allows sending anything other than NOT_ECT, so this is not flag-protected. PiperOrigin-RevId: 523154100
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc index 4e2e5cf..bfa2c5c 100644 --- a/quiche/quic/core/http/end_to_end_test.cc +++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -7182,10 +7182,11 @@ server_thread_->Resume(); } -TEST_P(EndToEndTest, EcnMarksReportedCorrectly) { +TEST_P(EndToEndTest, ServerReportsEcn) { // Client connects using not-ECT. ASSERT_TRUE(Initialize()); QuicConnection* client_connection = GetClientConnection(); + QuicConnectionPeer::DisableEcnCodepointValidation(client_connection); QuicEcnCounts* ecn = QuicSentPacketManagerPeer::GetPeerEcnCounts( QuicConnectionPeer::GetSentPacketManager(client_connection), APPLICATION_DATA); @@ -7229,6 +7230,32 @@ client_->Disconnect(); } +TEST_P(EndToEndTest, ClientReportsEcn) { + ASSERT_TRUE(Initialize()); + QuicConnection* server_connection = GetServerConnection(); + QuicConnectionPeer::DisableEcnCodepointValidation(server_connection); + QuicEcnCounts* ecn = QuicSentPacketManagerPeer::GetPeerEcnCounts( + QuicConnectionPeer::GetSentPacketManager(server_connection), + APPLICATION_DATA); + TestPerPacketOptions options; + options.ecn_codepoint = ECN_ECT1; + server_connection->set_per_packet_options(&options); + client_->SendSynchronousRequest("/foo"); + // A second request provides a packet for the client ACKs to go with. + client_->SendSynchronousRequest("/foo"); + EXPECT_EQ(ecn->ect0, 0); + EXPECT_EQ(ecn->ce, 0); + if (!GetQuicRestartFlag(quic_receive_ecn) || + !GetQuicRestartFlag(quic_quiche_ecn_sockets) || + !VersionHasIetfQuicFrames(version_.transport_version)) { + EXPECT_EQ(ecn->ect1, 0); + } else { + EXPECT_GT(ecn->ect1, 0); + } + server_connection->set_per_packet_options(nullptr); + client_->Disconnect(); +} + TEST_P(EndToEndTest, ClientMigrationAfterHalfwayServerMigration) { use_preferred_address_ = true; ASSERT_TRUE(Initialize());
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 271f172..0d42b43 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -3115,7 +3115,6 @@ void QuicConnection::WriteQueuedPackets() { QUICHE_DCHECK(!writer_->IsWriteBlocked()); - QUIC_CLIENT_HISTOGRAM_COUNTS("QuicSession.NumQueuedPacketsBeforeWrite", buffered_packets_.size(), 1, 1000, 50, ""); @@ -3124,7 +3123,7 @@ break; } const BufferedPacket& packet = buffered_packets_.front(); - WriteResult result = writer_->WritePacket( + WriteResult result = SendPacketToWriter( packet.data.get(), packet.length, packet.self_address.host(), packet.peer_address, per_packet_options_); QUIC_DVLOG(1) << ENDPOINT << "Sending buffered packet, result: " << result; @@ -3491,9 +3490,9 @@ // // writer_->WritePacket transfers buffer ownership back to the writer. packet->release_encrypted_buffer = nullptr; - result = writer_->WritePacket(packet->encrypted_buffer, encrypted_length, - send_from_address.host(), send_to_address, - per_packet_options_); + result = SendPacketToWriter(packet->encrypted_buffer, encrypted_length, + send_from_address.host(), send_to_address, + per_packet_options_); // This is a work around for an issue with linux UDP GSO batch writers. // When sending a GSO packet with 2 segments, if the first segment is // larger than the path MTU, instead of EMSGSIZE, the linux kernel returns @@ -4148,6 +4147,38 @@ address) != known_server_addresses_.cend(); } +void QuicConnection::ClearEcnCodepoint() { + if (per_packet_options_ != nullptr) { + per_packet_options_->ecn_codepoint = ECN_NOT_ECT; + } +} + +WriteResult QuicConnection::SendPacketToWriter( + const char* buffer, size_t buf_len, const QuicIpAddress& self_address, + const QuicSocketAddress& peer_address, PerPacketOptions* options) { + if (!disable_ecn_codepoint_validation_) { + switch (GetNextEcnCodepoint()) { + case ECN_NOT_ECT: + break; + case ECN_ECT0: + if (!sent_packet_manager_.GetSendAlgorithm()->SupportsECT0()) { + ClearEcnCodepoint(); + } + break; + case ECN_ECT1: + if (!sent_packet_manager_.GetSendAlgorithm()->SupportsECT1()) { + ClearEcnCodepoint(); + } + break; + case ECN_CE: + ClearEcnCodepoint(); + break; + } + } + return writer_->WritePacket(buffer, buf_len, self_address, peer_address, + options); +} + void QuicConnection::OnRetransmissionTimeout() { ScopedRetransmissionTimeoutIndicator indicator(this); #ifndef NDEBUG @@ -5966,7 +5997,7 @@ buffer, static_cast<QuicPacketLength>(length), coalesced_packet_.self_address(), coalesced_packet_.peer_address()); } else { - WriteResult result = writer_->WritePacket( + WriteResult result = SendPacketToWriter( buffer, length, coalesced_packet_.self_address().host(), coalesced_packet_.peer_address(), per_packet_options_); if (IsWriteError(result.status)) {
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 79de604..1e1f9b6 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1961,6 +1961,22 @@ // Returns true if |address| is known server address. bool IsKnownServerAddress(const QuicSocketAddress& address) const; + // Retrieves the ECN codepoint to be sent on the next packet. + QuicEcnCodepoint GetNextEcnCodepoint() const { + return (per_packet_options_ != nullptr) ? per_packet_options_->ecn_codepoint + : ECN_NOT_ECT; + } + + // Sets the ECN codepoint to Not-ECT. + void ClearEcnCodepoint(); + + // Writes the packet to the writer and clears the ECN codepoint in |options| + // if it is invalid. + WriteResult SendPacketToWriter(const char* buffer, size_t buf_len, + const QuicIpAddress& self_address, + const QuicSocketAddress& peer_address, + PerPacketOptions* options); + QuicConnectionContext context_; QuicFramer framer_; @@ -2357,6 +2373,14 @@ // original address. QuicLRUCache<QuicSocketAddress, bool, QuicSocketAddressHash> received_client_addresses_cache_; + + // Endpoints should never mark packets with Congestion Experienced (CE), as + // this is only done by routers. Endpoints cannot send ECT(0) or ECT(1) if + // their congestion control cannot respond to these signals in accordance with + // the spec, or if the QUIC implementation doesn't validate ECN feedback. When + // true, the connection will not verify that the requested codepoint adheres + // to these policies. This is only accessible through QuicConnectionPeer. + bool disable_ecn_codepoint_validation_ = false; }; } // namespace quic
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 27cf706..f0fa10b 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -17465,6 +17465,57 @@ EXPECT_EQ(kSelfAddress, connection_.self_address()); } +TEST_P(QuicConnectionTest, EcnCodepointsRejected) { + TestPerPacketOptions per_packet_options; + connection_.set_per_packet_options(&per_packet_options); + for (QuicEcnCodepoint ecn : {ECN_NOT_ECT, ECN_ECT0, ECN_ECT1, ECN_CE}) { + per_packet_options.ecn_codepoint = ecn; + if (ecn == ECN_ECT0) { + EXPECT_CALL(*send_algorithm_, SupportsECT0()).WillOnce(Return(false)); + } else if (ecn == ECN_ECT1) { + EXPECT_CALL(*send_algorithm_, SupportsECT1()).WillOnce(Return(false)); + } + EXPECT_CALL(connection_, OnSerializedPacket(_)); + SendPing(); + EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_NOT_ECT); + EXPECT_EQ(writer_->last_ecn_sent(), ECN_NOT_ECT); + } +} + +TEST_P(QuicConnectionTest, EcnCodepointsAccepted) { + TestPerPacketOptions per_packet_options; + connection_.set_per_packet_options(&per_packet_options); + for (QuicEcnCodepoint ecn : {ECN_NOT_ECT, ECN_ECT0, ECN_ECT1, ECN_CE}) { + per_packet_options.ecn_codepoint = ecn; + if (ecn == ECN_ECT0) { + EXPECT_CALL(*send_algorithm_, SupportsECT0()).WillOnce(Return(true)); + } else if (ecn == ECN_ECT1) { + EXPECT_CALL(*send_algorithm_, SupportsECT1()).WillOnce(Return(true)); + } + EXPECT_CALL(connection_, OnSerializedPacket(_)); + SendPing(); + QuicEcnCodepoint expected_codepoint = ecn; + if (ecn == ECN_CE) { + expected_codepoint = ECN_NOT_ECT; + } + EXPECT_EQ(per_packet_options.ecn_codepoint, expected_codepoint); + EXPECT_EQ(writer_->last_ecn_sent(), expected_codepoint); + } +} + +TEST_P(QuicConnectionTest, EcnValidationDisabled) { + TestPerPacketOptions per_packet_options; + connection_.set_per_packet_options(&per_packet_options); + QuicConnectionPeer::DisableEcnCodepointValidation(&connection_); + for (QuicEcnCodepoint ecn : {ECN_NOT_ECT, ECN_ECT0, ECN_ECT1, ECN_CE}) { + per_packet_options.ecn_codepoint = ecn; + EXPECT_CALL(connection_, OnSerializedPacket(_)); + SendPing(); + EXPECT_EQ(per_packet_options.ecn_codepoint, ecn); + EXPECT_EQ(writer_->last_ecn_sent(), ecn); + } +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/quic_types.cc b/quiche/quic/core/quic_types.cc index e046092..9dae069 100644 --- a/quiche/quic/core/quic_types.cc +++ b/quiche/quic/core/quic_types.cc
@@ -446,6 +446,19 @@ return os; } +std::string EcnCodepointToString(QuicEcnCodepoint ecn) { + switch (ecn) { + case ECN_NOT_ECT: + return "Not-ECT"; + case ECN_ECT0: + return "ECT(0)"; + case ECN_ECT1: + return "ECT(1)"; + case ECN_CE: + return "CE"; + } +} + #undef RETURN_STRING_LITERAL // undef for jumbo builds } // namespace quic
diff --git a/quiche/quic/core/quic_types.h b/quiche/quic/core/quic_types.h index 6e2531b..b2a100b 100644 --- a/quiche/quic/core/quic_types.h +++ b/quiche/quic/core/quic_types.h
@@ -885,6 +885,8 @@ ECN_CE = 3, }; +QUICHE_EXPORT std::string EcnCodepointToString(QuicEcnCodepoint ecn); + // This struct reports the Explicit Congestion Notification (ECN) contents of // the ACK_ECN frame. They are the cumulative number of QUIC packets received // for that codepoint in a given Packet Number Space.
diff --git a/quiche/quic/core/quic_utils_test.cc b/quiche/quic/core/quic_utils_test.cc index a4a194f..543c8ea 100644 --- a/quiche/quic/core/quic_utils_test.cc +++ b/quiche/quic/core/quic_utils_test.cc
@@ -233,6 +233,13 @@ EXPECT_FALSE(QuicUtils::AreStatelessResetTokensEqual(token1a, token2)); } +TEST_F(QuicUtilsTest, EcnCodepointToString) { + EXPECT_EQ(EcnCodepointToString(ECN_NOT_ECT), "Not-ECT"); + EXPECT_EQ(EcnCodepointToString(ECN_ECT0), "ECT(0)"); + EXPECT_EQ(EcnCodepointToString(ECN_ECT1), "ECT(1)"); + EXPECT_EQ(EcnCodepointToString(ECN_CE), "CE"); +} + enum class TestEnumClassBit : uint8_t { BIT_ZERO = 0, BIT_ONE,
diff --git a/quiche/quic/test_tools/quic_connection_peer.cc b/quiche/quic/test_tools/quic_connection_peer.cc index 038f8d7..8bddb19 100644 --- a/quiche/quic/test_tools/quic_connection_peer.cc +++ b/quiche/quic/test_tools/quic_connection_peer.cc
@@ -618,5 +618,11 @@ sizeof(QuicConnection::ReceivedPacketInfo) == 280); } +// static +void QuicConnectionPeer::DisableEcnCodepointValidation( + QuicConnection* connection) { + connection->disable_ecn_codepoint_validation_ = true; +} + } // namespace test } // namespace quic
diff --git a/quiche/quic/test_tools/quic_connection_peer.h b/quiche/quic/test_tools/quic_connection_peer.h index 087fcc0..cf8a5a6 100644 --- a/quiche/quic/test_tools/quic_connection_peer.h +++ b/quiche/quic/test_tools/quic_connection_peer.h
@@ -241,6 +241,9 @@ QuicConnection* connection); static bool TestLastReceivedPacketInfoDefaults(); + + // Overrides restrictions on sending ECN for test purposes. + static void DisableEcnCodepointValidation(QuicConnection* connection); }; } // namespace test
diff --git a/quiche/quic/test_tools/quic_test_utils.cc b/quiche/quic/test_tools/quic_test_utils.cc index 7eac021..e622ed8 100644 --- a/quiche/quic/test_tools/quic_test_utils.cc +++ b/quiche/quic/test_tools/quic_test_utils.cc
@@ -1304,7 +1304,7 @@ WriteResult TestPacketWriter::WritePacket(const char* buffer, size_t buf_len, const QuicIpAddress& self_address, const QuicSocketAddress& peer_address, - PerPacketOptions* /*options*/) { + PerPacketOptions* options) { last_write_source_address_ = self_address; last_write_peer_address_ = peer_address; // If the buffer is allocated from the pool, return it back to the pool. @@ -1377,6 +1377,7 @@ bytes_buffered_ += last_packet_size_; return WriteResult(WRITE_STATUS_OK, 0); } + last_ecn_sent_ = (options == nullptr) ? ECN_NOT_ECT : options->ecn_codepoint; return WriteResult(WRITE_STATUS_OK, last_packet_size_); }
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h index 6d7e395..56cf981 100644 --- a/quiche/quic/test_tools/quic_test_utils.h +++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -2010,6 +2010,8 @@ return last_write_peer_address_; } + const QuicEcnCodepoint last_ecn_sent() const { return last_ecn_sent_; } + private: char* AllocPacketBuffer(); @@ -2055,6 +2057,7 @@ QuicIpAddress last_write_source_address_; QuicSocketAddress last_write_peer_address_; int write_error_code_{0}; + QuicEcnCodepoint last_ecn_sent_ = ECN_NOT_ECT; }; // Parses a packet generated by