Fix bad interaction between various migration CLs
cl/360703267 has caused QuicConnectionTest.EffectivePeerAddressChangeAtServer to fail when the right combination of flags is set, because the test would send PATH_CHALLENGE at ENCRYPTION_INITIAL, which we no longer allow. This CL fixes that, and adds a DCHECK to allow us to catch this kind of test issue sooner.
Additionally, this CL fixes the error message from cl/360703267 which was incorrectly logging numbers instead if the enum's string representation.
This CL only changes logging (or debug builds) so it does not have flag protection.
PiperOrigin-RevId: 360924433
Change-Id: I1556241ad1fefc71b0387d9d9b500603970ce2f8
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 32889da..ccea95b 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1847,6 +1847,7 @@
if (version().SupportsAntiAmplificationLimit()) {
QuicConnectionPeer::SetAddressValidated(&connection_);
}
+ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
EXPECT_CALL(visitor_, GetHandshakeState())
.WillRepeatedly(Return(HANDSHAKE_CONFIRMED));
@@ -1866,7 +1867,7 @@
EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(AnyNumber());
}
ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, kPeerAddress,
- ENCRYPTION_INITIAL);
+ ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kEffectivePeerAddress, connection_.effective_peer_address());
@@ -1877,7 +1878,7 @@
connection_.ReturnEffectivePeerAddressForNextPacket(kNewEffectivePeerAddress);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress, kPeerAddress,
- ENCRYPTION_INITIAL);
+ ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(kPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewEffectivePeerAddress, connection_.effective_peer_address());
EXPECT_EQ(kPeerAddress, writer_->last_write_peer_address());
@@ -1900,7 +1901,7 @@
QuicAckFrame ack_frame = InitAckFrame(1);
EXPECT_CALL(*send_algorithm_, OnCongestionEvent(_, _, _, _, _));
ProcessFramePacketWithAddresses(QuicFrame(&ack_frame), kSelfAddress,
- kNewPeerAddress, ENCRYPTION_INITIAL);
+ kNewPeerAddress, ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(kNewPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewEffectivePeerAddress, connection_.effective_peer_address());
EXPECT_EQ(NO_CHANGE, connection_.active_effective_peer_migration_type());
@@ -1916,7 +1917,7 @@
kNewerEffectivePeerAddress);
EXPECT_CALL(visitor_, OnConnectionMigration(PORT_CHANGE)).Times(1);
ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
- kFinalPeerAddress, ENCRYPTION_INITIAL);
+ kFinalPeerAddress, ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(kFinalPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewerEffectivePeerAddress, connection_.effective_peer_address());
if (connection_.validate_client_address()) {
@@ -1938,7 +1939,7 @@
EXPECT_CALL(*send_algorithm_, OnConnectionMigration()).Times(1);
}
ProcessFramePacketWithAddresses(MakeCryptoFrame(), kSelfAddress,
- kFinalPeerAddress, ENCRYPTION_INITIAL);
+ kFinalPeerAddress, ENCRYPTION_FORWARD_SECURE);
EXPECT_EQ(kFinalPeerAddress, connection_.peer_address());
EXPECT_EQ(kNewestEffectivePeerAddress, connection_.effective_peer_address());
EXPECT_EQ(IPV6_TO_IPV4_CHANGE,
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 51fa3ea..e7565f6 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -3150,9 +3150,11 @@
decrypted_level)) {
QUIC_RELOADABLE_FLAG_COUNT_N(quic_reject_unexpected_ietf_frame_types, 2,
2);
- set_detailed_error(absl::StrCat("IETF frame type ", frame_type,
- " is unexpected at encryption level ",
- decrypted_level));
+ set_detailed_error(absl::StrCat(
+ "IETF frame type ",
+ QuicIetfFrameTypeString(static_cast<QuicIetfFrameType>(frame_type)),
+ " is unexpected at encryption level ",
+ EncryptionLevelToString(decrypted_level)));
return RaiseError(IETF_QUIC_PROTOCOL_VIOLATION);
}
}
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 9f829e5..02201b3 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -15237,8 +15237,10 @@
EXPECT_FALSE(framer_.ProcessPacket(encrypted));
EXPECT_THAT(framer_.error(), IsError(IETF_QUIC_PROTOCOL_VIOLATION));
- EXPECT_EQ("IETF frame type 2 is unexpected at encryption level 2",
- framer_.detailed_error());
+ EXPECT_EQ(
+ "IETF frame type IETF_ACK is unexpected at encryption level "
+ "ENCRYPTION_ZERO_RTT",
+ framer_.detailed_error());
}
} // namespace
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 9dabd3a..d5f7912 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -1604,6 +1604,21 @@
return false;
}
+ // Sanity check to ensure we don't send frames at the wrong encryption level.
+ QUICHE_DCHECK(
+ packet_.encryption_level == ENCRYPTION_ZERO_RTT ||
+ packet_.encryption_level == ENCRYPTION_FORWARD_SECURE ||
+ (frame.type != GOAWAY_FRAME && frame.type != WINDOW_UPDATE_FRAME &&
+ frame.type != HANDSHAKE_DONE_FRAME &&
+ frame.type != NEW_CONNECTION_ID_FRAME &&
+ frame.type != MAX_STREAMS_FRAME && frame.type != STREAMS_BLOCKED_FRAME &&
+ frame.type != PATH_RESPONSE_FRAME &&
+ frame.type != PATH_CHALLENGE_FRAME && frame.type != STOP_SENDING_FRAME &&
+ frame.type != MESSAGE_FRAME && frame.type != NEW_TOKEN_FRAME &&
+ frame.type != RETIRE_CONNECTION_ID_FRAME &&
+ frame.type != ACK_FREQUENCY_FRAME))
+ << frame.type << " not allowed at " << packet_.encryption_level;
+
if (frame.type == STREAM_FRAME) {
if (MaybeCoalesceStreamFrame(frame.stream_frame)) {
LogCoalesceStreamFrameStatus(true);
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h
index 7344cea..0e13f56 100644
--- a/quic/core/quic_types.h
+++ b/quic/core/quic_types.h
@@ -278,7 +278,7 @@
// byte, with the two most significant bits being 0. Thus, the following
// enumerations are valid as both the numeric values of frame types AND their
// encodings.
-enum QuicIetfFrameType : uint8_t {
+enum QuicIetfFrameType : uint64_t {
IETF_PADDING = 0x00,
IETF_PING = 0x01,
IETF_ACK = 0x02,
