VersionCountingBlockStore::forEachBlock() has an integrity check checking that all expected blocks are still existing.

src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp

@@ -1,5 +1,6 @@
 #include <fstream>
 #include <cpp-utils/random/Random.h>
+#include <unordered_set>
 #include "KnownBlockVersions.h"
 
 namespace bf = boost::filesystem;
@@ -205,6 +206,16 @@ bool KnownBlockVersions::blockShouldExist(const Key &key) const {
     return found->second != CLIENT_ID_FOR_DELETED_BLOCK;
 }
 
+std::unordered_set<Key> KnownBlockVersions::existingBlocks() const {
+    std::unordered_set<Key> result;
+    for (const auto &entry : _lastUpdateClientId) {
+        if (entry.second != CLIENT_ID_FOR_DELETED_BLOCK) {
+            result.insert(entry.first);
+        }
+    }
+    return result;
+}
+
 const bf::path &KnownBlockVersions::path() const {
     return _stateFilePath;
 }

src/blockstore/implementations/versioncounting/KnownBlockVersions.h

@@ -10,6 +10,7 @@
 #include <cpp-utils/data/Deserializer.h>
 #include <cpp-utils/data/Serializer.h>
 #include <mutex>
+#include <unordered_set>
 
 namespace blockstore {
     namespace versioncounting {
@@ -28,6 +29,7 @@ namespace blockstore {
             void markBlockAsDeleted(const Key &key);
 
             bool blockShouldExist(const Key &key) const;
+            std::unordered_set<Key> existingBlocks() const;
 
             uint64_t getBlockVersion(uint32_t clientId, const Key &key) const;
 

src/blockstore/implementations/versioncounting/VersionCountingBlockStore.cpp

@@ -1,3 +1,4 @@
+#include <unordered_set>
 #include "VersionCountingBlockStore.h"
 #include "VersionCountingBlock.h"
 
@@ -78,7 +79,22 @@ namespace blockstore {
         }
 
         void VersionCountingBlockStore::forEachBlock(std::function<void (const Key &)> callback) const {
-            return _baseBlockStore->forEachBlock(callback);
+            if (!_missingBlockIsIntegrityViolation) {
+                return _baseBlockStore->forEachBlock(callback);
+            }
+
+            std::unordered_set<blockstore::Key> existingBlocks = _knownBlockVersions.existingBlocks();
+            _baseBlockStore->forEachBlock([&existingBlocks, callback] (const Key &key) {
+                callback(key);
+
+                auto found = existingBlocks.find(key);
+                if (found != existingBlocks.end()) {
+                    existingBlocks.erase(found);
+                }
+            });
+            if (!existingBlocks.empty()) {
+                throw IntegrityViolationError("A block that should have existed wasn't found.");
+            }
         }
 
 #ifndef CRYFS_NO_COMPATIBILITY

test/blockstore/implementations/versioncounting/KnownBlockVersionsTest.cpp

@@ -5,7 +5,9 @@
 
 using blockstore::versioncounting::KnownBlockVersions;
 using blockstore::versioncounting::VersionCountingBlock;
+using blockstore::Key;
 using cpputils::TempFile;
+using std::unordered_set;
 
 class KnownBlockVersionsTest : public ::testing::Test {
 public:
@@ -339,4 +341,40 @@ TEST_F(KnownBlockVersionsTest, blockShouldExist_deletedBlock) {
 TEST_F(KnownBlockVersionsTest, path) {
     KnownBlockVersions obj(stateFile.path());
     EXPECT_EQ(stateFile.path(), obj.path());
-}
+}
+
+TEST_F(KnownBlockVersionsTest, existingBlocks_empty) {
+    EXPECT_EQ(unordered_set<Key>({}), testobj.existingBlocks());
+}
+
+TEST_F(KnownBlockVersionsTest, existingBlocks_oneentry) {
+    setVersion(&testobj, clientId, key, 5);
+    EXPECT_EQ(unordered_set<Key>({key}), testobj.existingBlocks());
+}
+
+TEST_F(KnownBlockVersionsTest, existingBlocks_twoentries) {
+    setVersion(&testobj, clientId, key, 5);
+    setVersion(&testobj, clientId2, key2, 5);
+    EXPECT_EQ(unordered_set<Key>({key, key2}), testobj.existingBlocks());
+}
+
+TEST_F(KnownBlockVersionsTest, existingBlocks_twoentries_sameKey) {
+    setVersion(&testobj, clientId, key, 5);
+    setVersion(&testobj, clientId2, key, 5);
+    EXPECT_EQ(unordered_set<Key>({key}), testobj.existingBlocks());
+}
+
+TEST_F(KnownBlockVersionsTest, existingBlocks_deletedEntry) {
+    setVersion(&testobj, clientId, key, 5);
+    setVersion(&testobj, clientId2, key2, 5);
+    testobj.markBlockAsDeleted(key2);
+    EXPECT_EQ(unordered_set<Key>({key}), testobj.existingBlocks());
+}
+
+TEST_F(KnownBlockVersionsTest, existingBlocks_deletedEntries) {
+    setVersion(&testobj, clientId, key, 5);
+    setVersion(&testobj, clientId2, key2, 5);
+    testobj.markBlockAsDeleted(key);
+    testobj.markBlockAsDeleted(key2);
+    EXPECT_EQ(unordered_set<Key>({}), testobj.existingBlocks());
+}

test/blockstore/implementations/versioncounting/VersionCountingBlockStoreTest_Specific.cpp

@@ -212,6 +212,33 @@ TEST_F(VersionCountingBlockStoreTest, DeletionPrevention_DoesntAllowDeletingBloc
   );
 }
 
+// Check that in a multi-client scenario, missing blocks are not integrity errors, because another client might have deleted them.
+TEST_F(VersionCountingBlockStoreTest, DeletionPrevention_InForEachBlock_AllowsDeletingBlocksWhenDeactivated) {
+  FakeBlockStore *baseBlockStore;
+  unique_ptr<VersionCountingBlockStore> blockStore;
+  std::tie(baseBlockStore, blockStore) = makeBlockStoreWithoutDeletionPrevention();
+  auto key = blockStore->create(Data(0))->key();
+  baseBlockStore->remove(baseBlockStore->load(key).value());
+  int count = 0;
+  blockStore->forEachBlock([&count] (const blockstore::Key &) {
+      ++count;
+  });
+  EXPECT_EQ(0, count);
+}
+
+// Check that in a single-client scenario, missing blocks are integrity errors.
+TEST_F(VersionCountingBlockStoreTest, DeletionPrevention_InForEachBlock_DoesntAllowDeletingBlocksWhenActivated) {
+  FakeBlockStore *baseBlockStore;
+  unique_ptr<VersionCountingBlockStore> blockStore;
+  std::tie(baseBlockStore, blockStore) = makeBlockStoreWithDeletionPrevention();
+  auto key = blockStore->create(Data(0))->key();
+  baseBlockStore->remove(baseBlockStore->load(key).value());
+  EXPECT_THROW(
+      blockStore->forEachBlock([] (const blockstore::Key &) {}),
+      IntegrityViolationError
+  );
+}
+
 // TODO Test more integrity cases:
 //   - RollbackPrevention_DoesntAllowReintroducingDeletedBlocks with different client id (i.e. trying to re-introduce the newest block of a different client)
 //   - RollbackPrevention_AllowsReintroducingDeletedBlocksWithNewVersionNumber with different client id