Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new command line options #167

Closed
wants to merge 1 commit into from
Closed

Conversation

jetwhiz
Copy link
Contributor

@jetwhiz jetwhiz commented May 11, 2016

Add new command line options

--mount - only mount FS (do not create)
--unmount - unmount FS
--config=path - specify config file (override ENV)
--password=passwd - specify password

VLOG(1) << "useStdin: " << useStdin;
if (useStdin) {
VLOG(1) << "passSrc: " << opts->passSrc;
if (opts->passSrc != Pass_Prompt) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be opts->passSrc == Pass_Stdin ?

Copy link
Contributor Author

@jetwhiz jetwhiz May 11, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right -- on Windows we don't have passwordProgram, so I didn't see that this will never be executed with the current layout on Linux.

We want to fall into userKey = config->getUserKey(opts); for either Pass_Stdin or Pass_Cmd. What do you think about changing this if-statement to:

if (opts->passSrc == Pass_Cmd || opts->passSrc == Pass_Stdin) {
...
else if (opts->passSrc == Pass_Ext)
...
else

@gh223682
Copy link

@vgough @jetwhiz I don't think that having an option to specify the password from the command-line is a good idea due to process security issues. This shouldn't be in the core code.

@jetwhiz
Copy link
Contributor Author

jetwhiz commented May 23, 2016

TrueCrypt/VeraCrypt both allow for passing the password in via command line options (see https://veracrypt.codeplex.com/wikipage?title=Command%20Line%20Usage).

It can be useful when you want to create encrypted filesystems in an automated fashion in software (e.g., encfs --standard --password="$usrpass" /home/me /tmp/crypt-view) without needing to create pipes to the program to send the password to it. Usage of it should be discouraged (in documentation) on the command line, since it may maintain a history, though.

@Aikhjarto
Copy link
Contributor

Usage of it should be discouraged (in documentation) on the command line, since it may maintain a history, though.

Not only history of the shell is a problem. Under Linux the command ps aux shows the command line of all running processes. This will exhibit the password while encfs is running.

@pepa65
Copy link

pepa65 commented May 23, 2016

For scripting, the option to pass some form of password to the binary is essential, either through a string, a pipe, an external password utility, or any means. I am currently using the -S|--stdinpass option, and hope that it stays.

@gh223682
Copy link

TrueCrypt/VeraCrypt both allow for passing the password in via command line options

@jetwhiz Hmm, that's interesting. There are other programs like fetchmail which don't have an option like this explicitly for process security reasons.

It can be useful when you want to create encrypted filesystems in an automated fashion in software (e.g., encfs --standard --password="$usrpass" /home/me /tmp/crypt-view) without needing to create pipes to the program to send the password to it. Usage of it should be discouraged (in documentation) on the command line, since it may maintain a history, though.

I was going to propose an alternate solution for people who want to launch EncFS with an unattended script (which includes me): an option to specify a permissions-restricted file which contains the password, e.g. "--password-file".

@rfjakob
Copy link
Collaborator

rfjakob commented May 23, 2016 via email

@gh223682
Copy link

gh223682 commented May 23, 2016

@rfjakob For the --extpass= option, an executable program is required, even if it is just to cat a file.

On certain platforms like Windows, there is no default system binary which can do this. Commands like "echo" and "type" are built-in cmd.exe shell commands and do not work.

@Aikhjarto
Copy link
Contributor

Commands like "echo" and "type" are built-in cmd.exe shell commands and do not work.

Why not invoking cmd.exe with something like cmd /c type mypassword.txt?

@gh223682
Copy link

gh223682 commented May 23, 2016

@Aikhjarto That'll work.

Mind you, I'm just trying to suggest an acceptable/equivalent alternative to a "--password=" option.

If there are no objections to implementing a "--password=" option, then there's no point for a "--password-file=" option.

@jetwhiz
Copy link
Contributor Author

jetwhiz commented May 23, 2016

I think cmd /c type mypassword.txt is a decent enough solution coupled with --extpass. If you guys prefer, I can remove the "--password=passwd" portion of this PR, unless this functionality is helpful to someone else.

@rfjakob
Copy link
Collaborator

rfjakob commented May 23, 2016

Yes please, there is no way to use --password securely.

@jetwhiz
Copy link
Contributor Author

jetwhiz commented May 24, 2016

The latest version should now include:

--mount - only mount FS (do not create)
--unmount - unmount FS
--config=path - specify config file (override ENV)

@imperative
Copy link

imperative commented Jun 25, 2016

We should really allow providing the password in command line.
Encfs is a solution that is already geared towards sophisticated users. We should trust the users to know the implications of this. Especially when we put a warning in the documentation.

At the same time, there are many legitimate use cases for this.
For example when using encfs to reverse-encrypt backups in the cloud. In that case we want the mounting to be automatic and we don't care if the password is easily readable locally, we only care that the cloud gets encrypted files.

As an option, using "passfile" (which would work on other platforms, even windows cmd) solution would only be a bit more complicated for the ones who need this functionality, but it would at least eliminate "ps aux" and shell history implications.

@pepa65
Copy link

pepa65 commented Jun 25, 2016

Hear hear! Please don't take essential options away.

@rfjakob
Copy link
Collaborator

rfjakob commented Jun 25, 2016

Something like --passfile would be acceptable.

@rfjakob
Copy link
Collaborator

rfjakob commented Jun 26, 2016

@jetwhiz Looks good to me, but can you please squash this into a single commit?
@vgough Ok to merge?

@pepa65
Copy link

pepa65 commented Jun 26, 2016

I do hope the --stdinpass option stays, but I don't understand why you would remove options that users want, have a good use case for, and don't take a lot of code.

@rfjakob
Copy link
Collaborator

rfjakob commented Jun 26, 2016

Dear @pepa65 , no options will be removed, this discussion is strictly about adding new ones!

@jetwhiz
Copy link
Contributor Author

jetwhiz commented Jun 26, 2016

@rfjakob Should I open a new pull request with the squashed commits, or is there a way to do that on GitHub?

@rfjakob
Copy link
Collaborator

rfjakob commented Jun 26, 2016 via email

--mount  - only mount FS (do not create)
--unmount  - unmount FS
--config=path  - specify config file (override ENV)
@Zero3
Copy link

Zero3 commented Jul 17, 2016

Good idea with these new command-line options @jetwhiz! I've been missing the mount/unmount commands.

I'd like to add some thoughts about passing passwords through files, though. This technique does not appear to be secure. I see a number of potential issues:

  1. Even if the file is deleted from the file system, the file contents may be recoverable. As far as I know, recovery is pretty trivial on spinning disks unless contents are overwritten before file deletion. On SSDs, you cannot reliably overwrite file contents as the disk might save the new contents to different blocks instead of overwriting. Recovery is probably more difficult though.

  2. The user might have backup software running that automatically backs up the password file.

  3. OS-level file caching and swap might pose risks too.

One could probably work around these, but I'm a little concerned about adding features that let users use their passwords in insecure ways. If such features are added, I hope you add some big fat warning messages next to them.

@imperative
Copy link

IMO there should be warnings, but the options should definitely be there. Otherwise it's very hard to automate the operation, which is an essential functionality for some users.

If these options are not implemented, it will force users to either use other encryption software (which leads to segmentation of the field and ultimately more code which is spread out, which has more bugs and thus overall less security), or implement some external addons (like sshpass) which will introduce all the problems we discuss here, and even more, because those addons are not very likely to be implemented securely.

This is opensource, we should give users the most flexibility with decent defaults, and hide dangerous options behind warnings and manuals. If they wanted a dull solution they would use bitlocker.

@Zero3
Copy link

Zero3 commented Jul 17, 2016

@imperative I completely agree that it should be possible to integrate/automate encfs! I'm working on a project about this myself right now :).

I don't mind adding this feature, I'm just a little concerned, that's all. I don't know what the best practice for doing this kind of thing would be, it's just that passing the password through command line or file on disk seems somewhat insecure. Maybe there is a better way?

@jetwhiz
Copy link
Contributor Author

jetwhiz commented Nov 20, 2016

Hi @rfjakob @vgough -- is there any update on merging these new options into encfs?

@benrubson
Copy link
Contributor

please see #474 for a rework 👍

@benrubson benrubson closed this Mar 8, 2018
@jetwhiz jetwhiz deleted the fork-cmd-opts branch February 2, 2019 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants