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

Implement running shell commands for getting passwords #315

Merged
merged 17 commits into from
Dec 18, 2022
Merged

Conversation

osa1
Copy link
Owner

@osa1 osa1 commented Apr 14, 2021

Password fields can now be a map with a command key and a string value. The
value is used the shell command to run to get the password.

To keep things simple, all external commands are run on startup.

Fixes #246

crates/tiny/src/config.rs Outdated Show resolved Hide resolved
@kmaasrud
Copy link

@osa1 What is the status on this PR? I would really like to run tiny without storing my password directly in the config 😄

@osa1
Copy link
Owner Author

osa1 commented Dec 14, 2022

@kmaasrud I'm a bit low on bandwidth but I have time this week to make some progress. Maybe we can finish this.

Before implementing, there are some design questions to answer though.

This works fine when the password store is unlocked before running tiny. If not unlocked I think most password managers will want to print something to stdout first, something like Enter master key password: to unlock the password store, which won't work because we capture the stdout, to be able to read the printed password. Not sure how to work around this issue yet.

We have two options:

  1. We could run commands before running the TUI.
  2. We could use suspend/activate, similar to how we run $EDITOR.

I feel like (1) should be good enough, and it will certainly make the implementation simpler.

I think the syntax ($ prefix) for commands (to distinguish them from passwords) is fine.

@kmaasrud
Copy link

I feel like (1) should be good enough, and it will certainly make the implementation simpler.

I think the syntax ($ prefix) for commands (to distinguish them from passwords) is fine.

+1 on this---it sounds like a very good solution to the problem! Better to keep it simple where possible.

@osa1
Copy link
Owner Author

osa1 commented Dec 15, 2022

I think this is done, just need to update README, CHANGELOG, and commit message.

However I'm having trouble using this feature. If I use pass show irc_pass as a password command, I get this error when running tiny:

Running server password command for server irc.oftc.net (`pass show irc_pass`)
Command returned non-zero: 2
stdout is empty
stderr:
--------------------------------------
gpg: public key decryption failed: No such file or directory
gpg: decryption failed: No such file or directory

--------------------------------------

But when I run pass show irc_pass in shell it works as expected.

Once I run the pass command to unlock the key store (so that pass show irc_pass directly prints the password) I can run tiny.

Does anyone know why this happens?

@kmaasrud
Copy link

Does anyone know why this happens?

@osa1 Some environment variable that the binary cannot access? Have you tested with different password commands (e.g., a simple echo?)

@osa1
Copy link
Owner Author

osa1 commented Dec 18, 2022

Yeah, simple commands like nickserv_ident: "$ echo '...'", or cat ... etc. work fine.

We should figure out how to make this work with pass. I'm assuming most people will use some kind of password manager, instead of a simple echo/cat etc.

@trevarj
Copy link
Contributor

trevarj commented Dec 18, 2022

I just tried and it works for me with the $ pass show irc-pass in the password field.

The keyring unlock pops up and I type my unlock password and then it starts tiny.

@kmaasrud
Copy link

Using security on MacOS also works for me! The keychain unlocks nicely and tiny successfully uses the requested password.

@osa1
Copy link
Owner Author

osa1 commented Dec 18, 2022

I just tried this on my Linux system and it also works there.

I will merge this today, we can debug any platform-specific issues later.

README.md Outdated
@@ -108,23 +108,21 @@ join require identification. To use this method enter your nick password to the

### Using external commands for passwords

When a password field in the config file starts with `$`, tiny uses rest of the
field as the shell command to run to get the password.
When a password field in the config file is a map with a `cmd` key, the value
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we update the sample config.yml too?

Copy link
Owner Author

Choose a reason for hiding this comment

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

We show an example in the README so I'm not sure if it's necessary.

@osa1 osa1 merged commit 8acbfdc into master Dec 18, 2022
@osa1 osa1 deleted the pass_cmd branch December 18, 2022 18:44
@kmaasrud
Copy link

Awesome work, @osa1! Thanks for fixing this so quickly 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement getting passwords from external commands
4 participants