Move the management of the certificate verifier and the fizz::client::FizzClientContext object to FizzClientContext (#63)

Summary:
This allows to remove various fizz specific parts of the API.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/63

Test Plan:
Imported from GitHub, without a `Test Plan:` line.

 ---
## Proxygen Canary
Traffic Canary: https://our.intern.facebook.com/intern/traffic/canary?fbid=2326668697645016
* elb.prod.sju1c01 - binary - 2019-11-14 15:00 - https://fburl.com/dyndash/7m8qfbm6
* flb.prod.flhe2c01 - binary - 2019-11-14 15:00 - https://fburl.com/dyndash/alba0iv1
* olb.prod.rpnb0c01 - binary - 2019-11-14 15:00 - https://fburl.com/dyndash/f5eogqg5
* slb.prod_regional.rodn0c00 - binary - 2019-11-14 15:00 - https://fburl.com/dyndash/vtit218f
 ---

Reviewed By: yangchi

Differential Revision: D18303967

Pulled By: mjoras

fbshipit-source-id: 9bb7ed6ab608f9c2d1e8d5b0b533bda69f5d9a71

quic/api/test/IoBufQuicBatchTest.cpp

@@ -56,7 +56,7 @@ void RunTest(int numBatch) {
   auto batchWriter = std::make_unique<TestPacketBatchWriter>(numBatch);
   folly::SocketAddress peerAddress{"127.0.0.1", 1234};
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   QuicConnectionStateBase::HappyEyeballsState happyEyeballsState;
 
   IOBufQuicBatch ioBufBatch(

quic/api/test/QuicPacketSchedulerTest.cpp

@@ -117,7 +117,7 @@ TEST_F(QuicPacketSchedulerTest, NoopScheduler) {
 
 TEST_F(QuicPacketSchedulerTest, CryptoPaddingInitialPacket) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   auto connId = getTestConnectionId();
   LongHeader longHeader1(
       LongHeader::Types::Initial,
@@ -177,7 +177,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoServerInitialNotPadded) {
 
 TEST_F(QuicPacketSchedulerTest, CryptoPaddingRetransmissionClientInitial) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   auto connId = getTestConnectionId();
   LongHeader longHeader(
       LongHeader::Types::Initial,
@@ -225,7 +225,7 @@ TEST_F(QuicPacketSchedulerTest, CryptoSchedulerOnlySingleLossFits) {
 
 TEST_F(QuicPacketSchedulerTest, CryptoWritePartialLossBuffer) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   auto connId = getTestConnectionId();
   LongHeader longHeader(
       LongHeader::Types::Initial,
@@ -313,7 +313,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerStreamNotExists) {
 
 TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   FrameScheduler noopScheduler("frame");
   ASSERT_FALSE(noopScheduler.hasData());
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
@@ -341,7 +341,7 @@ TEST_F(QuicPacketSchedulerTest, CloningSchedulerTest) {
 
 TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   FrameScheduler noopScheduler("frame");
   ASSERT_FALSE(noopScheduler.hasData());
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
@@ -409,7 +409,7 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) {
 
 TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   // Add two outstanding packets, but then mark the second one processed by
@@ -440,7 +440,7 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneProcessedClonedPacket) {
 
 TEST_F(QuicPacketSchedulerTest, DoNotClonePureAck) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   // Add two outstanding packets, with second one being pureAck
@@ -467,7 +467,7 @@ TEST_F(QuicPacketSchedulerTest, DoNotClonePureAck) {
 
 TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasDataIgnoresNonAppData) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   EXPECT_FALSE(cloningScheduler.hasData());
@@ -484,7 +484,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerHasDataIgnoresNonAppData) {
 
 TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   FrameScheduler noopScheduler("frame");
   CloningScheduler cloningScheduler(noopScheduler, conn, "CopyCat", 0);
   // Add two outstanding packets, with second one being handshake
@@ -512,7 +512,7 @@ TEST_F(QuicPacketSchedulerTest, DoNotCloneHandshake) {
 
 TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   MockFrameScheduler mockScheduler;
   CloningScheduler cloningScheduler(mockScheduler, conn, "Mocker", 0);
   ShortHeader header(
@@ -561,7 +561,7 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) {
 
 TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   auto stream = conn.streamManager->createNextBidirectionalStream().value();
   FrameScheduler noopScheduler("frame");
@@ -643,7 +643,7 @@ class AckSchedulingTest : public TestWithParam<PacketNumberSpace> {};
 
 TEST_F(QuicPacketSchedulerTest, AckStateHasAcksToSchedule) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   EXPECT_FALSE(hasAcksToSchedule(conn.ackStates.initialAckState));
   EXPECT_FALSE(hasAcksToSchedule(conn.ackStates.handshakeAckState));
   EXPECT_FALSE(hasAcksToSchedule(conn.ackStates.appDataAckState));
@@ -661,7 +661,7 @@ TEST_F(QuicPacketSchedulerTest, AckStateHasAcksToSchedule) {
 
 TEST_F(QuicPacketSchedulerTest, AckSchedulerHasAcksToSchedule) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   AckScheduler initialAckScheduler(
       conn, getAckState(conn, PacketNumberSpace::Initial));
   AckScheduler handshakeAckScheduler(
@@ -685,7 +685,7 @@ TEST_F(QuicPacketSchedulerTest, AckSchedulerHasAcksToSchedule) {
 
 TEST_F(QuicPacketSchedulerTest, LargestAckToSend) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   EXPECT_EQ(folly::none, largestAckToSend(conn.ackStates.initialAckState));
   EXPECT_EQ(folly::none, largestAckToSend(conn.ackStates.handshakeAckState));
   EXPECT_EQ(folly::none, largestAckToSend(conn.ackStates.appDataAckState));
@@ -701,7 +701,7 @@ TEST_F(QuicPacketSchedulerTest, LargestAckToSend) {
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;
@@ -727,7 +727,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerAllFit) {
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;
@@ -781,7 +781,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobin) {
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;
@@ -846,7 +846,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRoundRobinControl) {
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;
@@ -868,7 +868,7 @@ TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerOneStream) {
 
 TEST_F(QuicPacketSchedulerTest, StreamFrameSchedulerRemoveOne) {
   QuicClientConnectionState conn(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   conn.streamManager->setMaxLocalBidirectionalStreams(10);
   conn.flowControlState.peerAdvertisedMaxOffset = 100000;
   conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote = 100000;

quic/client/QuicClientTransport.cpp

@@ -11,8 +11,8 @@
 #include <folly/portability/Sockets.h>
 
 #include <quic/api/QuicTransportFunctions.h>
+#include <quic/client/handshake/ClientHandshakeFactory.h>
 #include <quic/client/handshake/ClientTransportParametersExtension.h>
-#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/client/state/ClientStateMachine.h>
 #include <quic/flowcontrol/QuicFlowController.h>
 #include <quic/handshake/FizzCryptoFactory.h>
@@ -30,9 +30,11 @@ namespace quic {
 QuicClientTransport::QuicClientTransport(
     folly::EventBase* evb,
     std::unique_ptr<folly::AsyncUDPSocket> socket,
+    std::shared_ptr<ClientHandshakeFactory> handshakeFactory,
     size_t connectionIdSize)
     : QuicTransportBase(evb, std::move(socket)),
       happyEyeballsConnAttemptDelayTimeout_(this) {
+  DCHECK(handshakeFactory);
   // TODO(T53612743) Only enforce that the initial destination connection id
   // is at least kMinInitialDestinationConnIdLength.
   // All subsequent connection ids should be in between
@@ -41,8 +43,8 @@ QuicClientTransport::QuicClientTransport(
       connectionIdSize == 0 ||
       (connectionIdSize >= kMinInitialDestinationConnIdLength &&
        connectionIdSize <= kMaxConnectionIdSize));
-  auto tempConn = std::make_unique<QuicClientConnectionState>(
+  auto tempConn =
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      std::make_unique<QuicClientConnectionState>(std::move(handshakeFactory));
   clientConn_ = tempConn.get();
   conn_ = std::move(tempConn);
   std::vector<uint8_t> connIdData(
@@ -891,12 +893,7 @@ void QuicClientTransport::startCryptoHandshake() {
   conn_->transportParametersEncoded = true;
   auto handshakeLayer = clientConn_->clientHandshakeLayer;
   handshakeLayer->connect(
-      ctx_,
+      hostname_, std::move(cachedPsk), std::move(paramsExtension), this);
-      verifier_,
-      hostname_,
-      std::move(cachedPsk),
-      std::move(paramsExtension),
-      this);
 
   auto zeroRttWriteCipher = handshakeLayer->getZeroRttWriteCipher();
   auto zeroRttWriteHeaderCipher = handshakeLayer->getZeroRttWriteHeaderCipher();
@@ -1093,14 +1090,6 @@ void QuicClientTransport::start(ConnectionCallback* cb) {
 
   CHECK(conn_->peerAddress.isInitialized());
 
-  if (!ctx_) {
-    ctx_ = std::make_shared<const fizz::client::FizzClientContext>();
-  }
-  if (!verifier_) {
-    verifier_ = std::make_shared<const fizz::DefaultCertificateVerifier>(
-        fizz::VerificationContext::Client);
-  }
-
   if (conn_->qLogger) {
     conn_->qLogger->addTransportStateUpdate(kStart);
   }
@@ -1170,16 +1159,6 @@ void QuicClientTransport::setHostname(const std::string& hostname) {
   hostname_ = hostname;
 }
 
-void QuicClientTransport::setFizzClientQuicHandshakeContext(
-    std::shared_ptr<const fizz::client::FizzClientContext> ctx) {
-  ctx_ = std::move(ctx);
-}
-
-void QuicClientTransport::setCertificateVerifier(
-    std::shared_ptr<const fizz::CertificateVerifier> verifier) {
-  verifier_ = std::move(verifier);
-}
-
 void QuicClientTransport::setPskCache(std::shared_ptr<QuicPskCache> pskCache) {
   pskCache_ = std::move(pskCache);
 }

quic/client/QuicClientTransport.h

@@ -18,6 +18,8 @@
 
 namespace quic {
 
+class ClientHandshakeFactory;
+
 class QuicClientTransport
     : public QuicTransportBase,
       public folly::AsyncUDPSocket::ReadCallback,
@@ -28,6 +30,7 @@ class QuicClientTransport
   QuicClientTransport(
       folly::EventBase* evb,
       std::unique_ptr<folly::AsyncUDPSocket> socket,
+      std::shared_ptr<ClientHandshakeFactory> handshakeFactory,
       size_t connectionIdSize = kDefaultConnectionIdSize);
 
   ~QuicClientTransport() override;
@@ -46,9 +49,10 @@ class QuicClientTransport
   static std::shared_ptr<TransportType> newClient(
       folly::EventBase* evb,
       std::unique_ptr<folly::AsyncUDPSocket> sock,
+      std::shared_ptr<ClientHandshakeFactory> handshakeFactory,
       size_t connectionIdSize = kDefaultConnectionIdSize) {
-    auto client =
+    auto client = std::make_shared<TransportType>(
-        std::make_shared<TransportType>(evb, std::move(sock), connectionIdSize);
+        evb, std::move(sock), std::move(handshakeFactory), connectionIdSize);
     client->setSelfOwning();
     return client;
   }
@@ -59,18 +63,6 @@ class QuicClientTransport
    */
   void setHostname(const std::string& hostname);
 
-  /**
-   * Set the client context for fizz. Must be set before start()
-   */
-  void setFizzClientQuicHandshakeContext(
-      std::shared_ptr<const fizz::client::FizzClientContext> ctx);
-
-  /**
-   * Set a custom certificate verifier. Must be set before start().
-   */
-  void setCertificateVerifier(
-      std::shared_ptr<const fizz::CertificateVerifier> verifier);
-
   /**
    * Supplies a new peer address to use for the connection. This must be called
    * at least once before start().
@@ -182,8 +174,6 @@ class QuicClientTransport
 
   Buf readBuffer_;
   folly::Optional<std::string> hostname_;
-  std::shared_ptr<const fizz::client::FizzClientContext> ctx_;
-  std::shared_ptr<const fizz::CertificateVerifier> verifier_;
   HappyEyeballsConnAttemptDelayTimeout happyEyeballsConnAttemptDelayTimeout_;
   bool serverInitialParamsSet_{false};
   uint64_t peerAdvertisedInitialMaxData_{0};

quic/client/handshake/ClientHandshake.h

@@ -10,9 +10,7 @@
 
 #include <fizz/client/ClientProtocol.h>
 #include <fizz/client/EarlyDataRejectionPolicy.h>
-#include <fizz/client/FizzClientContext.h>
 #include <fizz/client/PskCache.h>
-#include <fizz/protocol/DefaultCertificateVerifier.h>
 
 #include <folly/io/IOBufQueue.h>
 #include <folly/io/async/DelayedDestruction.h>
@@ -45,8 +43,6 @@ class ClientHandshake : public Handshake {
    * Initiate the handshake with the supplied parameters.
    */
   virtual void connect(
-      std::shared_ptr<const fizz::client::FizzClientContext> context,
-      std::shared_ptr<const fizz::CertificateVerifier> verifier,
       folly::Optional<std::string> hostname,
       folly::Optional<fizz::client::CachedPsk> cachedPsk,
       const std::shared_ptr<ClientTransportParametersExtension>&

quic/client/handshake/FizzClientHandshake.cpp

@@ -8,6 +8,7 @@
 
 #include <quic/client/handshake/FizzClientHandshake.h>
 
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/handshake/FizzCryptoFactory.h>
 
 namespace quic {
@@ -18,26 +19,28 @@ FizzClientHandshake::FizzClientHandshake(
     : ClientHandshake(cryptoState), fizzContext_(std::move(fizzContext)) {}
 
 void FizzClientHandshake::connect(
-    std::shared_ptr<const fizz::client::FizzClientContext> context,
-    std::shared_ptr<const fizz::CertificateVerifier> verifier,
     folly::Optional<std::string> hostname,
     folly::Optional<fizz::client::CachedPsk> cachedPsk,
     const std::shared_ptr<ClientTransportParametersExtension>& transportParams,
     HandshakeCallback* callback) {
   transportParams_ = transportParams;
   callback_ = callback;
-  auto ctx = std::make_shared<fizz::client::FizzClientContext>(*context);
+
   auto cryptoFactory = std::make_shared<FizzCryptoFactory>();
-  ctx->setFactory(cryptoFactory);
+
+  // Setup context for this handshake.
+  auto context = std::make_shared<fizz::client::FizzClientContext>(
+      *fizzContext_->getContext());
+  context->setFactory(cryptoFactory);
   cryptoFactory_ = std::move(cryptoFactory);
-  ctx->setSupportedCiphers({fizz::CipherSuite::TLS_AES_128_GCM_SHA256});
+  context->setSupportedCiphers({fizz::CipherSuite::TLS_AES_128_GCM_SHA256});
-  ctx->setCompatibilityMode(false);
+  context->setCompatibilityMode(false);
   // Since Draft-17, EOED should not be sent
-  ctx->setOmitEarlyRecordLayer(true);
+  context->setOmitEarlyRecordLayer(true);
   processActions(machine_.processConnect(
       state_,
-      std::move(ctx),
+      std::move(context),
-      std::move(verifier),
+      fizzContext_->getCertificateVerifier(),
       std::move(hostname),
       std::move(cachedPsk),
       transportParams));

quic/client/handshake/FizzClientHandshake.h

@@ -21,8 +21,6 @@ class FizzClientHandshake : public ClientHandshake {
       std::shared_ptr<FizzClientQuicHandshakeContext> fizzContext);
 
   void connect(
-      std::shared_ptr<const fizz::client::FizzClientContext> context,
-      std::shared_ptr<const fizz::CertificateVerifier> verifier,
       folly::Optional<std::string> hostname,
       folly::Optional<fizz::client::CachedPsk> cachedPsk,
       const std::shared_ptr<ClientTransportParametersExtension>&

quic/client/handshake/FizzClientQuicHandshakeContext.cpp

@@ -12,10 +12,30 @@
 
 namespace quic {
 
+FizzClientQuicHandshakeContext::FizzClientQuicHandshakeContext(
+    std::shared_ptr<const fizz::client::FizzClientContext> context,
+    std::shared_ptr<const fizz::CertificateVerifier> verifier)
+    : context_(std::move(context)), verifier_(std::move(verifier)) {}
+
 std::unique_ptr<ClientHandshake>
 FizzClientQuicHandshakeContext::makeClientHandshake(
     QuicCryptoState& cryptoState) {
   return std::make_unique<FizzClientHandshake>(cryptoState, shared_from_this());
 }
 
+std::shared_ptr<FizzClientQuicHandshakeContext>
+FizzClientQuicHandshakeContext::Builder::build() {
+  if (!context_) {
+    context_ = std::make_shared<const fizz::client::FizzClientContext>();
+  }
+  if (!verifier_) {
+    verifier_ = std::make_shared<const fizz::DefaultCertificateVerifier>(
+        fizz::VerificationContext::Client);
+  }
+
+  return std::shared_ptr<FizzClientQuicHandshakeContext>(
+      new FizzClientQuicHandshakeContext(
+          std::move(context_), std::move(verifier_)));
+}
+
 } // namespace quic

quic/client/handshake/FizzClientQuicHandshakeContext.h

@@ -10,6 +10,9 @@
 
 #include <quic/client/handshake/ClientHandshakeFactory.h>
 
+#include <fizz/client/FizzClientContext.h>
+#include <fizz/protocol/DefaultCertificateVerifier.h>
+
 namespace quic {
 
 class FizzClientHandshake;
@@ -20,6 +23,54 @@ class FizzClientQuicHandshakeContext
  public:
   std::unique_ptr<ClientHandshake> makeClientHandshake(
       QuicCryptoState& cryptoState) override;
+
+  const std::shared_ptr<const fizz::client::FizzClientContext>& getContext()
+      const {
+    return context_;
+  }
+
+  const std::shared_ptr<const fizz::CertificateVerifier>&
+  getCertificateVerifier() const {
+    return verifier_;
+  }
+
+ private:
+  /**
+   * We make the constructor private so that users have to use the Builder
+   * facility. This ensures that
+   *   - This will ALWAYS be managed by a shared_ptr, which the implementation
+   * expects.
+   *   - We can enforce that the internal state of FizzClientContext is always
+   * sane.
+   */
+  FizzClientQuicHandshakeContext(
+      std::shared_ptr<const fizz::client::FizzClientContext> context,
+      std::shared_ptr<const fizz::CertificateVerifier> verifier);
+
+  std::shared_ptr<const fizz::client::FizzClientContext> context_;
+  std::shared_ptr<const fizz::CertificateVerifier> verifier_;
+
+ public:
+  class Builder {
+   public:
+    Builder& setFizzClientContext(
+        std::shared_ptr<const fizz::client::FizzClientContext> context) {
+      context_ = std::move(context);
+      return *this;
+    }
+
+    Builder& setCertificateVerifier(
+        std::shared_ptr<const fizz::CertificateVerifier> verifier) {
+      verifier_ = std::move(verifier);
+      return *this;
+    }
+
+    std::shared_ptr<FizzClientQuicHandshakeContext> build();
+
+   private:
+    std::shared_ptr<const fizz::client::FizzClientContext> context_;
+    std::shared_ptr<const fizz::CertificateVerifier> verifier_;
+  };
 };
 
 } // namespace quic

quic/client/handshake/test/ClientHandshakeTest.cpp

@@ -56,8 +56,6 @@ class ClientHandshakeTest : public Test, public boost::static_visitor<> {
 
   virtual void connect() {
     handshake->connect(
-        clientCtx,
-        verifier,
         hostname,
         folly::none,
         std::make_shared<ClientTransportParametersExtension>(
@@ -81,10 +79,13 @@ class ClientHandshakeTest : public Test, public boost::static_visitor<> {
     // Fizz is the name of the identity for our server certificate.
     hostname = "Fizz";
     setupClientAndServerContext();
+
     verifier = std::make_shared<fizz::test::MockCertificateVerifier>();
-    handshake =
+    handshake = FizzClientQuicHandshakeContext::Builder()
-        std::make_shared<FizzClientQuicHandshakeContext>()->makeClientHandshake(
+                    .setFizzClientContext(clientCtx)
-            cryptoState);
+                    .setCertificateVerifier(verifier)
+                    .build()
+                    ->makeClientHandshake(cryptoState);
     std::vector<QuicVersion> supportedVersions = {getVersion()};
     auto serverTransportParameters =
         std::make_shared<ServerTransportParametersExtension>(
@@ -348,8 +349,6 @@ class ClientHandshakeCallbackTest : public ClientHandshakeTest {
 
   void connect() override {
     handshake->connect(
-        clientCtx,
-        verifier,
         hostname,
         folly::none,
         std::make_shared<ClientTransportParametersExtension>(
@@ -450,8 +449,6 @@ class ClientHandshakeZeroRttTest : public ClientHandshakeTest {
 
   void connect() override {
     handshake->connect(
-        clientCtx,
-        verifier,
         hostname,
         psk.cachedPsk,
         std::make_shared<ClientTransportParametersExtension>(

quic/client/test/QuicClientTransportTest.cpp

@@ -19,6 +19,7 @@
 #include <folly/io/Cursor.h>
 #include <folly/io/async/ScopedEventBaseThread.h>
 #include <folly/io/async/test/MockAsyncUDPSocket.h>
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/client/handshake/test/MockQuicPskCache.h>
 #include <quic/codec/DefaultConnectionIdAlgo.h>
 #include <quic/common/test/TestUtils.h>
@@ -72,8 +73,13 @@ class TestingQuicClientTransport : public QuicClientTransport {
   TestingQuicClientTransport(
       folly::EventBase* evb,
       std::unique_ptr<folly::AsyncUDPSocket> socket,
+      std::shared_ptr<ClientHandshakeFactory> handshakeFactory,
       size_t connIdSize = kDefaultConnectionIdSize)
-      : QuicClientTransport(evb, std::move(socket), connIdSize) {}
+      : QuicClientTransport(
+            evb,
+            std::move(socket),
+            std::move(handshakeFactory),
+            connIdSize) {}
 
   ~TestingQuicClientTransport() override {
     if (destructionCallback_) {
@@ -183,6 +189,8 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
       connected_ = true;
     }));
 
+    clientCtx = createClientContext();
+    verifier = createTestCertificateVerifier();
     client = createClient();
   }
 
@@ -190,21 +198,30 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
     return GetParam().version;
   }
 
-  std::shared_ptr<TestingQuicClientTransport> createClient() {
+  std::shared_ptr<fizz::client::FizzClientContext> createClientContext() {
     clientCtx = std::make_shared<fizz::client::FizzClientContext>();
     clientCtx->setSupportedAlpns({"h1q-fb"});
     clientCtx->setClock(std::make_shared<fizz::test::MockClock>());
+    return clientCtx;
+  }
+
+  std::shared_ptr<TestingQuicClientTransport> createClient() {
     pskCache_ = std::make_shared<BasicQuicPskCache>();
 
     auto sock = std::make_unique<folly::AsyncUDPSocket>(&eventbase_);
+    auto fizzClientContext = FizzClientQuicHandshakeContext::Builder()
+                                 .setFizzClientContext(clientCtx)
+                                 .setCertificateVerifier(verifier)
+                                 .build();
     client = std::make_shared<TestingQuicClientTransport>(
-        &eventbase_, std::move(sock), GetParam().dstConnIdSize);
+        &eventbase_,
+        std::move(sock),
+        std::move(fizzClientContext),
+        GetParam().dstConnIdSize);
     client->setSupportedVersions({getVersion()});
     client->setCongestionControllerFactory(
         std::make_shared<DefaultCongestionControllerFactory>());
     client->setHostname(hostname);
-    client->setFizzClientQuicHandshakeContext(clientCtx);
-    client->setCertificateVerifier(createTestCertificateVerifier());
     client->addNewPeerAddress(serverAddr);
     client->setPskCache(pskCache_);
     return client;
@@ -317,6 +334,7 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
   std::shared_ptr<TestingQuicClientTransport> client;
   std::shared_ptr<fizz::server::FizzServerContext> serverCtx;
   std::shared_ptr<fizz::client::FizzClientContext> clientCtx;
+  std::shared_ptr<fizz::CertificateVerifier> verifier;
   std::shared_ptr<QuicPskCache> pskCache_;
   std::shared_ptr<QuicServer> server_;
   bool connected_{false};
@@ -449,6 +467,9 @@ TEST_P(QuicClientTransportIntegrationTest, ALPNTest) {
 }
 
 TEST_P(QuicClientTransportIntegrationTest, TLSAlert) {
+  verifier = nullptr;
+  client = createClient();
+
   auto qLogger = std::make_shared<FileQLogger>(VantagePoint::CLIENT);
   client->getNonConstConn().qLogger = qLogger;
   EXPECT_CALL(clientConnCallback, onConnectionError(_))
@@ -465,7 +486,6 @@ TEST_P(QuicClientTransportIntegrationTest, TLSAlert) {
 
   ASSERT_EQ(client->getAppProtocol(), folly::none);
 
-  client->setCertificateVerifier(nullptr);
   client->start(&clientConnCallback);
   eventbase_.loopForever();
 }
@@ -1102,8 +1122,6 @@ class FakeOneRttHandshakeLayer : public ClientHandshake {
       : ClientHandshake(cryptoState) {}
 
   void connect(
-      std::shared_ptr<const fizz::client::FizzClientContext>,
-      std::shared_ptr<const fizz::CertificateVerifier>,
       folly::Optional<std::string>,
       folly::Optional<fizz::client::CachedPsk>,
       const std::shared_ptr<ClientTransportParametersExtension>&,
@@ -1251,13 +1269,17 @@ class QuicClientTransportTest : public Test {
     auto socket =
         std::make_unique<folly::test::MockAsyncUDPSocket>(eventbase_.get());
     sock = socket.get();
+
+    auto fizzClientContext =
+        FizzClientQuicHandshakeContext::Builder()
+            .setCertificateVerifier(createTestCertificateVerifier())
+            .build();
     client = TestingQuicClientTransport::newClient<TestingQuicClientTransport>(
-        eventbase_.get(), std::move(socket));
+        eventbase_.get(), std::move(socket), std::move(fizzClientContext));
     destructionCallback = std::make_shared<DestructionCallback>();
     client->setDestructionCallback(destructionCallback);
     client->setSupportedVersions(
         {QuicVersion::MVFST, MVFST1, QuicVersion::QUIC_DRAFT});
-    client->setCertificateVerifier(createTestCertificateVerifier());
     connIdAlgo_ = std::make_unique<DefaultConnectionIdAlgo>();
     ON_CALL(*sock, resumeRead(_))
         .WillByDefault(SaveArg<0>(&networkReadCallback));

quic/loss/test/QuicLossFunctionsTest.cpp

@@ -87,7 +87,7 @@ class QuicLossFunctionsTest : public TestWithParam<PacketNumberSpace> {
 
   std::unique_ptr<QuicClientConnectionState> createClientConn() {
     auto conn = std::make_unique<QuicClientConnectionState>(
-        std::make_shared<FizzClientQuicHandshakeContext>());
+        FizzClientQuicHandshakeContext::Builder().build());
     conn->clientConnectionId = getTestConnectionId();
     conn->version = QuicVersion::MVFST;
     conn->ackStates.initialAckState.nextPacketNum = 1;

quic/samples/echo/EchoClient.h

@@ -18,6 +18,7 @@
 
 #include <quic/api/QuicSocket.h>
 #include <quic/client/QuicClientTransport.h>
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/common/test/TestUtils.h>
 
 namespace quic {
@@ -111,11 +112,13 @@ class EchoClient : public quic::QuicSocket::ConnectionCallback,
 
     evb->runInEventBaseThreadAndWait([&] {
       auto sock = std::make_unique<folly::AsyncUDPSocket>(evb);
-      quicClient_ =
+      auto fizzClientContext =
-          std::make_shared<quic::QuicClientTransport>(evb, std::move(sock));
+          FizzClientQuicHandshakeContext::Builder()
+              .setCertificateVerifier(test::createTestCertificateVerifier())
+              .build();
+      quicClient_ = std::make_shared<quic::QuicClientTransport>(
+          evb, std::move(sock), std::move(fizzClientContext));
       quicClient_->setHostname("echo.com");
-      quicClient_->setCertificateVerifier(
-          test::createTestCertificateVerifier());
       quicClient_->addNewPeerAddress(addr);
 
       TransportSettings settings;

quic/state/test/QuicStreamFunctionsTest.cpp

@@ -29,7 +29,7 @@ using PeekIterator = std::deque<StreamBuffer>::const_iterator;
 class QuicStreamFunctionsTest : public Test {
  public:
   QuicStreamFunctionsTest()
-      : conn(std::make_shared<FizzClientQuicHandshakeContext>()) {}
+      : conn(FizzClientQuicHandshakeContext::Builder().build()) {}
 
   void SetUp() override {
     conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiLocal =
@@ -1088,7 +1088,7 @@ TEST_F(QuicStreamFunctionsTest, IsBidirectionalStream) {
 
 TEST_F(QuicStreamFunctionsTest, IsSendingStream) {
   QuicClientConnectionState clientState(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   QuicServerConnectionState serverState;
   QuicNodeType nodeType;
   StreamId id;
@@ -1116,7 +1116,7 @@ TEST_F(QuicStreamFunctionsTest, IsSendingStream) {
 
 TEST_F(QuicStreamFunctionsTest, IsReceivingStream) {
   QuicClientConnectionState clientState(
-      std::make_shared<FizzClientQuicHandshakeContext>());
+      FizzClientQuicHandshakeContext::Builder().build());
   QuicServerConnectionState serverState;
   QuicNodeType nodeType;
   StreamId id;

quic/tools/tperf/tperf.cpp

@@ -15,6 +15,7 @@
 #include <folly/stats/Histogram.h>
 
 #include <quic/client/QuicClientTransport.h>
+#include <quic/client/handshake/FizzClientQuicHandshakeContext.h>
 #include <quic/common/test/TestUtils.h>
 #include <quic/congestion_control/CongestionControllerFactory.h>
 #include <quic/server/QuicServer.h>
@@ -431,10 +432,13 @@ class TPerfClient : public quic::QuicSocket::ConnectionCallback,
     folly::SocketAddress addr(host_.c_str(), port_);
 
     auto sock = std::make_unique<folly::AsyncUDPSocket>(&eventBase_);
+    auto fizzClientContext =
+        FizzClientQuicHandshakeContext::Builder()
+            .setCertificateVerifier(test::createTestCertificateVerifier())
+            .build();
     quicClient_ = std::make_shared<quic::QuicClientTransport>(
-        &eventBase_, std::move(sock));
+        &eventBase_, std::move(sock), std::move(fizzClientContext));
     quicClient_->setHostname("tperf");
-    quicClient_->setCertificateVerifier(test::createTestCertificateVerifier());
     quicClient_->addNewPeerAddress(addr);
     quicClient_->setCongestionControllerFactory(
         std::make_shared<DefaultCongestionControllerFactory>());