From 58fc26002b249d6b8dc2fa67d77aa007a6414404 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 1 Feb 2018 18:09:28 -0800 Subject: [PATCH] Exit codes for integrity errors --- src/cryfs-cli/Cli.cpp | 42 +++++++++++---------- src/cryfs/ErrorCodes.h | 12 ++++++ src/cryfs/config/CryConfigLoader.cpp | 6 +-- src/cryfs/localstate/LocalStateMetadata.cpp | 3 +- test/cryfs-cli/CliTest_IntegrityCheck.cpp | 8 +++- 5 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/cryfs-cli/Cli.cpp b/src/cryfs-cli/Cli.cpp index 915b73b9..1dcd9979 100644 --- a/src/cryfs-cli/Cli.cpp +++ b/src/cryfs-cli/Cli.cpp @@ -193,11 +193,11 @@ namespace cryfs { } void Cli::_checkConfigIntegrity(const bf::path& basedir, const CryConfigFile& config) { - auto basedirMetadata = BasedirMetadata::load(); + auto basedirMetadata = BasedirMetadata::load(); if (!basedirMetadata.filesystemIdForBasedirIsCorrect(basedir, config.config()->FilesystemId())) { if (!_console->askYesNo("The filesystem id in the config file is different to the last time we loaded a filesystem from this basedir. This can be genuine if you replaced the filesystem with a different one. If you didn't do that, it is possible that an attacker did. Do you want to continue loading the file system?", false)) { - throw std::runtime_error( - "The filesystem id in the config file is different to the last time we loaded a filesystem from this basedir."); + throw CryfsException( + "The filesystem id in the config file is different to the last time we loaded a filesystem from this basedir.", ErrorCode::FilesystemIdChanged); } } // Update local state (or create it if it didn't exist yet) @@ -231,28 +231,32 @@ namespace cryfs { void Cli::_runFilesystem(const ProgramOptions &options) { try { - auto blockStore = make_unique_ref(options.baseDir()); - auto config = _loadOrCreateConfig(options); - CryDevice device(std::move(config.configFile), std::move(blockStore), config.myClientId, options.noIntegrityChecks(), config.configFile.config()->missingBlockIsIntegrityViolation()); - _sanityCheckFilesystem(&device); - fspp::FilesystemImpl fsimpl(&device); - fspp::fuse::Fuse fuse(&fsimpl, "cryfs", "cryfs@"+options.baseDir().native()); + auto blockStore = make_unique_ref(options.baseDir()); + auto config = _loadOrCreateConfig(options); + CryDevice device(std::move(config.configFile), std::move(blockStore), config.myClientId, + options.noIntegrityChecks(), config.configFile.config()->missingBlockIsIntegrityViolation()); + _sanityCheckFilesystem(&device); + fspp::FilesystemImpl fsimpl(&device); + fspp::fuse::Fuse fuse(&fsimpl, "cryfs", "cryfs@" + options.baseDir().native()); - _initLogfile(options); + _initLogfile(options); - //TODO Test auto unmounting after idle timeout - //TODO This can fail due to a race condition if the filesystem isn't started yet (e.g. passing --unmount-idle 0"). - auto idleUnmounter = _createIdleCallback(options.unmountAfterIdleMinutes(), [&fuse] {fuse.stop();}); - if (idleUnmounter != none) { - device.onFsAction(std::bind(&CallAfterTimeout::resetTimer, idleUnmounter->get())); - } + //TODO Test auto unmounting after idle timeout + //TODO This can fail due to a race condition if the filesystem isn't started yet (e.g. passing --unmount-idle 0"). + auto idleUnmounter = _createIdleCallback(options.unmountAfterIdleMinutes(), [&fuse] { fuse.stop(); }); + if (idleUnmounter != none) { + device.onFsAction(std::bind(&CallAfterTimeout::resetTimer, idleUnmounter->get())); + } #ifdef __APPLE__ - std::cout << "\nMounting filesystem. To unmount, call:\n$ umount " << options.mountDir() << "\n" << std::endl; + std::cout << "\nMounting filesystem. To unmount, call:\n$ umount " << options.mountDir() << "\n" << std::endl; #else - std::cout << "\nMounting filesystem. To unmount, call:\n$ fusermount -u " << options.mountDir() << "\n" << std::endl; + std::cout << "\nMounting filesystem. To unmount, call:\n$ fusermount -u " << options.mountDir() << "\n" + << std::endl; #endif - fuse.run(options.mountDir(), options.fuseOptions()); + fuse.run(options.mountDir(), options.fuseOptions()); + } catch (const CryfsException &e) { + throw; // CryfsException is only thrown if setup goes wrong. Throw it through so that we get the correct process exit code. } catch (const std::exception &e) { LOG(ERROR, "Crashed: {}", e.what()); } catch (...) { diff --git a/src/cryfs/ErrorCodes.h b/src/cryfs/ErrorCodes.h index de170ab9..203e8b46 100644 --- a/src/cryfs/ErrorCodes.h +++ b/src/cryfs/ErrorCodes.h @@ -39,6 +39,18 @@ enum class ErrorCode : int { // Something's wrong with the file system. InvalidFilesystem = 19, + + // The filesystem id in the config file is different to the last time we loaded a filesystem from this basedir. This could mean an attacker replaced the file system with a different one. + FilesystemIdChanged = 20, + + // The filesystem encryption key differs from the last time we loaded this filesystem. This could mean an attacker replaced the file system with a different one. + EncryptionKeyChanged = 21, + + // The command line options and the file system disagree on whether missing blocks should be treated as integrity violations. + FilesystemHasDifferentIntegritySetup = 22, + + // File system is in single-client mode and can only be used from the client that created it. + SingleClientFileSystem = 23, }; inline int exitCode(ErrorCode code) { diff --git a/src/cryfs/config/CryConfigLoader.cpp b/src/cryfs/config/CryConfigLoader.cpp index 070eb3cf..b217370e 100644 --- a/src/cryfs/config/CryConfigLoader.cpp +++ b/src/cryfs/config/CryConfigLoader.cpp @@ -79,17 +79,17 @@ void CryConfigLoader::_checkCipher(const CryConfig &config) const { void CryConfigLoader::_checkMissingBlocksAreIntegrityViolations(CryConfigFile *configFile, uint32_t myClientId) { if (_missingBlockIsIntegrityViolationFromCommandLine == optional(true) && configFile->config()->ExclusiveClientId() == none) { - throw std::runtime_error("You specified on the command line to treat missing blocks as integrity violations, but the file system is not setup to do that."); + throw CryfsException("You specified on the command line to treat missing blocks as integrity violations, but the file system is not setup to do that.", ErrorCode::FilesystemHasDifferentIntegritySetup); } if (_missingBlockIsIntegrityViolationFromCommandLine == optional(false) && configFile->config()->ExclusiveClientId() != none) { - throw std::runtime_error("You specified on the command line to not treat missing blocks as integrity violations, but the file system is setup to do that."); + throw CryfsException("You specified on the command line to not treat missing blocks as integrity violations, but the file system is setup to do that.", ErrorCode::FilesystemHasDifferentIntegritySetup); } // If the file system is set up to treat missing blocks as integrity violations, but we're accessing from a different client, ask whether they want to disable the feature. auto exclusiveClientId = configFile->config()->ExclusiveClientId(); if (exclusiveClientId != none && *exclusiveClientId != myClientId) { if (!_console->askYesNo("\nThis filesystem is setup to treat missing blocks as integrity violations and therefore only works in single-client mode. You are trying to access it from a different client.\nDo you want to disable this integrity feature and stop treating missing blocks as integrity violations?\nChoosing yes will not affect the confidentiality of your data, but in future you might not notice if an attacker deletes one of your files.", false)) { - throw std::runtime_error("File system is in single-client mode and can only be used from the client that created it."); + throw CryfsException("File system is in single-client mode and can only be used from the client that created it.", ErrorCode::SingleClientFileSystem); } configFile->config()->SetExclusiveClientId(none); configFile->save(); diff --git a/src/cryfs/localstate/LocalStateMetadata.cpp b/src/cryfs/localstate/LocalStateMetadata.cpp index ca221eac..cf877f40 100644 --- a/src/cryfs/localstate/LocalStateMetadata.cpp +++ b/src/cryfs/localstate/LocalStateMetadata.cpp @@ -4,6 +4,7 @@ #include #include #include +#include using boost::optional; using boost::none; @@ -35,7 +36,7 @@ LocalStateMetadata LocalStateMetadata::loadOrGenerate(const bf::path &statePath, } if (loaded->_encryptionKeyHash.digest != cpputils::hash::hash(encryptionKey, loaded->_encryptionKeyHash.salt).digest) { - throw std::runtime_error("The filesystem encryption key differs from the last time we loaded this filesystem. Did an attacker replace the file system?"); + throw CryfsException("The filesystem encryption key differs from the last time we loaded this filesystem. Did an attacker replace the file system?", ErrorCode::EncryptionKeyChanged); } return *loaded; } diff --git a/test/cryfs-cli/CliTest_IntegrityCheck.cpp b/test/cryfs-cli/CliTest_IntegrityCheck.cpp index 24c2641a..840ebe62 100644 --- a/test/cryfs-cli/CliTest_IntegrityCheck.cpp +++ b/test/cryfs-cli/CliTest_IntegrityCheck.cpp @@ -1,10 +1,12 @@ #include "testutils/CliTest.h" #include +#include using std::vector; using std::string; using cryfs::CryConfig; using cryfs::CryConfigFile; +using cryfs::ErrorCode; class CliTest_IntegrityCheck: public CliTest { public: @@ -28,7 +30,8 @@ TEST_F(CliTest_IntegrityCheck, givenIncorrectFilesystemId_thenFails) { modifyFilesystemId(); EXPECT_RUN_ERROR( args, - "Error: The filesystem id in the config file is different to the last time we loaded a filesystem from this basedir." + "Error: The filesystem id in the config file is different to the last time we loaded a filesystem from this basedir.", + ErrorCode::FilesystemIdChanged ); } @@ -39,6 +42,7 @@ TEST_F(CliTest_IntegrityCheck, givenIncorrectFilesystemKey_thenFails) { modifyFilesystemKey(); EXPECT_RUN_ERROR( args, - "Error: The filesystem encryption key differs from the last time we loaded this filesystem. Did an attacker replace the file system?" + "Error: The filesystem encryption key differs from the last time we loaded this filesystem. Did an attacker replace the file system?", + ErrorCode::EncryptionKeyChanged ); }