Merge branch 'feature/cleanup_keyprovider' into develop

src/cryfs/CMakeLists.txt

@@ -18,6 +18,7 @@ set(LIB_SOURCES
         config/CryConfigCreator.cpp
         config/CryKeyProvider.cpp
         config/CryPasswordBasedKeyProvider.cpp
+        config/CryPresetPasswordBasedKeyProvider.cpp
         filesystem/CryOpenFile.cpp
         filesystem/fsblobstore/utils/DirEntry.cpp
         filesystem/fsblobstore/utils/DirEntryList.cpp

src/cryfs/config/CryPasswordBasedKeyProvider.h

@@ -4,14 +4,14 @@
 
 #include "CryKeyProvider.h"
 #include <functional>
-#include <cpp-utils/crypto/kdf/Scrypt.h>
+#include <cpp-utils/crypto/kdf/PasswordBasedKDF.h>
 #include <cpp-utils/io/Console.h>
 
 namespace cryfs {
 
+// TODO Remove duplication with CryPresetPasswordBasedKeyProvider
 class CryPasswordBasedKeyProvider final : public CryKeyProvider {
 public:
-  // TODO Pass in KDF as dependency (needs changes in the KDF interface because of the static functions ::forNewKey and ::forExistingKey)
   explicit CryPasswordBasedKeyProvider(std::shared_ptr<cpputils::Console> console, std::function<std::string()> askPasswordForExistingFilesystem, std::function<std::string()> askPasswordForNewFilesystem, cpputils::unique_ref<cpputils::PasswordBasedKDF> kdf);
 
   cpputils::EncryptionKey requestKeyForExistingFilesystem(size_t keySize, const cpputils::Data& kdfParameters) override;

src/cryfs/config/CryPresetPasswordBasedKeyProvider.cpp

@@ -0,0 +1,23 @@
+#include "CryPresetPasswordBasedKeyProvider.h"
+
+using cpputils::unique_ref;
+using cpputils::EncryptionKey;
+using cpputils::unique_ref;
+using cpputils::PasswordBasedKDF;
+using cpputils::Data;
+
+namespace cryfs {
+
+CryPresetPasswordBasedKeyProvider::CryPresetPasswordBasedKeyProvider(std::string password, unique_ref<PasswordBasedKDF> kdf)
+: _password(std::move(password)), _kdf(std::move(kdf)) {}
+
+EncryptionKey CryPresetPasswordBasedKeyProvider::requestKeyForExistingFilesystem(size_t keySize, const Data& kdfParameters) {
+    return _kdf->deriveExistingKey(keySize, _password, kdfParameters);
+}
+
+CryPresetPasswordBasedKeyProvider::KeyResult CryPresetPasswordBasedKeyProvider::requestKeyForNewFilesystem(size_t keySize) {
+    auto keyResult = _kdf->deriveNewKey(keySize, _password);
+    return {std::move(keyResult.key), std::move(keyResult.kdfParameters)};
+}
+
+}

src/cryfs/config/CryPresetPasswordBasedKeyProvider.h

@@ -0,0 +1,27 @@
+#pragma once
+#ifndef CRYFS_CRYPRESETPASSWORDFROMCONSOLEKEYPROVIDER_H
+#define CRYFS_CRYPRESETPASSWORDFROMCONSOLEKEYPROVIDER_H
+
+#include "CryKeyProvider.h"
+#include <functional>
+#include <cpp-utils/crypto/kdf/PasswordBasedKDF.h>
+
+namespace cryfs {
+
+    class CryPresetPasswordBasedKeyProvider final : public CryKeyProvider {
+    public:
+        explicit CryPresetPasswordBasedKeyProvider(std::string password, cpputils::unique_ref<cpputils::PasswordBasedKDF> kdf);
+
+        cpputils::EncryptionKey requestKeyForExistingFilesystem(size_t keySize, const cpputils::Data& kdfParameters) override;
+        KeyResult requestKeyForNewFilesystem(size_t keySize) override;
+
+    private:
+        std::string _password;
+        cpputils::unique_ref<cpputils::PasswordBasedKDF> _kdf;
+
+        DISALLOW_COPY_AND_ASSIGN(CryPresetPasswordBasedKeyProvider);
+    };
+
+}
+
+#endif

test/cryfs/CMakeLists.txt

@@ -15,6 +15,7 @@ set(SOURCES
     config/CryConfigLoaderTest.cpp
     config/CryConfigConsoleTest.cpp
     config/CryPasswordBasedKeyProviderTest.cpp
+    config/CryPresetPasswordBasedKeyProviderTest.cpp
     filesystem/CryFsTest.cpp
     filesystem/CryNodeTest.cpp
     filesystem/FileSystemTest.cpp

test/cryfs/config/CompatibilityTest.cpp

@@ -7,7 +7,7 @@
 #include <cpp-utils/crypto/symmetric/ciphers.h>
 #include <cpp-utils/tempfile/TempFile.h>
 #include <cryfs/config/CryConfigFile.h>
-#include <cryfs/config/CryPasswordBasedKeyProvider.h>
+#include <cryfs/config/CryPresetPasswordBasedKeyProvider.h>
 #include "../testutils/MockConsole.h"
 
 using cpputils::Data;
@@ -16,7 +16,6 @@ using cpputils::Serpent128_CFB;
 using cpputils::TempFile;
 using cpputils::make_unique_ref;
 using cpputils::SCrypt;
-using std::make_shared;
 using namespace cryfs;
 
 // Test that config files created with (old) versions of cryfs are still loadable.
@@ -28,12 +27,7 @@ public:
 
     CryConfigFile loadConfigFromHex(const string &configFileContentHex) {
         storeHexToFile(configFileContentHex);
-        CryPasswordBasedKeyProvider keyProvider(
+        CryPresetPasswordBasedKeyProvider keyProvider("mypassword", make_unique_ref<SCrypt>(SCrypt::DefaultSettings));
-            make_shared<MockConsole>(),
-            [] () {return "mypassword"; },
-            [] () {return "mypassword"; },
-            make_unique_ref<SCrypt>(SCrypt::DefaultSettings)
-        );
         return CryConfigFile::load(file.path(), &keyProvider).value();
     }
 

test/cryfs/config/CryConfigLoaderTest.cpp

@@ -1,6 +1,6 @@
 #include <gtest/gtest.h>
 #include <cryfs/config/CryConfigLoader.h>
-#include <cryfs/config/CryPasswordBasedKeyProvider.h>
+#include <cryfs/config/CryPresetPasswordBasedKeyProvider.h>
 #include "../testutils/MockConsole.h"
 #include "../testutils/TestWithFakeHomeDirectory.h"
 #include <cpp-utils/tempfile/TempFile.h>
@@ -20,7 +20,7 @@ using cpputils::NoninteractiveConsole;
 using cpputils::make_unique_ref;
 using cpputils::Console;
 using cpputils::unique_ref;
-using cryfs::CryPasswordBasedKeyProvider;
+using cryfs::CryPresetPasswordBasedKeyProvider;
 using boost::optional;
 using boost::none;
 using std::string;
@@ -63,13 +63,7 @@ private:
 class CryConfigLoaderTest: public ::testing::Test, public TestWithMockConsole, TestWithFakeHomeDirectory {
 public:
     unique_ref<CryKeyProvider> keyProvider(const string& password) {
-      auto askPassword = [password] { return password;};
+      return make_unique_ref<CryPresetPasswordBasedKeyProvider>(password, make_unique_ref<SCrypt>(SCrypt::TestSettings));
-      return make_unique_ref<CryPasswordBasedKeyProvider>(
-          console,
-          askPassword,
-          askPassword,
-          make_unique_ref<SCrypt>(SCrypt::TestSettings)
-      );
     }
 
     CryConfigLoaderTest(): file(false), tempLocalStateDir(), localStateDir(tempLocalStateDir.path()) {

test/cryfs/config/CryPresetPasswordBasedKeyProviderTest.cpp

@@ -0,0 +1,56 @@
+#include <cryfs/config/CryPresetPasswordBasedKeyProvider.h>
+#include <gmock/gmock.h>
+#include "../testutils/MockConsole.h"
+#include <cpp-utils/data/DataFixture.h>
+
+using cpputils::make_unique_ref;
+using cpputils::EncryptionKey;
+using cpputils::PasswordBasedKDF;
+using cpputils::Data;
+using cpputils::DataFixture;
+using std::string;
+using cryfs::CryPresetPasswordBasedKeyProvider;
+using testing::Invoke;
+using testing::Eq;
+using testing::StrEq;
+using testing::_;
+
+class MockKDF : public PasswordBasedKDF {
+public:
+    MOCK_METHOD3(deriveExistingKey, EncryptionKey(size_t keySize, const string& password, const Data& kdfParameters));
+    MOCK_METHOD2(deriveNewKey, KeyResult(size_t keySize, const string& password));
+};
+
+TEST(CryPresetPasswordBasedKeyProviderTest, requestKeyForNewFilesystem) {
+    constexpr size_t keySize = 512;
+    constexpr const char* password = "mypassword";
+    const EncryptionKey key = EncryptionKey::FromString(DataFixture::generate(keySize).ToString());
+    auto kdf = make_unique_ref<MockKDF>();
+    const Data kdfParameters = DataFixture::generate(100);
+
+    EXPECT_CALL(*kdf, deriveNewKey(Eq(keySize), StrEq(password))).Times(1).WillOnce(Invoke([&] (auto, auto) {return PasswordBasedKDF::KeyResult{key, kdfParameters.copy()};}));
+
+    CryPresetPasswordBasedKeyProvider keyProvider(password, std::move(kdf));
+    auto returned_key = keyProvider.requestKeyForNewFilesystem(keySize);
+
+    EXPECT_EQ(key.ToString(), returned_key.key.ToString());
+    EXPECT_EQ(kdfParameters, returned_key.kdfParameters);
+}
+
+TEST(CryPresetPasswordBasedKeyProviderTest, requestKeyForExistingFilesystem) {
+    constexpr size_t keySize = 512;
+    constexpr const char* password = "mypassword";
+    const EncryptionKey key = EncryptionKey::FromString(DataFixture::generate(keySize).ToString());
+    auto kdf = make_unique_ref<MockKDF>();
+    const Data kdfParameters = DataFixture::generate(100);
+
+    EXPECT_CALL(*kdf, deriveExistingKey(Eq(keySize), StrEq(password), _)).Times(1).WillOnce(Invoke([&] (auto, auto, const auto& kdfParams) {
+        EXPECT_EQ(kdfParameters, kdfParams);
+        return key;
+    }));
+
+    CryPresetPasswordBasedKeyProvider keyProvider(password, std::move(kdf));
+    EncryptionKey returned_key = keyProvider.requestKeyForExistingFilesystem(keySize, kdfParameters);
+
+    EXPECT_EQ(key.ToString(), returned_key.ToString());
+}

test/cryfs/filesystem/CryFsTest.cpp

@@ -9,7 +9,7 @@
 #include <cryfs/filesystem/CryOpenFile.h>
 #include "../testutils/MockConsole.h"
 #include <cryfs/config/CryConfigLoader.h>
-#include <cryfs/config/CryPasswordBasedKeyProvider.h>
+#include <cryfs/config/CryPresetPasswordBasedKeyProvider.h>
 #include <cpp-utils/system/homedir.h>
 #include "../testutils/TestWithFakeHomeDirectory.h"
 #include <cpp-utils/io/NoninteractiveConsole.h>
@@ -28,7 +28,7 @@ using cpputils::Data;
 using cpputils::NoninteractiveConsole;
 using blockstore::ondisk::OnDiskBlockStore2;
 using boost::none;
-using cryfs::CryPasswordBasedKeyProvider;
+using cryfs::CryPresetPasswordBasedKeyProvider;
 
 namespace bf = boost::filesystem;
 using namespace cryfs;
@@ -39,13 +39,7 @@ public:
   }
 
   CryConfigFile loadOrCreateConfig() {
-    auto askPassword = [] {return "mypassword";};
+    auto keyProvider = make_unique_ref<CryPresetPasswordBasedKeyProvider>("mypassword", make_unique_ref<SCrypt>(SCrypt::TestSettings));
-    auto keyProvider = make_unique_ref<CryPasswordBasedKeyProvider>(
-        make_shared<MockConsole>(),
-        askPassword,
-        askPassword,
-        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;
   }
 

test/cryfs/filesystem/FileSystemTest.cpp

@@ -4,7 +4,7 @@
 #include <cpp-utils/io/NoninteractiveConsole.h>
 #include <cryfs/filesystem/CryDevice.h>
 #include <cryfs/config/CryConfigLoader.h>
-#include <cryfs/config/CryPasswordBasedKeyProvider.h>
+#include <cryfs/config/CryPresetPasswordBasedKeyProvider.h>
 #include "../testutils/MockConsole.h"
 #include "../testutils/TestWithFakeHomeDirectory.h"
 
@@ -17,7 +17,7 @@ using fspp::Device;
 using boost::none;
 using std::make_shared;
 using blockstore::inmemory::InMemoryBlockStore2;
-using cryfs::CryPasswordBasedKeyProvider;
+using cryfs::CryPresetPasswordBasedKeyProvider;
 
 using namespace cryfs;
 
@@ -29,14 +29,8 @@ public:
 
   unique_ref<Device> createDevice() override {
     auto blockStore = cpputils::make_unique_ref<InMemoryBlockStore2>();
-    auto askPassword = [] {return "mypassword";};
     auto _console = make_shared<NoninteractiveConsole>(mockConsole());
-    auto keyProvider = make_unique_ref<CryPasswordBasedKeyProvider>(
+    auto keyProvider = make_unique_ref<CryPresetPasswordBasedKeyProvider>("mypassword", make_unique_ref<SCrypt>(SCrypt::TestSettings));
-        _console,
-        askPassword,
-        askPassword,
-        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();
     return make_unique_ref<CryDevice>(std::move(config.configFile), std::move(blockStore), localStateDir, config.myClientId, false, false);

test/cryfs/filesystem/testutils/CryTestBase.h

@@ -2,7 +2,7 @@
 #define MESSMER_CRYFS_TEST_CRYFS_FILESYSTEM_CRYTESTBASE_H
 
 #include <cryfs/filesystem/CryDevice.h>
-#include <cryfs/config/CryPasswordBasedKeyProvider.h>
+#include <cryfs/config/CryPresetPasswordBasedKeyProvider.h>
 #include <blockstore/implementations/inmemory/InMemoryBlockStore2.h>
 #include <cpp-utils/tempfile/TempFile.h>
 #include <cpp-utils/crypto/kdf/Scrypt.h>
@@ -21,12 +21,7 @@ public:
         config.SetCipher("aes-256-gcm");
         config.SetEncryptionKey(cpputils::AES256_GCM::EncryptionKey::CreateKey(cpputils::Random::PseudoRandom(), cpputils::AES256_GCM::KEYSIZE).ToString());
         config.SetBlocksizeBytes(10240);
-        cryfs::CryPasswordBasedKeyProvider keyProvider(
+        cryfs::CryPresetPasswordBasedKeyProvider keyProvider("mypassword", cpputils::make_unique_ref<cpputils::SCrypt>(cpputils::SCrypt::TestSettings));
-            std::make_shared<MockConsole>(),
-            [] () {return "mypassword";},
-            [] () {return "mypassword";},
-            cpputils::make_unique_ref<cpputils::SCrypt>(cpputils::SCrypt::TestSettings)
-        );
         return cryfs::CryConfigFile::create(_configFile.path(), std::move(config), &keyProvider);
     }