From 0b7182f095579ff500ec7ecb5f14f537ec606f58 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Tue, 3 Nov 2015 20:27:00 -0800 Subject: [PATCH] Make test cases faster by using SCrypt::TestSettings --- src/Cli.cpp | 10 ++++++--- src/Cli.h | 3 ++- src/config/CryConfigFile.cpp | 11 ++++++++++ src/config/CryConfigFile.h | 14 +----------- src/config/CryConfigLoader.cpp | 22 +++++++++++++++++-- src/config/CryConfigLoader.h | 22 ++----------------- src/config/crypto/CryConfigEncryptorFactory.h | 10 ++++----- src/main.cpp | 7 ++++-- test/cli/testutils/CliTest.h | 6 ++--- test/config/CryConfigFileTest.cpp | 6 ++--- test/config/CryConfigLoaderTest.cpp | 14 ++++++------ test/filesystem/CryFsTest.cpp | 4 ++-- test/filesystem/FileSystemTest.cpp | 7 +++--- 13 files changed, 71 insertions(+), 65 deletions(-) diff --git a/src/Cli.cpp b/src/Cli.cpp index 7e0e2924..15ed7663 100644 --- a/src/Cli.cpp +++ b/src/Cli.cpp @@ -39,6 +39,9 @@ using cpputils::make_unique_ref; using cpputils::Random; using cpputils::IOStreamConsole; using cpputils::TempFile; +using cpputils::RandomGenerator; +using cpputils::unique_ref; +using cpputils::SCryptSettings; using std::cout; using std::string; using std::endl; @@ -52,11 +55,12 @@ using boost::none; //TODO Did deadlock in bonnie++ second run (in the create files sequentially) - maybe also in a later run or different step? //TODO Improve error message when root blob wasn't found. //TODO Replace ASSERTs with other error handling when it is not a programming error but an environment influence (e.g. a block is missing) -//TODO Fuse error messages like "fuse: bad mount point `...': Transport endpoint is not connected" go missing when running in background +//TODO Test cases fail indeterministically. Fix! namespace cryfs { - Cli::Cli(cpputils::RandomGenerator &keyGenerator): _keyGenerator(keyGenerator) {} + Cli::Cli(RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings): + _keyGenerator(keyGenerator), _scryptSettings(scryptSettings) {} void Cli::_showVersion() { cout << "CryFS Version " << version::VERSION_STRING << endl; @@ -122,7 +126,7 @@ namespace cryfs { auto configFile = _determineConfigFile(options); auto console = make_unique_ref(); std::cout << "Loading config file..." << std::endl; - auto config = CryConfigLoader(std::move(console), _keyGenerator, std::bind(&Cli::_getPassword, this, std::cref(options)), options.cipher()).loadOrCreate(configFile); + auto config = CryConfigLoader(std::move(console), _keyGenerator, _scryptSettings, std::bind(&Cli::_getPassword, this, std::cref(options)), options.cipher()).loadOrCreate(configFile); std::cout << "Loading config file...done" << std::endl; if (config == none) { std::cerr << "Could not load config file. Did you enter the correct password?" << std::endl; diff --git a/src/Cli.h b/src/Cli.h index 2597ed19..552e22fe 100644 --- a/src/Cli.h +++ b/src/Cli.h @@ -11,7 +11,7 @@ namespace cryfs { class Cli final { public: - Cli(cpputils::RandomGenerator &keyGenerator); + Cli(cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings); int main(int argc, char *argv[]); private: @@ -31,6 +31,7 @@ namespace cryfs { void _checkDirReadable(const boost::filesystem::path &dir, std::shared_ptr tempfile, const std::string &name); cpputils::RandomGenerator &_keyGenerator; + cpputils::SCryptSettings _scryptSettings; }; } diff --git a/src/config/CryConfigFile.cpp b/src/config/CryConfigFile.cpp index 8fb59b83..c66a9900 100644 --- a/src/config/CryConfigFile.cpp +++ b/src/config/CryConfigFile.cpp @@ -15,6 +15,7 @@ using std::stringstream; using std::istream; using cpputils::Data; using cpputils::unique_ref; +using cpputils::SCryptSettings; namespace bf = boost::filesystem; using namespace cpputils::logging; @@ -42,6 +43,16 @@ optional CryConfigFile::load(const bf::path &path, const string & return CryConfigFile(path, std::move(config), std::move(*encryptor)); } +CryConfigFile CryConfigFile::create(const bf::path &path, CryConfig config, const string &password, const SCryptSettings &scryptSettings) { + using ConfigCipher = cpputils::AES256_GCM; // TODO Take cipher from config instead + if (bf::exists(path)) { + throw std::runtime_error("Config file exists already."); + } + auto result = CryConfigFile(path, std::move(config), CryConfigEncryptorFactory::deriveKey(password, scryptSettings)); + result.save(); + return result; +} + CryConfigFile::CryConfigFile(const bf::path &path, CryConfig config, unique_ref encryptor) : _path (path), _config(std::move(config)), _encryptor(std::move(encryptor)) { } diff --git a/src/config/CryConfigFile.h b/src/config/CryConfigFile.h index 18748d68..49978797 100644 --- a/src/config/CryConfigFile.h +++ b/src/config/CryConfigFile.h @@ -14,8 +14,7 @@ namespace cryfs { CryConfigFile(CryConfigFile &&rhs) = default; ~CryConfigFile(); - template - static CryConfigFile create(const boost::filesystem::path &path, CryConfig config, const std::string &password); + static CryConfigFile create(const boost::filesystem::path &path, CryConfig config, const std::string &password, const cpputils::SCryptSettings &scryptSettings); static boost::optional load(const boost::filesystem::path &path, const std::string &password); void save() const; @@ -30,17 +29,6 @@ namespace cryfs { DISALLOW_COPY_AND_ASSIGN(CryConfigFile); }; - - template - CryConfigFile CryConfigFile::create(const boost::filesystem::path &path, CryConfig config, const std::string &password) { - using ConfigCipher = cpputils::AES256_GCM; // TODO Take cipher from config instead - if (boost::filesystem::exists(path)) { - throw std::runtime_error("Config file exists already."); - } - auto result = CryConfigFile(path, std::move(config), CryConfigEncryptorFactory::deriveKey(password)); - result.save(); - return result; - } } #endif diff --git a/src/config/CryConfigLoader.cpp b/src/config/CryConfigLoader.cpp index a47299a5..b8b5f094 100644 --- a/src/config/CryConfigLoader.cpp +++ b/src/config/CryConfigLoader.cpp @@ -11,17 +11,19 @@ using cpputils::Console; using cpputils::IOStreamConsole; using cpputils::Random; using cpputils::RandomGenerator; +using cpputils::SCryptSettings; using boost::optional; using boost::none; using std::vector; using std::string; using std::function; +using std::shared_ptr; using namespace cpputils::logging; namespace cryfs { -CryConfigLoader::CryConfigLoader(unique_ref console, RandomGenerator &keyGenerator, function askPassword, const optional &cipher) - : _creator(std::move(console), keyGenerator), _askPassword(askPassword), _cipher(cipher) { +CryConfigLoader::CryConfigLoader(unique_ref console, RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings, function askPassword, const optional &cipher) + : _creator(std::move(console), keyGenerator), _scryptSettings(scryptSettings), _askPassword(askPassword), _cipher(cipher) { } optional CryConfigLoader::_loadConfig(const bf::path &filename) { @@ -37,4 +39,20 @@ optional CryConfigLoader::_loadConfig(const bf::path &filename) { return std::move(*config); } +optional CryConfigLoader::loadOrCreate(const bf::path &filename) { + if (bf::exists(filename)) { + return _loadConfig(filename); + } else { + return _createConfig(filename); + } +} + +CryConfigFile CryConfigLoader::_createConfig(const bf::path &filename) { + auto config = _creator.create(_cipher); + //TODO Ask confirmation if using insecure password (<8 characters) + string password = _askPassword(); + return CryConfigFile::create(filename, std::move(config), password, _scryptSettings); +} + + } diff --git a/src/config/CryConfigLoader.h b/src/config/CryConfigLoader.h index 062495ec..6398848c 100644 --- a/src/config/CryConfigLoader.h +++ b/src/config/CryConfigLoader.h @@ -13,41 +13,23 @@ namespace cryfs { class CryConfigLoader { public: - CryConfigLoader(cpputils::unique_ref console, cpputils::RandomGenerator &keyGenerator, std::function askPassword, const boost::optional &cipher); + CryConfigLoader(cpputils::unique_ref console, cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings, std::function askPassword, const boost::optional &cipher); CryConfigLoader(CryConfigLoader &&rhs) = default; - template boost::optional loadOrCreate(const boost::filesystem::path &filename); private: boost::optional _loadConfig(const boost::filesystem::path &filename); - template CryConfigFile _createConfig(const boost::filesystem::path &filename); CryConfigCreator _creator; + cpputils::SCryptSettings _scryptSettings; std::function _askPassword; boost::optional _cipher; DISALLOW_COPY_AND_ASSIGN(CryConfigLoader); }; -template -boost::optional CryConfigLoader::loadOrCreate(const boost::filesystem::path &filename) { - if (boost::filesystem::exists(filename)) { - return _loadConfig(filename); - } else { - return _createConfig(filename); - } -} - -template -CryConfigFile CryConfigLoader::_createConfig(const boost::filesystem::path &filename) { - auto config = _creator.create(_cipher); - //TODO Ask confirmation if using insecure password (<8 characters) - std::string password = _askPassword(); - return CryConfigFile::create(filename, std::move(config), password); -} - } #endif diff --git a/src/config/crypto/CryConfigEncryptorFactory.h b/src/config/crypto/CryConfigEncryptorFactory.h index a45aa01d..b1d7135c 100644 --- a/src/config/crypto/CryConfigEncryptorFactory.h +++ b/src/config/crypto/CryConfigEncryptorFactory.h @@ -11,8 +11,8 @@ namespace cryfs { //TODO Test class CryConfigEncryptorFactory { public: - template - static cpputils::unique_ref deriveKey(const std::string &password); + template + static cpputils::unique_ref deriveKey(const std::string &password, const cpputils::SCryptSettings &scryptSettings); static boost::optional > loadKey(const cpputils::Data &ciphertext, const std::string &password); @@ -30,9 +30,9 @@ namespace cryfs { return OuterKeySize + Cipher::EncryptionKey::BINARY_LENGTH; } - template - cpputils::unique_ref CryConfigEncryptorFactory::deriveKey(const std::string &password) { - auto derivedKey = cpputils::SCrypt().generateKey(), SCryptConfig>(password); + template + cpputils::unique_ref CryConfigEncryptorFactory::deriveKey(const std::string &password, const cpputils::SCryptSettings &scryptSettings) { + auto derivedKey = cpputils::SCrypt().generateKey()>(password, scryptSettings); auto outerKey = derivedKey.key().template take(); auto innerKey = derivedKey.key().template drop(); return cpputils::make_unique_ref( diff --git a/src/main.cpp b/src/main.cpp index c93baca2..62c11a7a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,9 +1,12 @@ #include "Cli.h" #include +#include using namespace cryfs; +using cpputils::Random; +using cpputils::SCrypt; int main(int argc, char *argv[]) { - auto &keyGenerator = cpputils::Random::OSRandom(); - return Cli(keyGenerator).main(argc, argv); + auto &keyGenerator = Random::OSRandom(); + return Cli(keyGenerator, SCrypt::DefaultSettings).main(argc, argv); } diff --git a/test/cli/testutils/CliTest.h b/test/cli/testutils/CliTest.h index 4ee392ce..18e1ff0d 100644 --- a/test/cli/testutils/CliTest.h +++ b/test/cli/testutils/CliTest.h @@ -29,7 +29,7 @@ public: _args.push_back(const_cast(arg)); } auto &keyGenerator = cpputils::Random::PseudoRandom(); - cryfs::Cli(keyGenerator).main(_args.size(), _args.data()); + cryfs::Cli(keyGenerator, cpputils::SCrypt::TestSettings).main(_args.size(), _args.data()); } void EXPECT_EXIT_WITH_HELP_MESSAGE(std::vector args) { @@ -54,10 +54,10 @@ public: std::this_thread::sleep_for(std::chrono::milliseconds(50)); // TODO Is this the test case duration? Does a shorter interval make the test case faster? } }); - //testing::internal::CaptureStdout(); + testing::internal::CaptureStdout(); run(args); unmountThread.join(); - //EXPECT_THAT(testing::internal::GetCapturedStdout(), testing::MatchesRegex(".*Mounting filesystem.*")); + EXPECT_THAT(testing::internal::GetCapturedStdout(), testing::MatchesRegex(".*Mounting filesystem.*")); } }; diff --git a/test/config/CryConfigFileTest.cpp b/test/config/CryConfigFileTest.cpp index b585004d..0e0a14e3 100644 --- a/test/config/CryConfigFileTest.cpp +++ b/test/config/CryConfigFileTest.cpp @@ -3,13 +3,13 @@ #include "../../src/config/CryConfigFile.h" #include #include -#include using namespace cryfs; using cpputils::TempFile; using std::string; using boost::optional; using boost::none; +using cpputils::SCrypt; namespace bf = boost::filesystem; //gtest/boost::optional workaround for working with optional @@ -33,7 +33,7 @@ public: } void Create(CryConfig cfg, const string &password = "mypassword") { - CryConfigFile::create(file.path(), std::move(cfg), password); + CryConfigFile::create(file.path(), std::move(cfg), password, SCrypt::TestSettings); } optional Load(const string &password = "mypassword") { @@ -43,7 +43,7 @@ public: void CreateWithCipher(const string &cipher, const TempFile &tempFile) { CryConfig cfg; cfg.SetCipher(cipher); - CryConfigFile::create(tempFile.path(), std::move(cfg), "mypassword"); + CryConfigFile::create(tempFile.path(), std::move(cfg), "mypassword", SCrypt::TestSettings); } }; diff --git a/test/config/CryConfigLoaderTest.cpp b/test/config/CryConfigLoaderTest.cpp index 3062906d..52c77f2a 100644 --- a/test/config/CryConfigLoaderTest.cpp +++ b/test/config/CryConfigLoaderTest.cpp @@ -4,11 +4,11 @@ #include #include #include -#include using cpputils::unique_ref; using cpputils::make_unique_ref; using cpputils::TempFile; +using cpputils::SCrypt; using boost::optional; using boost::none; using std::string; @@ -24,33 +24,33 @@ public: CryConfigLoaderTest(): file(false) {} CryConfigLoader loader(const string &password, const optional &cipher = none) { - return CryConfigLoader(mockConsole(), cpputils::Random::PseudoRandom(), [password] {return password;}, cipher); + return CryConfigLoader(mockConsole(), cpputils::Random::PseudoRandom(), SCrypt::TestSettings, [password] {return password;}, cipher); } CryConfigFile Create(const string &password = "mypassword", const optional &cipher = none) { EXPECT_FALSE(file.exists()); - return loader(password, cipher).loadOrCreate(file.path()).value(); + return loader(password, cipher).loadOrCreate(file.path()).value(); } optional Load(const string &password = "mypassword", const optional &cipher = none) { EXPECT_TRUE(file.exists()); - return loader(password, cipher).loadOrCreate(file.path()); + return loader(password, cipher).loadOrCreate(file.path()); } void CreateWithRootBlob(const string &rootBlob, const string &password = "mypassword") { - auto cfg = loader(password).loadOrCreate(file.path()).value(); + auto cfg = loader(password).loadOrCreate(file.path()).value(); cfg.config()->SetRootBlob(rootBlob); cfg.save(); } void CreateWithCipher(const string &cipher, const string &password = "mypassword") { - auto cfg = loader(password).loadOrCreate(file.path()).value(); + auto cfg = loader(password).loadOrCreate(file.path()).value(); cfg.config()->SetCipher(cipher); cfg.save(); } void CreateWithEncryptionKey(const string &encKey, const string &password = "mypassword") { - auto cfg = loader(password).loadOrCreate(file.path()).value(); + auto cfg = loader(password).loadOrCreate(file.path()).value(); cfg.config()->SetEncryptionKey(encKey); cfg.save(); } diff --git a/test/filesystem/CryFsTest.cpp b/test/filesystem/CryFsTest.cpp index 70a07eb4..8f2e66dd 100644 --- a/test/filesystem/CryFsTest.cpp +++ b/test/filesystem/CryFsTest.cpp @@ -9,7 +9,6 @@ #include "../../src/filesystem/CryOpenFile.h" #include "../testutils/MockConsole.h" #include "../../src/config/CryConfigLoader.h" -#include //TODO (whole project) Make constructors explicit when implicit construction not needed @@ -23,6 +22,7 @@ using cpputils::make_unique_ref; using cpputils::unique_ref; using cpputils::Console; using cpputils::Random; +using cpputils::SCrypt; using blockstore::ondisk::OnDiskBlockStore; using boost::none; @@ -35,7 +35,7 @@ public: } CryConfigFile loadOrCreateConfig() { - return CryConfigLoader(mockConsole(), Random::PseudoRandom(), [] {return "mypassword";}, none).loadOrCreate(config.path()).value(); + return CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, [] {return "mypassword";}, none).loadOrCreate(config.path()).value(); } unique_ref blockStore() { diff --git a/test/filesystem/FileSystemTest.cpp b/test/filesystem/FileSystemTest.cpp index 12f027ae..ebfc77f8 100644 --- a/test/filesystem/FileSystemTest.cpp +++ b/test/filesystem/FileSystemTest.cpp @@ -5,12 +5,11 @@ #include "../../src/filesystem/CryDevice.h" #include "../../src/config/CryConfigLoader.h" #include "../testutils/MockConsole.h" -#include using cpputils::unique_ref; using cpputils::make_unique_ref; using cpputils::Random; - +using cpputils::SCrypt; using fspp::Device; using ::testing::Return; using ::testing::_; @@ -28,8 +27,8 @@ public: unique_ref createDevice() override { auto blockStore = cpputils::make_unique_ref(); - auto config = CryConfigLoader(mockConsole(), Random::PseudoRandom(), [] {return "mypassword";}, none) - .loadOrCreate(configFile.path()).value(); + auto config = CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, [] {return "mypassword";}, none) + .loadOrCreate(configFile.path()).value(); return make_unique_ref(std::move(config), std::move(blockStore)); }