diff --git a/ChangeLog.txt b/ChangeLog.txt index 1d53e5b1..8688e4e5 100644 --- a/ChangeLog.txt +++ b/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). diff --git a/src/blockstore/CMakeLists.txt b/src/blockstore/CMakeLists.txt index 80857186..434c05cf 100644 --- a/src/blockstore/CMakeLists.txt +++ b/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 diff --git a/src/blockstore/implementations/readonly/ReadOnlyBlockStore2.cpp b/src/blockstore/implementations/readonly/ReadOnlyBlockStore2.cpp new file mode 100644 index 00000000..354c29b3 --- /dev/null +++ b/src/blockstore/implementations/readonly/ReadOnlyBlockStore2.cpp @@ -0,0 +1 @@ +#include "ReadOnlyBlockStore2.h" diff --git a/src/blockstore/implementations/readonly/ReadOnlyBlockStore2.h b/src/blockstore/implementations/readonly/ReadOnlyBlockStore2.h new file mode 100644 index 00000000..f4033342 --- /dev/null +++ b/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 + +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 baseBlockStore); + + bool tryCreate(const BlockId &blockId, const cpputils::Data &data) override; + bool remove(const BlockId &blockId) override; + boost::optional 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 callback) const override; + +private: + cpputils::unique_ref _baseBlockStore; + + DISALLOW_COPY_AND_ASSIGN(ReadOnlyBlockStore2); +}; + +inline ReadOnlyBlockStore2::ReadOnlyBlockStore2(cpputils::unique_ref 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 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 callback) const { + return _baseBlockStore->forEachBlock(std::move(callback)); +} + +} +} + +#endif diff --git a/src/cryfs-cli/Cli.cpp b/src/cryfs-cli/Cli.cpp index 4eaf73af..0b8ce9ec 100644 --- a/src/cryfs-cli/Cli.cpp +++ b/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) { - throw CryfsException("Could not load config file. Did you enter the correct password?", ErrorCode::WrongPassword); + if (config.is_left()) { + 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()); - return std::move(*config); + _checkConfigIntegrity(options.baseDir(), localStateDir, *config.right().configFile, options.allowReplacedFilesystem()); + return std::move(config.right()); } - optional Cli::_loadOrCreateConfigFile(bf::path configFilePath, LocalStateDir localStateDir, const optional &cipher, const optional &blocksizeBytes, bool allowFilesystemUpgrade, const optional &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem) { + either Cli::_loadOrCreateConfigFile(bf::path configFilePath, LocalStateDir localStateDir, const optional &cipher, const optional &blocksizeBytes, bool allowFilesystemUpgrade, const optional &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( diff --git a/src/cryfs-cli/Cli.h b/src/cryfs-cli/Cli.h index 9a31fd53..2c6209ed 100644 --- a/src/cryfs-cli/Cli.h +++ b/src/cryfs-cli/Cli.h @@ -25,7 +25,7 @@ namespace cryfs_cli { void _runFilesystem(const program_options::ProgramOptions &options, std::function 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 _loadOrCreateConfigFile(boost::filesystem::path configFilePath, cryfs::LocalStateDir localStateDir, const boost::optional &cipher, const boost::optional &blocksizeBytes, bool allowFilesystemUpgrade, const boost::optional &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem); + cpputils::either _loadOrCreateConfigFile(boost::filesystem::path configFilePath, cryfs::LocalStateDir localStateDir, const boost::optional &cipher, const boost::optional &blocksizeBytes, bool allowFilesystemUpgrade, const boost::optional &missingBlockIsIntegrityViolation, bool allowReplacedFilesystem); boost::filesystem::path _determineConfigFile(const program_options::ProgramOptions &options); static std::function _askPasswordForExistingFilesystem(std::shared_ptr console); static std::function _askPasswordForNewFilesystem(std::shared_ptr console); diff --git a/src/cryfs/impl/config/CryConfigFile.cpp b/src/cryfs/impl/config/CryConfigFile.cpp index a7b270ad..185ee3c0 100644 --- a/src/cryfs/impl/config/CryConfigFile.cpp +++ b/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::load(bf::path path, CryKeyProvider* keyProvider) { +either> 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::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(std::move(path), std::move(config), std::move(*encryptor))); + auto configFile = make_unique_ref(CryConfigFile(std::move(path), std::move(config), std::move(*encryptor), access)); if (decrypted->wasInDeprecatedConfigFormat) { - // Migrate it to new format - configFile->save(); + if (access == Access::ReadWrite) { + // 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::create(bf::path path, CryConfig config, if (bf::exists(path)) { throw std::runtime_error("Config file exists already."); } - auto result = make_unique_ref(std::move(path), std::move(config), CryConfigEncryptorFactory::deriveNewKey(keyProvider)); + auto result = make_unique_ref(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 encryptor) - : _path(std::move(path)), _config(std::move(config)), _encryptor(std::move(encryptor)) { +CryConfigFile::CryConfigFile(bf::path path, CryConfig config, unique_ref encryptor, Access access) + : _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); diff --git a/src/cryfs/impl/config/CryConfigFile.h b/src/cryfs/impl/config/CryConfigFile.h index a9fd2ed9..898fa5ff 100644 --- a/src/cryfs/impl/config/CryConfigFile.h +++ b/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 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 encryptor, Access access); CryConfigFile(CryConfigFile &&rhs) = default; ~CryConfigFile(); @@ -20,7 +34,7 @@ namespace cryfs { static cpputils::unique_ref create(boost::filesystem::path path, CryConfig config, CryKeyProvider* keyProvider); enum class LoadError {ConfigFileNotFound, DecryptionFailed}; - static cpputils::either> load(boost::filesystem::path path, CryKeyProvider* keyProvider); + static cpputils::either> 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 _encryptor; + Access _access; DISALLOW_COPY_AND_ASSIGN(CryConfigFile); }; diff --git a/src/cryfs/impl/config/CryConfigLoader.cpp b/src/cryfs/impl/config/CryConfigLoader.cpp index f1acd5d4..32fd2551 100644 --- a/src/cryfs/impl/config/CryConfigLoader.cpp +++ b/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, RandomGenerator &k _localStateDir(std::move(localStateDir)) { } -optional CryConfigLoader::_loadConfig(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem) { - auto config = CryConfigFile::load(std::move(filename), _keyProvider.get()); +either CryConfigLoader::_loadConfig(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem, CryConfigFile::Access access) { + 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::_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::load(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem) { - return _loadConfig(std::move(filename), allowFilesystemUpgrade, allowReplacedFilesystem); +either CryConfigLoader::load(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem, CryConfigFile::Access access) { + return _loadConfig(std::move(filename), allowFilesystemUpgrade, allowReplacedFilesystem, access); } -optional CryConfigLoader::loadOrCreate(bf::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem) { +either 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); } diff --git a/src/cryfs/impl/config/CryConfigLoader.h b/src/cryfs/impl/config/CryConfigLoader.h index 21cc97ab..fb26efb9 100644 --- a/src/cryfs/impl/config/CryConfigLoader.h +++ b/src/cryfs/impl/config/CryConfigLoader.h @@ -4,6 +4,7 @@ #include #include +#include #include "CryConfigFile.h" #include "CryCipher.h" #include "CryConfigCreator.h" @@ -22,11 +23,11 @@ public: uint32_t myClientId; }; - boost::optional loadOrCreate(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem); - boost::optional load(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem); + cpputils::either loadOrCreate(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem); + cpputils::either load(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem, CryConfigFile::Access access); private: - boost::optional _loadConfig(boost::filesystem::path filename, bool allowFilesystemUpgrade, bool allowReplacedFilesystem); + cpputils::either _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; diff --git a/src/stats/main.cpp b/src/stats/main.cpp index 7e82160d..d20e94e3 100644 --- a/src/stats/main.cpp +++ b/src/stats/main.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -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 node) { unique_ref makeBlockStore(const path& basedir, const CryConfigLoader::ConfigLoadResult& config, LocalStateDir& localStateDir) { auto onDiskBlockStore = make_unique_ref(basedir); - auto encryptedBlockStore = CryCiphers::find(config.configFile->config()->Cipher()).createEncryptedBlockstore(std::move(onDiskBlockStore), config.configFile->config()->EncryptionKey()); + auto readOnlyBlockStore = make_unique_ref(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); - if (config == boost::none) { - // TODO Show more info about error - throw std::runtime_error("Error loading config file."); + auto config = config_loader.load(config_path, false, true, CryConfigFile::Access::ReadOnly); + if (config.is_left()) { + switch (config.left()) { + 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 unaccountedBlocks = _getAllBlockIds(basedir, *config, localStateDir); + set unaccountedBlocks = _getAllBlockIds(basedir, config.right(), localStateDir); cout << "done" << endl; - vector accountedBlocks = _getKnownBlockIds(basedir, *config, localStateDir); + vector 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 nodeStore = make_unique_ref(std::move(blockStore), config->configFile->config()->BlocksizeBytes()); + auto blockStore = makeBlockStore(basedir, config.right(), localStateDir); + auto nodeStore = make_unique_ref(std::move(blockStore), config.right().configFile->config()->BlocksizeBytes()); uint32_t numUnaccountedBlocks = unaccountedBlocks.size(); uint32_t numLeaves = 0; diff --git a/test/cryfs-cli/CliTest_IntegrityCheck.cpp b/test/cryfs-cli/CliTest_IntegrityCheck.cpp index 45bb1333..762caa6a 100644 --- a/test/cryfs-cli/CliTest_IntegrityCheck.cpp +++ b/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(); } diff --git a/test/cryfs/impl/config/CompatibilityTest.cpp b/test/cryfs/impl/config/CompatibilityTest.cpp index f001d998..07b931b3 100644 --- a/test/cryfs/impl/config/CompatibilityTest.cpp +++ b/test/cryfs/impl/config/CompatibilityTest.cpp @@ -29,7 +29,7 @@ public: unique_ref loadConfigFromHex(const string &configFileContentHex) { storeHexToFile(configFileContentHex); CryPresetPasswordBasedKeyProvider keyProvider("mypassword", make_unique_ref(SCrypt::DefaultSettings)); - return CryConfigFile::load(file.path(), &keyProvider).right(); + return CryConfigFile::load(file.path(), &keyProvider, CryConfigFile::Access::ReadWrite).right(); } private: diff --git a/test/cryfs/impl/config/CryConfigFileTest.cpp b/test/cryfs/impl/config/CryConfigFileTest.cpp index 4e8a549a..c43ca34c 100644 --- a/test/cryfs/impl/config/CryConfigFileTest.cpp +++ b/test/cryfs/impl/config/CryConfigFileTest.cpp @@ -47,7 +47,7 @@ public: optional> 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) { diff --git a/test/cryfs/impl/config/CryConfigLoaderTest.cpp b/test/cryfs/impl/config/CryConfigLoaderTest.cpp index 1e4ec52c..1ac878a2 100644 --- a/test/cryfs/impl/config/CryConfigLoaderTest.cpp +++ b/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 Create(const string &password = "mypassword", const optional &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> Load(const string &password = "mypassword", const optional &cipher = none, bool noninteractive = false, bool allowFilesystemUpgrade = false) { + either> LoadOrCreate(const string &password = "mypassword", const optional &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) { - return none; + if (loadResult.is_left()) { + return loadResult.left(); } - return std::move(loadResult->configFile); + return std::move(loadResult.right().configFile); + } + + either> 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"); - EXPECT_EQ(none, loaded); + auto loaded = LoadOrCreate("mypassword2"); + 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(); - auto configFile = CryConfigFile::load(file.path(), keyProvider("mypassword").get()).right_opt().value(); + LoadOrCreate().right(); + 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(); 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; - EXPECT_NE(myClientId, loader("mypassword", true).loadOrCreate(file2.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).right().myClientId); } TEST_F(CryConfigLoaderTest, MyClientIdIsLoadedCorrectly) { TempFile file(false); - uint32_t myClientId = loader("mypassword", true).loadOrCreate(file.path(), false, false).value().myClientId; - EXPECT_EQ(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).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); } diff --git a/test/cryfs/impl/filesystem/CryFsTest.cpp b/test/cryfs/impl/filesystem/CryFsTest.cpp index 0de82e1b..e44e79f0 100644 --- a/test/cryfs/impl/filesystem/CryFsTest.cpp +++ b/test/cryfs/impl/filesystem/CryFsTest.cpp @@ -43,7 +43,7 @@ public: shared_ptr loadOrCreateConfig() { auto keyProvider = make_unique_ref("mypassword", make_unique_ref(SCrypt::TestSettings)); - return CryConfigLoader(make_shared(mockConsole()), Random::PseudoRandom(), std::move(keyProvider), localStateDir, none, none, none).loadOrCreate(config.path(), false, false).value().configFile; + return CryConfigLoader(make_shared(mockConsole()), Random::PseudoRandom(), std::move(keyProvider), localStateDir, none, none, none).loadOrCreate(config.path(), false, false).right().configFile; } unique_ref blockStore() { diff --git a/test/cryfs/impl/filesystem/FileSystemTest.cpp b/test/cryfs/impl/filesystem/FileSystemTest.cpp index ab1e286b..45be78da 100644 --- a/test/cryfs/impl/filesystem/FileSystemTest.cpp +++ b/test/cryfs/impl/filesystem/FileSystemTest.cpp @@ -40,7 +40,7 @@ public: auto _console = make_shared(mockConsole()); auto keyProvider = make_unique_ref("mypassword", make_unique_ref(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(std::move(config.configFile), std::move(blockStore), localStateDir, config.myClientId, false, false, failOnIntegrityViolation()); }