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

Cannot add new secrets after adding additional recipients in case of age+gitfs configuration #2544

Closed
ckolumbus opened this issue Feb 8, 2023 · 4 comments · Fixed by #2545
Assignees
Labels
bug Defects
Milestone

Comments

@ckolumbus
Copy link
Contributor

ckolumbus commented Feb 8, 2023

Summary

After initializing a age+gitfs gopass storage new encrypted entries can be added and read, so far, so good.
After adding a second recipient with gopass recipient add a re-encryption happens BUT
adding new secrets with gopass edit -c test fails with ".. no valid keys (expired?)"

Steps To Reproduce

zsh❯ gopass setup --crypto age --storage gitfs

   __     _    _ _      _ _   ___   ___
 /'_ '\ /'_'\ ( '_'\  /'_' )/',__)/',__)
( (_) |( (_) )| (_) )( (_| |\__, \\__, \
'\__  |'\___/'| ,__/''\__,_)(____/(____/
( )_) |       | |
 \___/'       (_)

🌟 Welcome to gopass!
🌟 Initializing a new password store ...
🔐 No useable cryptographic keys. Generating new key pair
🧪 Creating cryptographic key pair (age) ...
⚠ Do you want to enter a passphrase? (otherwise we generate one for you) [y/N/q]: y
Enter passphrase for your new keypair: 
Retype passphrase for your new keypair: 
✅ Key pair for age generated
⚠ 🔐 We need to unlock your newly created private key now! Please enter the passphrase you just generated.
✅ Key pair age1f4h8pammn3ex34v53mmdsl6axvnug70mclqahqt22fmjc07x7pxshj77k7 validated
🔐 Cryptographic keys generated
🌟 Configuring your password store ...
Please enter an email address for password store git config []: xxxx
❓ Do you want to add a git remote? [y/N/q]: n
✅ Configuration written

zsh❯ gopass edit -c a
Warning: Password is too short

zsh❯ ./gopass recipients
Hint: run 'gopass sync' to import any missing public keys
gopass
└── age1f4h8pammn3ex34v53mmdsl6axvnug70mclqahqt22fmjc07x7pxshj77k7

zsh❯ gopass  a
Secret: a

test


zsh❯ ./gopass recipients add age1lm0kmca6mg92q2vkfnf023rhlcwf9yevygumgyqr2df5e4dc8dtqgnuy7m
  
Do you want to add "age1lm0kmca6mg92q2vkfnf023rhlcwf9yevygumgyqr2df5e4dc8dtqgnuy7m" (key "age1lm0kmca6mg92q2vkfnf023rhlcwf9yevygumgyqr2df5e4dc8dtqgnuy7m") as a recipient to the store ""? [y/N/q]: y
Reencrypting existing secrets. This may take some time ...
Starting reencrypt
] 1 / 1 [Goooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooopass] 100.00% ids = append(ids, r)

Added 1 recipients
You need to run 'gopass sync' to push these changes

zsh❯ gopass a
Secret: a

test

zsh❯ gopass edit -c b

Error: Invalid recipients detected: Invalid Recipients: age1lm0kmca6mg92q2vkfnf023rhlcwf9yevygumgyqr2df5e4dc8dtqgnuy7m: no valid keys (expired?), 

zsh❯ gopass recipients                                                                                                         ❮   17 
Hint: run 'gopass sync' to import any missing public keys
gopass
├── age1f4h8pammn3ex34v53mmdsl6axvnug70mclqahqt22fmjc07x7pxshj77k7
└── age1lm0kmca6mg92q2vkfnf023rhlcwf9yevygumgyqr2df5e4dc8dtqgnuy7m

Expected behavior

new secrets can be added an further more read by both recipients

Environment

  • OS: Windows 10 + Linux Manjaro
  • OS version: Linux pc-intel-nuc 5.15.91-1-MANJARO # 1 SMP PREEMPT Wed Feb 1 12:03:19 UTC 2023 x86_64 GNU/Linux
  • gopass Version: gopass 1.15.3 go1.19.5 linux amd64
  • Installation method: build from source (ref 3c66135)

Additional context

Preliminary analysis result:

in FindRecipients there is a loop that checks the recipient list against the entries found in the identities file. gopass add recipients only add the .age-recipients in the store and not to the identities (which it shouldn't and can't as identities contains secrets only). But so the check in Line 39 never succeeds as the additionally added recipients are never found in the list of loaded identities.

My assumption: the content of the identities file is different for the age backend and the code in FindRecipients should stay closer to the statement in the comment // For native age keys this is a no-op since they are self-contained (i.e. the ID is the full key already). in Line 13. A test with removing the two loops over recps and local and just calling ids = append(ids, r) without any check instead fixes the problem above. But I'm not sure whether there are any side effects of this "fix" that I don't see.

If you agree with the general approach of fixing this issue then I'd happily provide a pull request.

THANKS for gopass + age (+fossil ;-) )

@dominikschulz dominikschulz self-assigned this Feb 9, 2023
@dominikschulz dominikschulz added the bug Defects label Feb 9, 2023
@dominikschulz dominikschulz added this to the 1.15.4 milestone Feb 9, 2023
@dominikschulz
Copy link
Member

dominikschulz commented Feb 9, 2023

Thanks a lot for reporting this issue. This check is new and I have to admit that I did only test it with GPG so far.

Going back to my FindRecipients implementation is not fun. It's overly complicated (due to the shared legacy with the GPG backend). Let me try to unwrap it in my head and see if we can simplify things.

Feel free to send a PR if you want, but I'll need to also touch the code. It shouldn't remain as-is (too complicated for no reason).

dominikschulz added a commit to dominikschulz/gopass that referenced this issue Feb 9, 2023
The new recipients check for unusable GPG keys did not work for
age. In fact most age keys can be used as-is. At the same time
this is cleaning up the recipients handling for age a bit.

Fixes gopasspw#2544

RELEASE_NOTES=[BUGFIX] Fix recipients check for age.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
@dominikschulz
Copy link
Member

@ckolumbus I have opened #2545, but I can't add you as a reviewer.

dominikschulz added a commit to dominikschulz/gopass that referenced this issue Feb 9, 2023
The new recipients check for unusable GPG keys did not work for
age. In fact most age keys can be used as-is. At the same time
this is cleaning up the recipients handling for age a bit.

Fixes gopasspw#2544

RELEASE_NOTES=[BUGFIX] Fix recipients check for age.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
@ckolumbus
Copy link
Contributor Author

Hi @dominikschulz THANKS for the very quick fix and PR. I guess I'm not part of the project and therefor cannot be a reviewer. I'll have a look

dominikschulz added a commit that referenced this issue Feb 9, 2023
The new recipients check for unusable GPG keys did not work for
age. In fact most age keys can be used as-is. At the same time
this is cleaning up the recipients handling for age a bit.

Fixes #2544

RELEASE_NOTES=[BUGFIX] Fix recipients check for age.

Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
@ckolumbus
Copy link
Contributor Author

@dominikschulz just verified the merged PR at home... works 👍

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

Successfully merging a pull request may close this issue.

2 participants