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

Check all the keys in .gpg-id are valid before adding a new key #1918

Closed
hackaugusto opened this issue May 4, 2021 · 2 comments · Fixed by #2487
Closed

Check all the keys in .gpg-id are valid before adding a new key #1918

hackaugusto opened this issue May 4, 2021 · 2 comments · Fixed by #2487
Assignees
Labels
Milestone

Comments

@hackaugusto
Copy link

Summary

There are two things here:

  1. This is not a bug, but an usability issue. When adding a new key with gopass recipients add --store <store> <user_id> the secrets will be re-encrypted for every key in .gpg-id. If one of this keys is expired the encryption will fail.
  2. This I think it is a bug, if the encryption failed on the previous step, the new key will be added to .gpg-id even though it can't decrypt anything, running gopass fsck won't fix the issue.

Steps To Reproduce

I had this issue with a key that actually expired, I suppose these steps can reproduce the issue:

  • Create a new key pair
  • Add it to the password store and encrypt a secret with it
  • Edit the previous key and set the expiration in the past
  • Create a second key pair
  • Try to add the non-expired key to the keystore

Expected behavior

A nice error message saying that one of the keys is expired, and user intervention is necessary. Either the key has to be removed from the password store, or the owner of the key has to update the expiration.

Environment

  • OS: Fedora
  • OS version: Linux .. 5.11.15-200.fc33.x86_64 Add template feature #1 SMP Fri Apr 16 13:41:20 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
  • gopass Version: gopass 1.8.6 (d5b0d3b 2021-01-27 21:11:07) go1.15.6 linux amd64
  • Installation method: compiled

Additional context

@hackaugusto
Copy link
Author

hackaugusto commented May 4, 2021

This is similar to #1917 (comment) , here the field 7 should do the trick

@dominikschulz dominikschulz added crypto Crypto Backends gpg GPG related help-wanted labels May 4, 2021
@dominikschulz dominikschulz added this to the 1.x.x milestone May 4, 2021
@AnomalRoil
Copy link
Member

We should at least support using an expired key to add a new recipients.

Todo:

  • Add a test case for this
  • Add a warning when there is such a thing, and proper documentation about what is possible to do in that case (namely: extend the private key expiration date OR remove the expired key and add a new key with a expiration date in the future)
  • Add a flag to "ignore" expired keys upon re-encryption as long as there are still valid keys in the recipient list (or maybe provided we are working with a non-expired subkey related to the same master key as the expired key?)
  • Make sure we cannot add or create or generate new secrets if we have an expired key: Warn the user they need to add a new non-expired key and remove the expired key.

Somewhat related to #2015 too.

@AnomalRoil AnomalRoil modified the milestones: 1.x.x, 1.14.0 Dec 21, 2021
@dominikschulz dominikschulz modified the milestones: 1.14.0, 1.x.x Jan 17, 2022
@dominikschulz dominikschulz self-assigned this Dec 23, 2022
dominikschulz added a commit to dominikschulz/gopass that referenced this issue Dec 23, 2022
Fixes gopasspw#1918

RELEASE_NOTES=[ENHANCEMENT] Check recipients before adding a new one.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
@dominikschulz dominikschulz modified the milestones: 1.x.x, 1.15.3 Dec 23, 2022
dominikschulz added a commit that referenced this issue Dec 24, 2022
* Check existing recipients before trying to add a new one

Fixes #1918

RELEASE_NOTES=[ENHANCEMENT] Check recipients before adding a new one.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

* Add test for CheckRecipients with an invalid key.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

* Add custom error type and a better error message.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

* Initialize InvalidRecipientsError

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>

* Skip CheckRecipients tests on Windows

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants