Google Git
Sign in
quiche/quiche.git/44ae4e913eba0887b162e1298de82ff7b24f0b71^!/.
commit
44ae4e913eba0887b162e1298de82ff7b24f0b71
[log] [tgz]
author
fayang <fayang@google.com>
Tue Apr 28 13:09:42 2020 -0700
committer
Copybara-Service <copybara-worker@google.com>
Tue Apr 28 13:10:11 2020 -0700
tree
45c80a3296133b464a72ad7334ab0b5694cc9f87
parent
aecfd53825790b87df1aaf7fbf1afe7f7cf002bf [diff]
gfe-relnote: Let QUIC client drops initial key when a handshake packet has been sent. protected by existing gfe2_reloadable_flag_quic_enable_version_draft_25_v3 and gfe2_reloadable_flag_quic_enable_version_draft_27.

PiperOrigin-RevId: 308882771
Change-Id: Icbae03467ebc7845dacb8d6398cd2657dcf11632

diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index 4049caf..7bc452f 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc

@@ -137,6 +137,7 @@
   }
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() override {}
   void OnHandshakeDoneReceived() override {}
 
   MOCK_METHOD(void, OnCanWrite, (), (override));

diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index cdee5ef..6535328 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc

@@ -124,6 +124,7 @@
   }
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() override {}
   void OnHandshakeDoneReceived() override {}
 
   MOCK_METHOD(void, OnCanWrite, (), (override));

diff --git a/quic/core/quic_coalesced_packet.h b/quic/core/quic_coalesced_packet.h
index 1974f77..447ab95 100644
--- a/quic/core/quic_coalesced_packet.h
+++ b/quic/core/quic_coalesced_packet.h

@@ -36,6 +36,9 @@
 
   std::string ToString(size_t serialized_length) const;
 
+  // Returns true if this coalesced packet contains packet of |level|.
+  bool ContainsPacketOfEncryptionLevel(EncryptionLevel level) const;
+
   const SerializedPacket* initial_packet() const {
     return initial_packet_.get();
   }
@@ -49,9 +52,6 @@
   QuicPacketLength max_packet_length() const { return max_packet_length_; }
 
  private:
-  // Returns true if this coalesced packet contains packet of |level|.
-  bool ContainsPacketOfEncryptionLevel(EncryptionLevel level) const;
-
   // self/peer addresses are set when trying to coalesce the first packet.
   // Packets with different self/peer addresses cannot be coalesced.
   QuicSocketAddress self_address_;

diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 7a23582..e8dd94b 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc

@@ -4032,6 +4032,12 @@
     if (debug_visitor_ != nullptr) {
       debug_visitor_->OnCoalescedPacketSent(coalesced_packet_, length);
     }
+    if (coalesced_packet_.ContainsPacketOfEncryptionLevel(
+            ENCRYPTION_HANDSHAKE)) {
+      // This is only called in coalescer because all ENCRYPTION_HANDSHAKE
+      // packets go through the coalescer.
+      visitor_->OnHandshakePacketSent();
+    }
     return true;
   }
 
@@ -4055,6 +4061,11 @@
   if (debug_visitor_ != nullptr) {
     debug_visitor_->OnCoalescedPacketSent(coalesced_packet_, length);
   }
+  if (coalesced_packet_.ContainsPacketOfEncryptionLevel(ENCRYPTION_HANDSHAKE)) {
+    // This is only called in coalescer because all ENCRYPTION_HANDSHAKE
+    // packets go through the coalescer.
+    visitor_->OnHandshakePacketSent();
+  }
   // Account for added padding.
   if (length > coalesced_packet_.length()) {
     size_t padding_size = length - coalesced_packet_.length();

diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index e653352..aa68288 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h

@@ -181,6 +181,9 @@
 
   // Called when a 1RTT packet has been acknowledged.
   virtual void OnOneRttPacketAcknowledged() = 0;
+
+  // Called when a packet of ENCRYPTION_HANDSHAKE gets sent.
+  virtual void OnHandshakePacketSent() = 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 4ad005d..11977e6 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc

@@ -10209,6 +10209,7 @@
   connection_.set_debug_visitor(&debug_visitor);
   EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(3);
   EXPECT_CALL(debug_visitor, OnCoalescedPacketSent(_, _)).Times(1);
+  EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
   {
     QuicConnection::ScopedPacketFlusher flusher(&connection_);
     use_tagging_decrypter();
@@ -10280,6 +10281,7 @@
   connection_.SetEncrypter(ENCRYPTION_HANDSHAKE,
                            std::make_unique<TaggingEncrypter>(0x02));
   connection_.SetDefaultEncryptionLevel(ENCRYPTION_HANDSHAKE);
+  EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
   connection_.SendCryptoDataWithString("foo", 0, ENCRYPTION_HANDSHAKE);
   EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet());
 
@@ -10294,6 +10296,7 @@
   // Retransmit handshake data.
   clock_.AdvanceTime(retransmission_time - clock_.Now());
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(4), _, _));
+  EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
   connection_.GetRetransmissionAlarm()->Fire();
   EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet());
 
@@ -10307,6 +10310,7 @@
   // Retransmit handshake data again.
   clock_.AdvanceTime(retransmission_time - clock_.Now());
   EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, QuicPacketNumber(7), _, _));
+  EXPECT_CALL(visitor_, OnHandshakePacketSent()).Times(1);
   connection_.GetRetransmissionAlarm()->Fire();
   EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet());
 

diff --git a/quic/core/quic_crypto_client_handshaker.h b/quic/core/quic_crypto_client_handshaker.h
index ed8ddc7..40b490e 100644
--- a/quic/core/quic_crypto_client_handshaker.h
+++ b/quic/core/quic_crypto_client_handshaker.h

@@ -51,6 +51,7 @@
   HandshakeState GetHandshakeState() const override;
   size_t BufferSizeLimitForLevel(EncryptionLevel level) const override;
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() override {}
   void OnHandshakeDoneReceived() override;
   void OnApplicationState(
       std::unique_ptr<ApplicationState> /*application_state*/) override {

diff --git a/quic/core/quic_crypto_client_stream.cc b/quic/core/quic_crypto_client_stream.cc
index 6f2ffcf..532a982 100644
--- a/quic/core/quic_crypto_client_stream.cc
+++ b/quic/core/quic_crypto_client_stream.cc

@@ -113,6 +113,10 @@
   handshaker_->OnOneRttPacketAcknowledged();
 }
 
+void QuicCryptoClientStream::OnHandshakePacketSent() {
+  handshaker_->OnHandshakePacketSent();
+}
+
 void QuicCryptoClientStream::OnHandshakeDoneReceived() {
   handshaker_->OnHandshakeDoneReceived();
 }

diff --git a/quic/core/quic_crypto_client_stream.h b/quic/core/quic_crypto_client_stream.h
index d095deb..82b0ef9 100644
--- a/quic/core/quic_crypto_client_stream.h
+++ b/quic/core/quic_crypto_client_stream.h

@@ -156,6 +156,9 @@
     // Called when a 1RTT packet has been acknowledged.
     virtual void OnOneRttPacketAcknowledged() = 0;
 
+    // Called when a packet of ENCRYPTION_HANDSHAKE gets sent.
+    virtual void OnHandshakePacketSent() = 0;
+
     // Called when handshake done has been received.
     virtual void OnHandshakeDoneReceived() = 0;
 
@@ -211,6 +214,7 @@
   CryptoMessageParser* crypto_message_parser() override;
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override;
+  void OnHandshakePacketSent() 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 fd14d27..8eef07c 100644
--- a/quic/core/quic_crypto_server_stream.h
+++ b/quic/core/quic_crypto_server_stream.h

@@ -42,6 +42,7 @@
       CachedNetworkParameters cached_network_params) override;
   void OnPacketDecrypted(EncryptionLevel level) override;
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() override {}
   void OnHandshakeDoneReceived() override;
   bool ShouldSendExpectCTHeader() const override;
 

diff --git a/quic/core/quic_crypto_stream.h b/quic/core/quic_crypto_stream.h
index 94e4730..d3d1082 100644
--- a/quic/core/quic_crypto_stream.h
+++ b/quic/core/quic_crypto_stream.h

@@ -91,6 +91,9 @@
   // Called when a 1RTT packet has been acknowledged.
   virtual void OnOneRttPacketAcknowledged() = 0;
 
+  // Called when a packet of ENCRYPTION_HANDSHAKE gets sent.
+  virtual void OnHandshakePacketSent() = 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 6365bc7..5f414e6 100644
--- a/quic/core/quic_crypto_stream_test.cc
+++ b/quic/core/quic_crypto_stream_test.cc

@@ -58,6 +58,7 @@
   }
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() override {}
   void OnHandshakeDoneReceived() override {}
   HandshakeState GetHandshakeState() const override { return HANDSHAKE_START; }
 

diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 3524242..930fc2f 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc

@@ -289,6 +289,10 @@
   GetMutableCryptoStream()->OnOneRttPacketAcknowledged();
 }
 
+void QuicSession::OnHandshakePacketSent() {
+  GetMutableCryptoStream()->OnHandshakePacketSent();
+}
+
 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 85c1a5f..1825322 100644
--- a/quic/core/quic_session.h
+++ b/quic/core/quic_session.h

@@ -126,6 +126,7 @@
   void OnStopSendingFrame(const QuicStopSendingFrame& frame) override;
   void OnPacketDecrypted(EncryptionLevel level) override;
   void OnOneRttPacketAcknowledged() override;
+  void OnHandshakePacketSent() override;
 
   // QuicStreamFrameDataProducer
   WriteStreamDataResult WriteStreamData(QuicStreamId id,

diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc
index db24eca..8c6e9dd 100644
--- a/quic/core/quic_session_test.cc
+++ b/quic/core/quic_session_test.cc

@@ -117,6 +117,7 @@
   }
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() override {}
   void OnHandshakeDoneReceived() override {}
   HandshakeState GetHandshakeState() const override {
     return one_rtt_keys_available() ? HANDSHAKE_COMPLETE : HANDSHAKE_START;

diff --git a/quic/core/tls_client_handshaker.cc b/quic/core/tls_client_handshaker.cc
index 3770ebd..9379087 100644
--- a/quic/core/tls_client_handshaker.cc
+++ b/quic/core/tls_client_handshaker.cc

@@ -296,6 +296,15 @@
   OnHandshakeConfirmed();
 }
 
+void TlsClientHandshaker::OnHandshakePacketSent() {
+  if (initial_keys_dropped_) {
+    return;
+  }
+  handshaker_delegate()->DiscardOldEncryptionKey(ENCRYPTION_INITIAL);
+  handshaker_delegate()->DiscardOldDecryptionKey(ENCRYPTION_INITIAL);
+  initial_keys_dropped_ = true;
+}
+
 void TlsClientHandshaker::OnHandshakeDoneReceived() {
   if (!one_rtt_keys_available_) {
     CloseConnection(QUIC_HANDSHAKE_FAILED,
@@ -529,8 +538,6 @@
   if (level == ENCRYPTION_HANDSHAKE &&
       state_ < STATE_ENCRYPTION_HANDSHAKE_DATA_SENT) {
     state_ = STATE_ENCRYPTION_HANDSHAKE_DATA_SENT;
-    handshaker_delegate()->DiscardOldEncryptionKey(ENCRYPTION_INITIAL);
-    handshaker_delegate()->DiscardOldDecryptionKey(ENCRYPTION_INITIAL);
   }
   TlsHandshaker::WriteMessage(level, data);
 }

diff --git a/quic/core/tls_client_handshaker.h b/quic/core/tls_client_handshaker.h
index 7acec4f..91b8be8 100644
--- a/quic/core/tls_client_handshaker.h
+++ b/quic/core/tls_client_handshaker.h

@@ -59,6 +59,7 @@
   HandshakeState GetHandshakeState() const override;
   size_t BufferSizeLimitForLevel(EncryptionLevel level) const override;
   void OnOneRttPacketAcknowledged() override;
+  void OnHandshakePacketSent() override;
   void OnHandshakeDoneReceived() override;
   void SetWriteSecret(EncryptionLevel level,
                       const SSL_CIPHER* cipher,
@@ -159,6 +160,7 @@
   std::string cert_verify_error_details_;
 
   bool encryption_established_ = false;
+  bool initial_keys_dropped_ = false;
   bool one_rtt_keys_available_ = false;
   bool handshake_confirmed_ = false;
   QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters>

diff --git a/quic/core/tls_handshaker_test.cc b/quic/core/tls_handshaker_test.cc
index 147d62f..1bd9fae 100644
--- a/quic/core/tls_handshaker_test.cc
+++ b/quic/core/tls_handshaker_test.cc

@@ -183,6 +183,7 @@
 
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() 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 9d72d30..28fa121 100644
--- a/quic/core/tls_server_handshaker.h
+++ b/quic/core/tls_server_handshaker.h

@@ -47,6 +47,7 @@
       CachedNetworkParameters cached_network_params) override;
   void OnPacketDecrypted(EncryptionLevel level) override;
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() 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 5a24909..c4976b0 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h

@@ -536,6 +536,7 @@
               (override));
   MOCK_METHOD(void, OnPacketDecrypted, (EncryptionLevel), (override));
   MOCK_METHOD(void, OnOneRttPacketAcknowledged, (), (override));
+  MOCK_METHOD(void, OnHandshakePacketSent, (), (override));
 };
 
 class MockQuicConnectionHelper : public QuicConnectionHelperInterface {
@@ -830,6 +831,7 @@
   CryptoMessageParser* crypto_message_parser() override;
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() 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 f3a9f02..8b9fd1f 100644
--- a/quic/test_tools/simulator/quic_endpoint.h
+++ b/quic/test_tools/simulator/quic_endpoint.h

@@ -87,6 +87,7 @@
   void OnStopSendingFrame(const QuicStopSendingFrame& /*frame*/) override {}
   void OnPacketDecrypted(EncryptionLevel /*level*/) override {}
   void OnOneRttPacketAcknowledged() override {}
+  void OnHandshakePacketSent() override {}
 
   // End QuicConnectionVisitorInterface implementation.