Introduce version flags for file system entities to allow future CryFS versions to be backwards-compatible even if the format changes.

src/blobstore/implementations/onblocks/datanodestore/DataInnerNode.cpp

@@ -15,6 +15,9 @@ namespace datanodestore {
 DataInnerNode::DataInnerNode(DataNodeView view)
 : DataNode(std::move(view)) {
   ASSERT(depth() > 0, "Inner node can't have depth 0. Is this a leaf maybe?");
+  if (node().FormatVersion() != FORMAT_VERSION_HEADER) {
+    throw std::runtime_error("This node format is not supported. Was it created with a newer version of CryFS?");
+  }
 }
 
 DataInnerNode::~DataInnerNode() {
@@ -22,6 +25,7 @@ DataInnerNode::~DataInnerNode() {
 
 unique_ref<DataInnerNode> DataInnerNode::InitializeNewNode(unique_ref<Block> block, const DataNode &first_child) {
   DataNodeView node(std::move(block));
+  node.setFormatVersion(DataNode::FORMAT_VERSION_HEADER);
   node.setDepth(first_child.depth() + 1);
   node.setSize(1);
   auto result = make_unique_ref<DataInnerNode>(std::move(node));

src/blobstore/implementations/onblocks/datanodestore/DataLeafNode.cpp

@@ -16,6 +16,9 @@ DataLeafNode::DataLeafNode(DataNodeView view)
 : DataNode(std::move(view)) {
   ASSERT(node().Depth() == 0, "Leaf node must have depth 0. Is it an inner node instead?");
   ASSERT(numBytes() <= maxStoreableBytes(), "Leaf says it stores more bytes than it has space for");
+  if (node().FormatVersion() != FORMAT_VERSION_HEADER) {
+    throw std::runtime_error("This node format is not supported. Was it created with a newer version of CryFS?");
+  }
 }
 
 DataLeafNode::~DataLeafNode() {
@@ -23,6 +26,7 @@ DataLeafNode::~DataLeafNode() {
 
 unique_ref<DataLeafNode> DataLeafNode::InitializeNewNode(unique_ref<Block> block) {
   DataNodeView node(std::move(block));
+  node.setFormatVersion(DataNode::FORMAT_VERSION_HEADER);
   node.setDepth(0);
   node.setSize(0);
   //fillDataWithZeroes(); not needed, because a newly created block will be zeroed out. DataLeafNodeTest.SpaceIsZeroFilledWhenGrowing ensures this.

src/blobstore/implementations/onblocks/datanodestore/DataNode.cpp

@@ -14,6 +14,8 @@ namespace blobstore {
 namespace onblocks {
 namespace datanodestore {
 
+constexpr uint16_t DataNode::FORMAT_VERSION_HEADER;
+
 DataNode::DataNode(DataNodeView node)
 : _node(std::move(node)) {
 }

src/blobstore/implementations/onblocks/datanodestore/DataNode.h

@@ -24,6 +24,9 @@ public:
   void flush() const;
 
 protected:
+  // The FORMAT_VERSION_HEADER is used to allow future versions to have compatibility.
+  static constexpr uint16_t FORMAT_VERSION_HEADER = 0;
+
   DataNode(DataNodeView block);
 
   DataNodeView &node();

src/blobstore/implementations/onblocks/datanodestore/DataNodeView.h

@@ -28,10 +28,12 @@ public:
 
   //Total size of the header
   static constexpr uint32_t HEADERSIZE_BYTES = 8;
+  //Where in the header is the format version field (used to allow compatibility with future versions of CryFS)
+  static constexpr uint32_t FORMAT_VERSION_OFFSET_BYTES = 0; //format version uses 2 bytes
   //Where in the header is the depth field
-  static constexpr uint32_t DEPTH_OFFSET_BYTES = 0;
+  static constexpr uint32_t DEPTH_OFFSET_BYTES = 3; // depth uses 1 byte
   //Where in the header is the size field (for inner nodes: number of children, for leafs: content data size)
-  static constexpr uint32_t SIZE_OFFSET_BYTES = 4;
+  static constexpr uint32_t SIZE_OFFSET_BYTES = 4; // size uses 4 bytes
 
 
   //Size of a block (header + data region)
@@ -66,6 +68,14 @@ public:
 
   DataNodeView(DataNodeView &&rhs) = default;
 
+  uint16_t FormatVersion() const {
+    return *((uint8_t*)_block->data()+DataNodeLayout::FORMAT_VERSION_OFFSET_BYTES);
+  }
+
+  void setFormatVersion(uint16_t value) {
+    _block->write(&value, DataNodeLayout::FORMAT_VERSION_OFFSET_BYTES, sizeof(value));
+  }
+
   uint8_t Depth() const {
     return *((uint8_t*)_block->data()+DataNodeLayout::DEPTH_OFFSET_BYTES);
   }

src/blockstore/implementations/encrypted/EncryptedBlock.h

@@ -55,6 +55,11 @@ private:
   void _encryptToBaseBlock();
   static cpputils::Data _prependKeyHeaderToData(const Key &key, cpputils::Data data);
   static bool _keyHeaderIsCorrect(const Key &key, const cpputils::Data &data);
+  static cpputils::Data _prependFormatHeader(const cpputils::Data &data);
+  static void _checkFormatHeader(const void *data);
+
+  // This header is prepended to blocks to allow future versions to have compatibility.
+  static constexpr uint16_t FORMAT_VERSION_HEADER = 0;
 
   std::mutex _mutex;
 
@@ -64,12 +69,16 @@ private:
 template<class Cipher>
 constexpr unsigned int EncryptedBlock<Cipher>::HEADER_LENGTH;
 
+template<class Cipher>
+constexpr uint16_t EncryptedBlock<Cipher>::FORMAT_VERSION_HEADER;
+
 
 template<class Cipher>
 boost::optional<cpputils::unique_ref<EncryptedBlock<Cipher>>> EncryptedBlock<Cipher>::TryCreateNew(BlockStore *baseBlockStore, const Key &key, cpputils::Data data, const typename Cipher::EncryptionKey &encKey) {
   cpputils::Data plaintextWithHeader = _prependKeyHeaderToData(key, std::move(data));
   cpputils::Data encrypted = Cipher::encrypt((byte*)plaintextWithHeader.data(), plaintextWithHeader.size(), encKey);
-  auto baseBlock = baseBlockStore->tryCreate(key, std::move(encrypted));
+  cpputils::Data encryptedWithFormatHeader = _prependFormatHeader(std::move(encrypted));
+  auto baseBlock = baseBlockStore->tryCreate(key, std::move(encryptedWithFormatHeader));
   if (baseBlock == boost::none) {
     //TODO Test this code branch
     return boost::none;
@@ -78,10 +87,18 @@ boost::optional<cpputils::unique_ref<EncryptedBlock<Cipher>>> EncryptedBlock<Cip
   return cpputils::make_unique_ref<EncryptedBlock>(std::move(*baseBlock), encKey, std::move(plaintextWithHeader));
 }
 
+template<class Cipher>
+cpputils::Data EncryptedBlock<Cipher>::_prependFormatHeader(const cpputils::Data &data) {
+  cpputils::Data dataWithHeader(sizeof(FORMAT_VERSION_HEADER) + data.size());
+  std::memcpy(dataWithHeader.dataOffset(0), &FORMAT_VERSION_HEADER, sizeof(FORMAT_VERSION_HEADER));
+  std::memcpy(dataWithHeader.dataOffset(sizeof(FORMAT_VERSION_HEADER)), data.data(), data.size());
+  return dataWithHeader;
+}
+
 template<class Cipher>
 boost::optional<cpputils::unique_ref<EncryptedBlock<Cipher>>> EncryptedBlock<Cipher>::TryDecrypt(cpputils::unique_ref<Block> baseBlock, const typename Cipher::EncryptionKey &encKey) {
-  //TODO Change BlockStore so we can read their "class Data" objects instead of "void *data()", and then we can change the Cipher interface to take Data objects instead of "byte *" + size
+  _checkFormatHeader(baseBlock->data());
-  boost::optional<cpputils::Data> plaintextWithHeader = Cipher::decrypt((byte*)baseBlock->data(), baseBlock->size(), encKey);
+  boost::optional<cpputils::Data> plaintextWithHeader = Cipher::decrypt((byte*)baseBlock->data() + sizeof(FORMAT_VERSION_HEADER), baseBlock->size() - sizeof(FORMAT_VERSION_HEADER), encKey);
   if(plaintextWithHeader == boost::none) {
     //Decryption failed (e.g. an authenticated cipher detected modifications to the ciphertext)
     cpputils::logging::LOG(cpputils::logging::WARN) << "Decrypting block " << baseBlock->key().ToString() << " failed. Was the block modified by an attacker?";
@@ -95,6 +112,13 @@ boost::optional<cpputils::unique_ref<EncryptedBlock<Cipher>>> EncryptedBlock<Cip
   return cpputils::make_unique_ref<EncryptedBlock<Cipher>>(std::move(baseBlock), encKey, std::move(*plaintextWithHeader));
 }
 
+template<class Cipher>
+void EncryptedBlock<Cipher>::_checkFormatHeader(const void *data) {
+  if (*reinterpret_cast<decltype(FORMAT_VERSION_HEADER)*>(data) != FORMAT_VERSION_HEADER) {
+    throw std::runtime_error("The encrypted block has the wrong format. Was it created with a newer version of CryFS?");
+  }
+}
+
 template<class Cipher>
 cpputils::Data EncryptedBlock<Cipher>::_prependKeyHeaderToData(const Key &key, cpputils::Data data) {
   static_assert(HEADER_LENGTH >= Key::BINARY_LENGTH, "Key doesn't fit into the header");
@@ -159,7 +183,8 @@ template<class Cipher>
 void EncryptedBlock<Cipher>::_encryptToBaseBlock() {
   if (_dataChanged) {
     cpputils::Data encrypted = Cipher::encrypt((byte*)_plaintextWithHeader.data(), _plaintextWithHeader.size(), _encKey);
-    _baseBlock->write(encrypted.data(), 0, encrypted.size());
+    _baseBlock->write(&FORMAT_VERSION_HEADER, 0, sizeof(FORMAT_VERSION_HEADER));
+    _baseBlock->write(encrypted.data(), sizeof(FORMAT_VERSION_HEADER), encrypted.size());
     _dataChanged = false;
   }
 }

src/blockstore/implementations/ondisk/OnDiskBlock.cpp

@@ -12,6 +12,7 @@ using std::ostream;
 using std::ifstream;
 using std::ofstream;
 using std::ios;
+using std::string;
 using cpputils::Data;
 using cpputils::make_unique_ref;
 using cpputils::unique_ref;
@@ -23,6 +24,9 @@ namespace bf = boost::filesystem;
 namespace blockstore {
 namespace ondisk {
 
+const string OnDiskBlock::FORMAT_VERSION_HEADER_PREFIX = "cryfs;block;";
+const string OnDiskBlock::FORMAT_VERSION_HEADER = OnDiskBlock::FORMAT_VERSION_HEADER_PREFIX + "0";
+
 OnDiskBlock::OnDiskBlock(const Key &key, const bf::path &filepath, Data data)
  : Block(key), _filepath(filepath), _data(std::move(data)), _dataChanged(false), _mutex() {
 }
@@ -37,7 +41,7 @@ const void *OnDiskBlock::data() const {
 
 void OnDiskBlock::write(const void *source, uint64_t offset, uint64_t size) {
   ASSERT(offset <= _data.size() && offset + size <= _data.size(), "Write outside of valid area"); //Also check offset < _data->size() because of possible overflow in the addition
-  std::memcpy((uint8_t*)_data.data()+offset, source, size);
+  std::memcpy(_data.dataOffset(offset), source, size);
   _dataChanged = true;
 }
 
@@ -53,15 +57,8 @@ void OnDiskBlock::resize(size_t newSize) {
 optional<unique_ref<OnDiskBlock>> OnDiskBlock::LoadFromDisk(const bf::path &rootdir, const Key &key) {
   auto filepath = rootdir / key.ToString();
   try {
-    //If it isn't a file, Data::LoadFromFile() would usually also crash. We still need this extra check
+    boost::optional<Data> data = _loadFromDisk(filepath);
-    //upfront, because Data::LoadFromFile() doesn't crash if we give it the path of a directory
+    if (data == none) {
-    //instead the path of a file.
-    //TODO Data::LoadFromFile now returns boost::optional. Do we then still need this?
-    if(!bf::is_regular_file(filepath)) {
-      return none;
-    }
-    boost::optional<Data> data = Data::LoadFromFile(filepath);
-    if (!data) {
       return none;
     }
     return make_unique_ref<OnDiskBlock>(key, filepath, std::move(*data));
@@ -87,13 +84,61 @@ void OnDiskBlock::RemoveFromDisk(const bf::path &rootdir, const Key &key) {
   bf::remove(filepath);
 }
 
-void OnDiskBlock::_fillDataWithZeroes() {
+void OnDiskBlock::_storeToDisk() const {
-  _data.FillWithZeroes();
+  std::ofstream file(_filepath.c_str(), std::ios::binary | std::ios::trunc);
-  _dataChanged = true;
+  if (!file.good()) {
+    throw std::runtime_error("Could not open file for writing");
+  }
+  file.write(FORMAT_VERSION_HEADER.c_str(), formatVersionHeaderSize());
+  if (!file.good()) {
+    throw std::runtime_error("Error writing block header");
+  }
+  _data.StoreToStream(file);
+  if (!file.good()) {
+    throw std::runtime_error("Error writing block data");
+  }
 }
 
-void OnDiskBlock::_storeToDisk() const {
+optional<Data> OnDiskBlock::_loadFromDisk(const bf::path &filepath) {
-  _data.StoreToFile(_filepath);
+  //If it isn't a file, ifstream::good() would return false. We still need this extra check
+  //upfront, because ifstream::good() doesn't crash if we give it the path of a directory
+  //instead the path of a file.
+  if(!bf::is_regular_file(filepath)) {
+    return none;
+  }
+  ifstream file(filepath.c_str(), ios::binary);
+  if (!file.good()) {
+    return none;
+  }
+  _checkHeader(&file);
+  Data result = Data::LoadFromStream(file);
+  return result;
+}
+
+void OnDiskBlock::_checkHeader(istream *str) {
+  Data header(formatVersionHeaderSize());
+  str->read(reinterpret_cast<char*>(header.data()), formatVersionHeaderSize());
+  if (!_isAcceptedCryfsHeader(header)) {
+    if (_isOtherCryfsHeader(header)) {
+      throw std::runtime_error("This block is not supported yet. Maybe it was created with a newer version of CryFS?");
+    } else {
+      throw std::runtime_error("This is not a valid block.");
+    }
+  }
+}
+
+bool OnDiskBlock::_isAcceptedCryfsHeader(const Data &data) {
+  ASSERT(data.size() == formatVersionHeaderSize(), "We extracted the wrong header size from the block.");
+  return 0 == std::memcmp(data.data(), FORMAT_VERSION_HEADER.c_str(), formatVersionHeaderSize());
+}
+
+bool OnDiskBlock::_isOtherCryfsHeader(const Data &data) {
+  ASSERT(data.size() >= FORMAT_VERSION_HEADER_PREFIX.size(), "We extracted the wrong header size from the block.");
+  return 0 == std::memcmp(data.data(), FORMAT_VERSION_HEADER_PREFIX.c_str(), FORMAT_VERSION_HEADER_PREFIX.size());
+}
+
+unsigned int OnDiskBlock::formatVersionHeaderSize() {
+  return FORMAT_VERSION_HEADER.size() + 1; // +1 because of the null byte
 }
 
 void OnDiskBlock::flush() {

src/blockstore/implementations/ondisk/OnDiskBlock.h

@@ -19,6 +19,10 @@ public:
   OnDiskBlock(const Key &key, const boost::filesystem::path &filepath, cpputils::Data data);
   ~OnDiskBlock();
 
+  static const std::string FORMAT_VERSION_HEADER_PREFIX;
+  static const std::string FORMAT_VERSION_HEADER;
+  static unsigned int formatVersionHeaderSize();
+
   static boost::optional<cpputils::unique_ref<OnDiskBlock>> LoadFromDisk(const boost::filesystem::path &rootdir, const Key &key);
   static boost::optional<cpputils::unique_ref<OnDiskBlock>> CreateOnDisk(const boost::filesystem::path &rootdir, const Key &key, cpputils::Data data);
   static void RemoveFromDisk(const boost::filesystem::path &rootdir, const Key &key);
@@ -32,11 +36,16 @@ public:
   void resize(size_t newSize) override;
 
 private:
+
+  static bool _isAcceptedCryfsHeader(const cpputils::Data &data);
+  static bool _isOtherCryfsHeader(const cpputils::Data &data);
+  static void _checkHeader(std::istream *str);
+
   const boost::filesystem::path _filepath;
   cpputils::Data _data;
   bool _dataChanged;
 
-  void _fillDataWithZeroes();
+  static boost::optional<cpputils::Data> _loadFromDisk(const boost::filesystem::path &filepath);
   void _storeToDisk() const;
 
   std::mutex _mutex;

src/cryfs/CMakeLists.txt

@@ -26,6 +26,7 @@ set(SOURCES
     filesystem/fsblobstore/utils/DirEntry.cpp
     filesystem/fsblobstore/utils/DirEntryList.cpp
     filesystem/fsblobstore/FsBlobStore.cpp
+    filesystem/fsblobstore/FsBlobView.cpp
     filesystem/fsblobstore/FileBlob.cpp
     filesystem/fsblobstore/FsBlob.cpp
     filesystem/fsblobstore/SymlinkBlob.cpp

src/cryfs/filesystem/fsblobstore/DirBlob.cpp

@@ -6,7 +6,6 @@
 
 #include <blobstore/implementations/onblocks/utils/Math.h>
 #include <cpp-utils/data/Data.h>
-#include "MagicNumbers.h"
 #include "../CryDevice.h"
 #include "FileBlob.h"
 #include "SymlinkBlob.h"
@@ -30,7 +29,7 @@ constexpr off_t DirBlob::DIR_LSTAT_SIZE;
 
 DirBlob::DirBlob(unique_ref<Blob> blob, std::function<off_t (const blockstore::Key&)> getLstatSize) :
     FsBlob(std::move(blob)), _getLstatSize(getLstatSize), _entries(), _mutex(), _changed(false) {
-  ASSERT(magicNumber() == MagicNumbers::DIR, "Loaded blob is not a directory");
+  ASSERT(baseBlob().blobType() == FsBlobView::BlobType::DIR, "Loaded blob is not a directory");
   _readEntriesFromBlob();
 }
 
@@ -46,15 +45,15 @@ void DirBlob::flush() {
 }
 
 unique_ref<DirBlob> DirBlob::InitializeEmptyDir(unique_ref<Blob> blob, std::function<off_t(const blockstore::Key&)> getLstatSize) {
-  InitializeBlobWithMagicNumber(blob.get(), MagicNumbers::DIR);
+  InitializeBlob(blob.get(), FsBlobView::BlobType::DIR);
   return make_unique_ref<DirBlob>(std::move(blob), getLstatSize);
 }
 
 void DirBlob::_writeEntriesToBlob() {
   if (_changed) {
     Data serialized = _entries.serialize();
-    baseBlob().resize(1 + serialized.size());
+    baseBlob().resize(serialized.size());
-    baseBlob().write(serialized.data(), 1, serialized.size());
+    baseBlob().write(serialized.data(), 0, serialized.size());
     _changed = false;
   }
 }
@@ -62,7 +61,7 @@ void DirBlob::_writeEntriesToBlob() {
 void DirBlob::_readEntriesFromBlob() {
   //No lock needed, because this is only called from the constructor.
   Data data = baseBlob().readAll();
-  _entries.deserializeFrom(static_cast<uint8_t*>(data.data()) + 1, data.size() - 1);  // data+1/size-1 because the first byte is the magic number
+  _entries.deserializeFrom(static_cast<uint8_t*>(data.data()), data.size());
 }
 
 void DirBlob::AddChildDir(const std::string &name, const Key &blobKey, mode_t mode, uid_t uid, gid_t gid) {

src/cryfs/filesystem/fsblobstore/FileBlob.cpp

@@ -1,6 +1,5 @@
 #include "FileBlob.h"
 
-#include "MagicNumbers.h"
 #include <blockstore/utils/Key.h>
 #include <cassert>
 
@@ -14,19 +13,20 @@ namespace fsblobstore {
 
 FileBlob::FileBlob(unique_ref<Blob> blob)
 : FsBlob(std::move(blob)) {
+  ASSERT(baseBlob().blobType() == FsBlobView::BlobType::FILE, "Loaded blob is not a file");
 }
 
 unique_ref<FileBlob> FileBlob::InitializeEmptyFile(unique_ref<Blob> blob) {
-  InitializeBlobWithMagicNumber(blob.get(), MagicNumbers::FILE);
+  InitializeBlob(blob.get(), FsBlobView::BlobType::FILE);
   return make_unique_ref<FileBlob>(std::move(blob));
 }
 
 ssize_t FileBlob::read(void *target, uint64_t offset, uint64_t count) const {
-  return baseBlob().tryRead(target, offset + 1, count);
+  return baseBlob().tryRead(target, offset, count);
 }
 
 void FileBlob::write(const void *source, uint64_t offset, uint64_t count) {
-  baseBlob().write(source, offset + 1, count);
+  baseBlob().write(source, offset, count);
 }
 
 void FileBlob::flush() {
@@ -34,7 +34,7 @@ void FileBlob::flush() {
 }
 
 void FileBlob::resize(off_t size) {
-  baseBlob().resize(size+1);
+  baseBlob().resize(size);
 }
 
 off_t FileBlob::lstat_size() const {
@@ -42,7 +42,7 @@ off_t FileBlob::lstat_size() const {
 }
 
 off_t FileBlob::size() const {
-  return baseBlob().size()-1;
+  return baseBlob().size();
 }
 
 }

src/cryfs/filesystem/fsblobstore/FsBlob.h

@@ -4,6 +4,7 @@
 
 #include <cpp-utils/pointer/unique_ref.h>
 #include <blobstore/interface/Blob.h>
+#include "FsBlobView.h"
 
 namespace cryfs {
     namespace fsblobstore {
@@ -17,20 +18,17 @@ namespace cryfs {
         protected:
             FsBlob(cpputils::unique_ref<blobstore::Blob> baseBlob);
 
-            blobstore::Blob &baseBlob();
+            FsBlobView &baseBlob();
-            const blobstore::Blob &baseBlob() const;
+            const FsBlobView &baseBlob() const;
 
-            unsigned char magicNumber() const;
+            static void InitializeBlob(blobstore::Blob *blob, FsBlobView::BlobType magicNumber);
-            static unsigned char magicNumber(const blobstore::Blob &blob);
-
-            static void InitializeBlobWithMagicNumber(blobstore::Blob *blob, unsigned char magicNumber);
 
             friend class FsBlobStore;
             virtual cpputils::unique_ref<blobstore::Blob> releaseBaseBlob();
 
         private:
 
-            cpputils::unique_ref<blobstore::Blob> _baseBlob;
+            FsBlobView _baseBlob;
 
             DISALLOW_COPY_AND_ASSIGN(FsBlob);
         };
@@ -48,34 +46,23 @@ namespace cryfs {
         }
 
         inline const blockstore::Key &FsBlob::key() const {
-            return _baseBlob->key();
+            return _baseBlob.key();
         }
 
-        inline const blobstore::Blob &FsBlob::baseBlob() const {
+        inline const FsBlobView &FsBlob::baseBlob() const {
-            return *_baseBlob;
+            return _baseBlob;
         }
 
-        inline blobstore::Blob &FsBlob::baseBlob() {
+        inline FsBlobView &FsBlob::baseBlob() {
-            return *_baseBlob;
+            return _baseBlob;
         }
 
-        inline unsigned char FsBlob::magicNumber(const blobstore::Blob &blob) {
+        inline void FsBlob::InitializeBlob(blobstore::Blob *blob, FsBlobView::BlobType magicNumber) {
-            unsigned char value;
+            FsBlobView::InitializeBlob(blob, magicNumber);
-            blob.read(&value, 0, 1);
-            return value;
-        }
-
-        inline unsigned char FsBlob::magicNumber() const {
-            return magicNumber(*_baseBlob);
-        }
-
-        inline void FsBlob::InitializeBlobWithMagicNumber(blobstore::Blob *blob, unsigned char magicNumber) {
-            blob->resize(1);
-            blob->write(&magicNumber, 0, 1);
         }
 
         inline cpputils::unique_ref<blobstore::Blob> FsBlob::releaseBaseBlob() {
-            return std::move(_baseBlob);
+            return _baseBlob.releaseBaseBlob();
         }
     }
 }

src/cryfs/filesystem/fsblobstore/FsBlobStore.cpp

@@ -2,7 +2,6 @@
 #include "FileBlob.h"
 #include "DirBlob.h"
 #include "SymlinkBlob.h"
-#include "MagicNumbers.h"
 
 namespace bf = boost::filesystem;
 using cpputils::unique_ref;
@@ -38,12 +37,12 @@ boost::optional<unique_ref<FsBlob>> FsBlobStore::load(const blockstore::Key &key
     if (blob == none) {
         return none;
     }
-    unsigned char magicNumber = FsBlob::magicNumber(**blob);
+    FsBlobView::BlobType blobType = FsBlobView::blobType(**blob);
-    if (magicNumber == MagicNumbers::FILE) {
+    if (blobType == FsBlobView::BlobType::FILE) {
         return unique_ref<FsBlob>(make_unique_ref<FileBlob>(std::move(*blob)));
-    } else if (magicNumber == MagicNumbers::DIR) {
+    } else if (blobType == FsBlobView::BlobType::DIR) {
         return unique_ref<FsBlob>(make_unique_ref<DirBlob>(std::move(*blob), _getLstatSize()));
-    } else if (magicNumber == MagicNumbers::SYMLINK) {
+    } else if (blobType == FsBlobView::BlobType::SYMLINK) {
         return unique_ref<FsBlob>(make_unique_ref<SymlinkBlob>(std::move(*blob)));
     } else {
         ASSERT(false, "Unknown magic number");

src/cryfs/filesystem/fsblobstore/FsBlobView.cpp

@@ -0,0 +1,5 @@
+#include "FsBlobView.h"
+
+namespace cryfs {
+    constexpr uint16_t FsBlobView::FORMAT_VERSION_HEADER;
+}

src/cryfs/filesystem/fsblobstore/FsBlobView.h

@@ -0,0 +1,105 @@
+#pragma once
+#ifndef MESSMER_CRYFS_FILESYSTEM_FSBLOBSTORE_FSBLOBVIEW_H
+#define MESSMER_CRYFS_FILESYSTEM_FSBLOBSTORE_FSBLOBVIEW_H
+
+#include <blobstore/interface/Blob.h>
+#include <cpp-utils/pointer/unique_ref.h>
+
+namespace cryfs {
+
+    //TODO Test
+    class FsBlobView final : public blobstore::Blob {
+    public:
+        //TODO Rename to "Type" or similar
+        enum class BlobType : uint8_t {
+            DIR = 0x00,
+            FILE = 0x01,
+            SYMLINK = 0x02
+        };
+
+        FsBlobView(cpputils::unique_ref<blobstore::Blob> baseBlob): _baseBlob(std::move(baseBlob)) {
+            _checkHeader(*_baseBlob);
+        }
+
+        static void InitializeBlob(blobstore::Blob *baseBlob, BlobType blobType) {
+            baseBlob->resize(sizeof(FORMAT_VERSION_HEADER) + 1);
+            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), 1);
+        }
+
+        static BlobType blobType(const blobstore::Blob &blob) {
+            _checkHeader(blob);
+            return _blobType(blob);
+        }
+
+        BlobType blobType() const {
+            return _blobType(*_baseBlob);
+        }
+
+        const blockstore::Key &key() const override {
+            return _baseBlob->key();
+        }
+
+        uint64_t size() const override {
+            return _baseBlob->size() - sizeof(FORMAT_VERSION_HEADER) - 1;
+        }
+
+        void resize(uint64_t numBytes) override {
+            return _baseBlob->resize(numBytes + sizeof(FORMAT_VERSION_HEADER) + 1);
+        }
+
+        cpputils::Data readAll() const override {
+            cpputils::Data data = _baseBlob->readAll();
+            cpputils::Data dataWithoutHeader(data.size() - sizeof(FORMAT_VERSION_HEADER) - 1);
+            std::memcpy(dataWithoutHeader.data(), data.dataOffset(sizeof(FORMAT_VERSION_HEADER) + 1), dataWithoutHeader.size());
+            return dataWithoutHeader;
+        }
+
+        void read(void *target, uint64_t offset, uint64_t size) const override {
+            return _baseBlob->read(target, offset + sizeof(FORMAT_VERSION_HEADER) + 1, size);
+        }
+
+        uint64_t tryRead(void *target, uint64_t offset, uint64_t size) const override {
+            return _baseBlob->tryRead(target, offset + sizeof(FORMAT_VERSION_HEADER) + 1, size);
+        }
+
+        void write(const void *source, uint64_t offset, uint64_t size) override {
+            return _baseBlob->write(source, offset + sizeof(FORMAT_VERSION_HEADER) + 1, size);
+        }
+
+        void flush() override {
+            return _baseBlob->flush();
+        }
+
+        cpputils::unique_ref<blobstore::Blob> releaseBaseBlob() {
+            return std::move(_baseBlob);
+        }
+
+    private:
+        static constexpr uint16_t FORMAT_VERSION_HEADER = 0;
+
+        static void _checkHeader(const blobstore::Blob &blob) {
+            static_assert(sizeof(uint16_t) == sizeof(FORMAT_VERSION_HEADER), "Wrong type used to read format version header");
+            uint16_t actualFormatVersion;
+            blob.read(&actualFormatVersion, 0, sizeof(FORMAT_VERSION_HEADER));
+            if (FORMAT_VERSION_HEADER != actualFormatVersion) {
+                throw std::runtime_error("This file system entity has the wrong format. Was it created with a newer version of CryFS?");
+            }
+        }
+
+        static BlobType _blobType(const blobstore::Blob &blob) {
+            uint8_t result;
+            blob.read(&result, sizeof(FORMAT_VERSION_HEADER), 1);
+            return static_cast<BlobType>(result);
+        }
+
+        cpputils::unique_ref<blobstore::Blob> _baseBlob;
+
+        DISALLOW_COPY_AND_ASSIGN(FsBlobView);
+    };
+
+}
+
+
+#endif

src/cryfs/filesystem/fsblobstore/MagicNumbers.h

@@ -1,20 +0,0 @@
-#pragma once
-#ifndef MESSMER_CRYFS_FILESYSTEM_FSBLOBSTORE_MAGICNUMBERS_H_
-#define MESSMER_CRYFS_FILESYSTEM_FSBLOBSTORE_MAGICNUMBERS_H_
-
-namespace cryfs {
-namespace fsblobstore {
-
-//TODO enum class
-enum MagicNumbers {
-  DIR = 0x00,
-  FILE = 0x01,
-  SYMLINK = 0x02
-};
-
-}
-}
-
-
-
-#endif

src/cryfs/filesystem/fsblobstore/SymlinkBlob.cpp

@@ -1,6 +1,5 @@
 #include "SymlinkBlob.h"
 
-#include "MagicNumbers.h"
 #include <blockstore/utils/Key.h>
 #include <cassert>
 
@@ -16,29 +15,22 @@ namespace fsblobstore {
 
 SymlinkBlob::SymlinkBlob(unique_ref<Blob> blob)
 : FsBlob(std::move(blob)), _target(_readTargetFromBlob(baseBlob())) {
+  ASSERT(baseBlob().blobType() == FsBlobView::BlobType::SYMLINK, "Loaded blob is not a symlink");
 }
 
 unique_ref<SymlinkBlob> SymlinkBlob::InitializeSymlink(unique_ref<Blob> blob, const bf::path &target) {
+  InitializeBlob(blob.get(), FsBlobView::BlobType::SYMLINK);
+  FsBlobView symlinkBlobView(std::move(blob));
   string targetStr = target.native();
-  blob->resize(1 + targetStr.size());
+  symlinkBlobView.resize(targetStr.size());
-  unsigned char magicNumber = MagicNumbers::SYMLINK;
+  symlinkBlobView.write(targetStr.c_str(), 0, targetStr.size());
-  blob->write(&magicNumber, 0, 1);
+  return make_unique_ref<SymlinkBlob>(symlinkBlobView.releaseBaseBlob());
-  blob->write(targetStr.c_str(), 1, targetStr.size());
-  return make_unique_ref<SymlinkBlob>(std::move(blob));
 }
 
-void SymlinkBlob::_checkMagicNumber(const Blob &blob) {
+bf::path SymlinkBlob::_readTargetFromBlob(const FsBlobView &blob) {
-  unsigned char value;
+  char targetStr[blob.size() + 1]; // +1 because of the nullbyte
-  blob.read(&value, 0, 1);
+  blob.read(targetStr, 0, blob.size());
-  ASSERT(value == MagicNumbers::SYMLINK, "Blob is not a symlink blob");
+  targetStr[blob.size()] = '\0';
-}
-
-bf::path SymlinkBlob::_readTargetFromBlob(const blobstore::Blob &blob) {
-  _checkMagicNumber(blob);
-  size_t targetStrSize = blob.size() - 1; // -1 because of the magic number
-  char targetStr[targetStrSize + 1]; // +1 because of the nullbyte
-  blob.read(targetStr, 1, targetStrSize);
-  targetStr[targetStrSize] = '\0';
   return targetStr;
 }
 

src/cryfs/filesystem/fsblobstore/SymlinkBlob.h

@@ -22,9 +22,7 @@ namespace cryfs {
         private:
             boost::filesystem::path _target;
 
-            static void _checkMagicNumber(const blobstore::Blob &blob);
+            static boost::filesystem::path _readTargetFromBlob(const FsBlobView &blob);
-
-            static boost::filesystem::path _readTargetFromBlob(const blobstore::Blob &blob);
 
             DISALLOW_COPY_AND_ASSIGN(SymlinkBlob);
         };

test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockCreateTest.cpp

@@ -62,12 +62,14 @@ INSTANTIATE_TEST_CASE_P(OnDiskBlockCreateSizeTest, OnDiskBlockCreateSizeTest, Va
 
 TEST_P(OnDiskBlockCreateSizeTest, OnDiskSizeIsCorrect) {
   Data fileContent = Data::LoadFromFile(file.path()).value();
-  EXPECT_EQ(GetParam(), fileContent.size());
+  EXPECT_EQ(GetParam() + OnDiskBlock::formatVersionHeaderSize(), fileContent.size());
 }
 
 TEST_P(OnDiskBlockCreateSizeTest, OnDiskBlockIsZeroedOut) {
   Data fileContent = Data::LoadFromFile(file.path()).value();
-  EXPECT_EQ(ZEROES, fileContent);
+  Data fileContentWithoutHeader(fileContent.size() - OnDiskBlock::formatVersionHeaderSize());
+  std::memcpy(fileContentWithoutHeader.data(), fileContent.dataOffset(OnDiskBlock::formatVersionHeaderSize()), fileContentWithoutHeader.size());
+  EXPECT_EQ(ZEROES, fileContentWithoutHeader);
 }
 
 // This test is also tested by OnDiskBlockStoreTest, but there the block is created using the BlockStore interface.

test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockFlushTest.cpp

@@ -56,8 +56,10 @@ public:
   }
 
   void EXPECT_STORED_FILE_DATA_CORRECT() {
-    Data actual = Data::LoadFromFile(file.path()).value();
+    Data fileContent = Data::LoadFromFile(file.path()).value();
-    EXPECT_EQ(randomData, actual);
+    Data fileContentWithoutHeader(fileContent.size() - OnDiskBlock::formatVersionHeaderSize());
+    std::memcpy(fileContentWithoutHeader.data(), fileContent.dataOffset(OnDiskBlock::formatVersionHeaderSize()), fileContentWithoutHeader.size());
+    EXPECT_EQ(randomData, fileContentWithoutHeader);
   }
 };
 INSTANTIATE_TEST_CASE_P(OnDiskBlockFlushTest, OnDiskBlockFlushTest, Values((size_t)0, (size_t)1, (size_t)1024, (size_t)4096, (size_t)10*1024*1024));

test/blockstore/implementations/ondisk/OnDiskBlockTest/OnDiskBlockLoadTest.cpp

@@ -31,19 +31,19 @@ public:
   OnDiskBlockLoadTest():
     dir(),
     key(Key::FromString("1491BB4932A389EE14BC7090AC772972")),
-    file(dir.path() / key.ToString()) {
+    file(dir.path() / key.ToString(), false) {
   }
   TempDir dir;
   Key key;
   TempFile file;
 
-  void SetFileSize(size_t size) {
+  void CreateBlockWithSize(size_t size) {
     Data data(size);
-    data.StoreToFile(file.path());
+    OnDiskBlock::CreateOnDisk(dir.path(), key, std::move(data));
   }
 
-  void StoreData(const Data &data) {
+  void StoreData(Data data) {
-    data.StoreToFile(file.path());
+    OnDiskBlock::CreateOnDisk(dir.path(), key, std::move(data));
   }
 
   unique_ref<OnDiskBlock> LoadBlock() {
@@ -57,8 +57,8 @@ public:
 };
 INSTANTIATE_TEST_CASE_P(OnDiskBlockLoadTest, OnDiskBlockLoadTest, Values(0, 1, 5, 1024, 10*1024*1024));
 
-TEST_P(OnDiskBlockLoadTest, FileSizeIsCorrect) {
+TEST_P(OnDiskBlockLoadTest, LoadsCorrectSize) {
-  SetFileSize(GetParam());
+  CreateBlockWithSize(GetParam());
 
   auto block = LoadBlock();
 
@@ -67,7 +67,7 @@ TEST_P(OnDiskBlockLoadTest, FileSizeIsCorrect) {
 
 TEST_P(OnDiskBlockLoadTest, LoadedDataIsCorrect) {
   Data randomData = DataFixture::generate(GetParam());
-  StoreData(randomData);
+  StoreData(randomData.copy());
 
   auto block = LoadBlock();
 

test/cryfs/cli/CliTest_Setup.cpp

@@ -6,34 +6,34 @@ using cpputils::TempFile;
 using CliTest_Setup = CliTest;
 
 TEST_F(CliTest_Setup, NoSpecialOptions) {
-    //Specify --cipher and --extpass parameters to make it non-interactive
+    //Specify --cipher parameter to make it non-interactive
     //TODO Remove "-f" parameter, once EXPECT_RUN_SUCCESS can handle that
-    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "--cipher", "aes-256-gcm", "--extpass", "echo mypassword", "-f"}, mountdir);
+    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "--cipher", "aes-256-gcm", "-f"}, mountdir);
 }
 
 TEST_F(CliTest_Setup, NotexistingLogfileGiven) {
     TempFile notexisting_logfile(false);
-    //Specify --cipher and --extpass parameters to make it non-interactive
+    //Specify --cipher parameter to make it non-interactive
     //TODO Remove "-f" parameter, once EXPECT_RUN_SUCCESS can handle that
-    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--extpass", "echo mypassword", "--logfile", notexisting_logfile.path().c_str()}, mountdir);
+    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--logfile", notexisting_logfile.path().c_str()}, mountdir);
     //TODO Expect logfile is used (check logfile content)
 }
 
 TEST_F(CliTest_Setup, ExistingLogfileGiven) {
-    //Specify --cipher and --extpass parameters to make it non-interactive
+    //Specify --cipher parameter to make it non-interactive
     //TODO Remove "-f" parameter, once EXPECT_RUN_SUCCESS can handle that
-    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--extpass", "echo mypassword", "--logfile", logfile.path().c_str()}, mountdir);
+    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--logfile", logfile.path().c_str()}, mountdir);
     //TODO Expect logfile is used (check logfile content)
 }
 
 TEST_F(CliTest_Setup, ConfigfileGiven) {
-    //Specify --cipher and --extpass parameters to make it non-interactive
+    //Specify --cipher parameter to make it non-interactive
     //TODO Remove "-f" parameter, once EXPECT_RUN_SUCCESS can handle that
-    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--extpass", "echo mypassword", "--config", configfile.path().c_str()}, mountdir);
+    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--config", configfile.path().c_str()}, mountdir);
 }
 
 TEST_F(CliTest_Setup, FuseOptionGiven) {
-    //Specify --cipher and --extpass parameters to make it non-interactive
+    //Specify --cipher parameter to make it non-interactive
     //TODO Remove "-f" parameter, once EXPECT_RUN_SUCCESS can handle that
-    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--extpass", "echo mypassword", "--", "-f"}, mountdir);
+    EXPECT_RUN_SUCCESS({basedir.c_str(), mountdir.c_str(), "-f", "--cipher", "aes-256-gcm", "--", "-f"}, mountdir);
 }

test/cryfs/cli/CliTest_WrongEnvironment.cpp

@@ -60,11 +60,9 @@ public:
         if (GetParam().runningInForeground) {
             result.push_back("-f");
         }
-        // Test case should be non-interactive, so don't ask for cipher or password.
+        // Test case should be non-interactive, so don't ask for cipher.
         result.push_back("--cipher");
         result.push_back("aes-256-gcm");
-        result.push_back("--extpass");
-        result.push_back("echo mypassword");
         return result;
     }
 };

test/cryfs/cli/testutils/CliTest.h

@@ -39,6 +39,10 @@ public:
             _args.push_back(const_cast<char*>(arg));
         }
         auto &keyGenerator = cpputils::Random::PseudoRandom();
+        // Write 2x 'pass\n' to stdin so Cryfs can read it as password (+ password confirmation prompt)
+        std::cin.putback('\n'); std::cin.putback('s'); std::cin.putback('s'); std::cin.putback('a'); std::cin.putback('p');
+        std::cin.putback('\n'); std::cin.putback('s'); std::cin.putback('s'); std::cin.putback('a'); std::cin.putback('p');
+        // Run Cryfs
         cryfs::Cli(keyGenerator, cpputils::SCrypt::TestSettings, console, _httpClient()).main(_args.size(), _args.data());
     }