Add pass-tomb support - Githubissues

adworacz commented 3 years ago

Admin edit: tracking issue, merge request

First off - love what you're doing with this project, very nice job.

Second, please consider adding pass-tomb support/similar functionality. The ability to keep passwords in a tomb is a very nice security add. I know not all users will want to use it, but it's a brilliant extension for pass to solve the "metadata leakage" that one might have with secret names.

Update: After posting this, I did realize there's a potential work around by simply using pass-tomb direction for managing the tomb itself. And then prs will remain oblivious to the fact that it's operating on a tomb while the tomb is mounted.

Workaround

Here's a potential workaround (not tested, hypothetical)

pass tomb user@example.com #only required once to create tomb
pass open #assuming not already open from above command
prs add foo/bar
prs sync
prs ....
pass close

There's some caveats here, particularly around use by multiple recipients etc, but those are likely quite small issues as each recipient would just create their own personal tomb and then prs sync appropriately. This doesn't cover every threat model, but it may be an idea. Also, pass tomb accepts multiple gpg-id's, so it may just take some finagling to clear out any existing pass data in the .password-store directory and then run prs init as desired.

timvisee commented 3 years ago

Yes, supporting something like this would definitely be nice. Metadata leakage isn't much of a problem for me, but I agree that having this would be a great security addition.

I guess that using Tomb is fine, though I wonder what other approaches there are.

Based on the fact that it mounts the password store to a directory I think this is actually quite easy to implement. Which is great!

I imagine prs managing the Tomb automatically. Meaning it automatically mounts/unmounts, so no manual opening/closing is required. I haven't looked into the details of it with regard to pass compatibility and speed but I hope something like that is possible.

Another problem I see is that this likely needs some form of configuration (file). I haven't decided yet on what the best approach for this. Other pass clients use a simple YAML file.

Though I'm definitely a fan of implementing this, I'm not sure when I have time to give this a proper take.

First off - love what you're doing with this project, very nice job.

Thanks!

timvisee commented 3 years ago

I did some quick testing with pass-tomb in combination with prs. It definitely works together at the moment so that's great.

I've found the following so far:

Opening the tomb takes a few seconds on my machine
The tomb created by pass-tomb is too small to fit my current password store (being 13MB with git history)
Aliases are supported

It appears that tombs can be automatically closed after some specified timer. I think that automatically opening tombs with prs to keep it open for about a minute would be a great middle ground for security, performance and usability. With this, when using a tomb, the first command invocation would take a few seconds, the following commands will be instant.

The tomb CLI supports growing the tomb file, and so prs will definitely have the ability to automatically grow the tomb in case it gets too small.

adworacz commented 3 years ago

Thanks for giving this suggestion some thought!

Another problem I see is that this likely needs some form of configuration (file). I haven't decided yet on what the best approach for this. Other pass clients use a simple YAML file.

I'm not sure this is true. As you likely just experienced, pass-tomb works just fine without a configuration file. I could see a configuration file being useful if needing to manage multiple stores, but that's kind of a separate issue.

And it looks like you're already experienced some of the tomb features. Yes, it can auto-close. I think pass-tomb uses a systemd timer that is automatically registered when using the -t flag for tomb creation or opening.

timvisee commented 3 years ago

Another problem I see is that this likely needs some form of configuration (file). I haven't decided yet on what the best approach for this. Other pass clients use a simple YAML file.

I'm not sure this is true. As you likely just experienced, pass-tomb works just fine without a configuration file. I could see a configuration file being useful if needing to manage multiple stores, but that's kind of a separate issue.

You're right. The simplest tomb implementation can do with some simple path scanning, though it won't support any form of configuration then.

(...) I think pass-tomb uses a systemd timer that is automatically registered when using the -t flag for tomb creation or opening.

Thanks for linking the service file! Do you know whether placing this in the proper systemd service directory would be enough?

By the way, I've created an issue for this over on GitLab (which I use as main repository) here. You might find it useful to follow. Feel free to keep discussing things here though.

adworacz commented 3 years ago

Thanks for linking the service file! Do you know whether placing this in the proper systemd service directory would be enough?

I believe it's 50% of the required work. The other 50% is actually starting that service when desired. It uses systemd-run to actually handle the timer functionality via the --on-active flag.

By the way, I've created an issue for this over on GitLab

Sounds good!

timvisee commented 3 years ago

@adworacz FYI, I've started with this today, and it has now been partially implemented.

What works:

Automatically opening Tomb's on use
Automatically closing after 5 minutes through systemd
Manual opening/closing with prs tomb {open,close}
Works seamlessly alongside pass-tomb

Initializing a Tomb with prs and automatic resizing is not implemented yet.

It's still in early stages, and there are some problems I have to iron out. But it is definitely usable now, so I thought you might like it to know.

See branch: https://gitlab.com/timvisee/prs/-/tree/36-add-tomb-support

adworacz commented 3 years ago

Wow, awesome!!

I’ll check out the branch soon and mess around with it. I’ll let you know any issues that I encounter.

adworacz commented 3 years ago

Okay, I'm up way past my bed time, so apologies if my reports seem incoherent.

Running `prs tomb status` on closed tomb results in error

╰─ ./.cargo/bin/prs tomb status --verbose
error: failed to calcualte password store size
caused by: Permission denied (os error 13)

For detailed errors add '--verbose'
For more information add '--help'

Running `prs tomb open` with gpg key not unlocked/loaded gives no obvious error, no pinentry prompt

╰─ ./.cargo/bin/prs tomb open --verbose
Opening Tomb...
error: failed to open password store tomb
caused by: tomb operation exited with non-zero status code: exit status: 1

For detailed errors add '--verbose'
For more information add '--help'

Even adding the -f flag results in the same error.

`pass open -f` works just fine

╰─ pass open -f
 (*) Your password tomb has been opened in /home/user/.password-store/.
  .  You can now use pass as usual.
  .  This password store will be closed in 30m

This starts pinentry properly, asks for GPG password, and then unlocks the tomb properly.

`prs tomb open` seems to work after `pass open+close` is run.

Likely because the required GPG is already open/handled by gpg-agent.

timvisee commented 3 years ago

Thanks for testing! More problems than I expected actually, so it's a good thing we're testing this now.

The permission errors are likely caused by the Tomb mount point being owned by root. I'll be sure to change permissions after opening the Tomb through prs. This error happens when measuring the store size, that isn't too important anyway.

I'm not sure why the pinentry prompt doesn't come up, when it does with pass-tomb. I assume you're running prs as your current user. I do see a pinentry (from GPG) prompt on my machine.

adworacz commented 3 years ago

Yup, I’m running prs as my current user. Everything was done as the same user in the same shell session.

I’ll try and retest it with a ‘cold’ gpg-agent to make sure it’s reproducible.

If it helps, I’m using Sway as a WM, and pass-tomb gave me a terminal-based (curses) pinentry.

timvisee commented 3 years ago

I've made some more changes, available in cc882450. Notably, --verbose is now properly propagated to tomb, so this might provide useful debug information with prs tomb open --verbose.

Still not sure why pinentry doesn't come up for you. prs basically just invokes the tomb binary to open a Tomb, and so it should function the same as just calling tomb yourself without any weird shenanigans. Maybe Wayland is the culprit somehow.

If opening the Tomb through prs still fails, you might need to set yourself as owner on the mountpoint: open the Tomb with pass open to mount, then chown your ~/.password-store directory to your current user recursively.

In any case, it would be awesome if you could check the following:

Does running prs tomb open and close work now?
When the tomb is closed, and you open it with prs tomb open -t5, does it properly close after 5 seconds? (see prs tomb status)
When the tomb is closed, and you run a regular command such as prs list, does it automatically open the Tomb correctly?
You mention pinentry doesn't come up with Tomb operations. Does it come up when reading secrets? (when the Tomb is already open, having a flushed gpg-agent, with prs show for example)

By the way, I use gpgconf --reload gpg-agent to flush my gpg-agent.

Edit: as you might have noticed, when opening some output from tomb is shown because it doesn't properly stay quiet. I've opened this PR for it: https://github.com/dyne/Tomb/pull/418

adworacz commented 3 years ago

Roger, I'll try and get this tested tonight after work.

adworacz commented 3 years ago

Sorry for the delay, had some personal errands yesterday.

Updated testing based on the latest commit:

prs tomb status works just fine with a closed tomb now. Woot!

prs tomb open is still failing for me with a closed tomb and flushed gpg agent.


╰─ ./.cargo/bin/prs tomb open --verbose
Opening Tomb...
tomb [D] Identified caller: user (1000:1000)
tomb [D] Tomb command: open /home/user/.password.tomb
tomb [D] Caller: uid[1000], gid[1000], tty[/dev/pts/14].
tomb [D] Temporary directory: /tmp
tomb  .  Commanded to open tomb /home/user/.password.tomb
tomb [D] is_valid_tomb /home/user/.password.tomb
tomb [D] tomb file is readable
tomb [D] tomb file is a regular file
tomb [D] tomb file is not empty
tomb [D] Mapper: tomb..password.a0cf0a3d708937e0b3b435723dc0c60ef7b4bf4d318d73b5b52d0bca9d1d7fb4.loop0
tomb [D] tomb file is not currently in use
tomb  .  Valid tomb file found: /home/user/.password.tomb
tomb [D] load_key argument: /home/user/.password.tomb.key
tomb [D] load_key: /home/user/.password.tomb.key
tomb [D] is_valid_key
tomb  .  Key is valid.
tomb (*) Opening .password on /home/user/.password-store
tomb  .  This tomb is a valid LUKS encrypted device.
tomb  .  Cipher is "aes" mode "xts-plain64" hash "sha512"
tomb [D] Tomb key: /home/user/.password.tomb.key
tomb [D] Tomb name: .password (to be engraved)
tomb [D] no password needed, using GPG key
tomb [D] get_lukskey
tomb [D] Created tempfile: /tmp/32526692737655005
tomb [D] [GNUPG:] ENC_TO 77E67CAD8BC3A6D1 1 0
tomb [D] [GNUPG:] KEY_CONSIDERED 7CA3B3346B0070233D6E5D391467A25CDC07E7E9 0
tomb [D] [GNUPG:] PINENTRY_LAUNCHED 181265 gtk2:curses 1.1.1 - xterm-256color - - 1000/1000 0
tomb [D] [GNUPG:] KEY_CONSIDERED 7CA3B3346B0070233D6E5D391467A25CDC07E7E9 0
tomb [D] gpg: encrypted with 4096-bit RSA key, ID 77E67CAD8BC3A6D1, created 2021-06-15
tomb [D]       "user-laptop <user-laptop@pass>"
tomb [D] gpg: public key decryption failed: Inappropriate ioctl for device
tomb [D] [GNUPG:] ERROR pkdecrypt_failed 83918950
tomb [D] [GNUPG:] BEGIN_DECRYPTION
tomb [D] [GNUPG:] DECRYPTION_FAILED
tomb [D] gpg: decryption failed: No secret key
tomb [D] [GNUPG:] END_DECRYPTION
tomb [D] get_lukskey returns 1
tomb [E] No valid password supplied.
tomb [D] Restoring access and modification time for /home/user/.password.tomb
tomb [D] Restoring access and modification time for /home/user/.password.tomb.key
error: failed to open password store tomb
caused by: failed to open password store tomb through tomb CLI
caused by: tomb operation exited with non-zero status code: exit status: 1

For a detailed log add '--verbose' For more information add '--help'

3. With a flushed gpg-agent, `prs show` does not display pinentry and the decryption fails:

╰─ ./.cargo/bin/prs show --verbose Secret: prs-test error: failed to read secret caused by: failed to decrypt ciphertext caused by: gpg command exited with non-zero status code: exit status: 2

For a detailed log add '--verbose' For more information add '--help'


4. Yes, with tomb closed, `prs tomb open -t5` opens the tomb and then properly closes it after 5 seconds.
5. Yes, with tomb closed, `prs list` properly opens the tomb and lists the contents.

adworacz commented 3 years ago

Hmm, looking at the output above, this line is interesting:

tomb [D] [GNUPG:] PINENTRY_LAUNCHED 181265 gtk2:curses 1.1.1 - xterm-256color - - 1000/1000 0

pass-tomb always uses the curses pinentry for me. I never see a gtk dialog of any kind. prs tomb open results in neither a curses nor a gtk pinentry dialog.

timvisee commented 3 years ago

@adworacz Fantastic! Thank you once again for doing such a comprehensive test.

GPG pinentry seems to be the problem left to solve then. I'm not sure when I have the time to take a look into this. Maybe pass-tomb provides an extra option to enforce a TTY prompt.

timvisee commented 3 years ago

@adworacz I've looked into this, but I can't figure out why the pinentry prompt from GPG fails.

It seems that GPG selects the GTK2 pinentry binary, which isn't supported on Wayland as far as I know. That would explain why nothing comes up. Display environment variables are properly communicated through, so GPG should be able to detect what pinentry to use. I therefore don't understand why this happens when calling gpg through prs/tomb.

Maybe globally setting a different pinentry client helps. I can't reproduce this behavior on my machine, which makes it very hard to debug.

adworacz commented 3 years ago

No worries, I’ll look into it a little more. I’m going to run a test in an X11 WM as well to see what happens and then I’ll try taking a look at pass-tombs src to see if anything obvious jumps out at me.

Edit: This method in Tomb looks important: https://github.com/dyne/Tomb/blob/master/tomb#L384

I'm going to mess around with my setup and see what I can do to debug the pinentry selection.

timvisee commented 3 years ago

Edit: This method in Tomb looks important: https://github.com/dyne/Tomb/blob/master/tomb#L384

I believe this is irrelevant for our implementation. Tomb supports protecting a key with a password or with GPG.

When protecting a key with a password, Tomb obtains the password with ask_password() and spawns a pinentry dialog.

With prs/pass-tomb GPG is used to protect the key instead. In that case, gpg (or your gpg-agent) obtains the password, and the password function is skipped. GPG chooses what method to use for obtaining the password, and selects what pinentry client to invoke. This is likely what fails.

Sadly, there doesn't seem to be a way to instruct GPG to use a specific pinentry client. Nor does there seem to be a way to send a password we obtain ourselves to GPG reliably, in which case we would have been able to spawn pinentry ourselves.

I haven't looked into GPG's codebase, but I believe that GPG chooses a compatible pinentry client based on some environment variables (DISPLAY, WAYLAND_DISPLAY) similar to what Tomb does and a system-wide default. This means we can use these to hint GPG on what to use. But, these should already be set correctly, and prs shouldn't mess with this environment when calling through it, which is why I don't understand why it fails.

adworacz commented 3 years ago

Ahhh my mistake, I'd forgotten that was a feature of Tomb. I'm going to look at my /usr/bin/pinentry file on the laptop in question when I get a moment. On some distro's it's a simple shell script, which might elucidate the issue.

adworacz commented 3 years ago

Okay, I've got some debugging details.

Workaround

The Inappropriate ioctl for device error message tipped me off to this keybase issue which provides a fix/workaround that does work with prs.

Specifically, doing the following works (just like pass-tomb):

GPG_TTY=$(tty) ./.cargo/bin/prs tomb open

Archlinux pinentry

/usr/bin/pinentry
---
#!/bin/sh

# user-defined pre-exec hook
test -r "${XDG_CONFIG_HOME:-$HOME/.config}"/pinentry/preexec &&
    . "${XDG_CONFIG_HOME:-$HOME/.config}"/pinentry/preexec

# site-defined pre-exec hook
test -r /etc/pinentry/preexec &&
    . /etc/pinentry/preexec

test -e /usr/lib/libgtk-x11-2.0.so.0 &&
    exec /usr/bin/pinentry-gtk-2 "$@"

exec /usr/bin/pinentry-curses "$@"

This might in part explain the use of gtk-2.0 and curses that we saw in the verbose tomb log above.

Suspect issue

Right now, I'm thinking that the communication of the TTY env variable is getting a little screwy when using prs when compared with pass-tomb + tomb, both of which are shell scripts.

I'm going to try this on an X11 WM now and report back.

Edit:

X11 behavior

Yup, as I suspected, a floating dialog prompt appears when running prs inside a X11 WM (bspwm). I believe the dialog is the gtk version of the pinentry prompt. When running on Wayland, the gtk version "fails" and pinentry falls back on curses as a last ditch effort.

This works just fine with pass-tomb, but fails with prs. Again, I suspect it's something with how the TTY is being set/communicated.

timvisee commented 3 years ago

Great detective work! This is very useful.

Various places (#1, #2) apparently suggest to set GPG_TTY in your shell rc anyway, which does fix it. Because of that I wonder if I should see this as a prs issue.

I'm currently thinking about setting GPG_TTY inside prs (if possible) if WAYLAND_DISPLAY is set. So, for Wayland specifically because X11 already works. I hope this is a good balance between minimal environment changes and 'just works'.

~~Could you confirm WAYLAND_DISPLAY is set in your shell?~~

I'm not sure why the pinetry script isn't patched to consider Wayland. Others must have thought of this before, assuming your display variables reach the script.

timvisee commented 3 years ago

I've implemented the above in 8bd509f. I hope this finally fixes pinentry pompts on Wayland!

adworacz commented 3 years ago

Awesome! I'll test it out soon.

And what you say about setting the GPG_TTY in the shell rc makes sense. Turns out the Arch wiki literally has a troubleshooting entry about this very thing: https://wiki.archlinux.org/title/GnuPG#Invalid_IPC_response_and_Inappropriate_ioctl_for_device

With this in mind, we can either keep your fix, or perhaps add some documentation for other users to find if they experience similar issues.

Regardless, I'm going to try out your commit.

adworacz commented 3 years ago

I can confirm that your latest commit works successfully on Sway/Wayland!!

Congrats, nice job on the hard work!!

timvisee commented 3 years ago

I've just implemented init. Invoking prs tomb init is all you need to do to transform your existing password store into a Tomb.

Tomb support as a whole is now almost done. I've implemented some more improvements, checks and safe-guards to make using Tomb as seamless as possible.

I still have to decide on what to do with Tomb's -f flag for the swap error. After that I can merge to master and publish.

adworacz commented 3 years ago

Fabulous. Thank you for all of your hard work on this feature request!

timvisee commented 3 years ago

This has now been fully implemented.

I've released Tomb support with version v0.2.12.

It should be available as soon as this release pipeline succeeds: https://gitlab.com/timvisee/prs/-/pipelines/334121982

adworacz commented 3 years ago

Wow awesome!!

Thank you again for your hard work on this! I’ll download the new release and take it for a spin.

edvm commented 5 months ago

Hi! I'm a new user, installed prs using brewer (prs --version = 0.5.1) and had hit the same problem (Inappropriate ioctl for device, thanks @adworacz! Setting GPG_TTY fixed it!

$ prs generate me/test
Secret generated
For a detailed log add '--verbose'
For more information add '--help'

$ prs copy test --verbose
Secret: me/test
$ LANG=en_US.UTF-8 LANGUAGE=en_US.UTF-8 /opt/homebrew/bin/gpg --quiet --decrypt
= gnupg stderr: ================
gpg: public key decryption failed: Inappropriate ioctl for device
gpg: decryption failed: Inappropriate ioctl for device
================================
error: failed to read secret
caused by: failed to decrypt ciphertext
caused by: gpg command exited with non-zero status code: exit status: 2

For more information add '--help'

$ GPG_TTY=$(tty) prs copy test
Secret: me/test
Secret copied to clipboard. Clearing after 20 seconds...

Maybe the version installed by brewer is old? I'll check later 👍

timvisee / prs

Add pass-tomb support #8

Workaround

Running `prs tomb status` on closed tomb results in error

Running `prs tomb open` with gpg key not unlocked/loaded gives no obvious error, no pinentry prompt

`pass open -f` works just fine

`prs tomb open` seems to work after `pass open+close` is run.

Workaround

Archlinux pinentry

Suspect issue

X11 behavior

timvisee / prs

Add pass-tomb support #8

Workaround

Running prs tomb status on closed tomb results in error

Running prs tomb open with gpg key not unlocked/loaded gives no obvious error, no pinentry prompt

pass open -f works just fine

prs tomb open seems to work after pass open+close is run.

Workaround

Archlinux pinentry

Suspect issue

X11 behavior

Running `prs tomb status` on closed tomb results in error

Running `prs tomb open` with gpg key not unlocked/loaded gives no obvious error, no pinentry prompt

`pass open -f` works just fine

`prs tomb open` seems to work after `pass open+close` is run.