Extend CryptoFactory with makePacketNumberCipher (#40)

Summary:
The CryptoFactory is extended with makePacketNumberCipher . In order to support that feature, FizzCryptoFactory now explicitly takes a QuicFizzFactory as argument instead of a generic fizz::Factory, which is the only type that is used in practice anyways.

The cypher argument was removed because:
1/ Only one cypher is used at all. Fizz also supports ChaCha20, but using it in mvfst will throw an exception.
2/ it seems like the factory should know what cypher it is dealing with.

If a choice of cypher needs to be supported going forward, it can be done by adding state to FizzCryptoFactory.
Pull Request resolved: https://github.com/facebookincubator/mvfst/pull/40

Reviewed By: mjoras

Differential Revision: D16785274

Pulled By: yangchi

fbshipit-source-id: a1c490e34c5ddd107e8e068d8b127c1ed00a59ec

quic/client/QuicClientTransport.cpp

@@ -855,10 +855,11 @@ void QuicClientTransport::startCryptoHandshake() {
       *clientConn_->initialDestinationConnectionId, version);
   conn_->readCodec->setInitialReadCipher(cryptoFactory.getServerInitialCipher(
       *clientConn_->initialDestinationConnectionId, version));
-  conn_->readCodec->setInitialHeaderCipher(makeServerInitialHeaderCipher(
+  conn_->readCodec->setInitialHeaderCipher(
-      &fizzFactory, *clientConn_->initialDestinationConnectionId, version));
+      cryptoFactory.makeServerInitialHeaderCipher(
-  conn_->initialHeaderCipher = makeClientInitialHeaderCipher(
+          *clientConn_->initialDestinationConnectionId, version));
-      &fizzFactory, *clientConn_->initialDestinationConnectionId, version);
+  conn_->initialHeaderCipher = cryptoFactory.makeClientInitialHeaderCipher(
+      *clientConn_->initialDestinationConnectionId, version);
 
   // Add partial reliability parameter to customTransportParameters_.
   setPartialReliabilityTransportParameter();

quic/client/handshake/ClientHandshake.cpp

@@ -10,6 +10,7 @@
 
 #include <fizz/protocol/Protocol.h>
 #include <quic/handshake/FizzBridge.h>
+#include <quic/handshake/FizzCryptoFactory.h>
 #include <quic/state/QuicStreamFunctions.h>
 
 namespace quic {
@@ -328,7 +329,8 @@ void ClientHandshake::ActionMoveVisitor::operator()(fizz::EndOfData&) {
 
 void ClientHandshake::ActionMoveVisitor::operator()(
     fizz::SecretAvailable& secretAvailable) {
-  QuicFizzFactory factory;
+  QuicFizzFactory fizzFactory;
+  FizzCryptoFactory cryptoFactory(&fizzFactory);
   folly::variant_match(
       secretAvailable.secret.type,
       [&](fizz::EarlySecrets earlySecrets) {
@@ -346,8 +348,9 @@ void ClientHandshake::ActionMoveVisitor::operator()(
                     folly::range(secretAvailable.secret.secret),
                     kQuicKeyLabel,
                     kQuicIVLabel));
-            client_.zeroRttWriteHeaderCipher_ = makePacketNumberCipher(
+            client_.zeroRttWriteHeaderCipher_ =
-                &factory, folly::range(secretAvailable.secret.secret), cipher);
+                cryptoFactory.makePacketNumberCipher(
+                    folly::range(secretAvailable.secret.secret));
             break;
           }
           default:
@@ -362,10 +365,8 @@ void ClientHandshake::ActionMoveVisitor::operator()(
             folly::range(secretAvailable.secret.secret),
             kQuicKeyLabel,
             kQuicIVLabel);
-        auto headerCipher = makePacketNumberCipher(
+        auto headerCipher = cryptoFactory.makePacketNumberCipher(
-            &factory,
+            folly::range(secretAvailable.secret.secret));
-            folly::range(secretAvailable.secret.secret),
-            *client_.state_.cipher());
         switch (handshakeSecrets) {
           case fizz::HandshakeSecrets::ClientHandshakeTraffic:
             client_.handshakeWriteCipher_ = FizzAead::wrap(std::move(aead));
@@ -385,10 +386,8 @@ void ClientHandshake::ActionMoveVisitor::operator()(
             folly::range(secretAvailable.secret.secret),
             kQuicKeyLabel,
             kQuicIVLabel);
-        auto appHeaderCipher = makePacketNumberCipher(
+        auto appHeaderCipher = cryptoFactory.makePacketNumberCipher(
-            &factory,
+            folly::range(secretAvailable.secret.secret));
-            folly::range(secretAvailable.secret.secret),
-            *client_.state_.cipher());
         switch (appSecrets) {
           case fizz::AppTrafficSecrets::ClientAppTraffic:
             client_.oneRttWriteCipher_ = FizzAead::wrap(std::move(aead));

quic/client/handshake/test/ClientHandshakeTest.cpp

@@ -27,6 +27,7 @@
 #include <quic/client/handshake/test/MockQuicPskCache.h>
 #include <quic/common/test/TestUtils.h>
 #include <quic/handshake/FizzBridge.h>
+#include <quic/handshake/QuicFizzFactory.h>
 #include <quic/state/QuicStreamFunctions.h>
 #include <quic/state/StateData.h>
 

quic/client/test/QuicClientTransportTest.cpp

@@ -1534,10 +1534,8 @@ class QuicClientTransportTest : public Test {
     codec->setClientConnectionId(*originalConnId);
     codec->setInitialReadCipher(cryptoFactory.getClientInitialCipher(
         *client->getConn().initialDestinationConnectionId, QuicVersion::MVFST));
-    codec->setInitialHeaderCipher(makeClientInitialHeaderCipher(
+    codec->setInitialHeaderCipher(cryptoFactory.makeClientInitialHeaderCipher(
-        &fizzFactory,
+        *client->getConn().initialDestinationConnectionId, QuicVersion::MVFST));
-        *client->getConn().initialDestinationConnectionId,
-        QuicVersion::MVFST));
     codec->setHandshakeReadCipher(test::createNoOpAead());
     codec->setHandshakeHeaderCipher(test::createNoOpHeaderCipher());
     return codec;
@@ -1558,8 +1556,7 @@ class QuicClientTransportTest : public Test {
       codec->setInitialReadCipher(cryptoFactory.getClientInitialCipher(
           *client->getConn().initialDestinationConnectionId,
           QuicVersion::MVFST));
-      codec->setInitialHeaderCipher(makeClientInitialHeaderCipher(
+      codec->setInitialHeaderCipher(cryptoFactory.makeClientInitialHeaderCipher(
-          &fizzFactory,
           *client->getConn().initialDestinationConnectionId,
           QuicVersion::MVFST));
       codec->setHandshakeReadCipher(test::createNoOpAead());

quic/codec/test/QuicPacketBuilderTest.cpp

@@ -71,13 +71,13 @@ std::unique_ptr<QuicReadCodec> makeCodec(
   if (nodeType == QuicNodeType::Client) {
     codec->setInitialReadCipher(
         cryptoFactory.getServerInitialCipher(clientConnId, QuicVersion::MVFST));
-    codec->setInitialHeaderCipher(makeServerInitialHeaderCipher(
+    codec->setInitialHeaderCipher(cryptoFactory.makeServerInitialHeaderCipher(
-        &fizzFactory, clientConnId, QuicVersion::MVFST));
+        clientConnId, QuicVersion::MVFST));
   } else {
     codec->setInitialReadCipher(
         cryptoFactory.getClientInitialCipher(clientConnId, QuicVersion::MVFST));
-    codec->setInitialHeaderCipher(makeClientInitialHeaderCipher(
+    codec->setInitialHeaderCipher(cryptoFactory.makeClientInitialHeaderCipher(
-        &fizzFactory, clientConnId, QuicVersion::MVFST));
+        clientConnId, QuicVersion::MVFST));
   }
   return codec;
 }
@@ -196,7 +196,7 @@ TEST_F(QuicPacketBuilderTest, LongHeaderRegularPacket) {
   FizzCryptoFactory cryptoFactory(&fizzFactory);
   auto cleartextAead = cryptoFactory.getClientInitialCipher(serverConnId, ver);
   auto headerCipher =
-      makeClientInitialHeaderCipher(&fizzFactory, serverConnId, ver);
+      cryptoFactory.makeClientInitialHeaderCipher(serverConnId, ver);
 
   auto resultRegularPacket = createInitialCryptoPacket(
       serverConnId,

quic/codec/test/QuicReadCodecTest.cpp

@@ -51,8 +51,8 @@ std::unique_ptr<QuicReadCodec> makeEncryptedCodec(
   codec->setClientConnectionId(clientConnId);
   codec->setInitialReadCipher(
       cryptoFactory.getClientInitialCipher(clientConnId, QuicVersion::MVFST));
-  codec->setInitialHeaderCipher(makeClientInitialHeaderCipher(
+  codec->setInitialHeaderCipher(cryptoFactory.makeClientInitialHeaderCipher(
-      &fizzFactory, clientConnId, QuicVersion::MVFST));
+      clientConnId, QuicVersion::MVFST));
   codec->setZeroRttReadCipher(std::move(zeroRttAead));
   codec->setZeroRttHeaderCipher(test::createNoOpHeaderCipher());
   codec->setOneRttReadCipher(std::move(oneRttAead));
@@ -463,7 +463,7 @@ TEST_F(QuicReadCodecTest, TestInitialPacket) {
   uint64_t offset = 0;
   auto aead = cryptoFactory.getClientInitialCipher(connId, QuicVersion::MVFST);
   auto headerCipher =
-      makeClientInitialHeaderCipher(&fizzFactory, connId, QuicVersion::MVFST);
+      cryptoFactory.makeClientInitialHeaderCipher(connId, QuicVersion::MVFST);
   auto packet = createInitialCryptoPacket(
       getTestConnectionId(),
       connId,
@@ -501,7 +501,7 @@ TEST_F(QuicReadCodecTest, TestHandshakeDone) {
   uint64_t offset = 0;
   auto aead = cryptoFactory.getClientInitialCipher(connId, QuicVersion::MVFST);
   auto headerCipher =
-      makeClientInitialHeaderCipher(&fizzFactory, connId, QuicVersion::MVFST);
+      cryptoFactory.makeClientInitialHeaderCipher(connId, QuicVersion::MVFST);
   auto packet = createInitialCryptoPacket(
       getTestConnectionId(),
       connId,

quic/common/test/TestUtils.cpp

@@ -13,6 +13,7 @@
 #include <fizz/protocol/test/Mocks.h>
 #include <quic/api/QuicTransportFunctions.h>
 #include <quic/codec/DefaultConnectionIdAlgo.h>
+#include <quic/handshake/QuicFizzFactory.h>
 #include <quic/handshake/test/Mocks.h>
 #include <quic/server/handshake/StatelessResetGenerator.h>
 

quic/handshake/CryptoFactory.cpp

@@ -38,4 +38,22 @@ Buf CryptoFactory::makeClientInitialTrafficSecret(
       kClientInitialLabel, clientDestinationConnId, version);
 }
 
+std::unique_ptr<PacketNumberCipher>
+CryptoFactory::makeClientInitialHeaderCipher(
+    const ConnectionId& initialDestinationConnectionId,
+    QuicVersion version) const {
+  auto clientInitialTrafficSecret =
+      makeClientInitialTrafficSecret(initialDestinationConnectionId, version);
+  return makePacketNumberCipher(clientInitialTrafficSecret->coalesce());
+}
+
+std::unique_ptr<PacketNumberCipher>
+CryptoFactory::makeServerInitialHeaderCipher(
+    const ConnectionId& initialDestinationConnectionId,
+    QuicVersion version) const {
+  auto serverInitialTrafficSecret =
+      makeServerInitialTrafficSecret(initialDestinationConnectionId, version);
+  return makePacketNumberCipher(serverInitialTrafficSecret->coalesce());
+}
+
 } // namespace quic

quic/handshake/CryptoFactory.h

@@ -9,6 +9,7 @@
 #pragma once
 
 #include <quic/QuicConstants.h>
+#include <quic/codec/PacketNumberCipher.h>
 #include <quic/codec/QuicConnectionId.h>
 #include <quic/codec/Types.h>
 #include <quic/handshake/Aead.h>
@@ -36,6 +37,20 @@ class CryptoFactory {
       const ConnectionId& clientDestinationConnId,
       QuicVersion version) const;
 
+  /**
+   * Makes the header cipher for writing client initial packets.
+   */
+  std::unique_ptr<PacketNumberCipher> makeClientInitialHeaderCipher(
+      const ConnectionId& initialDestinationConnectionId,
+      QuicVersion version) const;
+
+  /**
+   * Makes the header cipher for writing server initial packets.
+   */
+  std::unique_ptr<PacketNumberCipher> makeServerInitialHeaderCipher(
+      const ConnectionId& initialDestinationConnectionId,
+      QuicVersion version) const;
+
   /**
    * Crypto layer specifc methods.
    */
@@ -49,6 +64,9 @@ class CryptoFactory {
       const ConnectionId& clientDestinationConnId,
       QuicVersion version) const = 0;
 
+  virtual std::unique_ptr<PacketNumberCipher> makePacketNumberCipher(
+      folly::ByteRange baseSecret) const = 0;
+
   virtual ~CryptoFactory() = default;
 };
 

quic/handshake/FizzCryptoFactory.cpp

@@ -58,4 +58,16 @@ std::unique_ptr<Aead> FizzCryptoFactory::makeInitialAead(
   return FizzAead::wrap(std::move(aead));
 }
 
+std::unique_ptr<PacketNumberCipher> FizzCryptoFactory::makePacketNumberCipher(
+    folly::ByteRange baseSecret) const {
+  auto pnCipher = factory_->makePacketNumberCipher(
+      fizz::CipherSuite::TLS_AES_128_GCM_SHA256);
+  auto deriver =
+      factory_->makeKeyDeriver(fizz::CipherSuite::TLS_AES_128_GCM_SHA256);
+  auto pnKey = deriver->expandLabel(
+      baseSecret, kQuicPNLabel, folly::IOBuf::create(0), pnCipher->keyLength());
+  pnCipher->setKey(pnKey->coalesce());
+  return pnCipher;
+}
+
 } // namespace quic

quic/handshake/FizzCryptoFactory.h

@@ -10,13 +10,13 @@
 
 #include <quic/handshake/CryptoFactory.h>
 
-#include <fizz/protocol/Factory.h>
+#include <quic/handshake/QuicFizzFactory.h>
 
 namespace quic {
 
 class FizzCryptoFactory : public CryptoFactory {
  public:
-  explicit FizzCryptoFactory(fizz::Factory* factory) : factory_(factory) {}
+  explicit FizzCryptoFactory(QuicFizzFactory* factory) : factory_(factory) {}
 
   Buf makeInitialTrafficSecret(
       folly::StringPiece label,
@@ -28,8 +28,11 @@ class FizzCryptoFactory : public CryptoFactory {
       const ConnectionId& clientDestinationConnId,
       QuicVersion version) const override;
 
+  std::unique_ptr<PacketNumberCipher> makePacketNumberCipher(
+      folly::ByteRange baseSecret) const override;
+
  private:
-  fizz::Factory* factory_{nullptr};
+  QuicFizzFactory* factory_{nullptr};
 };
 
 } // namespace quic

quic/handshake/HandshakeLayer.cpp

@@ -17,44 +17,6 @@
 
 namespace quic {
 
-std::unique_ptr<PacketNumberCipher> makeClientInitialHeaderCipher(
-    QuicFizzFactory* factory,
-    const ConnectionId& initialDestinationConnectionId,
-    QuicVersion version) {
-  auto clientInitialTrafficSecret =
-      FizzCryptoFactory(factory).makeClientInitialTrafficSecret(
-          initialDestinationConnectionId, version);
-  return makePacketNumberCipher(
-      factory,
-      clientInitialTrafficSecret->coalesce(),
-      fizz::CipherSuite::TLS_AES_128_GCM_SHA256);
-}
-
-std::unique_ptr<PacketNumberCipher> makeServerInitialHeaderCipher(
-    QuicFizzFactory* factory,
-    const ConnectionId& initialDestinationConnectionId,
-    QuicVersion version) {
-  auto serverInitialTrafficSecret =
-      FizzCryptoFactory(factory).makeServerInitialTrafficSecret(
-          initialDestinationConnectionId, version);
-  return makePacketNumberCipher(
-      factory,
-      serverInitialTrafficSecret->coalesce(),
-      fizz::CipherSuite::TLS_AES_128_GCM_SHA256);
-}
-
-std::unique_ptr<PacketNumberCipher> makePacketNumberCipher(
-    QuicFizzFactory* factory,
-    folly::ByteRange baseSecret,
-    fizz::CipherSuite cipher) {
-  auto pnCipher = factory->makePacketNumberCipher(cipher);
-  auto deriver = factory->makeKeyDeriver(cipher);
-  auto pnKey = deriver->expandLabel(
-      baseSecret, kQuicPNLabel, folly::IOBuf::create(0), pnCipher->keyLength());
-  pnCipher->setKey(pnKey->coalesce());
-  return pnCipher;
-}
-
 EncryptionLevel protectionTypeToEncryptionLevel(ProtectionType type) {
   switch (type) {
     case ProtectionType::Initial:
@@ -69,4 +31,5 @@ EncryptionLevel protectionTypeToEncryptionLevel(ProtectionType type) {
   }
   folly::assume_unreachable();
 }
+
 } // namespace quic

quic/handshake/HandshakeLayer.h

@@ -13,7 +13,6 @@
 #include <quic/QuicConstants.h>
 #include <quic/codec/PacketNumberCipher.h>
 #include <quic/codec/Types.h>
-#include <quic/handshake/QuicFizzFactory.h>
 
 namespace quic {
 
@@ -38,29 +37,9 @@ constexpr folly::StringPiece kQuicDraft22Salt =
 constexpr folly::StringPiece kClientInitialLabel = "client in";
 constexpr folly::StringPiece kServerInitialLabel = "server in";
 
-/**
- * Makes the header cipher for writing client initial packets.
- */
-std::unique_ptr<PacketNumberCipher> makeClientInitialHeaderCipher(
-    QuicFizzFactory* factory,
-    const ConnectionId& initialDestinationConnectionId,
-    QuicVersion version);
-
-/**
- * Makes the header cipher for writing server initial packets.
- */
-std::unique_ptr<PacketNumberCipher> makeServerInitialHeaderCipher(
-    QuicFizzFactory* factory,
-    const ConnectionId& initialDestinationConnectionId,
-    QuicVersion version);
-
-std::unique_ptr<PacketNumberCipher> makePacketNumberCipher(
-    QuicFizzFactory* factory,
-    folly::ByteRange baseSecret,
-    fizz::CipherSuite cipher);
-
 /**
  * Converts the protection type of QUIC to an encryption level.
  */
 EncryptionLevel protectionTypeToEncryptionLevel(ProtectionType type);
+
 } // namespace quic

quic/handshake/test/CMakeLists.txt

@@ -16,21 +16,12 @@ quic_add_test(TARGET TransportParametersTest
   mvfst_test_utils
 )
 
-quic_add_test(TARGET HandshakeLayerTest
+quic_add_test(TARGET FizzCryptoFactoryTest
   SOURCES
-  HandshakeLayerTest.cpp
+  FizzCryptoFactoryTest.cpp
   DEPENDS
   Folly::folly
   mvfst_codec_types
   mvfst_handshake
   mvfst_test_utils
 )
-
-quic_add_test(TARGET FizzCryptoFactoryTest
-  SOURCES
-  FizzCryptoFactoryTest.cpp
-  DEPENDS
-  Folly::folly
-  mvfst_handshake
-  mvfst_test_utils
-)

quic/handshake/test/FizzCryptoFactoryTest.cpp

@@ -33,7 +33,18 @@ class QuicTestFizzFactory : public QuicFizzFactory {
     aead_ = std::move(aead);
   }
 
+  std::unique_ptr<PacketNumberCipher> makePacketNumberCipher(
+      fizz::CipherSuite) const override {
+    return std::move(packetNumberCipher_);
+  }
+
+  void setMockPacketNumberCipher(
+      std::unique_ptr<MockPacketNumberCipher> packetNumberCipher) {
+    packetNumberCipher_ = std::move(packetNumberCipher);
+  }
+
   mutable std::unique_ptr<fizz::Aead> aead_;
+  mutable std::unique_ptr<MockPacketNumberCipher> packetNumberCipher_;
 };
 
 class FizzCryptoFactoryTest : public Test {
@@ -50,7 +61,19 @@ class FizzCryptoFactoryTest : public Test {
     return mockAead;
   }
 
+  std::unique_ptr<MockPacketNumberCipher> createMockPacketNumberCipher() {
+    auto mockPacketNumberCipher = std::make_unique<MockPacketNumberCipher>();
+    EXPECT_CALL(*mockPacketNumberCipher, setKey(_))
+        .WillOnce(Invoke([&](folly::ByteRange key) {
+          packetCipherKey_ = folly::IOBuf::copyBuffer(key);
+        }));
+    EXPECT_CALL(*mockPacketNumberCipher, keyLength())
+        .WillRepeatedly(Return(fizz::AESGCM128::kKeyLength));
+    return mockPacketNumberCipher;
+  }
+
   folly::Optional<fizz::TrafficKey> trafficKey_;
+  folly::Optional<std::unique_ptr<folly::IOBuf>> packetCipherKey_;
 };
 
 TEST_F(FizzCryptoFactoryTest, TestDraft17ClearTextCipher) {
@@ -76,5 +99,34 @@ TEST_F(FizzCryptoFactoryTest, TestDraft17ClearTextCipher) {
   EXPECT_EQ(trafficIvHex, expectedIv);
 }
 
+TEST_F(FizzCryptoFactoryTest, TestPacketEncryptionKey) {
+  QuicTestFizzFactory fizzFactory;
+  fizzFactory.setMockPacketNumberCipher(createMockPacketNumberCipher());
+  FizzCryptoFactory cryptoFactory(&fizzFactory);
+  auto clientKey = std::vector<uint8_t>(
+      {0x0c, 0x74, 0xbb, 0x95, 0xa1, 0x04, 0x8e, 0x52, 0xef, 0x3b, 0x72,
+       0xe1, 0x28, 0x89, 0x35, 0x1c, 0xd7, 0x3a, 0x55, 0x0f, 0xb6, 0x2c,
+       0x4b, 0xb0, 0x87, 0xe9, 0x15, 0xcc, 0xe9, 0x6c, 0xe3, 0xa0});
+  auto expectedHex = "cd253a36ff93937c469384a823af6c56";
+  auto packetCipher =
+      cryptoFactory.makePacketNumberCipher(folly::range(clientKey));
+  auto secretHex = folly::hexlify(packetCipherKey_.value()->coalesce());
+  EXPECT_EQ(secretHex, expectedHex);
+
+  // reset the cipher
+  fizzFactory.setMockPacketNumberCipher(createMockPacketNumberCipher());
+  auto serverKey = std::vector<uint8_t>(
+      {0x4c, 0x9e, 0xdf, 0x24, 0xb0, 0xe5, 0xe5, 0x06, 0xdd, 0x3b, 0xfa,
+       0x4e, 0x0a, 0x03, 0x11, 0xe8, 0xc4, 0x1f, 0x35, 0x42, 0x73, 0xd8,
+       0xcb, 0x49, 0xdd, 0xd8, 0x46, 0x41, 0x38, 0xd4, 0x7e, 0xc6});
+
+  auto expectedKey2 = "2579d8696f85eda68d3502b65596586b";
+
+  auto packetCipher2 =
+      cryptoFactory.makePacketNumberCipher(folly::range(serverKey));
+  auto secretHex2 = folly::hexlify(packetCipherKey_.value()->coalesce());
+  EXPECT_EQ(secretHex2, expectedKey2);
+}
+
 } // namespace test
 } // namespace quic

quic/handshake/test/HandshakeLayerTest.cpp

@@ -1,91 +0,0 @@
-/*
- * Copyright (c) Facebook, Inc. and its affiliates.
- *
- * This source code is licensed under the MIT license found in the
- * LICENSE file in the root directory of this source tree.
- *
- */
-
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-
-#include <fizz/crypto/aead/test/Mocks.h>
-#include <quic/codec/QuicConnectionId.h>
-#include <quic/common/test/TestUtils.h>
-#include <quic/handshake/HandshakeLayer.h>
-#include <quic/handshake/QuicFizzFactory.h>
-#include <quic/handshake/test/Mocks.h>
-#include <vector>
-
-using namespace folly;
-using namespace testing;
-
-namespace quic {
-namespace test {
-
-class QuicTestFizzFactory : public QuicFizzFactory {
- public:
-  ~QuicTestFizzFactory() override = default;
-
-  std::unique_ptr<PacketNumberCipher> makePacketNumberCipher(
-      fizz::CipherSuite) const override {
-    return std::move(packetNumberCipher_);
-  }
-
-  void setMockPacketNumberCipher(
-      std::unique_ptr<MockPacketNumberCipher> packetNumberCipher) {
-    packetNumberCipher_ = std::move(packetNumberCipher);
-  }
-
-  mutable std::unique_ptr<MockPacketNumberCipher> packetNumberCipher_;
-};
-
-class HandshakeLayerTest : public Test {
- public:
-  std::unique_ptr<MockPacketNumberCipher> createMockPacketNumberCipher() {
-    auto mockPacketNumberCipher = std::make_unique<MockPacketNumberCipher>();
-    EXPECT_CALL(*mockPacketNumberCipher, setKey(_))
-        .WillOnce(Invoke([&](folly::ByteRange key) {
-          packetCipherKey_ = folly::IOBuf::copyBuffer(key);
-        }));
-    EXPECT_CALL(*mockPacketNumberCipher, keyLength())
-        .WillRepeatedly(Return(fizz::AESGCM128::kKeyLength));
-    return mockPacketNumberCipher;
-  }
-
-  folly::Optional<std::unique_ptr<folly::IOBuf>> packetCipherKey_;
-};
-
-TEST_F(HandshakeLayerTest, TestPacketEncryptionKey) {
-  QuicTestFizzFactory factory;
-  factory.setMockPacketNumberCipher(createMockPacketNumberCipher());
-  auto clientKey = std::vector<uint8_t>(
-      {0x0c, 0x74, 0xbb, 0x95, 0xa1, 0x04, 0x8e, 0x52, 0xef, 0x3b, 0x72,
-       0xe1, 0x28, 0x89, 0x35, 0x1c, 0xd7, 0x3a, 0x55, 0x0f, 0xb6, 0x2c,
-       0x4b, 0xb0, 0x87, 0xe9, 0x15, 0xcc, 0xe9, 0x6c, 0xe3, 0xa0});
-  auto expectedHex = "cd253a36ff93937c469384a823af6c56";
-  auto packetCipher = makePacketNumberCipher(
-      &factory,
-      folly::range(clientKey),
-      fizz::CipherSuite::TLS_AES_128_GCM_SHA256);
-  auto secretHex = folly::hexlify(packetCipherKey_.value()->coalesce());
-  EXPECT_EQ(secretHex, expectedHex);
-
-  // reset the cipher
-  factory.setMockPacketNumberCipher(createMockPacketNumberCipher());
-  auto serverKey = std::vector<uint8_t>(
-      {0x4c, 0x9e, 0xdf, 0x24, 0xb0, 0xe5, 0xe5, 0x06, 0xdd, 0x3b, 0xfa,
-       0x4e, 0x0a, 0x03, 0x11, 0xe8, 0xc4, 0x1f, 0x35, 0x42, 0x73, 0xd8,
-       0xcb, 0x49, 0xdd, 0xd8, 0x46, 0x41, 0x38, 0xd4, 0x7e, 0xc6});
-
-  auto expectedKey2 = "2579d8696f85eda68d3502b65596586b";
-
-  auto packetCipher2 = makePacketNumberCipher(
-      &factory,
-      folly::range(serverKey),
-      fizz::CipherSuite::TLS_AES_128_GCM_SHA256);
-  auto secretHex2 = folly::hexlify(packetCipherKey_.value()->coalesce());
-  EXPECT_EQ(secretHex2, expectedKey2);
-}
-} // namespace test
-} // namespace quic

quic/server/handshake/ServerHandshake.cpp

@@ -10,6 +10,7 @@
 
 #include <fizz/protocol/Protocol.h>
 #include <quic/handshake/FizzBridge.h>
+#include <quic/handshake/FizzCryptoFactory.h>
 #include <quic/state/QuicStreamFunctions.h>
 
 namespace quic {
@@ -400,11 +401,10 @@ void ServerHandshake::ActionMoveVisitor::operator()(
       folly::range(secretAvailable.secret.secret),
       kQuicKeyLabel,
       kQuicIVLabel);
-  QuicFizzFactory factory;
+  QuicFizzFactory fizzFactory;
-  auto headerCipher = makePacketNumberCipher(
+  FizzCryptoFactory cryptoFactory(&fizzFactory);
-      &factory,
+  auto headerCipher = cryptoFactory.makePacketNumberCipher(
-      folly::range(secretAvailable.secret.secret),
+      folly::range(secretAvailable.secret.secret));
-      *server_.state_.cipher());
   folly::variant_match(
       secretAvailable.secret.type,
       [&](fizz::EarlySecrets earlySecrets) {

quic/server/handshake/test/ServerHandshakeTest.cpp

@@ -28,6 +28,7 @@
 #include <quic/common/test/TestUtils.h>
 #include <quic/handshake/FizzBridge.h>
 #include <quic/handshake/HandshakeLayer.h>
+#include <quic/handshake/QuicFizzFactory.h>
 #include <quic/server/handshake/AppToken.h>
 #include <quic/server/handshake/ServerHandshake.h>
 #include <quic/state/StateData.h>

quic/server/state/ServerStateMachine.cpp

@@ -495,6 +495,7 @@ void onServerReadDataFromOpen(
             conn.transportSettings.partialReliabilityEnabled,
             token));
     QuicFizzFactory fizzFactory;
+    FizzCryptoFactory cryptoFactory(&fizzFactory);
     conn.readCodec = std::make_unique<QuicReadCodec>(QuicNodeType::Server);
     conn.readCodec->setInitialReadCipher(
         FizzCryptoFactory(&fizzFactory)
@@ -506,14 +507,14 @@ void onServerReadDataFromOpen(
     }
     conn.readCodec->setCodecParameters(
         CodecParameters(conn.peerAckDelayExponent, version));
-    conn.initialWriteCipher =
+    conn.initialWriteCipher = cryptoFactory.getServerInitialCipher(
-        FizzCryptoFactory(&fizzFactory)
+        initialDestinationConnectionId, version);
-            .getServerInitialCipher(initialDestinationConnectionId, version);
 
-    conn.readCodec->setInitialHeaderCipher(makeClientInitialHeaderCipher(
+    conn.readCodec->setInitialHeaderCipher(
-        &fizzFactory, initialDestinationConnectionId, version));
+        cryptoFactory.makeClientInitialHeaderCipher(
-    conn.initialHeaderCipher = makeServerInitialHeaderCipher(
+            initialDestinationConnectionId, version));
-        &fizzFactory, initialDestinationConnectionId, version);
+    conn.initialHeaderCipher = cryptoFactory.makeServerInitialHeaderCipher(
+        initialDestinationConnectionId, version);
     conn.peerAddress = conn.originalPeerAddress;
   }
   folly::IOBufQueue udpData{folly::IOBufQueue::cacheChainLength()};

quic/server/test/QuicServerTransportTest.cpp

@@ -330,8 +330,9 @@ class QuicServerTransportTest : public Test {
 
   std::unique_ptr<PacketNumberCipher> getInitialHeaderCipher() {
     QuicFizzFactory fizzFactory;
-    return makeClientInitialHeaderCipher(
+    FizzCryptoFactory cryptoFactory(&fizzFactory);
-        &fizzFactory, *initialDestinationConnectionId, QuicVersion::MVFST);
+    return cryptoFactory.makeClientInitialHeaderCipher(
+        *initialDestinationConnectionId, QuicVersion::MVFST);
   }
 
   Buf recvEncryptedStream(
@@ -412,8 +413,9 @@ class QuicServerTransportTest : public Test {
     clientReadCodec->setClientConnectionId(*clientConnectionId);
     clientReadCodec->setInitialReadCipher(cryptoFactory.getServerInitialCipher(
         *initialDestinationConnectionId, QuicVersion::MVFST));
-    clientReadCodec->setInitialHeaderCipher(makeServerInitialHeaderCipher(
+    clientReadCodec->setInitialHeaderCipher(
-        &fizzFactory, *initialDestinationConnectionId, QuicVersion::MVFST));
+        cryptoFactory.makeServerInitialHeaderCipher(
+            *initialDestinationConnectionId, QuicVersion::MVFST));
     clientReadCodec->setCodecParameters(
         CodecParameters(kDefaultAckDelayExponent, QuicVersion::MVFST));
   }
@@ -590,8 +592,9 @@ class QuicServerTransportTest : public Test {
     if (handshakeCipher) {
       readCodec->setInitialReadCipher(cryptoFactory.getServerInitialCipher(
           *initialDestinationConnectionId, QuicVersion::MVFST));
-      readCodec->setInitialHeaderCipher(makeServerInitialHeaderCipher(
+      readCodec->setInitialHeaderCipher(
-          &fizzFactory, *initialDestinationConnectionId, QuicVersion::MVFST));
+          cryptoFactory.makeServerInitialHeaderCipher(
+              *initialDestinationConnectionId, QuicVersion::MVFST));
     }
     return readCodec;
   }