diff --git a/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp b/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp index 99a13010..6cada5f5 100644 --- a/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp +++ b/src/blockstore/implementations/versioncounting/KnownBlockVersions.cpp @@ -175,5 +175,9 @@ uint32_t KnownBlockVersions::myClientId() const { return _myClientId; } +uint64_t KnownBlockVersions::getBlockVersion(uint32_t clientId, const Key &key) const { + return _knownVersions.at({clientId, key}); +} + } } diff --git a/src/blockstore/implementations/versioncounting/KnownBlockVersions.h b/src/blockstore/implementations/versioncounting/KnownBlockVersions.h index 43e6deda..b97693c8 100644 --- a/src/blockstore/implementations/versioncounting/KnownBlockVersions.h +++ b/src/blockstore/implementations/versioncounting/KnownBlockVersions.h @@ -25,6 +25,8 @@ namespace blockstore { void updateVersion(const Key &key, uint64_t version); + uint64_t getBlockVersion(uint32_t clientId, const Key &key) const; + uint32_t myClientId() const; private: diff --git a/src/blockstore/implementations/versioncounting/VersionCountingBlock.cpp b/src/blockstore/implementations/versioncounting/VersionCountingBlock.cpp index d6e5388d..cd37c108 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlock.cpp +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlock.cpp @@ -7,5 +7,6 @@ namespace blockstore { constexpr unsigned int VersionCountingBlock::HEADER_LENGTH; constexpr uint16_t VersionCountingBlock::FORMAT_VERSION_HEADER; constexpr uint64_t VersionCountingBlock::VERSION_ZERO; + constexpr uint64_t VersionCountingBlock::VERSION_DELETED; } } diff --git a/src/blockstore/implementations/versioncounting/VersionCountingBlock.h b/src/blockstore/implementations/versioncounting/VersionCountingBlock.h index 3a554c0f..7ebdd8a7 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlock.h +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlock.h @@ -31,7 +31,7 @@ public: static uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize); //TODO Storing key twice (in parent class and in object pointed to). Once would be enough. - VersionCountingBlock(cpputils::unique_ref baseBlock, cpputils::Data dataWithHeader, uint64_t version, KnownBlockVersions *knownBlockVersions); + VersionCountingBlock(cpputils::unique_ref baseBlock, cpputils::Data dataWithHeader, KnownBlockVersions *knownBlockVersions); ~VersionCountingBlock(); const void *data() const override; @@ -55,17 +55,18 @@ 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 _versionIsNondecreasing(uint32_t clientId, const Key &key, uint64_t version, KnownBlockVersions *knownBlockVersions); + static bool _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; - static constexpr uint64_t VERSION_ZERO = 1; // lowest block version is '1', because that is required by class KnownBlockVersions. std::mutex _mutex; DISALLOW_COPY_AND_ASSIGN(VersionCountingBlock); public: + static constexpr uint64_t VERSION_ZERO = 1; // lowest block version is '1', because that is required by class KnownBlockVersions. + static constexpr uint64_t VERSION_DELETED = std::numeric_limits::max(); static constexpr unsigned int CLIENTID_HEADER_OFFSET = sizeof(FORMAT_VERSION_HEADER); static constexpr unsigned int VERSION_HEADER_OFFSET = sizeof(FORMAT_VERSION_HEADER) + sizeof(uint32_t); static constexpr unsigned int HEADER_LENGTH = sizeof(FORMAT_VERSION_HEADER) + sizeof(uint32_t) + sizeof(VERSION_ZERO); @@ -81,7 +82,7 @@ inline boost::optional> VersionCounti } knownBlockVersions->updateVersion(key, VERSION_ZERO); - return cpputils::make_unique_ref(std::move(*baseBlock), std::move(dataWithHeader), VERSION_ZERO, knownBlockVersions); + return cpputils::make_unique_ref(std::move(*baseBlock), std::move(dataWithHeader), knownBlockVersions); } inline cpputils::Data VersionCountingBlock::_prependHeaderToData(uint32_t myClientId, uint64_t version, cpputils::Data data) { @@ -98,14 +99,26 @@ inline boost::optional> VersionCounti cpputils::Data data(baseBlock->size()); std::memcpy(data.data(), baseBlock->data(), data.size()); _checkFormatHeader(data); - uint32_t lastClientId = _readClientId(data); - uint64_t version = _readVersion(data); - if(!_versionIsNondecreasing(lastClientId, baseBlock->key(), version, knownBlockVersions)) { - //The stored key in the block data is incorrect - an attacker might have exchanged the contents with the encrypted data from a different block - cpputils::logging::LOG(cpputils::logging::WARN) << "Decrypting block " << baseBlock->key().ToString() << " failed due to wrong version number. Was the block rolled back by an attacker?"; + if (!_checkVersion(data, baseBlock->key(), knownBlockVersions)) { return boost::none; } - return cpputils::make_unique_ref(std::move(baseBlock), std::move(data), version, knownBlockVersions); + return cpputils::make_unique_ref(std::move(baseBlock), std::move(data), knownBlockVersions); +} + +inline bool 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)) { + if (knownBlockVersions->getBlockVersion(lastClientId, key) == VERSION_DELETED) { + cpputils::logging::LOG(cpputils::logging::WARN) << "Decrypting block " << key.ToString() << + " failed because it was marked as deleted. Was the block reintroduced by an attacker?"; + } else { + cpputils::logging::LOG(cpputils::logging::WARN) << "Decrypting block " << key.ToString() << + " failed due to decreasing version number. Was the block rolled back by an attacker?"; + } + return false; + } + return true; } inline void VersionCountingBlock::_checkFormatHeader(const cpputils::Data &data) { @@ -126,18 +139,17 @@ inline uint64_t VersionCountingBlock::_readVersion(const cpputils::Data &data) { return version; } -inline bool VersionCountingBlock::_versionIsNondecreasing(uint32_t clientId, const Key &key, uint64_t version, KnownBlockVersions *knownBlockVersions) { - return knownBlockVersions->checkAndUpdateVersion(clientId, key, version); -} - -inline VersionCountingBlock::VersionCountingBlock(cpputils::unique_ref baseBlock, cpputils::Data dataWithHeader, uint64_t version, KnownBlockVersions *knownBlockVersions) +inline VersionCountingBlock::VersionCountingBlock(cpputils::unique_ref baseBlock, cpputils::Data dataWithHeader, KnownBlockVersions *knownBlockVersions) :Block(baseBlock->key()), _knownBlockVersions(knownBlockVersions), _baseBlock(std::move(baseBlock)), _dataWithHeader(std::move(dataWithHeader)), - _version(version), + _version(_readVersion(_dataWithHeader)), _dataChanged(false), _mutex() { + if (_version == VERSION_DELETED) { + throw std::runtime_error("Loaded block is marked as deleted. This shouldn't happen because in case of a version number overflow, the block isn't stored at all."); + } } inline VersionCountingBlock::~VersionCountingBlock() { @@ -173,6 +185,11 @@ inline void VersionCountingBlock::resize(size_t newSize) { inline void VersionCountingBlock::_storeToBaseBlock() { if (_dataChanged) { ++_version; + if (_version == VERSION_DELETED) { + // It's *very* unlikely we ever run out of version numbers in 64bit...but just to be sure... + throw std::runtime_error("Version overflow"); + static_assert(VERSION_DELETED == std::numeric_limits::max(), "The check above assumes VERSION_DELETE to be an upper bound."); + } uint32_t myClientId = _knownBlockVersions->myClientId(); std::memcpy(_dataWithHeader.dataOffset(CLIENTID_HEADER_OFFSET), &myClientId, sizeof(myClientId)); std::memcpy(_dataWithHeader.dataOffset(VERSION_HEADER_OFFSET), &_version, sizeof(_version)); diff --git a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h index 317243c4..13018c7b 100644 --- a/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h +++ b/src/blockstore/implementations/versioncounting/VersionCountingBlockStore.h @@ -59,10 +59,12 @@ inline boost::optional> VersionCountingBlockStore::l } inline void VersionCountingBlockStore::remove(cpputils::unique_ref block) { + Key key = block->key(); auto versionCountingBlock = cpputils::dynamic_pointer_move(block); ASSERT(versionCountingBlock != boost::none, "Block is not an VersionCountingBlock"); auto baseBlock = (*versionCountingBlock)->releaseBlock(); - return _baseBlockStore->remove(std::move(baseBlock)); + _baseBlockStore->remove(std::move(baseBlock)); + _knownBlockVersions.updateVersion(key, VersionCountingBlock::VERSION_DELETED); } inline uint64_t VersionCountingBlockStore::numBlocks() const { diff --git a/test/blockstore/implementations/versioncounting/VersionCountingBlockStoreTest_Specific.cpp b/test/blockstore/implementations/versioncounting/VersionCountingBlockStoreTest_Specific.cpp index 911f3596..61a3ff85 100644 --- a/test/blockstore/implementations/versioncounting/VersionCountingBlockStoreTest_Specific.cpp +++ b/test/blockstore/implementations/versioncounting/VersionCountingBlockStoreTest_Specific.cpp @@ -12,6 +12,7 @@ using cpputils::Data; using cpputils::unique_ref; using cpputils::make_unique_ref; using cpputils::TempFile; +using boost::none; using blockstore::testfake::FakeBlockStore; @@ -80,6 +81,14 @@ public: baseBlock->write((char*)&clientId, VersionCountingBlock::CLIENTID_HEADER_OFFSET, sizeof(clientId)); } + void deleteBlock(const blockstore::Key &key) { + blockStore->remove(blockStore->load(key).value()); + } + + void insertBaseBlock(const blockstore::Key &key, Data data) { + EXPECT_NE(none, baseBlockStore->tryCreate(key, std::move(data))); + } + private: DISALLOW_COPY_AND_ASSIGN(VersionCountingBlockStoreTest); }; @@ -127,6 +136,15 @@ TEST_F(VersionCountingBlockStoreTest, RollbackPrevention_DoesntAllowSameVersionN EXPECT_EQ(boost::none, blockStore->load(key)); } +// Test that deleted blocks cannot be re-introduced +TEST_F(VersionCountingBlockStoreTest, RollbackPrevention_DoesntAllowReintroducingDeletedBlocks) { + auto key = CreateBlockReturnKey(); + Data oldBaseBlock = loadBaseBlock(key); + deleteBlock(key); + insertBaseBlock(key, std::move(oldBaseBlock)); + EXPECT_EQ(boost::none, blockStore->load(key)); +} + TEST_F(VersionCountingBlockStoreTest, PhysicalBlockSize_zerophysical) { EXPECT_EQ(0u, blockStore->blockSizeFromPhysicalBlockSize(0)); }