Bug 1634436 - Make cipher-strategy stateful and keep mode, key and IV as state. r=dom-workers-and-storage-reviewers,janv

Differential Revision: https://phabricator.services.mozilla.com/D80014
2020-08-17 08:42:20 +00:00 · 2020-08-17 08:42:20 +00:00 · 1fccb4c60f
--- a/dom/quota/DecryptingInputStream.h
+++ b/dom/quota/DecryptingInputStream.h
@ -86,11 +86,11 @@ class DecryptingInputStream final : public DecryptingInputStreamBase {
  // base stream must also be blocking.  The base stream does not have to be
  // buffered.
  DecryptingInputStream(MovingNotNull<nsCOMPtr<nsIInputStream>> aBaseStream,
-                        size_t aBlockSize, CipherStrategy aCipherStrategy,
+                        size_t aBlockSize,
                        typename CipherStrategy::KeyType aKey);

  // For deserialization only.
-  explicit DecryptingInputStream(CipherStrategy aCipherStrategy);
+  explicit DecryptingInputStream();

  NS_IMETHOD Close() override;
  NS_IMETHOD Available(uint64_t* _retval) override;
@ -137,7 +137,7 @@ class DecryptingInputStream final : public DecryptingInputStreamBase {

  bool EnsureBuffers();

-  const CipherStrategy mCipherStrategy;
+  CipherStrategy mCipherStrategy;
  LazyInitializedOnce<const typename CipherStrategy::KeyType> mKey;

  // Buffer to hold encrypted data.  Must copy here since we need a
--- a/dom/quota/DecryptingInputStream_impl.h
+++ b/dom/quota/DecryptingInputStream_impl.h
@ -21,10 +21,13 @@ namespace mozilla::dom::quota {
 template <typename CipherStrategy>
 DecryptingInputStream<CipherStrategy>::DecryptingInputStream(
    MovingNotNull<nsCOMPtr<nsIInputStream>> aBaseStream, size_t aBlockSize,
-    CipherStrategy aCipherStrategy, typename CipherStrategy::KeyType aKey)
+    typename CipherStrategy::KeyType aKey)
    : DecryptingInputStreamBase(std::move(aBaseStream), aBlockSize),
-      mCipherStrategy(std::move(aCipherStrategy)),
      mKey(aKey) {
+  // XXX Move this to a fallible init function.
+  MOZ_ALWAYS_SUCCEEDS(mCipherStrategy.Init(CipherMode::Decrypt,
+                                           CipherStrategy::SerializeKey(aKey)));
+
  // This implementation only supports sync base streams.  Verify this in debug
  // builds.
 #ifdef DEBUG
@ -41,10 +44,8 @@ DecryptingInputStream<CipherStrategy>::~DecryptingInputStream() {
 }

 template <typename CipherStrategy>
-DecryptingInputStream<CipherStrategy>::DecryptingInputStream(
-    CipherStrategy aCipherStrategy)
-    : DecryptingInputStreamBase{},
-      mCipherStrategy(std::move(aCipherStrategy)) {}
+DecryptingInputStream<CipherStrategy>::DecryptingInputStream()
+    : DecryptingInputStreamBase{} {}

 template <typename CipherStrategy>
 NS_IMETHODIMP DecryptingInputStream<CipherStrategy>::Close() {
@ -190,9 +191,9 @@ nsresult DecryptingInputStream<CipherStrategy>::ParseNextChunk(
  }

  // XXX Do we need to know the actual decrypted size?
-  rv = mCipherStrategy.Cipher(
-      CipherMode::Decrypt, *mKey, mEncryptedBlock->MutableCipherPrefix(),
-      mEncryptedBlock->Payload(), AsWritableBytes(Span{mPlainBuffer}));
+  rv = mCipherStrategy.Cipher(mEncryptedBlock->MutableCipherPrefix(),
+                              mEncryptedBlock->Payload(),
+                              AsWritableBytes(Span{mPlainBuffer}));
  if (NS_WARN_IF(NS_FAILED(rv))) {
    return rv;
  }
@ -444,10 +445,9 @@ NS_IMETHODIMP DecryptingInputStream<CipherStrategy>::Clone(
    return rv;
  }

-  *_retval =
-      MakeAndAddRef<DecryptingInputStream>(WrapNotNull(std::move(clonedStream)),
-                                           *mBlockSize, mCipherStrategy, *mKey)
-          .take();
+  *_retval = MakeAndAddRef<DecryptingInputStream>(
+                 WrapNotNull(std::move(clonedStream)), *mBlockSize, *mKey)
+                 .take();

  return NS_OK;
 }
@ -501,6 +501,10 @@ bool DecryptingInputStream<CipherStrategy>::Deserialize(
  Init(WrapNotNull<nsCOMPtr<nsIInputStream>>(std::move(stream)),
       params.blockSize());
  mKey.init(mCipherStrategy.DeserializeKey(params.key()));
+  if (NS_WARN_IF(
+          NS_FAILED(mCipherStrategy.Init(CipherMode::Decrypt, params.key())))) {
+    return false;
+  }

  return true;
 }
--- a/dom/quota/DummyCipherStrategy.h
+++ b/dom/quota/DummyCipherStrategy.h
@ -28,9 +28,13 @@ struct DummyCipherStrategy {

  static Result<KeyType, nsresult> GenerateKey() { return KeyType{}; }

-  static nsresult Cipher(const CipherMode aMode, const KeyType& aKey,
-                         Span<uint8_t> aIv, Span<const uint8_t> aIn,
-                         Span<uint8_t> aOut) {
+  nsresult Init(CipherMode aCipherMode, Span<const uint8_t> aKey,
+                Span<const uint8_t> aInitialIv = Span<const uint8_t>{}) {
+    return NS_OK;
+  }
+
+  nsresult Cipher(Span<uint8_t> aIv, Span<const uint8_t> aIn,
+                  Span<uint8_t> aOut) {
    DummyTransform(aIn, aOut);
    return NS_OK;
  }
--- a/dom/quota/EncryptingOutputStream.h
+++ b/dom/quota/EncryptingOutputStream.h
@ -55,7 +55,6 @@ class EncryptingOutputStream final : public EncryptingOutputStreamBase {
  // up to kMaxBlockSize.
  explicit EncryptingOutputStream(nsCOMPtr<nsIOutputStream> aBaseStream,
                                  size_t aBlockSize,
-                                  CipherStrategy aCipherStrategy,
                                  typename CipherStrategy::KeyType aKey);

 private:
@ -65,8 +64,7 @@ class EncryptingOutputStream final : public EncryptingOutputStreamBase {

  bool EnsureBuffers();

-  const CipherStrategy mCipherStrategy;
-  const typename CipherStrategy::KeyType mKey;
+  CipherStrategy mCipherStrategy;

  // Buffer holding copied plain data.  This must be copied here
  // so that the encryption can be performed on a single flat buffer.
--- a/dom/quota/EncryptingOutputStream_impl.h
+++ b/dom/quota/EncryptingOutputStream_impl.h
@ -17,10 +17,13 @@ namespace mozilla::dom::quota {
 template <typename CipherStrategy>
 EncryptingOutputStream<CipherStrategy>::EncryptingOutputStream(
    nsCOMPtr<nsIOutputStream> aBaseStream, size_t aBlockSize,
-    CipherStrategy aCipherStrategy, typename CipherStrategy::KeyType aKey)
-    : EncryptingOutputStreamBase(std::move(aBaseStream), aBlockSize),
-      mCipherStrategy(std::move(aCipherStrategy)),
-      mKey(aKey) {
+    typename CipherStrategy::KeyType aKey)
+    : EncryptingOutputStreamBase(std::move(aBaseStream), aBlockSize) {
+  // XXX Move this to a fallible init function.
+  MOZ_ALWAYS_SUCCEEDS(mCipherStrategy.Init(CipherMode::Encrypt,
+                                           CipherStrategy::SerializeKey(aKey),
+                                           CipherStrategy::MakeBlockPrefix()));
+
  MOZ_ASSERT(mBlockSize > 0);
  MOZ_ASSERT(mBlockSize % CipherStrategy::BasicBlockSize == 0);
  static_assert(
@ -197,7 +200,7 @@ nsresult EncryptingOutputStream<CipherStrategy>::FlushToBaseStream() {
  // Encrypt the data to our internal encrypted buffer.
  // XXX Do we need to know the actual encrypted size?
  nsresult rv = mCipherStrategy.Cipher(
-      CipherMode::Encrypt, mKey, mEncryptedBlock->MutableCipherPrefix(),
+      mEncryptedBlock->MutableCipherPrefix(),
      mozilla::Span(reinterpret_cast<uint8_t*>(mBuffer.Elements()),
                    ((mNextByte + (CipherStrategy::BasicBlockSize - 1)) /
                     CipherStrategy::BasicBlockSize) *
--- a/dom/quota/test/gtest/TestEncryptedStream.cpp
+++ b/dom/quota/test/gtest/TestEncryptedStream.cpp
@ -290,11 +290,10 @@ template <typename CipherStrategy>
 static void WriteTestData(nsCOMPtr<nsIOutputStream>&& aBaseOutputStream,
                          const Span<const uint8_t> aData,
                          const size_t aWriteChunkSize, const size_t aBlockSize,
-                          const CipherStrategy& aCipherStrategy,
                          const typename CipherStrategy::KeyType& aKey,
                          const FlushMode aFlushMode) {
  auto outStream = MakeSafeRefPtr<EncryptingOutputStream<CipherStrategy>>(
-      std::move(aBaseOutputStream), aBlockSize, aCipherStrategy, aKey);
+      std::move(aBaseOutputStream), aBlockSize, aKey);

  for (auto remaining = aData; !remaining.IsEmpty();) {
    auto [currentChunk, newRemaining] =
@ -358,11 +357,10 @@ template <typename CipherStrategy,
 static auto ReadTestData(
    MovingNotNull<nsCOMPtr<nsIInputStream>>&& aBaseInputStream,
    const Span<const uint8_t> aExpectedData, const size_t aReadChunkSize,
-    const size_t aBlockSize, const CipherStrategy& aCipherStrategy,
-    const typename CipherStrategy::KeyType& aKey,
+    const size_t aBlockSize, const typename CipherStrategy::KeyType& aKey,
    const ExtraChecks& aExtraChecks = NoExtraChecks<CipherStrategy>) {
  auto inStream = MakeSafeRefPtr<DecryptingInputStream<CipherStrategy>>(
-      std::move(aBaseInputStream), aBlockSize, aCipherStrategy, aKey);
+      std::move(aBaseInputStream), aBlockSize, aKey);

  ReadTestData(*inStream, aExpectedData, aReadChunkSize, aExtraChecks);

@ -375,7 +373,6 @@ template <typename CipherStrategy,
 static RefPtr<dom::quota::MemoryOutputStream> DoRoundtripTest(
    const size_t aDataSize, const size_t aWriteChunkSize,
    const size_t aReadChunkSize, const size_t aBlockSize,
-    const CipherStrategy& aCipherStrategy,
    const typename CipherStrategy::KeyType& aKey, const FlushMode aFlushMode,
    const ExtraChecks& aExtraChecks = NoExtraChecks<CipherStrategy>) {
  // XXX Add deduction guide for RefPtr from already_AddRefed
@ -385,28 +382,28 @@ static RefPtr<dom::quota::MemoryOutputStream> DoRoundtripTest(

  const auto data = MakeTestData(aDataSize);

-  WriteTestData(nsCOMPtr<nsIOutputStream>{baseOutputStream.get()}, Span{data},
-                aWriteChunkSize, aBlockSize, aCipherStrategy, aKey, aFlushMode);
+  WriteTestData<CipherStrategy>(
+      nsCOMPtr<nsIOutputStream>{baseOutputStream.get()}, Span{data},
+      aWriteChunkSize, aBlockSize, aKey, aFlushMode);

  const auto baseInputStream =
      MakeRefPtr<ArrayBufferInputStream>(baseOutputStream->Data());

-  ReadTestData(WrapNotNull(nsCOMPtr<nsIInputStream>{baseInputStream}),
-               Span{data}, aReadChunkSize, aBlockSize, aCipherStrategy, aKey,
-               aExtraChecks);
+  ReadTestData<CipherStrategy>(
+      WrapNotNull(nsCOMPtr<nsIInputStream>{baseInputStream}), Span{data},
+      aReadChunkSize, aBlockSize, aKey, aExtraChecks);

  return baseOutputStream;
 }

 TEST_P(ParametrizedCryptTest, DummyCipherStrategy_CheckOutput) {
  using CipherStrategy = DummyCipherStrategy;
-  const CipherStrategy cipherStrategy;
  const TestParams& testParams = GetParam();

-  const auto encryptedDataStream = DoRoundtripTest(
+  const auto encryptedDataStream = DoRoundtripTest<CipherStrategy>(
      testParams.DataSize(), testParams.EffectiveWriteChunkSize(),
      testParams.EffectiveReadChunkSize(), testParams.BlockSize(),
-      cipherStrategy, CipherStrategy::KeyType{}, testParams.FlushMode());
+      CipherStrategy::KeyType{}, testParams.FlushMode());

  if (HasFailure()) {
    return;
@ -451,44 +448,41 @@ TEST_P(ParametrizedCryptTest, DummyCipherStrategy_CheckOutput) {

 TEST_P(ParametrizedCryptTest, DummyCipherStrategy_Tell) {
  using CipherStrategy = DummyCipherStrategy;
-  const CipherStrategy cipherStrategy;
  const TestParams& testParams = GetParam();

-  DoRoundtripTest(testParams.DataSize(), testParams.EffectiveWriteChunkSize(),
-                  testParams.EffectiveReadChunkSize(), testParams.BlockSize(),
-                  cipherStrategy, CipherStrategy::KeyType{},
-                  testParams.FlushMode(),
-                  [](auto& inStream, Span<const uint8_t> expectedData,
-                     Span<const uint8_t> remainder) {
-                    // Check that Tell tells the right position.
-                    int64_t pos;
-                    EXPECT_EQ(NS_OK, inStream.Tell(&pos));
-                    EXPECT_EQ(expectedData.Length() - remainder.Length(),
-                              static_cast<uint64_t>(pos));
-                  });
+  DoRoundtripTest<CipherStrategy>(
+      testParams.DataSize(), testParams.EffectiveWriteChunkSize(),
+      testParams.EffectiveReadChunkSize(), testParams.BlockSize(),
+      CipherStrategy::KeyType{}, testParams.FlushMode(),
+      [](auto& inStream, Span<const uint8_t> expectedData,
+         Span<const uint8_t> remainder) {
+        // Check that Tell tells the right position.
+        int64_t pos;
+        EXPECT_EQ(NS_OK, inStream.Tell(&pos));
+        EXPECT_EQ(expectedData.Length() - remainder.Length(),
+                  static_cast<uint64_t>(pos));
+      });
 }

 TEST_P(ParametrizedCryptTest, DummyCipherStrategy_Available) {
  using CipherStrategy = DummyCipherStrategy;
-  const CipherStrategy cipherStrategy;
  const TestParams& testParams = GetParam();

-  DoRoundtripTest(testParams.DataSize(), testParams.EffectiveWriteChunkSize(),
-                  testParams.EffectiveReadChunkSize(), testParams.BlockSize(),
-                  cipherStrategy, CipherStrategy::KeyType{},
-                  testParams.FlushMode(),
-                  [](auto& inStream, Span<const uint8_t> expectedData,
-                     Span<const uint8_t> remainder) {
-                    // Check that Available tells the right remainder.
-                    uint64_t available;
-                    EXPECT_EQ(NS_OK, inStream.Available(&available));
-                    EXPECT_EQ(remainder.Length(), available);
-                  });
+  DoRoundtripTest<CipherStrategy>(
+      testParams.DataSize(), testParams.EffectiveWriteChunkSize(),
+      testParams.EffectiveReadChunkSize(), testParams.BlockSize(),
+      CipherStrategy::KeyType{}, testParams.FlushMode(),
+      [](auto& inStream, Span<const uint8_t> expectedData,
+         Span<const uint8_t> remainder) {
+        // Check that Available tells the right remainder.
+        uint64_t available;
+        EXPECT_EQ(NS_OK, inStream.Available(&available));
+        EXPECT_EQ(remainder.Length(), available);
+      });
 }

 TEST_P(ParametrizedCryptTest, DummyCipherStrategy_Clone) {
  using CipherStrategy = DummyCipherStrategy;
-  const CipherStrategy cipherStrategy;
  const TestParams& testParams = GetParam();

  // XXX Add deduction guide for RefPtr from already_AddRefed
@ -498,18 +492,18 @@ TEST_P(ParametrizedCryptTest, DummyCipherStrategy_Clone) {

  const auto data = MakeTestData(testParams.DataSize());

-  WriteTestData(nsCOMPtr<nsIOutputStream>{baseOutputStream.get()}, Span{data},
-                testParams.EffectiveWriteChunkSize(), testParams.BlockSize(),
-                cipherStrategy, CipherStrategy::KeyType{},
-                testParams.FlushMode());
+  WriteTestData<CipherStrategy>(
+      nsCOMPtr<nsIOutputStream>{baseOutputStream.get()}, Span{data},
+      testParams.EffectiveWriteChunkSize(), testParams.BlockSize(),
+      CipherStrategy::KeyType{}, testParams.FlushMode());

  const auto baseInputStream =
      MakeRefPtr<ArrayBufferInputStream>(baseOutputStream->Data());

-  const auto inStream = ReadTestData(
+  const auto inStream = ReadTestData<CipherStrategy>(
      WrapNotNull(nsCOMPtr<nsIInputStream>{baseInputStream}), Span{data},
      testParams.EffectiveReadChunkSize(), testParams.BlockSize(),
-      cipherStrategy, CipherStrategy::KeyType{});
+      CipherStrategy::KeyType{});

  nsCOMPtr<nsIInputStream> clonedInputStream;
  EXPECT_EQ(NS_OK, inStream->Clone(getter_AddRefs(clonedInputStream)));
@ -522,7 +516,6 @@ TEST_P(ParametrizedCryptTest, DummyCipherStrategy_Clone) {
 // XXX This test is actually only parametrized on the block size.
 TEST_P(ParametrizedCryptTest, DummyCipherStrategy_IncompleteBlock) {
  using CipherStrategy = DummyCipherStrategy;
-  const CipherStrategy cipherStrategy;
  const TestParams& testParams = GetParam();

  // Provide half a block, content doesn't matter.
@ -533,7 +526,7 @@ TEST_P(ParametrizedCryptTest, DummyCipherStrategy_IncompleteBlock) {

  const auto inStream = MakeSafeRefPtr<DecryptingInputStream<CipherStrategy>>(
      WrapNotNull(nsCOMPtr<nsIInputStream>{baseInputStream}),
-      testParams.BlockSize(), cipherStrategy, CipherStrategy::KeyType{});
+      testParams.BlockSize(), CipherStrategy::KeyType{});

  nsTArray<uint8_t> readData;
  readData.SetLength(testParams.BlockSize());
@ -613,7 +606,6 @@ class ParametrizedSeekCryptTest

 TEST_P(ParametrizedSeekCryptTest, DummyCipherStrategy_Seek) {
  using CipherStrategy = DummyCipherStrategy;
-  const CipherStrategy cipherStrategy;
  const SeekTestParams& testParams = GetParam();

  const auto baseOutputStream =
@ -622,16 +614,17 @@ TEST_P(ParametrizedSeekCryptTest, DummyCipherStrategy_Seek) {

  const auto data = MakeTestData(testParams.mDataSize);

-  WriteTestData(nsCOMPtr<nsIOutputStream>{baseOutputStream.get()}, Span{data},
-                testParams.mDataSize, testParams.mBlockSize, cipherStrategy,
-                CipherStrategy::KeyType{}, FlushMode::Never);
+  WriteTestData<CipherStrategy>(
+      nsCOMPtr<nsIOutputStream>{baseOutputStream.get()}, Span{data},
+      testParams.mDataSize, testParams.mBlockSize, CipherStrategy::KeyType{},
+      FlushMode::Never);

  const auto baseInputStream =
      MakeRefPtr<ArrayBufferInputStream>(baseOutputStream->Data());

  const auto inStream = MakeSafeRefPtr<DecryptingInputStream<CipherStrategy>>(
      WrapNotNull(nsCOMPtr<nsIInputStream>{baseInputStream}),
-      testParams.mBlockSize, cipherStrategy, CipherStrategy::KeyType{});
+      testParams.mBlockSize, CipherStrategy::KeyType{});

  uint32_t accumulatedOffset = 0;
  for (const auto& seekOp : testParams.mSeekOps) {
--- a/ipc/glue/InputStreamUtils.cpp
+++ b/ipc/glue/InputStreamUtils.cpp
@ -375,8 +375,7 @@ already_AddRefed<nsIInputStream> InputStreamHelper::DeserializeInputStream(

    case InputStreamParams::TEncryptedFileInputStreamParams:
      serializable = new dom::quota::DecryptingInputStream<
-          dom::quota::IPCStreamCipherStrategy>(
-          dom::quota::IPCStreamCipherStrategy{});
+          dom::quota::IPCStreamCipherStrategy>();
      break;

    default: