Throw an IntegrityViolationError exception instead of just logging integrity violations. This makes sure the user notices.

src/blockstore/CMakeLists.txt

@@ -31,6 +31,7 @@ set(SOURCES
   implementations/versioncounting/VersionCountingBlockStore.cpp
   implementations/versioncounting/KnownBlockVersions.cpp
   implementations/versioncounting/ClientIdAndBlockKey.cpp
+  implementations/versioncounting/IntegrityViolationError.cpp
 )
 
 add_library(${PROJECT_NAME} STATIC ${SOURCES})

src/blockstore/implementations/versioncounting/IntegrityViolationError.cpp

@@ -0,0 +1 @@
+#include "IntegrityViolationError.h"

src/blockstore/implementations/versioncounting/IntegrityViolationError.h

@@ -0,0 +1,29 @@
+#pragma once
+#ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_VERSIONCOUNTING_INTEGRITYVIOLATIONERROR_H_
+#define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_VERSIONCOUNTING_INTEGRITYVIOLATIONERROR_H_
+
+#include <cpp-utils/macros.h>
+#include <string>
+
+namespace blockstore {
+    namespace versioncounting {
+
+        class IntegrityViolationError final : public std::exception {
+        public:
+            IntegrityViolationError(const std::string &reason)
+                    : _reason("Integrity violation: " + reason) {
+            }
+
+            const char *what() const throw() override {
+                return _reason.c_str();
+            }
+
+        private:
+            std::string _reason;
+        };
+
+
+    }
+}
+
+#endif

src/blockstore/implementations/versioncounting/VersionCountingBlock.h

@@ -17,6 +17,7 @@
 #include <mutex>
 #include <cpp-utils/logging/logging.h>
 #include "../../../../vendor/googletest/gtest-1.7.0/googletest/include/gtest/gtest_prod.h"
+#include "IntegrityViolationError.h"
 
 namespace blockstore {
 namespace versioncounting {
@@ -26,7 +27,7 @@ namespace versioncounting {
 class VersionCountingBlock final: public Block {
 public:
   static boost::optional<cpputils::unique_ref<VersionCountingBlock>> TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, KnownBlockVersions *knownBlockVersions);
-  static boost::optional<cpputils::unique_ref<VersionCountingBlock>> TryLoad(cpputils::unique_ref<Block> baseBlock, KnownBlockVersions *knownBlockVersions);
+  static cpputils::unique_ref<VersionCountingBlock> Load(cpputils::unique_ref<Block> baseBlock, KnownBlockVersions *knownBlockVersions);
 
   static uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize);
 
@@ -59,7 +60,7 @@ private:
   static void _checkFormatHeader(const cpputils::Data &data);
   static uint64_t _readVersion(const cpputils::Data &data);
   static uint32_t _readClientId(const cpputils::Data &data);
-  static bool _checkVersion(const cpputils::Data &data, const blockstore::Key &key, KnownBlockVersions *knownBlockVersions);
+  static void _checkVersion(const cpputils::Data &data, const blockstore::Key &key, KnownBlockVersions *knownBlockVersions);
 
   // This header is prepended to blocks to allow future versions to have compatibility.
   static constexpr uint16_t FORMAT_VERSION_HEADER = 0;
@@ -99,25 +100,20 @@ inline cpputils::Data VersionCountingBlock::_prependHeaderToData(uint32_t myClie
   return result;
 }
 
-inline boost::optional<cpputils::unique_ref<VersionCountingBlock>> VersionCountingBlock::TryLoad(cpputils::unique_ref<Block> baseBlock, KnownBlockVersions *knownBlockVersions) {
+inline cpputils::unique_ref<VersionCountingBlock> VersionCountingBlock::Load(cpputils::unique_ref<Block> baseBlock, KnownBlockVersions *knownBlockVersions) {
   cpputils::Data data(baseBlock->size());
   std::memcpy(data.data(), baseBlock->data(), data.size());
   _checkFormatHeader(data);
-  if (!_checkVersion(data, baseBlock->key(), knownBlockVersions)) {
-    return boost::none;
-  }
+  _checkVersion(data, baseBlock->key(), knownBlockVersions);
   return cpputils::make_unique_ref<VersionCountingBlock>(std::move(baseBlock), std::move(data), knownBlockVersions);
 }
 
-inline bool VersionCountingBlock::_checkVersion(const cpputils::Data &data, const blockstore::Key &key, KnownBlockVersions *knownBlockVersions) {
+inline void VersionCountingBlock::_checkVersion(const cpputils::Data &data, const blockstore::Key &key, KnownBlockVersions *knownBlockVersions) {
   uint32_t lastClientId = _readClientId(data);
   uint64_t version = _readVersion(data);
   if(!knownBlockVersions->checkAndUpdateVersion(lastClientId, key, version)) {
-      cpputils::logging::LOG(cpputils::logging::WARN) << "Decrypting block " << key.ToString() <<
-        " failed due to decreasing version number. Was the block rolled back or re-introduced by an attacker?";
-    return false;
+     throw IntegrityViolationError("The block version number is too low. Did an attacker try to roll back the block or to re-introduce a deleted block?");
   }
-  return true;
 }
 
 inline void VersionCountingBlock::_checkFormatHeader(const cpputils::Data &data) {

src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h

@@ -59,7 +59,7 @@ inline boost::optional<cpputils::unique_ref<Block>> VersionCountingBlockStore::l
   if (block == boost::none) {
     return boost::none;
   }
-  return boost::optional<cpputils::unique_ref<Block>>(VersionCountingBlock::TryLoad(std::move(*block), &_knownBlockVersions));
+  return boost::optional<cpputils::unique_ref<Block>>(VersionCountingBlock::Load(std::move(*block), &_knownBlockVersions));
 }
 
 inline void VersionCountingBlockStore::remove(cpputils::unique_ref<Block> block) {

test/blockstore/implementations/versioncounting/VersionCountingBlockStoreTest_Specific.cpp

@@ -106,7 +106,10 @@ TEST_F(VersionCountingBlockStoreTest, RollbackPrevention_DoesntAllowDecreasingVe
   Data oldBaseBlock = loadBaseBlock(key);
   modifyBlock(key);
   rollbackBaseBlock(key, oldBaseBlock);
-  EXPECT_EQ(boost::none, blockStore->load(key));
+  EXPECT_THROW(
+      blockStore->load(key),
+      IntegrityViolationError
+  );
 }
 
 TEST_F(VersionCountingBlockStoreTest, RollbackPrevention_DoesntAllowDecreasingVersionNumberForSameClient_2) {
@@ -115,7 +118,10 @@ TEST_F(VersionCountingBlockStoreTest, RollbackPrevention_DoesntAllowDecreasingVe
   modifyBlock(key);
   // Decrease the version number again
   decreaseVersionNumber(key);
-  EXPECT_EQ(boost::none, blockStore->load(key));
+  EXPECT_THROW(
+          blockStore->load(key),
+          IntegrityViolationError
+  );
 }
 
 // Test that a different client doesn't need to have a higher version number (i.e. version numbers are per client).
@@ -140,7 +146,10 @@ TEST_F(VersionCountingBlockStoreTest, RollbackPrevention_DoesntAllowSameVersionN
   loadBlock(key); // make the block store know about this other client's modification
   // Rollback to old client
   rollbackBaseBlock(key, oldBaseBlock);
-  EXPECT_EQ(boost::none, blockStore->load(key));
+  EXPECT_THROW(
+          blockStore->load(key),
+          IntegrityViolationError
+  );
 }
 
 // Test that deleted blocks cannot be re-introduced
@@ -149,7 +158,10 @@ TEST_F(VersionCountingBlockStoreTest, RollbackPrevention_DoesntAllowReintroducin
   Data oldBaseBlock = loadBaseBlock(key);
   deleteBlock(key);
   insertBaseBlock(key, std::move(oldBaseBlock));
-  EXPECT_EQ(boost::none, blockStore->load(key));
+  EXPECT_THROW(
+          blockStore->load(key),
+          IntegrityViolationError
+  );
 }
 
 // This can happen if a client synchronization is delayed. Another client might have won the conflict and pushed a new version for the deleted block.