Make blockstore::Key more typesafe

src/blockstore/CMakeLists.txt

@@ -2,6 +2,7 @@ project (blockstore)
 
 set(SOURCES
   utils/Key.cpp
+  utils/IdWrapper.cpp
   utils/BlockStoreUtils.cpp
   utils/FileDoesntExistException.cpp
   interface/helpers/BlockStoreWithRandomKeys.cpp

src/blockstore/implementations/encrypted/EncryptedBlockStore2.h

@@ -157,7 +157,7 @@ inline cpputils::Data EncryptedBlockStore2<Cipher>::_migrateBlock(const cpputils
 
 template<class Cipher>
 inline bool EncryptedBlockStore2<Cipher>::_keyHeaderIsCorrect(const Key &key, const cpputils::Data &data) {
-  return 0 == std::memcmp(key.data(), data.data(), Key::BINARY_LENGTH);
+  return key == Key::FromBinary(data.data());
 }
 #endif
 

src/blockstore/implementations/integrity/IntegrityBlockStore2.cpp

@@ -26,7 +26,7 @@ Data IntegrityBlockStore2::_prependHeaderToData(const Key& key, uint32_t myClien
   static_assert(HEADER_LENGTH == sizeof(FORMAT_VERSION_HEADER) + Key::BINARY_LENGTH + sizeof(myClientId) + sizeof(version), "Wrong header length");
   Data result(data.size() + HEADER_LENGTH);
   std::memcpy(result.dataOffset(0), &FORMAT_VERSION_HEADER, sizeof(FORMAT_VERSION_HEADER));
-  std::memcpy(result.dataOffset(ID_HEADER_OFFSET), key.data(), Key::BINARY_LENGTH);
+  std::memcpy(result.dataOffset(ID_HEADER_OFFSET), key.data().data(), Key::BINARY_LENGTH);
   std::memcpy(result.dataOffset(CLIENTID_HEADER_OFFSET), &myClientId, sizeof(myClientId));
   std::memcpy(result.dataOffset(VERSION_HEADER_OFFSET), &version, sizeof(version));
   std::memcpy((uint8_t*)result.dataOffset(HEADER_LENGTH), data.data(), data.size());
@@ -147,7 +147,7 @@ optional<Data> IntegrityBlockStore2::load(const Key &key) const {
 Data IntegrityBlockStore2::_migrateBlock(const Key &key, const Data &data) {
   Data migrated(data.size() + Key::BINARY_LENGTH);
   std::memcpy(migrated.dataOffset(0), &FORMAT_VERSION_HEADER, sizeof(FORMAT_VERSION_HEADER));
-  std::memcpy(migrated.dataOffset(ID_HEADER_OFFSET), key.data(), Key::BINARY_LENGTH);
+  std::memcpy(migrated.dataOffset(ID_HEADER_OFFSET), key.data().data(), Key::BINARY_LENGTH);
   std::memcpy(migrated.dataOffset(ID_HEADER_OFFSET + Key::BINARY_LENGTH), data.dataOffset(sizeof(FORMAT_VERSION_HEADER)), data.size() - sizeof(FORMAT_VERSION_HEADER));
   ASSERT(migrated.size() == sizeof(FORMAT_VERSION_HEADER) + Key::BINARY_LENGTH + (data.size() - sizeof(FORMAT_VERSION_HEADER)), "Wrong offset computation");
   return migrated;

src/blockstore/implementations/integrity/KnownBlockVersions.cpp

@@ -136,7 +136,7 @@ void KnownBlockVersions::_serializeKnownVersions(Serializer *serializer) const {
 
 pair<ClientIdAndBlockKey, uint64_t> KnownBlockVersions::_deserializeKnownVersionsEntry(Deserializer *deserializer) {
     uint32_t clientId = deserializer->readUint32();
-    Key blockKey = deserializer->readFixedSizeData<Key::BINARY_LENGTH>();
+    Key blockKey(deserializer->readFixedSizeData<Key::BINARY_LENGTH>());
     uint64_t version = deserializer->readUint64();
 
     return {{clientId, blockKey}, version};
@@ -144,7 +144,7 @@ pair<ClientIdAndBlockKey, uint64_t> KnownBlockVersions::_deserializeKnownVersion
 
 void KnownBlockVersions::_serializeKnownVersionsEntry(Serializer *serializer, const pair<ClientIdAndBlockKey, uint64_t> &entry) {
     serializer->writeUint32(entry.first.clientId);
-    serializer->writeFixedSizeData<Key::BINARY_LENGTH>(entry.first.blockKey);
+    serializer->writeFixedSizeData<Key::BINARY_LENGTH>(entry.first.blockKey.data());
     serializer->writeUint64(entry.second);
 }
 
@@ -168,14 +168,14 @@ void KnownBlockVersions::_serializeLastUpdateClientIds(Serializer *serializer) c
 }
 
 pair<Key, uint32_t> KnownBlockVersions::_deserializeLastUpdateClientIdEntry(Deserializer *deserializer) {
-    Key blockKey = deserializer->readFixedSizeData<Key::BINARY_LENGTH>();
+    Key blockKey(deserializer->readFixedSizeData<Key::BINARY_LENGTH>());
     uint32_t clientId = deserializer->readUint32();
 
     return {blockKey, clientId};
 };
 
 void KnownBlockVersions::_serializeLastUpdateClientIdEntry(Serializer *serializer, const pair<Key, uint32_t> &entry) {
-    serializer->writeFixedSizeData<Key::BINARY_LENGTH>(entry.first);
+    serializer->writeFixedSizeData<Key::BINARY_LENGTH>(entry.first.data());
     serializer->writeUint32(entry.second);
 }
 

src/blockstore/implementations/low2highlevel/LowToHighLevelBlockStore.cpp

@@ -19,7 +19,7 @@ LowToHighLevelBlockStore::LowToHighLevelBlockStore(unique_ref<BlockStore2> baseB
 
 Key LowToHighLevelBlockStore::createKey() {
     // TODO Is this the right way?
-    return cpputils::Random::PseudoRandom().getFixedSize<Key::BINARY_LENGTH>();
+    return Key::Random();
 }
 
 optional<unique_ref<Block>> LowToHighLevelBlockStore::tryCreate(const Key &key, Data data) {

src/blockstore/implementations/mock/MockBlockStore.h

@@ -105,7 +105,7 @@ namespace blockstore {
             std::vector<Key> distinctWrittenBlocks() const {
                 std::vector<Key> result(_writtenBlocks);
                 std::sort(result.begin(), result.end(), [](const Key &lhs, const Key &rhs) {
-                    return std::memcmp(lhs.data(), rhs.data(), lhs.BINARY_LENGTH) < 0;
+                    return std::memcmp(lhs.data().data(), rhs.data().data(), lhs.BINARY_LENGTH) < 0;
                 });
                 result.erase(std::unique(result.begin(), result.end() ), result.end());
                 return result;

src/blockstore/interface/BlockStore2.h

@@ -27,7 +27,7 @@ public:
   virtual void store(const Key &key, const cpputils::Data &data) = 0;
 
   Key create(const cpputils::Data& data) {
-    Key key = cpputils::Random::PseudoRandom().getFixedSize<Key::BINARY_LENGTH>();
+    Key key = Key::Random();
     bool success = tryCreate(key, data);
     if (success) {
       return key;

src/blockstore/interface/helpers/BlockStoreWithRandomKeys.h

@@ -11,10 +11,11 @@ namespace blockstore {
 // This is an implementation helpers for BlockStores that use random block keys.
 // You should never give this static type to the client. The client should always
 // work with the BlockStore interface instead.
+// TODO Delete this class
 class BlockStoreWithRandomKeys: public BlockStore {
 public:
   Key createKey() final {
-    return cpputils::Random::PseudoRandom().getFixedSize<Key::BINARY_LENGTH>();
+    return Key::Random();
   }
 };
 

src/blockstore/utils/BlockId.h

@@ -0,0 +1,18 @@
+#pragma once
+#ifndef MESSMER_BLOCKSTORE_UTILS_KEY_H_
+#define MESSMER_BLOCKSTORE_UTILS_KEY_H_
+
+#include "IdWrapper.h"
+
+namespace blockstore {
+
+struct _BlockIdTag final {};
+// A key here is NOT a key for encryption, but a key as used in key->value mappings ("access handle for a block").
+// TODO Rename to BlockId and split from a BlobId (i.e. IdWrapper<BlobIdTag>)
+using BlockId = IdWrapper<_BlockIdTag>;
+
+}
+
+DEFINE_IDWRAPPER(blockstore::BlockId);
+
+#endif

src/blockstore/utils/IdWrapper.cpp

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

src/blockstore/utils/IdWrapper.h

@@ -0,0 +1,109 @@
+#pragma once
+#ifndef MESSMER_BLOCKSTORE_UTILS_IDWRAPPER_H_
+#define MESSMER_BLOCKSTORE_UTILS_IDWRAPPER_H_
+
+#include <string>
+#include <cpp-utils/data/FixedSizeData.h>
+#include <cpp-utils/random/Random.h>
+
+namespace blockstore {
+
+// Tag is used to distinguish different concrete IdWrappers
+template<class Tag>
+class IdWrapper final {
+private:
+  using IdData = cpputils::FixedSizeData<16>;
+
+public:
+  static constexpr size_t BINARY_LENGTH = IdData::BINARY_LENGTH;
+  static constexpr size_t STRING_LENGTH = IdData::STRING_LENGTH;
+
+  explicit IdWrapper(const IdData& id);
+  const IdData& data() const;
+
+  static IdWrapper Random();
+  static IdWrapper Null();
+
+  static IdWrapper FromString(const std::string &data);
+  std::string ToString() const;
+
+  static IdWrapper FromBinary(const void *source);
+  void ToBinary(void *target) const;
+
+private:
+
+  IdData id_;
+  friend class std::hash<IdWrapper>;
+  friend class std::less<IdWrapper>;
+  template<class Tag2> friend bool operator==(const IdWrapper<Tag2>& lhs, const IdWrapper<Tag2>& rhs);
+  template<class Tag2> friend bool operator!=(const IdWrapper<Tag2>& lhs, const IdWrapper<Tag2>& rhs);
+};
+
+template<class Tag>
+inline IdWrapper<Tag>::IdWrapper(const IdData& id): id_(id) {}
+
+template<class Tag>
+inline IdWrapper<Tag> IdWrapper<Tag>::Random() {
+    return IdWrapper(cpputils::Random::PseudoRandom().getFixedSize<BINARY_LENGTH>());
+}
+
+template<class Tag>
+inline IdWrapper<Tag> IdWrapper<Tag>::Null() {
+    return IdWrapper(IdData::Null());
+}
+
+template<class Tag>
+inline IdWrapper<Tag> IdWrapper<Tag>::FromString(const std::string &data) {
+    return IdWrapper(IdData::FromString(data));
+}
+
+template<class Tag>
+inline std::string IdWrapper<Tag>::ToString() const {
+    return id_.ToString();
+}
+
+template<class Tag>
+inline IdWrapper<Tag> IdWrapper<Tag>::FromBinary(const void *source) {
+    return IdWrapper(IdData::FromBinary(source));
+}
+
+template<class Tag>
+inline void IdWrapper<Tag>::ToBinary(void *target) const {
+    id_.ToBinary(target);
+}
+
+template<class Tag>
+inline const typename IdWrapper<Tag>::IdData& IdWrapper<Tag>::data() const {
+    return id_;
+}
+
+template<class Tag>
+inline bool operator==(const IdWrapper<Tag>& lhs, const IdWrapper<Tag>& rhs) {
+  return lhs.id_ == rhs.id_;
+}
+
+template<class Tag>
+inline bool operator!=(const IdWrapper<Tag>& lhs, const IdWrapper<Tag>& rhs) {
+  return !operator==(lhs, rhs);
+}
+
+}
+
+#define DEFINE_IDWRAPPER(IdWrapper)                                                                                    \
+  namespace std {                                                                                                      \
+    /*Allow using IdWrapper in std::unordered_map / std::unordered_set */                                              \
+    template <> struct hash<IdWrapper> {                                                                               \
+      size_t operator()(const IdWrapper &idWrapper) const {                                                            \
+        /*Ids are random, so it is enough to use the first few bytes as a hash */                                      \
+        return *(size_t*)(idWrapper.id_.data());                                                                       \
+      }                                                                                                                \
+    };                                                                                                                 \
+    /*Allow using IdWrapper in std::map / std::set */                                                                  \
+    template <> struct less<IdWrapper> {                                                                               \
+      bool operator()(const IdWrapper &lhs, const IdWrapper &rhs) const {                                              \
+        return 0 > std::memcmp(lhs.id_.data(), rhs.id_.data(), IdWrapper::BINARY_LENGTH);                              \
+      }                                                                                                                \
+    };                                                                                                                 \
+  }                                                                                                                    \
+
+#endif

src/blockstore/utils/Key.h

@@ -1,32 +0,0 @@
-#pragma once
-#ifndef MESSMER_BLOCKSTORE_UTILS_KEY_H_
-#define MESSMER_BLOCKSTORE_UTILS_KEY_H_
-
-#include <string>
-#include <cpp-utils/data/FixedSizeData.h>
-
-namespace blockstore {
-
-  // A key here is NOT a key for encryption, but a key as used in key->value mappings ("access handle for a block").
-  //TODO Rename to BlockId/BlobId and make it a class containing a FixedSizeData<> member
-  using Key = cpputils::FixedSizeData<16>;
-}
-
-namespace std {
-  //Allow using blockstore::Key in std::unordered_map / std::unordered_set
-  template <> struct hash<blockstore::Key> {
-    size_t operator()(const blockstore::Key &key) const {
-      //Keys are random, so it is enough to use the first few bytes as a hash
-      return *(size_t*)(key.data());
-    }
-  };
-
-  //Allow using blockstore::Key in std::map / std::set
-  template <> struct less<blockstore::Key> {
-    bool operator()(const blockstore::Key &lhs, const blockstore::Key &rhs) const {
-      return 0 > std::memcmp(lhs.data(), rhs.data(), blockstore::Key::BINARY_LENGTH);
-    }
-  };
-}
-
-#endif

src/cryfs/filesystem/fsblobstore/FsBlobView.cpp

@@ -16,7 +16,7 @@ namespace cryfs {
         blob->resize(blob->size() + blockstore::Key::BINARY_LENGTH);
         blob->write(data.dataOffset(OLD_HEADER_SIZE), HEADER_SIZE, data.size() - OLD_HEADER_SIZE);
         // Write parent pointer
-        blob->write(parentKey.data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
+        blob->write(parentKey.data().data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
         // Update format version number
         blob->write(&FORMAT_VERSION_HEADER, 0, sizeof(FORMAT_VERSION_HEADER));
     }

src/cryfs/filesystem/fsblobstore/FsBlobView.h

@@ -27,7 +27,7 @@ namespace cryfs {
             baseBlob->write(&FORMAT_VERSION_HEADER, 0, sizeof(FORMAT_VERSION_HEADER));
             uint8_t blobTypeInt = static_cast<uint8_t>(blobType);
             baseBlob->write(&blobTypeInt, sizeof(FORMAT_VERSION_HEADER), sizeof(uint8_t));
-            baseBlob->write(parent.data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
+            baseBlob->write(parent.data().data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
             static_assert(HEADER_SIZE == sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t) + blockstore::Key::BINARY_LENGTH, "If this fails, the header is not initialized correctly in this function.");
         }
 
@@ -118,11 +118,13 @@ namespace cryfs {
         }
 
         void _loadParentPointer() {
-            _baseBlob->read(_parentPointer.data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
+            auto idData = cpputils::FixedSizeData<blockstore::Key::BINARY_LENGTH>::Null();
+            _baseBlob->read(idData.data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
+            _parentPointer = blockstore::Key(idData);
         }
 
         void _storeParentPointer() {
-            _baseBlob->write(_parentPointer.data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
+            _baseBlob->write(_parentPointer.data().data(), sizeof(FORMAT_VERSION_HEADER) + sizeof(uint8_t), blockstore::Key::BINARY_LENGTH);
         }
 
 

src/cryfs/filesystem/fsblobstore/utils/DirEntryList.cpp

@@ -174,7 +174,7 @@ vector<DirEntry>::iterator DirEntryList::_findFirst(const Key &hint, std::functi
     if (_entries.size() == 0) {
         return _entries.end();
     }
-    double startpos_percent = static_cast<double>(*static_cast<const unsigned char*>(hint.data())) / std::numeric_limits<unsigned char>::max();
+    double startpos_percent = static_cast<double>(*static_cast<const unsigned char*>(hint.data().data())) / std::numeric_limits<unsigned char>::max();
     auto iter = _entries.begin() + static_cast<int>(startpos_percent * (_entries.size()-1));
     ASSERT(iter >= _entries.begin() && iter < _entries.end(), "Startpos out of range");
     while(iter != _entries.begin() && pred(*iter)) {

test/blockstore/interface/helpers/BlockStoreWithRandomKeysTest.cpp

@@ -42,7 +42,7 @@ public:
 
 class BlockMock: public Block {
 public:
-  BlockMock(): Block(cpputils::Random::PseudoRandom().getFixedSize<Key::BINARY_LENGTH>()) {}
+  BlockMock(): Block(Key::Random()) {}
   MOCK_CONST_METHOD0(data, const void*());
   MOCK_METHOD3(write, void(const void*, uint64_t, uint64_t));
   MOCK_METHOD0(flush, void());

test/cryfs/config/CryCipherTest.cpp

@@ -43,7 +43,7 @@ public:
 
     template<class ExpectedCipher>
     void _EXPECT_ENCRYPTS_WITH_ACTUAL_BLOCKSTORE_DECRYPTS_CORRECTLY_WITH_EXPECTED_BLOCKSTORE(const CryCipher &actualCipher, const std::string &encKey, Data dataFixture) {
-        blockstore::Key key = cpputils::Random::PseudoRandom().getFixedSize<blockstore::Key::BINARY_LENGTH>();
+        blockstore::Key key = blockstore::Key::Random();
         Data encrypted = _encryptUsingEncryptedBlockStoreWithCipher(actualCipher, encKey, key, dataFixture.copy());
         Data decrypted = _decryptUsingEncryptedBlockStoreWithCipher<ExpectedCipher>(encKey, key, std::move(encrypted));
         EXPECT_EQ(dataFixture, decrypted);