Google Git
Sign in
quiche/quiche.git/2f2915d4697d68e0c28bf6392b8ba7ae48ecf95d^!/.
commit
2f2915d4697d68e0c28bf6392b8ba7ae48ecf95d
[log] [tgz]
author
fayang <fayang@google.com>
Fri Jan 24 06:47:15 2020 -0800
committer
Copybara-Service <copybara-worker@google.com>
Fri Jan 24 06:47:48 2020 -0800
tree
45c9561deff3026a486ba60ee9c170349af7d0fc
parent
cbfb93e0cd9ca2bca19884b497afbba208b29bbf [diff]
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.