When creating a new filesystem, ask password twice (second time for confirmation)

src/cli/Cli.cpp

@@ -27,6 +27,7 @@
 #endif
 
 //TODO Many functions accessing the ProgramOptions object. Factor out into class that stores it as a member.
+//TODO Factor out class handling askPassword
 
 using namespace cryfs;
 namespace bf = boost::filesystem;
@@ -95,10 +96,10 @@ namespace cryfs {
         return true;
     }
 
-    string Cli::_getPassword(const ProgramOptions &options) {
+    string Cli::_getPassword(const ProgramOptions &options, function<string()> askPassword) {
         string password;
         if (options.extPass() == none) {
-            password = _askPassword();
+            password = askPassword();
         } else {
             password = cpputils::Subprocess::call(*options.extPass());
         }
@@ -113,7 +114,7 @@ namespace cryfs {
         return password;
     }
 
-    string Cli::_askPassword() {
+    string Cli::_askPasswordForExistingFilesystem() {
         string password = getpass("Password: ");
         while (!_checkPassword(password)) {
             password = getpass("Password: ");
@@ -121,6 +122,33 @@ namespace cryfs {
         return password;
     };
 
+    string Cli::_askPasswordForNewFilesystem() {
+        string password;
+        bool again = false;
+        do {
+            password = getpass("Password: ");
+            if (!_checkPassword(password)) {
+                again = true;
+                continue;
+            }
+            if (!_confirmPassword(password)) {
+                again = true;
+                continue;
+            }
+            again = false;
+        } while(again);
+        return password;
+    }
+
+    bool Cli::_confirmPassword(const string &password) {
+        string confirmPassword = getpass("Confirm Password: ");
+        if (password != confirmPassword) {
+            std::cout << "Passwords don't match" << std::endl;
+            return false;
+        }
+        return true;
+    }
+
     bf::path Cli::_determineConfigFile(const ProgramOptions &options) {
         auto configFile = options.configFile();
         if (configFile == none) {
@@ -133,9 +161,10 @@ namespace cryfs {
         try {
             auto configFile = _determineConfigFile(options);
             auto console = make_unique_ref<IOStreamConsole>();
-            std::cout << "Loading config file..." << std::endl;
+            auto config = CryConfigLoader(std::move(console), _keyGenerator, _scryptSettings,
-            auto config = CryConfigLoader(std::move(console), _keyGenerator, _scryptSettings, std::bind(&Cli::_getPassword, this, std::cref(options)), options.cipher()).loadOrCreate(configFile);
+                                          std::bind(&Cli::_getPassword, this, std::cref(options), &Cli::_askPasswordForExistingFilesystem),
-            std::cout << "Loading config file...done" << std::endl;
+                                          std::bind(&Cli::_getPassword, this, std::cref(options), &Cli::_askPasswordForNewFilesystem),
+                                          options.cipher()).loadOrCreate(configFile);
             if (config == none) {
                 std::cerr << "Could not load config file. Did you enter the correct password?" << std::endl;
                 exit(1);

src/cli/Cli.h

@@ -19,9 +19,11 @@ namespace cryfs {
         void _runFilesystem(const program_options::ProgramOptions &options);
         CryConfigFile _loadOrCreateConfig(const program_options::ProgramOptions &options);
         boost::filesystem::path _determineConfigFile(const program_options::ProgramOptions &options);
-        std::string _getPassword(const program_options::ProgramOptions &options);
+        std::string _getPassword(const program_options::ProgramOptions &options, std::function<std::string()> askPassword);
-        std::string _askPassword();
+        static std::string _askPasswordForExistingFilesystem();
-        bool _checkPassword(const std::string &password);
+        static std::string _askPasswordForNewFilesystem();
+        static bool _confirmPassword(const std::string &password);
+        static bool _checkPassword(const std::string &password);
         void _showVersion();
         void _initLogfile(const program_options::ProgramOptions &options);
         void _sanityChecks(const program_options::ProgramOptions &options);

src/config/CryConfigLoader.cpp

@@ -22,17 +22,21 @@ using namespace cpputils::logging;
 
 namespace cryfs {
 
-CryConfigLoader::CryConfigLoader(unique_ref<Console> console, RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings, function<string()> askPassword, const optional<string> &cipher)
+CryConfigLoader::CryConfigLoader(unique_ref<Console> console, RandomGenerator &keyGenerator, const SCryptSettings &scryptSettings, function<string()> askPasswordForExistingFilesystem, function<string()> askPasswordForNewFilesystem, const optional<string> &cipher)
-    : _creator(std::move(console), keyGenerator), _scryptSettings(scryptSettings), _askPassword(askPassword), _cipher(cipher) {
+    : _creator(std::move(console), keyGenerator), _scryptSettings(scryptSettings),
+      _askPasswordForExistingFilesystem(askPasswordForExistingFilesystem), _askPasswordForNewFilesystem(askPasswordForNewFilesystem),
+      _cipher(cipher) {
 }
 
 optional<CryConfigFile> CryConfigLoader::_loadConfig(const bf::path &filename) {
-  string password = _askPassword();
+  string password = _askPasswordForExistingFilesystem();
+  std::cout << "Loading config file..." << std::flush;
   auto config = CryConfigFile::load(filename, password);
   if (config == none) {
     LOG(ERROR) << "Could not load config file. Wrong password?";
     return none;
   }
+  std::cout << "done" << std::endl;
   if (_cipher != none && config->config()->Cipher() != *_cipher) {
     throw std::runtime_error("Filesystem uses "+config->config()->Cipher()+" cipher and not "+*_cipher+" as specified.");
   }
@@ -50,8 +54,11 @@ optional<CryConfigFile> CryConfigLoader::loadOrCreate(const bf::path &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();
+  string password = _askPasswordForNewFilesystem();
-  return CryConfigFile::create(filename, std::move(config), password, _scryptSettings);
+  std::cout << "Creating config file..." << std::flush;
+  auto result = CryConfigFile::create(filename, std::move(config), password, _scryptSettings);
+  std::cout << "done" << std::endl;
+  return result;
 }
 
 

src/config/CryConfigLoader.h

@@ -13,7 +13,7 @@ namespace cryfs {
 
 class CryConfigLoader {
 public:
-  CryConfigLoader(cpputils::unique_ref<cpputils::Console> console, cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings, std::function<std::string()> askPassword, const boost::optional<std::string> &cipher);
+  CryConfigLoader(cpputils::unique_ref<cpputils::Console> console, cpputils::RandomGenerator &keyGenerator, const cpputils::SCryptSettings &scryptSettings, std::function<std::string()> askPasswordForExistingFilesystem, std::function<std::string()> askPasswordForNewFilesystem, const boost::optional<std::string> &cipher);
   CryConfigLoader(CryConfigLoader &&rhs) = default;
 
   boost::optional<CryConfigFile> loadOrCreate(const boost::filesystem::path &filename);
@@ -24,7 +24,8 @@ private:
 
     CryConfigCreator _creator;
     cpputils::SCryptSettings _scryptSettings;
-    std::function<std::string()> _askPassword;
+    std::function<std::string()> _askPasswordForExistingFilesystem;
+    std::function<std::string()> _askPasswordForNewFilesystem;
     boost::optional<std::string> _cipher;
 
     DISALLOW_COPY_AND_ASSIGN(CryConfigLoader);

test/config/CryConfigLoaderTest.cpp

@@ -24,7 +24,8 @@ public:
     CryConfigLoaderTest(): file(false) {}
 
     CryConfigLoader loader(const string &password, const optional<string> &cipher = none) {
-        return CryConfigLoader(mockConsole(), cpputils::Random::PseudoRandom(), SCrypt::TestSettings, [password] {return password;}, cipher);
+        auto askPassword = [password] {return password;};
+        return CryConfigLoader(mockConsole(), cpputils::Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, cipher);
     }
 
     CryConfigFile Create(const string &password = "mypassword", const optional<string> &cipher = none) {

test/filesystem/CryFsTest.cpp

@@ -36,7 +36,8 @@ public:
   }
 
   CryConfigFile loadOrCreateConfig() {
-    return CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, [] {return "mypassword";}, none).loadOrCreate(config.path()).value();
+    auto askPassword = [] {return "mypassword";};
+    return CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, none).loadOrCreate(config.path()).value();
   }
 
   unique_ref<OnDiskBlockStore> blockStore() {

test/filesystem/FileSystemTest.cpp

@@ -27,7 +27,8 @@ public:
 
   unique_ref<Device> createDevice() override {
     auto blockStore = cpputils::make_unique_ref<FakeBlockStore>();
-    auto config = CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, [] {return "mypassword";}, none)
+    auto askPassword = [] {return "mypassword";};
+    auto config = CryConfigLoader(mockConsole(), Random::PseudoRandom(), SCrypt::TestSettings, askPassword, askPassword, none)
             .loadOrCreate(configFile.path()).value();
     return make_unique_ref<CryDevice>(std::move(config), std::move(blockStore));
   }