Start refactor of QuicCryptoStream

QuicCryptoClientStream and QuicCryptoServerStream are refactored to delegate
their implementations to a CryptoClientStreamDelegate or
CryptoServerStreamDelegate, and the existing implementations are moved to new
subclasses of the new delegates.

The major work left to do in this refactor is for QuicCryptoStream to no longer
implement CryptoFramerVisitorInterface and to remove the OnHandshakeMessage
method from CryptoClientStreamDelegate and CryptoServerStreamDelegate
interfaces.

Merge internal change: 161549072

Change-Id: I3919d067028cdacd939c64be4362d4645d7e7e65
Reviewed-on: https://chromium-review.googlesource.com/565978
Commit-Queue: Nick Harper <nharper@chromium.org>
Reviewed-by: Ryan Hamilton <rch@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486185}

net/quic/chromium/quic_chromium_client_session_peer.cc

@@ -17,13 +17,6 @@ void QuicChromiumClientSessionPeer::SetMaxOpenStreams(
   session->config()->SetMaxStreamsPerConnection(max_streams, default_streams);
 }
 
-// static
-void QuicChromiumClientSessionPeer::SetChannelIDSent(
-    QuicChromiumClientSession* session,
-    bool channel_id_sent) {
-  session->crypto_stream_->channel_id_sent_ = channel_id_sent;
-}
-
 // static
 void QuicChromiumClientSessionPeer::SetHostname(
     QuicChromiumClientSession* session,

net/quic/chromium/quic_chromium_client_session_peer.h

@@ -24,9 +24,6 @@ class QuicChromiumClientSessionPeer {
                                 size_t max_streams,
                                 size_t default_streams);
 
-  static void SetChannelIDSent(QuicChromiumClientSession* session,
-                               bool channel_id_sent);
-
   static void SetHostname(QuicChromiumClientSession* session,
                           const std::string& hostname);
 

net/quic/chromium/quic_chromium_client_session_test.cc

@@ -58,6 +58,26 @@ const char kServerHostname[] = "test.example.com";
 const uint16_t kServerPort = 443;
 const size_t kMaxReadersPerQuicSession = 5;
 
+// A subclass of QuicChromiumClientSession with GetSSLInfo overriden to allow
+// forcing the value of SSLInfo::channel_id_sent to true.
+class TestingQuicChromiumClientSession : public QuicChromiumClientSession {
+ public:
+  using QuicChromiumClientSession::QuicChromiumClientSession;
+
+  bool GetSSLInfo(SSLInfo* ssl_info) const override {
+    bool ret = QuicChromiumClientSession::GetSSLInfo(ssl_info);
+    if (ret)
+      ssl_info->channel_id_sent =
+          ssl_info->channel_id_sent || force_channel_id_sent_;
+    return ret;
+  }
+
+  void OverrideChannelIDSent() { force_channel_id_sent_ = true; }
+
+ private:
+  bool force_channel_id_sent_ = false;
+};
+
 class QuicChromiumClientSessionTest
     : public ::testing::TestWithParam<QuicVersion> {
  protected:
@@ -97,7 +117,7 @@ class QuicChromiumClientSessionTest
         0, QuicSocketAddress(QuicSocketAddressImpl(kIpEndPoint)), &helper_,
         &alarm_factory_, writer, true, Perspective::IS_CLIENT,
         SupportedVersions(GetParam()));
-    session_.reset(new QuicChromiumClientSession(
+    session_.reset(new TestingQuicChromiumClientSession(
         connection, std::move(socket),
         /*stream_factory=*/nullptr, &crypto_client_stream_factory_, &clock_,
         &transport_security_state_,
@@ -159,7 +179,7 @@ class QuicChromiumClientSessionTest
   MockCryptoClientStreamFactory crypto_client_stream_factory_;
   QuicClientPushPromiseIndex push_promise_index_;
   QuicServerId server_id_;
-  std::unique_ptr<QuicChromiumClientSession> session_;
+  std::unique_ptr<TestingQuicChromiumClientSession> session_;
   TestServerPushDelegate test_push_delegate_;
   QuicConnectionVisitorInterface* visitor_;
   TestCompletionCallback callback_;
@@ -976,7 +996,7 @@ TEST_P(QuicChromiumClientSessionTest, ConnectionPooledWithTlsChannelId) {
   CompleteCryptoHandshake();
   session_->OnProofVerifyDetailsAvailable(details);
   QuicChromiumClientSessionPeer::SetHostname(session_.get(), "www.example.org");
-  QuicChromiumClientSessionPeer::SetChannelIDSent(session_.get(), true);
+  session_->OverrideChannelIDSent();
 
   EXPECT_TRUE(session_->CanPool("www.example.org", PRIVACY_MODE_DISABLED));
   EXPECT_TRUE(session_->CanPool("mail.example.org", PRIVACY_MODE_DISABLED));
@@ -1012,7 +1032,7 @@ TEST_P(QuicChromiumClientSessionTest, ConnectionNotPooledWithDifferentPin) {
   CompleteCryptoHandshake();
   session_->OnProofVerifyDetailsAvailable(details);
   QuicChromiumClientSessionPeer::SetHostname(session_.get(), "www.example.org");
-  QuicChromiumClientSessionPeer::SetChannelIDSent(session_.get(), true);
+  session_->OverrideChannelIDSent();
 
   EXPECT_FALSE(session_->CanPool("mail.example.org", PRIVACY_MODE_DISABLED));
 }
@@ -1044,7 +1064,7 @@ TEST_P(QuicChromiumClientSessionTest, ConnectionPooledWithMatchingPin) {
   CompleteCryptoHandshake();
   session_->OnProofVerifyDetailsAvailable(details);
   QuicChromiumClientSessionPeer::SetHostname(session_.get(), "www.example.org");
-  QuicChromiumClientSessionPeer::SetChannelIDSent(session_.get(), true);
+  session_->OverrideChannelIDSent();
 
   EXPECT_TRUE(session_->CanPool("mail.example.org", PRIVACY_MODE_DISABLED));
 }

net/quic/core/quic_crypto_client_stream.h

@@ -56,6 +56,58 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
   //     token.
   static const int kMaxClientHellos = 3;
 
+  // QuicCryptoClientStream creates a HandshakerDelegate at construction time
+  // based on the QuicVersion of the connection. Different HandshakerDelegates
+  // provide implementations of different crypto handshake protocols. Currently
+  // QUIC crypto is the only protocol implemented; a future HandshakerDelegate
+  // will use TLS as the handshake protocol. QuicCryptoClientStream delegates
+  // all of its public methods to its HandshakerDelegate.
+  //
+  // This setup of the crypto stream delegating its implementation to the
+  // handshaker results in the handshaker reading and writing bytes on the
+  // crypto stream, instead of the handshaker passing the stream bytes to send.
+  class QUIC_EXPORT_PRIVATE HandshakerDelegate {
+   public:
+    virtual ~HandshakerDelegate() {}
+
+    // Performs a crypto handshake with the server. Returns true if the
+    // connection is still connected.
+    virtual bool CryptoConnect() = 0;
+
+    // num_sent_client_hellos returns the number of client hello messages that
+    // have been sent. If the handshake has completed then this is one greater
+    // than the number of round-trips needed for the handshake.
+    virtual int num_sent_client_hellos() const = 0;
+
+    // The number of server config update messages received by the
+    // client.  Does not count update messages that were received prior
+    // to handshake confirmation.
+    virtual int num_scup_messages_received() const = 0;
+
+    // TODO(nharper): Move this to QuicCryptoClientHandshaker.
+    virtual void OnHandshakeMessage(const CryptoHandshakeMessage& message) = 0;
+
+    // Returns true if a channel ID was sent on this connection.
+    virtual bool WasChannelIDSent() const = 0;
+
+    // Returns true if our ChannelIDSourceCallback was run, which implies the
+    // ChannelIDSource operated asynchronously. Intended for testing.
+    virtual bool WasChannelIDSourceCallbackRun() const = 0;
+
+    virtual std::string chlo_hash() const = 0;
+
+    // Returns true once any encrypter (initial/0RTT or final/1RTT) has been set
+    // for the connection.
+    virtual bool encryption_established() const = 0;
+
+    // Returns true once the crypto handshake has completed.
+    virtual bool handshake_confirmed() const = 0;
+
+    // Returns the parameters negotiated in the crypto handshake.
+    virtual const QuicCryptoNegotiatedParameters& crypto_negotiated_params()
+        const = 0;
+  };
+
   // ProofHandler is an interface that handles callbacks from the crypto
   // stream when the client has proof verification details of the server.
   class QUIC_EXPORT_PRIVATE ProofHandler {
@@ -89,6 +141,12 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
 
   int num_scup_messages_received() const override;
 
+  // From QuicCryptoStream
+  bool encryption_established() const override;
+  bool handshake_confirmed() const override;
+  const QuicCryptoNegotiatedParameters& crypto_negotiated_params()
+      const override;
+
   // CryptoFramerVisitorInterface implementation
   void OnHandshakeMessage(const CryptoHandshakeMessage& message) override;
 
@@ -99,8 +157,37 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
   // ChannelIDSource operated asynchronously. Intended for testing.
   bool WasChannelIDSourceCallbackRun() const;
 
-  std::string chlo_hash() const { return chlo_hash_; }
+  std::string chlo_hash() const;
+
+ private:
+  std::unique_ptr<HandshakerDelegate> handshaker_;
+
+  DISALLOW_COPY_AND_ASSIGN(QuicCryptoClientStream);
+};
+
+// An implementation of QuicCryptoClientStream::HandshakerDelegate which uses
+// QUIC crypto as the crypto handshake protocol.
+class QUIC_EXPORT_PRIVATE QuicCryptoClientHandshaker
+    : public QuicCryptoClientStream::HandshakerDelegate {
+ public:
+  QuicCryptoClientHandshaker(
+      const QuicServerId& server_id,
+      QuicCryptoClientStream* stream,
+      QuicSession* session,
+      ProofVerifyContext* verify_context,
+      QuicCryptoClientConfig* crypto_config,
+      QuicCryptoClientStream::ProofHandler* proof_handler);
+
+  ~QuicCryptoClientHandshaker() override;
 
+  // From QuicCryptoClientStream::HandshakerDelegate
+  bool CryptoConnect() override;
+  int num_sent_client_hellos() const override;
+  int num_scup_messages_received() const override;
+  void OnHandshakeMessage(const CryptoHandshakeMessage& message) override;
+  bool WasChannelIDSent() const override;
+  bool WasChannelIDSourceCallbackRun() const override;
+  std::string chlo_hash() const override;
   bool encryption_established() const override;
   bool handshake_confirmed() const override;
   const QuicCryptoNegotiatedParameters& crypto_negotiated_params()
@@ -112,7 +199,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
   // channel ID lookup when lookup is performed asynchronously.
   class ChannelIDSourceCallbackImpl : public ChannelIDSourceCallback {
    public:
-    explicit ChannelIDSourceCallbackImpl(QuicCryptoClientStream* stream);
+    explicit ChannelIDSourceCallbackImpl(QuicCryptoClientHandshaker* parent);
     ~ChannelIDSourceCallbackImpl() override;
 
     // ChannelIDSourceCallback interface.
@@ -123,7 +210,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
     void Cancel();
 
    private:
-    QuicCryptoClientStream* stream_;
+    QuicCryptoClientHandshaker* parent_;
   };
 
   // ProofVerifierCallbackImpl is passed as the callback method to VerifyProof.
@@ -131,7 +218,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
   // when verification is performed asynchronously.
   class ProofVerifierCallbackImpl : public ProofVerifierCallback {
    public:
-    explicit ProofVerifierCallbackImpl(QuicCryptoClientStream* stream);
+    explicit ProofVerifierCallbackImpl(QuicCryptoClientHandshaker* parent);
     ~ProofVerifierCallbackImpl() override;
 
     // ProofVerifierCallback interface.
@@ -144,7 +231,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
     void Cancel();
 
    private:
-    QuicCryptoClientStream* stream_;
+    QuicCryptoClientHandshaker* parent_;
   };
 
   friend class test::QuicChromiumClientSessionPeer;
@@ -216,6 +303,13 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
   // and the client config settings also allow sending a ChannelID.
   bool RequiresChannelID(QuicCryptoClientConfig::CachedState* cached);
 
+  // Returns the QuicSession that this stream belongs to.
+  QuicSession* session() const { return session_; }
+
+  QuicCryptoClientStream* stream_;
+
+  QuicSession* session_;
+
   State next_state_;
   // num_client_hellos_ contains the number of client hello messages that this
   // connection has sent.
@@ -257,7 +351,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
   ProofVerifierCallbackImpl* proof_verify_callback_;
   // proof_handler_ contains the callback object used by a quic client
   // for proof verification. It is not owned by this class.
-  ProofHandler* proof_handler_;
+  QuicCryptoClientStream::ProofHandler* proof_handler_;
 
   // These members are used to store the result of an asynchronous proof
   // verification. These members must not be used after
@@ -281,7 +375,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoClientStream
   QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters>
       crypto_negotiated_params_;
 
-  DISALLOW_COPY_AND_ASSIGN(QuicCryptoClientStream);
+  DISALLOW_COPY_AND_ASSIGN(QuicCryptoClientHandshaker);
 };
 
 }  // namespace net

net/quic/core/quic_crypto_client_stream_test.cc

@@ -314,6 +314,14 @@ TEST_F(QuicCryptoClientStreamTest, ServerConfigUpdateBeforeHandshake) {
                                           /*offset=*/0, data->AsStringPiece()));
 }
 
+TEST_F(QuicCryptoClientStreamTest, NoChannelID) {
+  crypto_config_.SetChannelIDSource(nullptr);
+
+  CompleteCryptoHandshake();
+  EXPECT_FALSE(stream()->WasChannelIDSent());
+  EXPECT_FALSE(stream()->WasChannelIDSourceCallbackRun());
+}
+
 TEST_F(QuicCryptoClientStreamTest, TokenBindingNegotiation) {
   server_options_.token_binding_params = QuicTagVector{kTB10, kP256};
   crypto_config_.tb_key_params = QuicTagVector{kTB10};

net/quic/core/quic_crypto_server_stream.h

@@ -16,6 +16,7 @@
 #include "net/quic/core/proto/source_address_token.pb.h"
 #include "net/quic/core/quic_config.h"
 #include "net/quic/core/quic_crypto_stream.h"
+#include "net/quic/core/quic_session.h"
 #include "net/quic/platform/api/quic_export.h"
 
 namespace net {
@@ -73,6 +74,69 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStreamBase : public QuicCryptoStream {
 class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
     : public QuicCryptoServerStreamBase {
  public:
+  // QuicCryptoServerStream creates a HandshakerDelegate at construction time
+  // based on the QuicVersion of the connection. Different HandshakerDelegates
+  // provide implementations of different crypto handshake protocols. Currently
+  // QUIC crypto is the only protocol implemented; a future HandshakerDelegate
+  // will use TLS as the handshake protocol. QuicCryptoServerStream delegates
+  // all of its public methods to its HandshakerDelegate.
+  //
+  // This setup of the crypto stream delegating its implementation to the
+  // handshaker results in the handshaker reading and writing bytes on the
+  // crypto stream, instead of the handshake rpassing the stream bytes to send.
+  class QUIC_EXPORT_PRIVATE HandshakerDelegate {
+   public:
+    virtual ~HandshakerDelegate() {}
+
+    // Cancel any outstanding callbacks, such as asynchronous validation of
+    // client hello.
+    virtual void CancelOutstandingCallbacks() = 0;
+
+    // TODO(nharper): Move this to QuicCryptoServerHandshaker.
+    virtual void OnHandshakeMessage(const CryptoHandshakeMessage& message) = 0;
+
+    // GetBase64SHA256ClientChannelID sets |*output| to the base64 encoded,
+    // SHA-256 hash of the client's ChannelID key and returns true, if the
+    // client presented a ChannelID. Otherwise it returns false.
+    virtual bool GetBase64SHA256ClientChannelID(std::string* output) const = 0;
+
+    // Sends the latest server config and source-address token to the client.
+    virtual void SendServerConfigUpdate(
+        const CachedNetworkParameters* cached_network_params) = 0;
+
+    // These are all accessors and setters to their respective counters.
+    virtual uint8_t NumHandshakeMessages() const = 0;
+    virtual uint8_t NumHandshakeMessagesWithServerNonces() const = 0;
+    virtual int NumServerConfigUpdateMessagesSent() const = 0;
+    virtual const CachedNetworkParameters* PreviousCachedNetworkParams()
+        const = 0;
+    virtual bool UseStatelessRejectsIfPeerSupported() const = 0;
+    virtual bool PeerSupportsStatelessRejects() const = 0;
+    virtual bool ZeroRttAttempted() const = 0;
+    virtual void SetPeerSupportsStatelessRejects(
+        bool peer_supports_stateless_rejects) = 0;
+    virtual void SetPreviousCachedNetworkParams(
+        CachedNetworkParameters cached_network_params) = 0;
+
+    // NOTE: Indicating that the Expect-CT header should be sent here presents a
+    // layering violation to some extent. The Expect-CT header only applies to
+    // HTTP connections, while this class can be used for non-HTTP applications.
+    // However, it is exposed here because that is the only place where the
+    // configuration for the certificate used in the connection is accessible.
+    virtual bool ShouldSendExpectCTHeader() const = 0;
+
+    // Returns true once any encrypter (initial/0RTT or final/1RTT) has been set
+    // for the connection.
+    virtual bool encryption_established() const = 0;
+
+    // Returns true once the crypto handshake has completed.
+    virtual bool handshake_confirmed() const = 0;
+
+    // Returns the parameters negotiated in the crypto handshake.
+    virtual const QuicCryptoNegotiatedParameters& crypto_negotiated_params()
+        const = 0;
+  };
+
   class Helper {
    public:
     virtual ~Helper() {}
@@ -124,9 +188,56 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
   // HTTP connections, while this class can be used for non-HTTP applications.
   // However, it is exposed here because that is the only place where the
   // configuration for the certificate used in the connection is accessible.
-  bool ShouldSendExpectCTHeader() const {
-    return signed_config_->proof.send_expect_ct_header;
-  }
+  bool ShouldSendExpectCTHeader() const;
+
+  bool encryption_established() const override;
+  bool handshake_confirmed() const override;
+  const QuicCryptoNegotiatedParameters& crypto_negotiated_params()
+      const override;
+
+ protected:
+  // Provided so that subclasses can provide their own handshaker.
+  virtual HandshakerDelegate* handshaker() const;
+
+ private:
+  std::unique_ptr<HandshakerDelegate> handshaker_;
+
+  DISALLOW_COPY_AND_ASSIGN(QuicCryptoServerStream);
+};
+
+class QUIC_EXPORT_PRIVATE QuicCryptoServerHandshaker
+    : public QuicCryptoServerStream::HandshakerDelegate {
+ public:
+  // |crypto_config| must outlive the stream.
+  // |session| must outlive the stream.
+  // |helper| must outlive the stream.
+  QuicCryptoServerHandshaker(const QuicCryptoServerConfig* crypto_config,
+                             QuicCryptoServerStream* stream,
+                             QuicCompressedCertsCache* compressed_certs_cache,
+                             bool use_stateless_rejects_if_peer_supported,
+                             QuicSession* session,
+                             QuicCryptoServerStream::Helper* helper);
+
+  ~QuicCryptoServerHandshaker() override;
+
+  // From HandshakerDelegate
+  void CancelOutstandingCallbacks() override;
+  void OnHandshakeMessage(const CryptoHandshakeMessage& message) override;
+  bool GetBase64SHA256ClientChannelID(std::string* output) const override;
+  void SendServerConfigUpdate(
+      const CachedNetworkParameters* cached_network_params) override;
+  uint8_t NumHandshakeMessages() const override;
+  uint8_t NumHandshakeMessagesWithServerNonces() const override;
+  int NumServerConfigUpdateMessagesSent() const override;
+  const CachedNetworkParameters* PreviousCachedNetworkParams() const override;
+  bool UseStatelessRejectsIfPeerSupported() const override;
+  bool PeerSupportsStatelessRejects() const override;
+  bool ZeroRttAttempted() const override;
+  void SetPeerSupportsStatelessRejects(
+      bool peer_supports_stateless_rejects) override;
+  void SetPreviousCachedNetworkParams(
+      CachedNetworkParameters cached_network_params) override;
+  bool ShouldSendExpectCTHeader() const override;
 
   bool encryption_established() const override;
   bool handshake_confirmed() const override;
@@ -152,7 +263,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
 
   class ValidateCallback : public ValidateClientHelloResultCallback {
    public:
-    explicit ValidateCallback(QuicCryptoServerStream* parent);
+    explicit ValidateCallback(QuicCryptoServerHandshaker* parent);
     // To allow the parent to detach itself from the callback before deletion.
     void Cancel();
 
@@ -161,7 +272,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
              std::unique_ptr<ProofSource::Details> details) override;
 
    private:
-    QuicCryptoServerStream* parent_;
+    QuicCryptoServerHandshaker* parent_;
 
     DISALLOW_COPY_AND_ASSIGN(ValidateCallback);
   };
@@ -169,7 +280,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
   class SendServerConfigUpdateCallback
       : public BuildServerConfigUpdateMessageResultCallback {
    public:
-    explicit SendServerConfigUpdateCallback(QuicCryptoServerStream* parent);
+    explicit SendServerConfigUpdateCallback(QuicCryptoServerHandshaker* parent);
     SendServerConfigUpdateCallback(const SendServerConfigUpdateCallback&) =
         delete;
     void operator=(const SendServerConfigUpdateCallback&) = delete;
@@ -181,7 +292,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
     void Run(bool ok, const CryptoHandshakeMessage& message) override;
 
    private:
-    QuicCryptoServerStream* parent_;
+    QuicCryptoServerHandshaker* parent_;
   };
 
   // Invoked by ValidateCallback::RunImpl once initial validation of
@@ -216,6 +327,16 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
   // if |use_stateless_rejects| is true. Returns 0 otherwise.
   QuicConnectionId GenerateConnectionIdForReject(bool use_stateless_rejects);
 
+  // Returns the QuicSession that this stream belongs to.
+  QuicSession* session() const { return session_; }
+
+  // Returns the QuicVersion of the connection.
+  QuicVersion version() const { return session_->connection()->version(); }
+
+  QuicCryptoServerStream* stream_;
+
+  QuicSession* session_;
+
   // crypto_config_ contains crypto parameters for the handshake.
   const QuicCryptoServerConfig* crypto_config_;
 
@@ -232,7 +353,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
   std::string chlo_hash_;
 
   // Pointer to the helper for this crypto stream. Must outlive this stream.
-  Helper* helper_;
+  QuicCryptoServerStream::Helper* helper_;
 
   // Number of handshake messages received by this stream.
   uint8_t num_handshake_messages_;
@@ -295,7 +416,7 @@ class QUIC_EXPORT_PRIVATE QuicCryptoServerStream
   QuicReferenceCountedPointer<QuicCryptoNegotiatedParameters>
       crypto_negotiated_params_;
 
-  DISALLOW_COPY_AND_ASSIGN(QuicCryptoServerStream);
+  DISALLOW_COPY_AND_ASSIGN(QuicCryptoServerHandshaker);
 };
 
 }  // namespace net