Move CryptoFactory from SrverHandshake to FizzServerHandshake (#162)

Summary:
This is moving some fizz specific part of the server handshake in FizzServerHandshake, following a similar pattern as what was done for the client.

Depends on https://github.com/facebookincubator/mvfst/issues/161 and https://github.com/facebookincubator/mvfst/issues/160

Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/162

Reviewed By: yangchi

Differential Revision: D23560890

Pulled By: xttjsn

fbshipit-source-id: 7bc03f6726ed2e922838d0b8dcd6b5d99fe9c540

quic/fizz/server/handshake/FizzServerHandshake.cpp

@@ -20,4 +20,28 @@ FizzServerHandshake::FizzServerHandshake(
     std::shared_ptr<FizzServerQuicHandshakeContext> fizzContext)
     : ServerHandshake(conn), fizzContext_(std::move(fizzContext)) {}
 
+void FizzServerHandshake::initializeImpl(
+    std::shared_ptr<const fizz::server::FizzServerContext> context,
+    HandshakeCallback* callback,
+    std::unique_ptr<fizz::server::AppTokenValidator> validator) {
+  auto ctx = std::make_shared<fizz::server::FizzServerContext>(*context);
+  ctx->setFactory(cryptoFactory_.getFizzFactory());
+  ctx->setSupportedCiphers({{fizz::CipherSuite::TLS_AES_128_GCM_SHA256}});
+  ctx->setVersionFallbackEnabled(false);
+  // Since Draft-17, client won't sent EOED
+  ctx->setOmitEarlyRecordLayer(true);
+  context_ = std::move(ctx);
+  callback_ = callback;
+
+  if (validator) {
+    state_.appTokenValidator() = std::move(validator);
+  } else {
+    state_.appTokenValidator() = std::make_unique<FailingAppTokenValidator>();
+  }
+}
+
+const CryptoFactory& FizzServerHandshake::getCryptoFactory() const {
+  return cryptoFactory_;
+}
+
 } // namespace quic

quic/fizz/server/handshake/FizzServerHandshake.h

@@ -8,6 +8,7 @@
 
 #pragma once
 
+#include <quic/fizz/handshake/FizzCryptoFactory.h>
 #include <quic/server/handshake/ServerHandshake.h>
 
 namespace quic {
@@ -21,7 +22,17 @@ class FizzServerHandshake : public ServerHandshake {
       QuicServerConnectionState* conn,
       std::shared_ptr<FizzServerQuicHandshakeContext> fizzContext);
 
+  const CryptoFactory& getCryptoFactory() const override;
+
  private:
+  void initializeImpl(
+      std::shared_ptr<const fizz::server::FizzServerContext> context,
+      HandshakeCallback* callback,
+      std::unique_ptr<fizz::server::AppTokenValidator> validator) override;
+
+ private:
+  FizzCryptoFactory cryptoFactory_;
+
   std::shared_ptr<FizzServerQuicHandshakeContext> fizzContext_;
 };
 

quic/server/handshake/ServerHandshake.cpp

@@ -38,22 +38,7 @@ void ServerHandshake::initialize(
     HandshakeCallback* callback,
     std::unique_ptr<fizz::server::AppTokenValidator> validator) {
   executor_ = executor;
-  auto ctx = std::make_shared<fizz::server::FizzServerContext>(*context);
+  initializeImpl(std::move(context), callback, std::move(validator));
-  auto cryptoFactory = std::make_shared<FizzCryptoFactory>();
-  ctx->setFactory(cryptoFactory->getFizzFactory());
-  cryptoFactory_ = std::move(cryptoFactory);
-  ctx->setSupportedCiphers({{fizz::CipherSuite::TLS_AES_128_GCM_SHA256}});
-  ctx->setVersionFallbackEnabled(false);
-  // Since Draft-17, client won't sent EOED
-  ctx->setOmitEarlyRecordLayer(true);
-  context_ = std::move(ctx);
-  callback_ = callback;
-
-  if (validator) {
-    state_.appTokenValidator() = std::move(validator);
-  } else {
-    state_.appTokenValidator() = std::make_unique<FailingAppTokenValidator>();
-  }
 }
 
 void ServerHandshake::doHandshake(
@@ -435,7 +420,7 @@ void ServerHandshake::ActionMoveVisitor::operator()(
       folly::range(secretAvailable.secret.secret),
       kQuicKeyLabel,
       kQuicIVLabel);
-  auto headerCipher = server_.cryptoFactory_->makePacketNumberCipher(
+  auto headerCipher = server_.getCryptoFactory().makePacketNumberCipher(
       folly::range(secretAvailable.secret.secret));
   switch (secretAvailable.secret.type.type()) {
     case fizz::SecretType::Type::EarlySecrets_E:

quic/server/handshake/ServerHandshake.h

@@ -107,9 +107,7 @@ class ServerHandshake : public Handshake {
   /**
    * Returns a reference to the CryptoFactory used internaly.
    */
-  virtual const CryptoFactory& getCryptoFactory() const {
+  virtual const CryptoFactory& getCryptoFactory() const = 0;
-    return *cryptoFactory_;
-  }
 
   /**
    * An edge triggered API to get the handshakeWriteCipher. Once you receive the
@@ -294,7 +292,12 @@ class ServerHandshake : public Handshake {
 
   Phase phase_{Phase::Handshake};
 
-  std::shared_ptr<CryptoFactory> cryptoFactory_;
   std::shared_ptr<ServerTransportParametersExtension> transportParams_;
+
+ private:
+  virtual void initializeImpl(
+      std::shared_ptr<const fizz::server::FizzServerContext> context,
+      HandshakeCallback* callback,
+      std::unique_ptr<fizz::server::AppTokenValidator> validator) = 0;
 }; // namespace quic
 } // namespace quic

quic/server/test/QuicServerTransportTest.cpp

@@ -19,6 +19,8 @@
 #include <quic/common/test/TestUtils.h>
 #include <quic/congestion_control/ServerCongestionControllerFactory.h>
 #include <quic/fizz/handshake/FizzCryptoFactory.h>
+#include <quic/fizz/server/handshake/FizzServerHandshake.h>
+#include <quic/fizz/server/handshake/FizzServerQuicHandshakeContext.h>
 #include <quic/logging/FileQLogger.h>
 #include <quic/server/handshake/ServerHandshake.h>
 #include <quic/server/test/Mocks.h>
@@ -38,14 +40,15 @@ using ByteEvent = QuicTransportBase::ByteEvent;
 using PacketDropReason = QuicTransportStatsCallback::PacketDropReason;
 } // namespace
 
-class FakeServerHandshake : public ServerHandshake {
+class FakeServerHandshake : public FizzServerHandshake {
  public:
   explicit FakeServerHandshake(
       QuicServerConnectionState& conn,
+      std::shared_ptr<FizzServerQuicHandshakeContext> fizzContext,
       bool chloSync = false,
       bool cfinSync = false,
       folly::Optional<uint64_t> clientActiveConnectionIdLimit = folly::none)
-      : ServerHandshake(&conn),
+      : FizzServerHandshake(&conn, std::move(fizzContext)),
         conn_(conn),
         chloSync_(chloSync),
         cfinSync_(cfinSync),
@@ -341,7 +344,9 @@ class QuicServerTransportTest : public Test {
   }
 
   virtual void initializeServerHandshake() {
-    fakeHandshake = new FakeServerHandshake(server->getNonConstConn());
+    fakeHandshake = new FakeServerHandshake(
+        server->getNonConstConn(),
+        std::make_shared<FizzServerQuicHandshakeContext>());
   }
 
   virtual bool getDisableMigration() {
@@ -2195,6 +2200,7 @@ class QuicServerTransportAllowMigrationTest
   virtual void initializeServerHandshake() override {
     fakeHandshake = new FakeServerHandshake(
         server->getNonConstConn(),
+        std::make_shared<FizzServerQuicHandshakeContext>(),
         false,
         false,
         GetParam().clientSentActiveConnIdTransportParam);
@@ -3914,7 +3920,10 @@ class QuicServerTransportPendingDataTest
 
   void initializeServerHandshake() override {
     fakeHandshake = new FakeServerHandshake(
-        server->getNonConstConn(), GetParam().chloSync, GetParam().cfinSync);
+        server->getNonConstConn(),
+        std::make_shared<FizzServerQuicHandshakeContext>(),
+        GetParam().chloSync,
+        GetParam().cfinSync);
     if (GetParam().acceptZeroRtt) {
       fakeHandshake->allowZeroRttKeys();
     }
@@ -4081,7 +4090,10 @@ class QuicServerTransportHandshakeTest
 
   void initializeServerHandshake() override {
     fakeHandshake = new FakeServerHandshake(
-        server->getNonConstConn(), GetParam().chloSync, GetParam().cfinSync);
+        server->getNonConstConn(),
+        std::make_shared<FizzServerQuicHandshakeContext>(),
+        GetParam().chloSync,
+        GetParam().cfinSync);
     if (GetParam().acceptZeroRtt) {
       fakeHandshake->allowZeroRttKeys();
     }