gfe-relnote: In QUIC, add OnOneRttPacketAckowledged to TLS handshaker, and this is used to allow client mark handshake confirmed when handshake done frame is not supported. Not affecting prod, not protected.
PiperOrigin-RevId: 291363354
Change-Id: I2aa500244b1e443e17bc4585e983c666d5782afa
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 1522118..1082633 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -131,6 +131,7 @@
return QuicCryptoHandshaker::crypto_message_parser();
}
void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
+ void OnOneRttPacketAcknowledged() override {}
void OnHandshakeDoneReceived() override {}
MOCK_METHOD0(OnCanWrite, void());
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 2c3e43c..38b1dfb 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1032,6 +1032,8 @@
QUIC_DLOG(INFO) << ENDPOINT << "Received an old ack frame: ignoring";
return true;
}
+ const bool one_rtt_packet_was_acked =
+ sent_packet_manager_.one_rtt_packet_acked();
const AckResult ack_result = sent_packet_manager_.OnAckFrameEnd(
time_of_last_received_packet_, last_header_.packet_number,
last_decrypted_packet_level_);
@@ -1044,6 +1046,10 @@
<< QuicUtils::AckResultToString(ack_result);
return false;
}
+ if (SupportsMultiplePacketNumberSpaces() && !one_rtt_packet_was_acked &&
+ sent_packet_manager_.one_rtt_packet_acked()) {
+ visitor_->OnOneRttPacketAcknowledged();
+ }
// Cancel the send alarm because new packets likely have been acked, which
// may change the congestion window and/or pacing rate. Canceling the alarm
// causes CanWrite to recalculate the next send time.
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 7fc528f..b060f87 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -175,6 +175,9 @@
// Called when a packet of encryption |level| has been successfully decrypted.
virtual void OnPacketDecrypted(EncryptionLevel level) = 0;
+
+ // Called when a 1RTT packet has been acknowledged.
+ virtual void OnOneRttPacketAcknowledged() = 0;
};
// Interface which gets callbacks from the QuicConnection at interesting
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 0cb109e..4a041d1 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1071,7 +1071,8 @@
EXPECT_CALL(visitor_, OnPacketReceived(_, _, _)).Times(AnyNumber());
EXPECT_CALL(visitor_, OnForwardProgressConfirmed()).Times(AnyNumber());
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)).Times(AnyNumber());
-
+ EXPECT_CALL(visitor_, OnOneRttPacketAcknowledged())
+ .Times(testing::AtMost(1));
EXPECT_CALL(*loss_algorithm_, GetLossTimeout())
.WillRepeatedly(Return(QuicTime::Zero()));
EXPECT_CALL(*loss_algorithm_, DetectLosses(_, _, _, _, _, _))
@@ -2669,10 +2670,16 @@
QuicAckFrame ack1 = InitAckFrame(1);
QuicAckFrame ack2 = InitAckFrame(2);
EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _));
+ if (connection_.SupportsMultiplePacketNumberSpaces()) {
+ EXPECT_CALL(visitor_, OnOneRttPacketAcknowledged()).Times(1);
+ }
ProcessAckPacket(2, &ack2);
// Should ack immediately since we have missing packets.
EXPECT_EQ(2u, writer_->packets_write_attempts());
+ if (connection_.SupportsMultiplePacketNumberSpaces()) {
+ EXPECT_CALL(visitor_, OnOneRttPacketAcknowledged()).Times(0);
+ }
ProcessAckPacket(1, &ack1);
// Should not ack an ack filling a missing packet.
EXPECT_EQ(2u, writer_->packets_write_attempts());
diff --git a/quic/core/quic_crypto_client_handshaker.h b/quic/core/quic_crypto_client_handshaker.h
index dc799fb..be3907d 100644
--- a/quic/core/quic_crypto_client_handshaker.h
+++ b/quic/core/quic_crypto_client_handshaker.h
@@ -47,6 +47,7 @@
CryptoMessageParser* crypto_message_parser() override;
HandshakeState GetHandshakeState() const override;
size_t BufferSizeLimitForLevel(EncryptionLevel level) const override;
+ void OnOneRttPacketAcknowledged() override {}
void OnHandshakeDoneReceived() override;
// From QuicCryptoHandshaker
diff --git a/quic/core/quic_crypto_client_stream.cc b/quic/core/quic_crypto_client_stream.cc
index 92d396f..aa8c1b5 100644
--- a/quic/core/quic_crypto_client_stream.cc
+++ b/quic/core/quic_crypto_client_stream.cc
@@ -99,6 +99,10 @@
return handshaker_->chlo_hash();
}
+void QuicCryptoClientStream::OnOneRttPacketAcknowledged() {
+ handshaker_->OnOneRttPacketAcknowledged();
+}
+
void QuicCryptoClientStream::OnHandshakeDoneReceived() {
handshaker_->OnHandshakeDoneReceived();
}
diff --git a/quic/core/quic_crypto_client_stream.h b/quic/core/quic_crypto_client_stream.h
index fd81259..01f9612 100644
--- a/quic/core/quic_crypto_client_stream.h
+++ b/quic/core/quic_crypto_client_stream.h
@@ -121,6 +121,9 @@
// Returns current handshake state.
virtual HandshakeState GetHandshakeState() const = 0;
+ // Called when a 1RTT packet has been acknowledged.
+ virtual void OnOneRttPacketAcknowledged() = 0;
+
// Called when handshake done has been received.
virtual void OnHandshakeDoneReceived() = 0;
};
@@ -168,6 +171,7 @@
const override;
CryptoMessageParser* crypto_message_parser() override;
void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
+ void OnOneRttPacketAcknowledged() override;
void OnHandshakeDoneReceived() override;
HandshakeState GetHandshakeState() const override;
size_t BufferSizeLimitForLevel(EncryptionLevel level) const override;
diff --git a/quic/core/quic_crypto_server_stream.h b/quic/core/quic_crypto_server_stream.h
index 9f8d08d..3150036 100644
--- a/quic/core/quic_crypto_server_stream.h
+++ b/quic/core/quic_crypto_server_stream.h
@@ -175,6 +175,7 @@
const override;
CryptoMessageParser* crypto_message_parser() override;
void OnPacketDecrypted(EncryptionLevel level) override;
+ void OnOneRttPacketAcknowledged() override {}
void OnHandshakeDoneReceived() override;
HandshakeState GetHandshakeState() const override;
size_t BufferSizeLimitForLevel(EncryptionLevel level) const override;
diff --git a/quic/core/quic_crypto_stream.h b/quic/core/quic_crypto_stream.h
index 66bd461..17474a8 100644
--- a/quic/core/quic_crypto_stream.h
+++ b/quic/core/quic_crypto_stream.h
@@ -85,6 +85,9 @@
// Called when a packet of encryption |level| has been successfully decrypted.
virtual void OnPacketDecrypted(EncryptionLevel level) = 0;
+ // Called when a 1RTT packet has been acknowledged.
+ virtual void OnOneRttPacketAcknowledged() = 0;
+
// Called when a handshake done frame has been received.
virtual void OnHandshakeDoneReceived() = 0;
diff --git a/quic/core/quic_crypto_stream_test.cc b/quic/core/quic_crypto_stream_test.cc
index dfbc6ef..1e1967c 100644
--- a/quic/core/quic_crypto_stream_test.cc
+++ b/quic/core/quic_crypto_stream_test.cc
@@ -57,6 +57,7 @@
return QuicCryptoHandshaker::crypto_message_parser();
}
void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
+ void OnOneRttPacketAcknowledged() override {}
void OnHandshakeDoneReceived() override {}
HandshakeState GetHandshakeState() const override { return HANDSHAKE_START; }
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index f4b7973..d9f501c 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -106,6 +106,7 @@
pto_exponential_backoff_start_point_(0),
pto_rttvar_multiplier_(4),
num_tlp_timeout_ptos_(0),
+ one_rtt_packet_acked_(false),
sanitize_ack_delay_(GetQuicReloadableFlag(quic_sanitize_ack_delay)) {
SetSendAlgorithm(congestion_control_type);
}
@@ -1233,6 +1234,9 @@
return PACKETS_ACKED_IN_WRONG_PACKET_NUMBER_SPACE;
}
last_ack_frame_.packets.Add(acked_packet.packet_number);
+ if (info->encryption_level == ENCRYPTION_FORWARD_SECURE) {
+ one_rtt_packet_acked_ = true;
+ }
largest_packet_peer_knows_is_acked_.UpdateMax(info->largest_acked);
if (supports_multiple_packet_number_spaces()) {
largest_packets_peer_knows_is_acked_[packet_number_space].UpdateMax(
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index fdbd697..d6c9b83 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -402,6 +402,8 @@
return skip_packet_number_for_pto_;
}
+ bool one_rtt_packet_acked() const { return one_rtt_packet_acked_; }
+
private:
friend class test::QuicConnectionPeer;
friend class test::QuicSentPacketManagerPeer;
@@ -639,6 +641,9 @@
// Number of PTOs similar to TLPs.
size_t num_tlp_timeout_ptos_;
+ // True if any 1-RTT packet gets acknowledged.
+ bool one_rtt_packet_acked_;
+
// Latched value of quic_sanitize_ack_delay.
const bool sanitize_ack_delay_;
};
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index e3dbf73..4ab9f6c 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -299,6 +299,10 @@
GetMutableCryptoStream()->OnPacketDecrypted(level);
}
+void QuicSession::OnOneRttPacketAcknowledged() {
+ GetMutableCryptoStream()->OnOneRttPacketAcknowledged();
+}
+
void QuicSession::PendingStreamOnRstStream(const QuicRstStreamFrame& frame) {
DCHECK(VersionUsesHttp3(transport_version()));
QuicStreamId stream_id = frame.stream_id;
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h
index bd19714..d951912 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h
@@ -135,6 +135,7 @@
bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame) override;
void OnStopSendingFrame(const QuicStopSendingFrame& frame) override;
void OnPacketDecrypted(EncryptionLevel level) override;
+ void OnOneRttPacketAcknowledged() override;
// QuicStreamFrameDataProducer
WriteStreamDataResult WriteStreamData(QuicStreamId id,
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index 23cd87b..a287f55 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc
@@ -115,6 +115,7 @@
return QuicCryptoHandshaker::crypto_message_parser();
}
void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
+ void OnOneRttPacketAcknowledged() override {}
void OnHandshakeDoneReceived() override {}
HandshakeState GetHandshakeState() const override {
return one_rtt_keys_available() ? HANDSHAKE_COMPLETE : HANDSHAKE_START;
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h
index 42dc050..fd49da2 100644
--- a/quic/core/quic_types.h
+++ b/quic/core/quic_types.h
@@ -736,10 +736,9 @@
// has both 1-RTT send and receive keys.
HANDSHAKE_COMPLETE,
// Only used in IETF QUIC with TLS handshake. State proceeds to
- // HANDSHAKE_CONFIRMED if a client receives HANDSHAKE_DONE frame or server has
+ // HANDSHAKE_CONFIRMED if 1) a client receives HANDSHAKE_DONE frame or
+ // acknowledgment for 1-RTT packet or 2) server has
// 1-RTT send and receive keys.
- // TODO(fayang): on the client side, proceed state to HANDSHAKE_CONFIRMED once
- // 1-RTT packet gets acknowledged..
HANDSHAKE_CONFIRMED,
};
diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index 45d1849..42955ce 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc
@@ -257,12 +257,21 @@
return TlsHandshaker::BufferSizeLimitForLevel(level);
}
+void TlsClientHandshaker::OnOneRttPacketAcknowledged() {
+ OnHandshakeConfirmed();
+}
+
void TlsClientHandshaker::OnHandshakeDoneReceived() {
if (!one_rtt_keys_available_) {
CloseConnection(QUIC_HANDSHAKE_FAILED,
"Unexpected handshake done received");
return;
}
+ OnHandshakeConfirmed();
+}
+
+void TlsClientHandshaker::OnHandshakeConfirmed() {
+ DCHECK(one_rtt_keys_available_);
if (handshake_confirmed_) {
return;
}
diff --git a/quic/core/tls_client_handshaker.h b/quic/core/tls_client_handshaker.h
index 4c7028a..5c5390f 100644
--- a/quic/core/tls_client_handshaker.h
+++ b/quic/core/tls_client_handshaker.h
@@ -51,6 +51,7 @@
CryptoMessageParser* crypto_message_parser() override;
HandshakeState GetHandshakeState() const override;
size_t BufferSizeLimitForLevel(EncryptionLevel level) const override;
+ void OnOneRttPacketAcknowledged() override;
void OnHandshakeDoneReceived() override;
// Override to drop initial keys if trying to write ENCRYPTION_HANDSHAKE data.
@@ -107,6 +108,10 @@
bool ProcessTransportParameters(std::string* error_details);
void FinishHandshake();
+ // Called when server completes handshake (i.e., either handshake done is
+ // received or 1-RTT packet gets acknowledged).
+ void OnHandshakeConfirmed();
+
void InsertSession(bssl::UniquePtr<SSL_SESSION> session) override;
QuicSession* session() { return session_; }
diff --git a/quic/core/tls_handshaker_test.cc b/quic/core/tls_handshaker_test.cc
index 12b3299..03a899f 100644
--- a/quic/core/tls_handshaker_test.cc
+++ b/quic/core/tls_handshaker_test.cc
@@ -181,6 +181,7 @@
}
void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
+ void OnOneRttPacketAcknowledged() override {}
HandshakeState GetHandshakeState() const override {
return handshaker()->GetHandshakeState();
diff --git a/quic/core/tls_server_handshaker.h b/quic/core/tls_server_handshaker.h
index 8215bb7..d377f4d 100644
--- a/quic/core/tls_server_handshaker.h
+++ b/quic/core/tls_server_handshaker.h
@@ -47,6 +47,7 @@
void SetPreviousCachedNetworkParams(
CachedNetworkParameters cached_network_params) override;
void OnPacketDecrypted(EncryptionLevel level) override;
+ void OnOneRttPacketAcknowledged() override {}
void OnHandshakeDoneReceived() override;
bool ShouldSendExpectCTHeader() const override;
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index 30cbd5a..4ce21a8 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -413,6 +413,7 @@
bool(const QuicStreamsBlockedFrame& frame));
MOCK_METHOD1(OnStopSendingFrame, void(const QuicStopSendingFrame& frame));
MOCK_METHOD1(OnPacketDecrypted, void(EncryptionLevel));
+ MOCK_METHOD0(OnOneRttPacketAcknowledged, void());
};
class MockQuicConnectionHelper : public QuicConnectionHelperInterface {
@@ -690,6 +691,7 @@
const override;
CryptoMessageParser* crypto_message_parser() override;
void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
+ void OnOneRttPacketAcknowledged() override {}
void OnHandshakeDoneReceived() override {}
HandshakeState GetHandshakeState() const override { return HANDSHAKE_START; }
diff --git a/quic/test_tools/simulator/quic_endpoint.h b/quic/test_tools/simulator/quic_endpoint.h
index a65848f..f3a9f02 100644
--- a/quic/test_tools/simulator/quic_endpoint.h
+++ b/quic/test_tools/simulator/quic_endpoint.h
@@ -86,6 +86,7 @@
}
void OnStopSendingFrame(const QuicStopSendingFrame& /*frame*/) override {}
void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
+ void OnOneRttPacketAcknowledged() override {}
// End QuicConnectionVisitorInterface implementation.
