- cryfs-stats tool is guaranteed to be readonly and not modify the file system

- Now shows a better error message when failing to load the config file and distinguishes between 'wrong password' and 'config file not found'
- The cryfs-stats tool only reads and never writes the cryfs.config file

ChangeLog.txt

@@ -9,6 +9,7 @@ Other changes:
 
 Improvements:
 * Display the file system configuration when mounting a file system
+* Now shows a better error message when failing to load the config file and distinguishes between "wrong password" and "config file not found".
 
 New features:
 * Add support for atime mount options (noatime, strictatime, relatime, atime, nodiratime).

src/blockstore/CMakeLists.txt

@@ -11,6 +11,7 @@ set(SOURCES
   implementations/parallelaccess/ParallelAccessBlockStore.cpp
   implementations/parallelaccess/BlockRef.cpp
   implementations/parallelaccess/ParallelAccessBlockStoreAdapter.cpp
+  implementations/readonly/ReadOnlyBlockStore2.cpp
   implementations/compressing/CompressingBlockStore.cpp
   implementations/compressing/CompressedBlock.cpp
   implementations/compressing/compressors/RunLengthEncoding.cpp

src/blockstore/implementations/readonly/ReadOnlyBlockStore2.cpp

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

src/blockstore/implementations/readonly/ReadOnlyBlockStore2.h

@@ -0,0 +1,79 @@
+#pragma once
+#ifndef MESSMER_BLOCKSTORE_IMPLEMENTATIONS_ENCRYPTED_READONLYBLOCKSTORE2_H_
+#define MESSMER_BLOCKSTORE_IMPLEMENTATIONS_ENCRYPTED_READONLYBLOCKSTORE2_H_
+
+#include "../../interface/BlockStore2.h"
+#include <cpp-utils/macros.h>
+
+namespace blockstore {
+namespace readonly {
+
+// TODO Test
+
+/**
+ * Wraps another block store and makes it read-only.
+ * All read operations are passed through to the underlying
+ * blockstore, while all write operations just throw
+ * an exception. This can be used to protect a blockstore
+ * if we're in a mode that's supposed to be read-only,
+ * e.g. recovery after data corruption.
+ */
+class ReadOnlyBlockStore2 final: public BlockStore2 {
+public:
+  ReadOnlyBlockStore2(cpputils::unique_ref<BlockStore2> baseBlockStore);
+
+  bool tryCreate(const BlockId &blockId, const cpputils::Data &data) override;
+  bool remove(const BlockId &blockId) override;
+  boost::optional<cpputils::Data> load(const BlockId &blockId) const override;
+  void store(const BlockId &blockId, const cpputils::Data &data) override;
+  uint64_t numBlocks() const override;
+  uint64_t estimateNumFreeBytes() const override;
+  uint64_t blockSizeFromPhysicalBlockSize(uint64_t blockSize) const override;
+  void forEachBlock(std::function<void (const BlockId &)> callback) const override;
+
+private:
+  cpputils::unique_ref<BlockStore2> _baseBlockStore;
+
+  DISALLOW_COPY_AND_ASSIGN(ReadOnlyBlockStore2);
+};
+
+inline ReadOnlyBlockStore2::ReadOnlyBlockStore2(cpputils::unique_ref<BlockStore2> baseBlockStore)
+: _baseBlockStore(std::move(baseBlockStore)) {
+}
+
+inline bool ReadOnlyBlockStore2::tryCreate(const BlockId &/*blockId*/, const cpputils::Data &/*data*/) {
+    throw std::logic_error("Tried to call tryCreate on a ReadOnlyBlockStore. Writes to the block store aren't allowed.");
+}
+
+inline bool ReadOnlyBlockStore2::remove(const BlockId &/*blockId*/) {
+  throw std::logic_error("Tried to call remove on a ReadOnlyBlockStore. Writes to the block store aren't allowed.");
+}
+
+inline boost::optional<cpputils::Data> ReadOnlyBlockStore2::load(const BlockId &blockId) const {
+    return _baseBlockStore->load(blockId);
+}
+
+inline void ReadOnlyBlockStore2::store(const BlockId &/*blockId*/, const cpputils::Data &/*data*/) {
+  throw std::logic_error("Tried to call store on a ReadOnlyBlockStore. Writes to the block store aren't allowed.");
+}
+
+inline uint64_t ReadOnlyBlockStore2::numBlocks() const {
+  return _baseBlockStore->numBlocks();
+}
+
+inline uint64_t ReadOnlyBlockStore2::estimateNumFreeBytes() const {
+  return _baseBlockStore->estimateNumFreeBytes();
+}
+
+inline uint64_t ReadOnlyBlockStore2::blockSizeFromPhysicalBlockSize(uint64_t blockSize) const {
+    return _baseBlockStore->blockSizeFromPhysicalBlockSize(blockSize);
+}
+
+inline void ReadOnlyBlockStore2::forEachBlock(std::function<void (const BlockId &)> callback) const {
+  return _baseBlockStore->forEachBlock(std::move(callback));
+}
+
+}
+}
+
+#endif

src/cryfs-cli/Cli.cpp

@@ -45,6 +45,7 @@ using cpputils::TempFile;
 using cpputils::RandomGenerator;
 using cpputils::unique_ref;
 using cpputils::SCrypt;
+using cpputils::either;
 using cpputils::SCryptSettings;
 using cpputils::Console;
 using cpputils::HttpClient;
@@ -203,14 +204,19 @@ namespace cryfs_cli {
     CryConfigLoader::ConfigLoadResult Cli::_loadOrCreateConfig(const ProgramOptions &options, const LocalStateDir& localStateDir) {
         auto configFile = _determineConfigFile(options);
         auto config = _loadOrCreateConfigFile(std::move(configFile), localStateDir, options.cipher(), options.blocksizeBytes(), options.allowFilesystemUpgrade(), options.missingBlockIsIntegrityViolation(), options.allowReplacedFilesystem());
-        if (config == none) {
+        if (config.is_left()) {
-          throw CryfsException("Could not load config file. Did you enter the correct password?", ErrorCode::WrongPassword);
+            switch(config.left()) {
+                case CryConfigFile::LoadError::DecryptionFailed:
+                    throw CryfsException("Failed to decrypt the config file. Did you enter the correct password?", ErrorCode::WrongPassword);
+                case CryConfigFile::LoadError::ConfigFileNotFound:
+                    throw CryfsException("Could not find the cryfs.config file. Are you sure this is a valid CryFS file system?", ErrorCode::InvalidFilesystem);
+            }
         }
-        _checkConfigIntegrity(options.baseDir(), localStateDir, *config->configFile, options.allowReplacedFilesystem());
+        _checkConfigIntegrity(options.baseDir(), localStateDir, *config.right().configFile, options.allowReplacedFilesystem());
-        return std::move(*config);
+        return std::move(config.right());
     }
 
-    optional<CryConfigLoader::ConfigLoadResult> Cli::_loadOrCreateConfigFile(bf::path configFilePath, LocalStateDir localStateDir, const optional<string> &cipher, const optional<uint32_t> &blocksizeBytes, bool allowFilesystemUpgrade, const optional<bool> &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem) {
+    either<CryConfigFile::LoadError, CryConfigLoader::ConfigLoadResult> Cli::_loadOrCreateConfigFile(bf::path configFilePath, LocalStateDir localStateDir, const optional<string> &cipher, const optional<uint32_t> &blocksizeBytes, bool allowFilesystemUpgrade, const optional<bool> &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem) {
         // TODO Instead of passing in _askPasswordXXX functions to KeyProvider, only pass in console and move logic to the key provider,
         //      for example by having a separate CryPasswordBasedKeyProvider / CryNoninteractivePasswordBasedKeyProvider.
         auto keyProvider = make_unique_ref<CryPasswordBasedKeyProvider>(

src/cryfs-cli/Cli.h

@@ -25,7 +25,7 @@ namespace cryfs_cli {
         void _runFilesystem(const program_options::ProgramOptions &options, std::function<void()> onMounted);
         cryfs::CryConfigLoader::ConfigLoadResult _loadOrCreateConfig(const program_options::ProgramOptions &options, const cryfs::LocalStateDir& localStateDir);
         void _checkConfigIntegrity(const boost::filesystem::path& basedir, const cryfs::LocalStateDir& localStateDir, const cryfs::CryConfigFile& config, bool allowReplacedFilesystem);
-        boost::optional<cryfs::CryConfigLoader::ConfigLoadResult> _loadOrCreateConfigFile(boost::filesystem::path configFilePath, cryfs::LocalStateDir localStateDir, const boost::optional<std::string> &cipher, const boost::optional<uint32_t> &blocksizeBytes, bool allowFilesystemUpgrade, const boost::optional<bool> &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem);
+        cpputils::either<cryfs::CryConfigFile::LoadError, cryfs::CryConfigLoader::ConfigLoadResult> _loadOrCreateConfigFile(boost::filesystem::path configFilePath, cryfs::LocalStateDir localStateDir, const boost::optional<std::string> &cipher, const boost::optional<uint32_t> &blocksizeBytes, bool allowFilesystemUpgrade, const boost::optional<bool> &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem);
         boost::filesystem::path _determineConfigFile(const program_options::ProgramOptions &options);
         static std::function<std::string()> _askPasswordForExistingFilesystem(std::shared_ptr<cpputils::Console> console);
         static std::function<std::string()> _askPasswordForNewFilesystem(std::shared_ptr<cpputils::Console> console);

src/cryfs/impl/config/CryConfigFile.cpp

@@ -25,7 +25,7 @@ CryConfigFile::~CryConfigFile() {
     //We do not call save() here, because we do not want the config file to be re-encrypted on each filesystem run
 }
 
-either<CryConfigFile::LoadError, unique_ref<CryConfigFile>> CryConfigFile::load(bf::path path, CryKeyProvider* keyProvider) {
+either<CryConfigFile::LoadError, unique_ref<CryConfigFile>> CryConfigFile::load(bf::path path, CryKeyProvider* keyProvider, Access access) {
     auto encryptedConfigData = Data::LoadFromFile(path);
     if (encryptedConfigData == none) {
         return LoadError::ConfigFileNotFound;
@@ -43,10 +43,12 @@ either<CryConfigFile::LoadError, unique_ref<CryConfigFile>> CryConfigFile::load(
         LOG(ERR, "Inner cipher algorithm used to encrypt config file doesn't match config value");
         return LoadError::DecryptionFailed;
     }
-    auto configFile = make_unique_ref<CryConfigFile>(CryConfigFile(std::move(path), std::move(config), std::move(*encryptor)));
+    auto configFile = make_unique_ref<CryConfigFile>(CryConfigFile(std::move(path), std::move(config), std::move(*encryptor), access));
     if (decrypted->wasInDeprecatedConfigFormat) {
-        // Migrate it to new format
+        if (access == Access::ReadWrite) {
-        configFile->save();
+            // Migrate it to new format
+            configFile->save();
+        }
     }
     #if !defined(__clang__) && !defined(_MSC_VER) && defined(__GNUC__) && __GNUC__ < 8
         return std::move(configFile);
@@ -59,16 +61,19 @@ unique_ref<CryConfigFile> CryConfigFile::create(bf::path path, CryConfig config,
     if (bf::exists(path)) {
         throw std::runtime_error("Config file exists already.");
     }
-    auto result = make_unique_ref<CryConfigFile>(std::move(path), std::move(config), CryConfigEncryptorFactory::deriveNewKey(keyProvider));
+    auto result = make_unique_ref<CryConfigFile>(std::move(path), std::move(config), CryConfigEncryptorFactory::deriveNewKey(keyProvider), CryConfigFile::Access::ReadWrite);
     result->save();
     return result;
 }
 
-CryConfigFile::CryConfigFile(bf::path path, CryConfig config, unique_ref<CryConfigEncryptor> encryptor)
+CryConfigFile::CryConfigFile(bf::path path, CryConfig config, unique_ref<CryConfigEncryptor> encryptor, Access access)
-    : _path(std::move(path)), _config(std::move(config)), _encryptor(std::move(encryptor)) {
+    : _path(std::move(path)), _config(std::move(config)), _encryptor(std::move(encryptor)), _access(access) {
 }
 
 void CryConfigFile::save() const {
+    if (_access == Access::ReadOnly) {
+        throw std::logic_error("Tried to save the cryfs.config file while being in read only mode");
+    }
     Data configData = _config.save();
     auto encrypted = _encryptor->encrypt(configData, _config.Cipher());
     encrypted.StoreToFile(_path);

src/cryfs/impl/config/CryConfigFile.h

@@ -12,7 +12,21 @@
 namespace cryfs {
     class CryConfigFile final {
     public:
-        CryConfigFile(boost::filesystem::path path, CryConfig config, cpputils::unique_ref<CryConfigEncryptor> encryptor);
+        enum class Access : uint8_t {
+            // Never write to the config file, just read it.
+            // Note that this is only sound if the file system itself
+            // is also loaded read-only, or at least with migrations disabled.
+            // Otherwise, the file system might get migrated but the config
+            // file will still say it's the old version.
+            ReadOnly,
+
+            // Load the config file and update it if necessary,
+            // e.g. write the "last opened with" entry into it
+            // and potentially upgrade the version number.
+            ReadWrite,
+        };
+
+        CryConfigFile(boost::filesystem::path path, CryConfig config, cpputils::unique_ref<CryConfigEncryptor> encryptor, Access access);
 
         CryConfigFile(CryConfigFile &&rhs) = default;
         ~CryConfigFile();
@@ -20,7 +34,7 @@ namespace cryfs {
         static cpputils::unique_ref<CryConfigFile> create(boost::filesystem::path path, CryConfig config, CryKeyProvider* keyProvider);
 
         enum class LoadError {ConfigFileNotFound, DecryptionFailed};
-        static cpputils::either<LoadError, cpputils::unique_ref<CryConfigFile>> load(boost::filesystem::path path, CryKeyProvider* keyProvider);
+        static cpputils::either<LoadError, cpputils::unique_ref<CryConfigFile>> load(boost::filesystem::path path, CryKeyProvider* keyProvider, Access access);
 
         void save() const;
 
@@ -31,6 +45,7 @@ namespace cryfs {
         boost::filesystem::path _path;
         CryConfig _config;
         cpputils::unique_ref<CryConfigEncryptor> _encryptor;
+        Access _access;
 
         DISALLOW_COPY_AND_ASSIGN(CryConfigFile);
     };

src/cryfs/impl/config/CryConfigLoader.cpp

@@ -14,6 +14,7 @@ namespace bf = boost::filesystem;
 using cpputils::Console;
 using cpputils::RandomGenerator;
 using cpputils::unique_ref;
+using cpputils::either;
 using boost::optional;
 using boost::none;
 using std::shared_ptr;
@@ -31,10 +32,10 @@ CryConfigLoader::CryConfigLoader(shared_ptr<Console> console, RandomGenerator &k
       _localStateDir(std::move(localStateDir)) {
 }
 
-optional<CryConfigLoader::ConfigLoadResult> CryConfigLoader::_loadConfig(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem) {
+either<CryConfigFile::LoadError, CryConfigLoader::ConfigLoadResult> CryConfigLoader::_loadConfig(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem, CryConfigFile::Access access) {
-  auto config = CryConfigFile::load(std::move(filename), _keyProvider.get());
+  auto config = CryConfigFile::load(std::move(filename), _keyProvider.get(), access);
   if (config.is_left()) {
-    return none;
+    return config.left();
   }
 #ifndef CRYFS_NO_COMPATIBILITY
   //Since 0.9.7 and 0.9.8 set their own version to cryfs.version instead of the filesystem format version (which is 0.9.6), overwrite it
@@ -45,11 +46,15 @@ optional<CryConfigLoader::ConfigLoadResult> CryConfigLoader::_loadConfig(bf::pat
   _checkVersion(*config.right()->config(), allowFilesystemUpgrade);
   if (config.right()->config()->Version() != CryConfig::FilesystemFormatVersion) {
     config.right()->config()->SetVersion(CryConfig::FilesystemFormatVersion);
-    config.right()->save();
+    if (access == CryConfigFile::Access::ReadWrite) {
+      config.right()->save();
+    }
   }
   if (config.right()->config()->LastOpenedWithVersion() != gitversion::VersionString()) {
     config.right()->config()->SetLastOpenedWithVersion(gitversion::VersionString());
-    config.right()->save();
+    if (access == CryConfigFile::Access::ReadWrite) {
+      config.right()->save();
+    }
   }
   _checkCipher(*config.right()->config());
   auto localState = LocalStateMetadata::loadOrGenerate(_localStateDir.forFilesystemId(config.right()->config()->FilesystemId()), cpputils::Data::FromString(config.right()->config()->EncryptionKey()), allowReplacedFilesystem);
@@ -99,13 +104,13 @@ void CryConfigLoader::_checkMissingBlocksAreIntegrityViolations(CryConfigFile *c
   }
 }
 
-optional<CryConfigLoader::ConfigLoadResult> CryConfigLoader::load(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem) {
+either<CryConfigFile::LoadError, CryConfigLoader::ConfigLoadResult> CryConfigLoader::load(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem, CryConfigFile::Access access) {
-  return _loadConfig(std::move(filename), allowFilesystemUpgrade, allowReplacedFilesystem);
+  return _loadConfig(std::move(filename), allowFilesystemUpgrade, allowReplacedFilesystem, access);
 }
 
-optional<CryConfigLoader::ConfigLoadResult> CryConfigLoader::loadOrCreate(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem) {
+either<CryConfigFile::LoadError, CryConfigLoader::ConfigLoadResult> CryConfigLoader::loadOrCreate(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem) {
   if (bf::exists(filename)) {
-    return _loadConfig(std::move(filename), allowFilesystemUpgrade, allowReplacedFilesystem);
+    return _loadConfig(std::move(filename), allowFilesystemUpgrade, allowReplacedFilesystem, CryConfigFile::Access::ReadWrite);
   } else {
     return _createConfig(std::move(filename), allowReplacedFilesystem);
   }

src/cryfs/impl/config/CryConfigLoader.h

@@ -4,6 +4,7 @@
 
 #include <cpp-utils/pointer/unique_ref.h>
 #include <boost/filesystem.hpp>
+#include <cpp-utils/either.h>
 #include "CryConfigFile.h"
 #include "CryCipher.h"
 #include "CryConfigCreator.h"
@@ -22,11 +23,11 @@ public:
       uint32_t myClientId;
   };
 
-  boost::optional<ConfigLoadResult> loadOrCreate(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem);
+  cpputils::either<CryConfigFile::LoadError, CryConfigLoader::ConfigLoadResult> loadOrCreate(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem);
-  boost::optional<ConfigLoadResult> load(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem);
+  cpputils::either<CryConfigFile::LoadError, CryConfigLoader::ConfigLoadResult> load(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem, CryConfigFile::Access access);
 
 private:
-    boost::optional<ConfigLoadResult> _loadConfig(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem);
+    cpputils::either<CryConfigFile::LoadError, CryConfigLoader::ConfigLoadResult> _loadConfig(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem, CryConfigFile::Access access);
     ConfigLoadResult _createConfig(boost::filesystem::path filename, bool allowReplacedFilesystem);
     void _checkVersion(const CryConfig &config, bool allowFilesystemUpgrade);
     void _checkCipher(const CryConfig &config) const;

src/stats/main.cpp

@@ -3,6 +3,7 @@
 #include <cryfs/impl/config/CryConfigLoader.h>
 #include <cryfs/impl/config/CryPasswordBasedKeyProvider.h>
 #include <blockstore/implementations/ondisk/OnDiskBlockStore2.h>
+#include <blockstore/implementations/readonly/ReadOnlyBlockStore2.h>
 #include <blockstore/implementations/integrity/IntegrityBlockStore2.h>
 #include <blockstore/implementations/low2highlevel/LowToHighLevelBlockStore.h>
 #include <blobstore/implementations/onblocks/datanodestore/DataNodeStore.h>
@@ -31,6 +32,7 @@ using namespace cryfs;
 using namespace cpputils;
 using namespace blockstore;
 using namespace blockstore::ondisk;
+using namespace blockstore::readonly;
 using namespace blockstore::integrity;
 using namespace blockstore::lowtohighlevel;
 using namespace blobstore::onblocks;
@@ -55,7 +57,8 @@ void printNode(unique_ref<DataNode> node) {
 
 unique_ref<BlockStore> makeBlockStore(const path& basedir, const CryConfigLoader::ConfigLoadResult& config, LocalStateDir& localStateDir) {
     auto onDiskBlockStore = make_unique_ref<OnDiskBlockStore2>(basedir);
-    auto encryptedBlockStore = CryCiphers::find(config.configFile->config()->Cipher()).createEncryptedBlockstore(std::move(onDiskBlockStore), config.configFile->config()->EncryptionKey());
+    auto readOnlyBlockStore = make_unique_ref<ReadOnlyBlockStore2>(std::move(onDiskBlockStore));
+    auto encryptedBlockStore = CryCiphers::find(config.configFile->config()->Cipher()).createEncryptedBlockstore(std::move(readOnlyBlockStore), config.configFile->config()->EncryptionKey());
     auto statePath = localStateDir.forFilesystemId(config.configFile->config()->FilesystemId());
     auto integrityFilePath = statePath / "integritydata";
     auto onIntegrityViolation = [] () {
@@ -165,12 +168,16 @@ int main(int argc, char* argv[]) {
     LocalStateDir localStateDir(cpputils::system::HomeDirectory::getXDGDataDir() / "cryfs");
     CryConfigLoader config_loader(console, Random::OSRandom(), std::move(keyProvider), localStateDir, boost::none, boost::none, boost::none);
 
-    auto config = config_loader.load(config_path, false, true);
+    auto config = config_loader.load(config_path, false, true, CryConfigFile::Access::ReadOnly);
-    if (config == boost::none) {
+    if (config.is_left()) {
-        // TODO Show more info about error
+        switch (config.left()) {
-        throw std::runtime_error("Error loading config file.");
+            case CryConfigFile::LoadError::ConfigFileNotFound:
+                throw std::runtime_error("Error loading config file: Config file not found. Are you sure this is a valid CryFS file system?");
+            case CryConfigFile::LoadError::DecryptionFailed:
+                throw std::runtime_error("Error loading config file: Decryption failed. Did you maybe enter a wrong password?");
+        }
     }
-    const auto& config_ = config->configFile->config();
+    const auto& config_ = config.right().configFile->config();
     std::cout << "Loading filesystem of version " << config_->Version() << std::endl;
 #ifndef CRYFS_NO_COMPATIBILITY
     const bool is_correct_format = config_->Version() == CryConfig::FilesystemFormatVersion && config_->HasParentPointers() && config_->HasVersionNumbers();
@@ -178,16 +185,15 @@ int main(int argc, char* argv[]) {
     const bool is_correct_format = config_->Version() == CryConfig::FilesystemFormatVersion;
 #endif
     if (!is_correct_format) {
-        // TODO At this point, the cryfs.config file was already switched to 0.10 format. We should probably not do that.
         std::cerr << "The filesystem is not in the 0.10 format. It needs to be migrated. The cryfs-stats tool unfortunately can't handle this, please mount and unmount the filesystem once." << std::endl;
         exit(1);
     }
 
     cout << "Listing all blocks..." << flush;
-    set<BlockId> unaccountedBlocks = _getAllBlockIds(basedir, *config, localStateDir);
+    set<BlockId> unaccountedBlocks = _getAllBlockIds(basedir, config.right(), localStateDir);
     cout << "done" << endl;
 
-    vector<BlockId> accountedBlocks = _getKnownBlockIds(basedir, *config, localStateDir);
+    vector<BlockId> accountedBlocks = _getKnownBlockIds(basedir, config.right(), localStateDir);
     for (const BlockId& blockId : accountedBlocks) {
         auto num_erased = unaccountedBlocks.erase(blockId);
         ASSERT(1 == num_erased, "Blob id referenced by directory entry but didn't found it on disk? This can't happen.");
@@ -195,8 +201,8 @@ int main(int argc, char* argv[]) {
 
     console->print("Calculate statistics\n");
 
-    auto blockStore = makeBlockStore(basedir, *config, localStateDir);
+    auto blockStore = makeBlockStore(basedir, config.right(), localStateDir);
-    auto nodeStore = make_unique_ref<DataNodeStore>(std::move(blockStore), config->configFile->config()->BlocksizeBytes());
+    auto nodeStore = make_unique_ref<DataNodeStore>(std::move(blockStore), config.right().configFile->config()->BlocksizeBytes());
 
     uint32_t numUnaccountedBlocks = unaccountedBlocks.size();
     uint32_t numLeaves = 0;

test/cryfs-cli/CliTest_IntegrityCheck.cpp

@@ -69,14 +69,14 @@ class CliTest_IntegrityCheck : public CliTest {
 public:
   void modifyFilesystemId() {
     FakeCryKeyProvider keyProvider;
-    auto configFile = CryConfigFile::load(basedir / "cryfs.config", &keyProvider).right_opt().value();
+    auto configFile = CryConfigFile::load(basedir / "cryfs.config", &keyProvider, CryConfigFile::Access::ReadWrite).right_opt().value();
     configFile->config()->SetFilesystemId(CryConfig::FilesystemID::FromString("0123456789ABCDEF0123456789ABCDEF"));
     configFile->save();
   }
 
   void modifyFilesystemKey() {
     FakeCryKeyProvider keyProvider;
-    auto configFile = CryConfigFile::load(basedir / "cryfs.config", &keyProvider).right_opt().value();
+    auto configFile = CryConfigFile::load(basedir / "cryfs.config", &keyProvider, CryConfigFile::Access::ReadWrite).right_opt().value();
     configFile->config()->SetEncryptionKey("0123456789ABCDEF0123456789ABCDEF");
     configFile->save();
   }

test/cryfs/impl/config/CompatibilityTest.cpp

@@ -29,7 +29,7 @@ public:
     unique_ref<CryConfigFile> loadConfigFromHex(const string &configFileContentHex) {
         storeHexToFile(configFileContentHex);
         CryPresetPasswordBasedKeyProvider keyProvider("mypassword", make_unique_ref<SCrypt>(SCrypt::DefaultSettings));
-        return CryConfigFile::load(file.path(), &keyProvider).right();
+        return CryConfigFile::load(file.path(), &keyProvider, CryConfigFile::Access::ReadWrite).right();
     }
 
 private:

test/cryfs/impl/config/CryConfigFileTest.cpp

@@ -47,7 +47,7 @@ public:
 
     optional<unique_ref<CryConfigFile>> Load(unsigned char keySeed = 0) {
         FakeCryKeyProvider keyProvider(keySeed);
-        return CryConfigFile::load(file.path(), &keyProvider).right_opt();
+        return CryConfigFile::load(file.path(), &keyProvider, CryConfigFile::Access::ReadWrite).right_opt();
     }
 
     void CreateWithCipher(const string &cipher) {

test/cryfs/impl/config/CryConfigLoaderTest.cpp

@@ -20,6 +20,7 @@ using cpputils::DataFixture;
 using cpputils::Data;
 using cpputils::NoninteractiveConsole;
 using cpputils::unique_ref;
+using cpputils::either;
 using cpputils::make_unique_ref;
 using cpputils::Console;
 using cpputils::unique_ref;
@@ -80,26 +81,51 @@ public:
 
     unique_ref<CryConfigFile> Create(const string &password = "mypassword", const optional<string> &cipher = none, bool noninteractive = false) {
         EXPECT_FALSE(file.exists());
-        return loader(password, noninteractive, cipher).loadOrCreate(file.path(), false, false).value().configFile;
+        return loader(password, noninteractive, cipher).loadOrCreate(file.path(), false, false).right().configFile;
     }
 
-    optional<unique_ref<CryConfigFile>> Load(const string &password = "mypassword", const optional<string> &cipher = none, bool noninteractive = false, bool allowFilesystemUpgrade = false) {
+    either<CryConfigFile::LoadError, unique_ref<CryConfigFile>> LoadOrCreate(const string &password = "mypassword", const optional<string> &cipher = none, bool noninteractive = false, bool allowFilesystemUpgrade = false) {
         EXPECT_TRUE(file.exists());
         auto loadResult = loader(password, noninteractive, cipher).loadOrCreate(file.path(), allowFilesystemUpgrade, false);
-        if (loadResult == none) {
+        if (loadResult.is_left()) {
-            return none;
+            return loadResult.left();
         }
-        return std::move(loadResult->configFile);
+        return std::move(loadResult.right().configFile);
+    }
+
+    either<CryConfigFile::LoadError, unique_ref<CryConfigFile>> Load(CryConfigFile::Access access = CryConfigFile::Access::ReadWrite) {
+        EXPECT_TRUE(file.exists());
+        auto loadResult = loader("mypassword", false, none).load(file.path(), false, false, access);
+        if (loadResult.is_left()) {
+            return loadResult.left();
+        }
+        return std::move(loadResult.right().configFile);
+    }
+
+    void expectLoadingModifiesFile(CryConfigFile::Access access) {
+        Data contents_before_loading = Data::LoadFromFile(file.path()).value();
+        EXPECT_TRUE(Load(access).is_right());
+        Data contents_after_loading = Data::LoadFromFile(file.path()).value();
+        ASSERT_EQ(contents_before_loading.size(), contents_after_loading.size());
+        EXPECT_NE(0, std::memcmp(contents_before_loading.data(), contents_after_loading.data(), contents_before_loading.size()));
+    }
+
+    void expectLoadingDoesntModifyFile(CryConfigFile::Access access) {
+        Data contents_before_loading = Data::LoadFromFile(file.path()).value();
+        EXPECT_TRUE(Load(access).is_right());
+        Data contents_after_loading = Data::LoadFromFile(file.path()).value();
+        ASSERT_EQ(contents_before_loading.size(), contents_after_loading.size());
+        EXPECT_EQ(0, std::memcmp(contents_before_loading.data(), contents_after_loading.data(), contents_before_loading.size()));
     }
 
     void CreateWithRootBlob(const string &rootBlob, const string &password = "mypassword") {
-        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).value().configFile;
+        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).right().configFile;
         cfg->config()->SetRootBlob(rootBlob);
         cfg->save();
     }
 
     void CreateWithCipher(const string &cipher, const string &password = "mypassword") {
-        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).value().configFile;
+        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).right().configFile;
         cfg->config()->SetCipher(cipher);
         cfg->save();
     }
@@ -107,17 +133,17 @@ public:
     void CreateWithEncryptionKey(const string &encKey, const string &password = "mypassword") {
         FakeRandomGenerator generator(Data::FromString(encKey));
         auto loader = CryConfigLoader(console, generator, keyProvider(password), localStateDir, none, none, none);
-        ASSERT_NE(boost::none, loader.loadOrCreate(file.path(), false, false));
+        ASSERT_TRUE(loader.loadOrCreate(file.path(), false, false).is_right());
     }
 
     void ChangeEncryptionKey(const string &encKey, const string& password = "mypassword") {
-        auto cfg = CryConfigFile::load(file.path(), keyProvider(password).get()).right_opt().value();
+        auto cfg = CryConfigFile::load(file.path(), keyProvider(password).get(), CryConfigFile::Access::ReadWrite).right();
         cfg->config()->SetEncryptionKey(encKey);
         cfg->save();
     }
 
     void CreateWithVersion(const string &version, const string& formatVersion, const string &password = "mypassword") {
-        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).value().configFile;
+        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).right().configFile;
         cfg->config()->SetVersion(formatVersion);
         cfg->config()->SetLastOpenedWithVersion(version);
         cfg->config()->SetCreatedWithVersion(version);
@@ -125,13 +151,13 @@ public:
     }
   
     void CreateWithFilesystemID(const CryConfig::FilesystemID &filesystemId, const string &password = "mypassword") {
-        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).value().configFile;
+        auto cfg = loader(password, false).loadOrCreate(file.path(), false, false).right().configFile;
         cfg->config()->SetFilesystemId(filesystemId);
         cfg->save();
     }
 
     void ChangeFilesystemID(const CryConfig::FilesystemID &filesystemId, const string& password = "mypassword") {
-      auto cfg = CryConfigFile::load(file.path(), keyProvider(password).get()).right_opt().value();
+      auto cfg = CryConfigFile::load(file.path(), keyProvider(password).get(), CryConfigFile::Access::ReadWrite).right();
       cfg->config()->SetFilesystemId(filesystemId);
       cfg->save();
     }
@@ -164,24 +190,24 @@ public:
 TEST_F(CryConfigLoaderTest, CreatesNewIfNotExisting) {
     EXPECT_FALSE(file.exists());
     Create();
-    EXPECT_TRUE(file.exists());
+    ASSERT_TRUE(file.exists());
 }
 
 TEST_F(CryConfigLoaderTest, DoesntCrashIfExisting) {
     Create();
-    Load();
+    LoadOrCreate();
 }
 
 TEST_F(CryConfigLoaderTest, DoesntLoadIfWrongPassword) {
     Create("mypassword");
-    auto loaded = Load("mypassword2");
+    auto loaded = LoadOrCreate("mypassword2");
-    EXPECT_EQ(none, loaded);
+    EXPECT_TRUE(loaded.is_left());
 }
 
 TEST_F(CryConfigLoaderTest, DoesntLoadIfDifferentCipher) {
     Create("mypassword", string("aes-256-gcm"), false);
     try {
-        Load("mypassword", string("aes-256-cfb"), false);
+        LoadOrCreate("mypassword", string("aes-256-cfb"), false);
         EXPECT_TRUE(false); // Should throw exception
     } catch (const std::runtime_error &e) {
         EXPECT_EQ(string("Filesystem uses aes-256-gcm cipher and not aes-256-cfb as specified."), e.what());
@@ -191,7 +217,7 @@ TEST_F(CryConfigLoaderTest, DoesntLoadIfDifferentCipher) {
 TEST_F(CryConfigLoaderTest, DoesntLoadIfDifferentCipher_Noninteractive) {
     Create("mypassword", string("aes-256-gcm"), true);
     try {
-        Load("mypassword", string("aes-256-cfb"), true);
+        LoadOrCreate("mypassword", string("aes-256-cfb"), true);
         EXPECT_TRUE(false); // Should throw exception
     } catch (const std::runtime_error &e) {
         EXPECT_EQ(string("Filesystem uses aes-256-gcm cipher and not aes-256-cfb as specified."), e.what());
@@ -200,17 +226,17 @@ TEST_F(CryConfigLoaderTest, DoesntLoadIfDifferentCipher_Noninteractive) {
 
 TEST_F(CryConfigLoaderTest, DoesLoadIfSameCipher) {
     Create("mypassword", string("aes-256-gcm"));
-    Load("mypassword", string("aes-256-gcm"));
+    LoadOrCreate("mypassword", string("aes-256-gcm"));
 }
 
 TEST_F(CryConfigLoaderTest, DoesLoadIfSameCipher_Noninteractive) {
     Create("mypassword", string("aes-128-gcm"), true);
-    Load("mypassword", string("aes-128-gcm"), true);
+    LoadOrCreate("mypassword", string("aes-128-gcm"), true);
 }
 
 TEST_F(CryConfigLoaderTest, RootBlob_Load) {
     CreateWithRootBlob("rootblobid");
-    auto loaded = Load().value();
+    auto loaded = LoadOrCreate().right();
     EXPECT_EQ("rootblobid", loaded->config()->RootBlob());
 }
 
@@ -221,7 +247,7 @@ TEST_F(CryConfigLoaderTest, RootBlob_Create) {
 
 TEST_F(CryConfigLoaderTest, EncryptionKey_Load) {
     CreateWithEncryptionKey("3B4682CF22F3CA199E385729B9F3CA19D325229E385729B9443CA19D325229E3");
-    auto loaded = Load().value();
+    auto loaded = LoadOrCreate().right();
     EXPECT_EQ("3B4682CF22F3CA199E385729B9F3CA19D325229E385729B9443CA19D325229E3", loaded->config()->EncryptionKey());
 }
 
@@ -229,7 +255,7 @@ TEST_F(CryConfigLoaderTest, EncryptionKey_Load_whenKeyChanged_thenFails) {
   CreateWithEncryptionKey("3B4682CF22F3CA199E385729B9F3CA19D325229E385729B9443CA19D325229E3");
   ChangeEncryptionKey("3B4682CF22F3CA199E385729B9F3CA19D325229E385729B9443CA19D325229E4");
   EXPECT_THROW(
-      Load(),
+      LoadOrCreate(),
       std::runtime_error
   );
 }
@@ -242,7 +268,7 @@ TEST_F(CryConfigLoaderTest, EncryptionKey_Create) {
 
 TEST_F(CryConfigLoaderTest, Cipher_Load) {
     CreateWithCipher("twofish-128-cfb");
-    auto loaded = Load().value();
+    auto loaded = LoadOrCreate().right();
     EXPECT_EQ("twofish-128-cfb", loaded->config()->Cipher());
 }
 
@@ -254,7 +280,7 @@ TEST_F(CryConfigLoaderTest, Cipher_Create) {
 
 TEST_F(CryConfigLoaderTest, Version_Load) {
     CreateWithVersion("0.9.4", "0.9.4");
-    auto loaded = std::move(Load().value());
+    auto loaded = std::move(LoadOrCreate().right());
     EXPECT_EQ(CryConfig::FilesystemFormatVersion, loaded->config()->Version());
     EXPECT_EQ(gitversion::VersionString(), loaded->config()->LastOpenedWithVersion());
     EXPECT_EQ("0.9.4", loaded->config()->CreatedWithVersion());
@@ -262,8 +288,8 @@ TEST_F(CryConfigLoaderTest, Version_Load) {
 
 TEST_F(CryConfigLoaderTest, Version_Load_IsStoredAndNotOnlyOverwrittenInMemoryOnLoad) {
     CreateWithVersion("0.9.4", "0.9.4", "mypassword");
-    Load().value();
+    LoadOrCreate().right();
-    auto configFile = CryConfigFile::load(file.path(), keyProvider("mypassword").get()).right_opt().value();
+    auto configFile = CryConfigFile::load(file.path(), keyProvider("mypassword").get(), CryConfigFile::Access::ReadWrite).right();
     EXPECT_EQ(CryConfig::FilesystemFormatVersion, configFile->config()->Version());
     EXPECT_EQ(gitversion::VersionString(), configFile->config()->LastOpenedWithVersion());
     EXPECT_EQ("0.9.4", configFile->config()->CreatedWithVersion());
@@ -279,7 +305,7 @@ TEST_F(CryConfigLoaderTest, Version_Create) {
 TEST_F(CryConfigLoaderTest, FilesystemID_Load) {
     auto fixture = DataFixture::generateFixedSize<CryConfig::FilesystemID::BINARY_LENGTH>();
     CreateWithFilesystemID(fixture);
-    auto loaded = Load().value();
+    auto loaded = LoadOrCreate().right();
     EXPECT_EQ(fixture, loaded->config()->FilesystemId());
 }
 
@@ -293,7 +319,7 @@ TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerYes) {
 
     string version = newerVersion();
     CreateWithVersion(version, version);
-    EXPECT_NE(boost::none, Load());
+    EXPECT_TRUE(LoadOrCreate().is_right());
 }
 
 TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerNo) {
@@ -302,7 +328,7 @@ TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerNo) {
     string version = newerVersion();
     CreateWithVersion(version, version);
     try {
-        Load();
+        LoadOrCreate();
         EXPECT_TRUE(false); // expect throw
     } catch (const std::runtime_error &e) {
         EXPECT_THAT(e.what(), HasSubstr("Please update your CryFS version."));
@@ -314,14 +340,14 @@ TEST_F(CryConfigLoaderTest, AsksWhenMigratingOlderFilesystem) {
 
     string version = olderVersion();
     CreateWithVersion(version, version);
-    EXPECT_NE(boost::none, Load());
+    EXPECT_TRUE(LoadOrCreate().is_right());
 }
 
 TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenCorrectVersion) {
     EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to attempt a migration now?"), _)).Times(0);
 
     CreateWithVersion(gitversion::VersionString(), CryConfig::FilesystemFormatVersion);
-    EXPECT_NE(boost::none, Load());
+    EXPECT_TRUE(LoadOrCreate().is_right());
 }
 
 TEST_F(CryConfigLoaderTest, DontMigrateWhenAnsweredNo) {
@@ -330,7 +356,7 @@ TEST_F(CryConfigLoaderTest, DontMigrateWhenAnsweredNo) {
     string version = olderVersion();
     CreateWithVersion(version, version);
     try {
-        Load();
+        LoadOrCreate();
         EXPECT_TRUE(false); // expect throw
     } catch (const std::runtime_error &e) {
         EXPECT_THAT(e.what(), HasSubstr("It has to be migrated."));
@@ -340,14 +366,14 @@ TEST_F(CryConfigLoaderTest, DontMigrateWhenAnsweredNo) {
 TEST_F(CryConfigLoaderTest, MyClientIdIsIndeterministic) {
     TempFile file1(false);
     TempFile file2(false);
-    uint32_t myClientId = loader("mypassword", true).loadOrCreate(file1.path(), false, false).value().myClientId;
+    uint32_t myClientId = loader("mypassword", true).loadOrCreate(file1.path(), false, false).right().myClientId;
-    EXPECT_NE(myClientId, loader("mypassword", true).loadOrCreate(file2.path(), false, false).value().myClientId);
+    EXPECT_NE(myClientId, loader("mypassword", true).loadOrCreate(file2.path(), false, false).right().myClientId);
 }
 
 TEST_F(CryConfigLoaderTest, MyClientIdIsLoadedCorrectly) {
     TempFile file(false);
-    uint32_t myClientId = loader("mypassword", true).loadOrCreate(file.path(), false, false).value().myClientId;
+    uint32_t myClientId = loader("mypassword", true).loadOrCreate(file.path(), false, false).right().myClientId;
-    EXPECT_EQ(myClientId, loader("mypassword", true).loadOrCreate(file.path(), false, false).value().myClientId);
+    EXPECT_EQ(myClientId, loader("mypassword", true).loadOrCreate(file.path(), false, false).right().myClientId);
 }
 
 TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenUpgradesAllowedByProgramArguments_NoninteractiveMode) {
@@ -355,7 +381,7 @@ TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenUpgradesAllowedByProgramAr
 
     string version = olderVersion();
     CreateWithVersion(version, version);
-    EXPECT_NE(boost::none, Load("mypassword", none, true, true));
+    EXPECT_TRUE(LoadOrCreate("mypassword", none, true, true).is_right());
 }
 
 TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenUpgradesAllowedByProgramArguments_InteractiveMode) {
@@ -363,5 +389,26 @@ TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenUpgradesAllowedByProgramAr
 
   string version = olderVersion();
   CreateWithVersion(version, version);
-  EXPECT_NE(boost::none, Load("mypassword", none, false, true));
+  EXPECT_TRUE(LoadOrCreate("mypassword", none, false, true).is_right());
+}
+
+TEST_F(CryConfigLoaderTest, UpdatesConfigFileWithNewVersionWhenMigrated) {
+    EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to attempt a migration now?"), false)).Times(1).WillOnce(Return(true));
+
+    string version = olderVersion(); // this triggers a migration which should cause it to modify the config file on load
+    CreateWithVersion(version, version);
+
+    expectLoadingModifiesFile(CryConfigFile::Access::ReadWrite);
+
+    // If we load it again, it shouldn't modify again because it's already updated
+    expectLoadingDoesntModifyFile(CryConfigFile::Access::ReadWrite);
+}
+
+TEST_F(CryConfigLoaderTest, DoesntUpdatesConfigFileWithNewVersionWhenLoadingReadOnly) {
+    EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to attempt a migration now?"), false)).Times(1).WillOnce(Return(true));
+
+    string version = olderVersion(); // this triggers a migration which usually would cause it to modify the config file on load
+    CreateWithVersion(version, version);
+
+    expectLoadingDoesntModifyFile(CryConfigFile::Access::ReadOnly);
 }

test/cryfs/impl/filesystem/CryFsTest.cpp

@@ -43,7 +43,7 @@ public:
 
   shared_ptr<CryConfigFile> loadOrCreateConfig() {
     auto keyProvider = make_unique_ref<CryPresetPasswordBasedKeyProvider>("mypassword", make_unique_ref<SCrypt>(SCrypt::TestSettings));
-    return CryConfigLoader(make_shared<NoninteractiveConsole>(mockConsole()), Random::PseudoRandom(), std::move(keyProvider), localStateDir, none, none, none).loadOrCreate(config.path(), false, false).value().configFile;
+    return CryConfigLoader(make_shared<NoninteractiveConsole>(mockConsole()), Random::PseudoRandom(), std::move(keyProvider), localStateDir, none, none, none).loadOrCreate(config.path(), false, false).right().configFile;
   }
 
   unique_ref<OnDiskBlockStore2> blockStore() {

test/cryfs/impl/filesystem/FileSystemTest.cpp

@@ -40,7 +40,7 @@ public:
     auto _console = make_shared<NoninteractiveConsole>(mockConsole());
     auto keyProvider = make_unique_ref<CryPresetPasswordBasedKeyProvider>("mypassword", make_unique_ref<SCrypt>(SCrypt::TestSettings));
     auto config = CryConfigLoader(_console, Random::PseudoRandom(), std::move(keyProvider), localStateDir, none, none, none)
-            .loadOrCreate(configFile.path(), false, false).value();
+            .loadOrCreate(configFile.path(), false, false).right();
     return make_unique_ref<CryDevice>(std::move(config.configFile), std::move(blockStore), localStateDir, config.myClientId, false, false, failOnIntegrityViolation());
   }