Remove --extpass parameter, because that encourages tool writers to do bad things like storing a password in a file and using --extpass="cat filename".

The password can now be passed in to stdin without problems, so tools should use that.
This commit is contained in:
Sebastian Messmer 2016-02-13 10:46:05 +01:00
parent 098f16a4fe
commit f433da7dc1
6 changed files with 13 additions and 27 deletions

View File

@ -3,6 +3,9 @@ Version 0.9.0 (unreleased)
(warning) file systems created with earlier CryFS versions are incompatible with this release
* Fully support file access times
* Fix: Password is read from stdin, not from glibc getpass(). This enables external tools (e.g. GUIs) to pass in the password without problems.
* Remove --extpass parameter, because that encourages tool writers to do bad things like storing a password in a file and using --extpass="cat filename".
The password can now be passed in to stdin without problems, so tools should use that.
* Works with zuluMount GUI, https://mhogomchungu.github.io/zuluCrypt/
* (for developers) New repository layout. All subrepositories have been merged to one directory
* (for developers) Using CMake instead of biicode as build system

View File

@ -107,13 +107,8 @@ namespace cryfs {
return true;
}
string Cli::_getPassword(const ProgramOptions &options, function<string()> askPassword) {
string password;
if (options.extPass() == none) {
password = askPassword();
} else {
password = cpputils::Subprocess::call(*options.extPass());
}
string Cli::_getPassword(function<string()> askPassword) {
string password = askPassword();
//Remove trailing newline
if (password[password.size()-1] == '\n') {
password.resize(password.size()-1);
@ -183,8 +178,8 @@ namespace cryfs {
try {
auto configFile = _determineConfigFile(options);
auto config = CryConfigLoader(_console, _keyGenerator, _scryptSettings,
std::bind(&Cli::_getPassword, this, std::cref(options), &Cli::_askPasswordForExistingFilesystem),
std::bind(&Cli::_getPassword, this, std::cref(options), &Cli::_askPasswordForNewFilesystem),
std::bind(&Cli::_getPassword, this, &Cli::_askPasswordForExistingFilesystem),
std::bind(&Cli::_getPassword, this, &Cli::_askPasswordForNewFilesystem),
options.cipher()).loadOrCreate(configFile);
if (config == none) {
std::cerr << "Could not load config file. Did you enter the correct password?" << std::endl;

View File

@ -21,7 +21,7 @@ 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::function<std::string()> askPassword);
std::string _getPassword(std::function<std::string()> askPassword);
static std::string _askPasswordForExistingFilesystem();
static std::string _askPasswordForNewFilesystem();
static std::string _askPasswordFromStdin(const std::string &prompt);

View File

@ -61,12 +61,8 @@ ProgramOptions Parser::parse(const vector<string> &supportedCiphers) const {
cipher = vm["cipher"].as<string>();
_checkValidCipher(*cipher, supportedCiphers);
}
optional<string> extPass = none;
if (vm.count("extpass")) {
extPass = vm["extpass"].as<string>();
}
return ProgramOptions(baseDir, mountDir, configfile, foreground, unmountAfterIdleMinutes, logfile, cipher, extPass, options.second);
return ProgramOptions(baseDir, mountDir, configfile, foreground, unmountAfterIdleMinutes, logfile, cipher, options.second);
}
void Parser::_checkValidCipher(const string &cipher, const vector<string> &supportedCiphers) {
@ -114,7 +110,6 @@ void Parser::_addAllowedOptions(po::options_description *desc) {
("cipher", po::value<string>(), "Cipher to use for encryption. See possible values by calling cryfs with --show-ciphers")
("show-ciphers", "Show list of supported ciphers.")
("unmount-idle", po::value<double>(), "Automatically unmount after specified number of idle minutes.")
("extpass", po::value<string>(), "External program to use for password input")
("logfile", po::value<string>(), "Specify the file to write log messages to. If this is not specified, log messages will go to stdout, or syslog if CryFS is running in the background.")
;
desc->add(options);

View File

@ -11,10 +11,9 @@ namespace bf = boost::filesystem;
ProgramOptions::ProgramOptions(const bf::path &baseDir, const bf::path &mountDir, const optional<bf::path> &configFile,
bool foreground, const optional<double> &unmountAfterIdleMinutes,
const optional<bf::path> &logFile, const optional<string> &cipher,
const optional<string> &extPass, const vector<char*> &fuseOptions)
const vector<char*> &fuseOptions)
:_baseDir(baseDir), _mountDir(nullptr), _configFile(configFile), _foreground(foreground),
_cipher(cipher), _unmountAfterIdleMinutes(unmountAfterIdleMinutes), _logFile(logFile), _extPass(extPass),
_fuseOptions(fuseOptions) {
_cipher(cipher), _unmountAfterIdleMinutes(unmountAfterIdleMinutes), _logFile(logFile), _fuseOptions(fuseOptions) {
string mountDirStr = mountDir.native();
_mountDir = new char[mountDirStr.size()+1];
@ -28,7 +27,7 @@ ProgramOptions::ProgramOptions(ProgramOptions &&rhs)
:_baseDir(std::move(rhs._baseDir)), _mountDir(std::move(rhs._mountDir)), _configFile(std::move(rhs._configFile)),
_foreground(std::move(rhs._foreground)), _cipher(std::move(rhs._cipher)),
_unmountAfterIdleMinutes(std::move(rhs._unmountAfterIdleMinutes)), _logFile(std::move(rhs._logFile)),
_extPass(std::move(rhs._extPass)), _fuseOptions(std::move(rhs._fuseOptions)) {
_fuseOptions(std::move(rhs._fuseOptions)) {
rhs._mountDir = nullptr;
}
@ -66,10 +65,6 @@ const optional<string> &ProgramOptions::cipher() const {
return _cipher;
}
const optional<string> &ProgramOptions::extPass() const {
return _extPass;
}
const vector<char *> &ProgramOptions::fuseOptions() const {
return _fuseOptions;
}

View File

@ -16,7 +16,7 @@ namespace cryfs {
const boost::optional<boost::filesystem::path> &configFile,
bool foreground, const boost::optional<double> &unmountAfterIdleMinutes,
const boost::optional<boost::filesystem::path> &logFile,
const boost::optional<std::string> &cipher, const boost::optional<std::string> &extPass,
const boost::optional<std::string> &cipher,
const std::vector<char *> &fuseOptions);
ProgramOptions(ProgramOptions &&rhs);
~ProgramOptions();
@ -28,7 +28,6 @@ namespace cryfs {
const boost::optional<std::string> &cipher() const;
const boost::optional<double> &unmountAfterIdleMinutes() const;
const boost::optional<boost::filesystem::path> &logFile() const;
const boost::optional<std::string> &extPass() const;
const std::vector<char *> &fuseOptions() const;
private:
@ -39,7 +38,6 @@ namespace cryfs {
boost::optional<std::string> _cipher;
boost::optional<double> _unmountAfterIdleMinutes;
boost::optional<boost::filesystem::path> _logFile;
boost::optional<std::string> _extPass;
std::vector<char *> _fuseOptions;
DISALLOW_COPY_AND_ASSIGN(ProgramOptions);