From 9bf83a6fe704037939ef25fcd666f7d53d5cc88a Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 22 Oct 2015 18:48:04 +0200 Subject: [PATCH] Adapted to new key creation interface --- src/config/CryCipher.cpp | 4 +-- src/config/CryCipher.h | 3 +- src/config/CryConfigCreator.cpp | 24 +++------------- src/config/CryConfigCreator.h | 9 ++---- src/config/CryConfigLoader.cpp | 21 +++----------- src/config/CryConfigLoader.h | 6 +--- test/config/CryCipherTest.cpp | 12 ++++---- test/filesystem/CryFsTest.cpp | 45 ++++++++++++++---------------- test/filesystem/FileSystemTest.cpp | 13 ++++++++- test/testutils/MockConsole.h | 15 ++++++++++ 10 files changed, 71 insertions(+), 81 deletions(-) create mode 100644 test/testutils/MockConsole.h diff --git a/src/config/CryCipher.cpp b/src/config/CryCipher.cpp index a691e14b..a70136fe 100644 --- a/src/config/CryCipher.cpp +++ b/src/config/CryCipher.cpp @@ -36,8 +36,8 @@ public: return make_unique_ref>(std::move(baseBlockStore), Cipher::EncryptionKey::FromString(encKey)); } - string createKey() const override { - return Cipher::EncryptionKey::CreateOSRandom().ToString(); + string createKey(cpputils::RandomGenerator &randomGenerator) const override { + return Cipher::CreateKey(randomGenerator).ToString(); } private: diff --git a/src/config/CryCipher.h b/src/config/CryCipher.h index 4f43a055..04d36214 100644 --- a/src/config/CryCipher.h +++ b/src/config/CryCipher.h @@ -6,6 +6,7 @@ #include #include #include +#include namespace cryfs { @@ -16,7 +17,7 @@ public: virtual const std::string &cipherName() const = 0; virtual const boost::optional &warning() const = 0; virtual cpputils::unique_ref createEncryptedBlockstore(cpputils::unique_ref baseBlockStore, const std::string &encKey) const = 0; - virtual std::string createKey() const = 0; + virtual std::string createKey(cpputils::RandomGenerator &randomGenerator) const = 0; }; class CryCiphers { diff --git a/src/config/CryConfigCreator.cpp b/src/config/CryConfigCreator.cpp index 056f3e4d..55fdb090 100644 --- a/src/config/CryConfigCreator.cpp +++ b/src/config/CryConfigCreator.cpp @@ -4,13 +4,13 @@ using cpputils::Console; using cpputils::unique_ref; +using cpputils::RandomGenerator; using std::string; using std::vector; - namespace cryfs { - CryConfigCreator::CryConfigCreator(unique_ref console) - :_console(std::move(console)) { + CryConfigCreator::CryConfigCreator(unique_ref console, RandomGenerator &encryptionKeyGenerator) + :_console(std::move(console)), _encryptionKeyGenerator(encryptionKeyGenerator) { } CryConfig CryConfigCreator::create() { @@ -21,14 +21,6 @@ namespace cryfs { return config; } - CryConfig CryConfigCreator::createForTest() { - CryConfig config; - config.SetCipher(_generateCipherForTest()); - config.SetEncryptionKey(_generateEncKeyForTest(config.Cipher())); - config.SetRootBlob(_generateRootBlobKey()); - return config; - } - string CryConfigCreator::_generateCipher() { vector ciphers = CryCiphers::supportedCipherNames(); string cipherName = ""; @@ -51,19 +43,11 @@ namespace cryfs { string CryConfigCreator::_generateEncKey(const std::string &cipher) { _console->print("\nGenerating secure encryption key..."); - auto key = CryCiphers::find(cipher).createKey(); + auto key = CryCiphers::find(cipher).createKey(_encryptionKeyGenerator); _console->print("done\n"); return key; } - string CryConfigCreator::_generateCipherForTest() { - return "aes-256-gcm"; - } - - string CryConfigCreator::_generateEncKeyForTest(const std::string &) { - return blockstore::encrypted::AES256_GCM::EncryptionKey::CreatePseudoRandom().ToString(); - } - string CryConfigCreator::_generateRootBlobKey() { //An empty root blob entry will tell CryDevice to create a new root blob return ""; diff --git a/src/config/CryConfigCreator.h b/src/config/CryConfigCreator.h index 9a97ffbe..efd82960 100644 --- a/src/config/CryConfigCreator.h +++ b/src/config/CryConfigCreator.h @@ -2,27 +2,24 @@ #define CRYFS_SRC_CONFIG_CRYCONFIGCREATOR_H #include +#include #include #include "CryConfig.h" namespace cryfs { class CryConfigCreator final { public: - CryConfigCreator(cpputils::unique_ref console); + CryConfigCreator(cpputils::unique_ref console, cpputils::RandomGenerator &encryptionKeyGenerator); CryConfig create(); - CryConfig createForTest(); private: std::string _generateCipher(); std::string _generateEncKey(const std::string &cipher); std::string _generateRootBlobKey(); bool _showWarningForCipherAndReturnIfOk(const std::string &cipherName); - //TODO Don't have these functions here, but use a CryConfigCreator interface and mock it in the tests - std::string _generateEncKeyForTest(const std::string &cipher); - std::string _generateCipherForTest(); - cpputils::unique_ref _console; + cpputils::RandomGenerator &_encryptionKeyGenerator; DISALLOW_COPY_AND_ASSIGN(CryConfigCreator); }; diff --git a/src/config/CryConfigLoader.cpp b/src/config/CryConfigLoader.cpp index e9ff5ca7..6a037456 100644 --- a/src/config/CryConfigLoader.cpp +++ b/src/config/CryConfigLoader.cpp @@ -1,6 +1,7 @@ #include "CryConfigLoader.h" #include "CryConfigFile.h" #include +#include namespace bf = boost::filesystem; using cpputils::unique_ref; @@ -14,9 +15,10 @@ using std::string; namespace cryfs { -CryConfigLoader::CryConfigLoader(): CryConfigLoader(make_unique_ref()) {} +CryConfigLoader::CryConfigLoader(): CryConfigLoader(make_unique_ref(), cpputils::Random::OSRandom()) {} -CryConfigLoader::CryConfigLoader(unique_ref console) : _creator(std::move(console)) {} +CryConfigLoader::CryConfigLoader(unique_ref console, cpputils::RandomGenerator &keyGenerator) + : _creator(std::move(console), keyGenerator) {} CryConfigFile CryConfigLoader::loadOrCreate(const bf::path &filename) { auto config = CryConfigFile::load(filename); @@ -33,19 +35,4 @@ CryConfigFile CryConfigLoader::createNew(const bf::path &filename) { return configFile; } -CryConfigFile CryConfigLoader::loadOrCreateForTest(const bf::path &filename) { - auto config = CryConfigFile::load(filename); - if (config != none) { - return std::move(*config); - } - return createNewForTest(filename); -} - -CryConfigFile CryConfigLoader::createNewForTest(const bf::path &filename) { - auto config = _creator.createForTest(); - auto configFile = CryConfigFile::create(filename, std::move(config)); - configFile.save(); - return configFile; -} - } diff --git a/src/config/CryConfigLoader.h b/src/config/CryConfigLoader.h index b7abb6d2..abe623d0 100644 --- a/src/config/CryConfigLoader.h +++ b/src/config/CryConfigLoader.h @@ -13,15 +13,11 @@ namespace cryfs { class CryConfigLoader { public: CryConfigLoader(); - explicit CryConfigLoader(cpputils::unique_ref console); + explicit CryConfigLoader(cpputils::unique_ref console, cpputils::RandomGenerator &keyGenerator); CryConfigFile loadOrCreate(const boost::filesystem::path &filename); CryConfigFile createNew(const boost::filesystem::path &filename); - //This methods are only for testing purposes, because creating weak keys is much faster than creating strong keys. - CryConfigFile loadOrCreateForTest(const boost::filesystem::path &filename); - CryConfigFile createNewForTest(const boost::filesystem::path &filename); - private: CryConfigCreator _creator; }; diff --git a/test/config/CryCipherTest.cpp b/test/config/CryCipherTest.cpp index 08998bb0..049e299b 100644 --- a/test/config/CryCipherTest.cpp +++ b/test/config/CryCipherTest.cpp @@ -6,6 +6,7 @@ #include #include #include +#include using namespace cryfs; using namespace blockstore::encrypted; @@ -22,6 +23,7 @@ using cpputils::DataFixture; using cpputils::Data; using cpputils::unique_ref; using cpputils::make_unique_ref; +using cpputils::Random; class CryCipherTest : public ::testing::Test { public: @@ -39,13 +41,13 @@ public: void EXPECT_CREATES_CORRECT_ENCRYPTED_BLOCKSTORE(const string &cipherName) { const auto &actualCipher = CryCiphers::find(cipherName); Data dataFixture = DataFixture::generate(1024); - string encKey = ExpectedCipher::EncryptionKey::CreatePseudoRandom().ToString(); + string encKey = ExpectedCipher::CreateKey(Random::PseudoRandom()).ToString(); _EXPECT_ENCRYPTS_WITH_ACTUAL_BLOCKSTORE_DECRYPTS_CORRECTLY_WITH_EXPECTED_BLOCKSTORE(actualCipher, encKey, std::move(dataFixture)); } template void _EXPECT_ENCRYPTS_WITH_ACTUAL_BLOCKSTORE_DECRYPTS_CORRECTLY_WITH_EXPECTED_BLOCKSTORE(const CryCipher &actualCipher, const std::string &encKey, Data dataFixture) { - blockstore::Key key = blockstore::Key::CreatePseudoRandom(); + blockstore::Key key = cpputils::Random::PseudoRandom().getFixedSize(); Data encrypted = _encryptUsingEncryptedBlockStoreWithCipher(actualCipher, encKey, key, dataFixture.copy()); Data decrypted = _decryptUsingEncryptedBlockStoreWithCipher(encKey, key, std::move(encrypted)); EXPECT_EQ(dataFixture, decrypted); @@ -124,13 +126,13 @@ TEST_F(CryCipherTest, ThereIsACipherWithIntegrityWarning) { } TEST_F(CryCipherTest, EncryptionKeyHasCorrectSize_448) { - EXPECT_EQ(Mars448_GCM::EncryptionKey::STRING_LENGTH, CryCiphers::find("mars-448-gcm").createKey().size()); + EXPECT_EQ(Mars448_GCM::EncryptionKey::STRING_LENGTH, CryCiphers::find("mars-448-gcm").createKey(Random::PseudoRandom()).size()); } TEST_F(CryCipherTest, EncryptionKeyHasCorrectSize_256) { - EXPECT_EQ(AES256_GCM::EncryptionKey::STRING_LENGTH, CryCiphers::find("aes-256-gcm").createKey().size()); + EXPECT_EQ(AES256_GCM::EncryptionKey::STRING_LENGTH, CryCiphers::find("aes-256-gcm").createKey(Random::PseudoRandom()).size()); } TEST_F(CryCipherTest, EncryptionKeyHasCorrectSize_128) { - EXPECT_EQ(AES128_GCM::EncryptionKey::STRING_LENGTH, CryCiphers::find("aes-128-gcm").createKey().size()); + EXPECT_EQ(AES128_GCM::EncryptionKey::STRING_LENGTH, CryCiphers::find("aes-128-gcm").createKey(Random::PseudoRandom()).size()); } diff --git a/test/filesystem/CryFsTest.cpp b/test/filesystem/CryFsTest.cpp index 906c0896..0e6af198 100644 --- a/test/filesystem/CryFsTest.cpp +++ b/test/filesystem/CryFsTest.cpp @@ -7,10 +7,13 @@ #include "../../src/filesystem/CryDir.h" #include "../../src/filesystem/CryFile.h" #include "../../src/filesystem/CryOpenFile.h" +#include "../testutils/MockConsole.h" //TODO (whole project) Make constructors explicit when implicit construction not needed using ::testing::Test; +using ::testing::Return; +using ::testing::_; using cpputils::TempDir; using cpputils::TempFile; using cpputils::dynamic_pointer_move; @@ -22,44 +25,38 @@ using blockstore::ondisk::OnDiskBlockStore; namespace bf = boost::filesystem; using namespace cryfs; -class MockConsole: public Console { - void print(const std::string &) override {} - unsigned int ask(const std::string &, const std::vector &) override { - return 0; - } - bool askYesNo(const std::string &) override { - return true; - } -}; - class CryFsTest: public Test { public: - CryFsTest(): rootdir(), config(false) {} + CryFsTest(): rootdir(), config(false) { + } + unique_ref mockConsole() { + auto console = make_unique_ref(); + EXPECT_CALL(*console, ask(_, _)).WillRepeatedly(Return(0)); + EXPECT_CALL(*console, askYesNo(_)).WillRepeatedly(Return(true)); + return console; + } TempDir rootdir; TempFile config; }; -TEST_F(CryFsTest, CreatedRootdirIsLoadableAfterClosing) { +TEST_F(CryFsTest, CreatedRootdirIsLoadableAfterClosing_1) { { - CryDevice dev(CryConfigLoader().createNewForTest(config.path()), make_unique_ref(rootdir.path())); + CryDevice dev( + CryConfigLoader(mockConsole(), cpputils::Random::PseudoRandom()) + .createNew(config.path()), make_unique_ref(rootdir.path()) + ); } CryDevice dev(CryConfigFile::load(config.path()).value(), make_unique_ref(rootdir.path())); auto root = dev.Load(bf::path("/")); dynamic_pointer_move(root.get()).get()->children(); } -TEST_F(CryFsTest, UsingStrongKey1_CreatedRootdirIsLoadableAfterClosing) { +TEST_F(CryFsTest, CreatedRootdirIsLoadableAfterClosing_2) { { - CryDevice dev(CryConfigLoader(make_unique_ref()).createNew(config.path()), make_unique_ref(rootdir.path())); - } - CryDevice dev(CryConfigFile::load(config.path()).value(), make_unique_ref(rootdir.path())); - auto root = dev.Load(bf::path("/")); - dynamic_pointer_move(root.get()).get()->children(); -} - -TEST_F(CryFsTest, UsingStrongKey2_CreatedRootdirIsLoadableAfterClosing) { - { - CryDevice dev(CryConfigLoader(make_unique_ref()).loadOrCreate(config.path()), make_unique_ref(rootdir.path())); + CryDevice dev( + CryConfigLoader(mockConsole(), cpputils::Random::PseudoRandom()) + .loadOrCreate(config.path()), make_unique_ref(rootdir.path()) + ); } CryDevice dev(CryConfigLoader().loadOrCreate(config.path()), make_unique_ref(rootdir.path())); auto root = dev.Load(bf::path("/")); diff --git a/test/filesystem/FileSystemTest.cpp b/test/filesystem/FileSystemTest.cpp index 736c87c4..ef433b0a 100644 --- a/test/filesystem/FileSystemTest.cpp +++ b/test/filesystem/FileSystemTest.cpp @@ -3,11 +3,14 @@ #include #include "../../src/filesystem/CryDevice.h" +#include "../testutils/MockConsole.h" using cpputils::unique_ref; using cpputils::make_unique_ref; using fspp::Device; +using ::testing::Return; +using ::testing::_; using blockstore::testfake::FakeBlockStore; @@ -21,10 +24,18 @@ public: unique_ref createDevice() override { auto blockStore = cpputils::make_unique_ref(); - auto config = CryConfigLoader().loadOrCreateForTest(configFile.path()); + auto config = CryConfigLoader(mockConsole(), cpputils::Random::PseudoRandom()) + .loadOrCreate(configFile.path()); return make_unique_ref(std::move(config), std::move(blockStore)); } + unique_ref mockConsole() { + auto console = make_unique_ref(); + EXPECT_CALL(*console, ask(_, _)).WillRepeatedly(Return(0)); + EXPECT_CALL(*console, askYesNo(_)).WillRepeatedly(Return(true)); + return console; + } + cpputils::TempFile configFile; }; diff --git a/test/testutils/MockConsole.h b/test/testutils/MockConsole.h new file mode 100644 index 00000000..3f4c9f3b --- /dev/null +++ b/test/testutils/MockConsole.h @@ -0,0 +1,15 @@ +#pragma once +#ifndef MESSMER_CRYFS_TEST_TESTUTILS_MOCKCONSOLE_H +#define MESSMER_CRYFS_TEST_TESTUTILS_MOCKCONSOLE_H + +#include +#include + +class MockConsole: public cpputils::Console { +public: + MOCK_METHOD1(print, void(const std::string&)); + MOCK_METHOD2(ask, unsigned int(const std::string&, const std::vector&)); + MOCK_METHOD1(askYesNo, bool(const std::string&)); +}; + +#endif