-
-
Notifications
You must be signed in to change notification settings - Fork 18
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
gpg: add APIs for subkey interactions, revocations and key signing. #18
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18 +/- ##
==========================================
- Coverage 87.72% 81.68% -6.05%
==========================================
Files 6 12 +6
Lines 554 1092 +538
==========================================
+ Hits 486 892 +406
- Misses 34 102 +68
- Partials 34 98 +64
Continue to review full report at Codecov.
|
This commit addresses LeSuisse#10 by introducing subkey support in the API. The corresponding golang crypto library changes are in process of being merged upstream at https://go-review.googlesource.com/c/crypto/+/161817/. Documentation for the new APIs has been added in the README file. A brief description of the changes in the commit can be seen below: 1. Handling concurrent key read/write for all APIs. 2. Subkey create, read, list and delete APIs. 3. Key and subkey revocation API. 4. Key signing API to show trust in another key stored in vault. 5. Breadth tests for all newly introduced APIs.
50986f8
to
3c495f2
Compare
I tried to increase the coverage offered by the tests in 3c495f2 and the |
Hi, First of all, thank you for the contribution, awesome work!
Thanks for taking the time of doing that. It looks good. I will do a more in depth review by (hopefully) the end of the week. I will however not merge the PR until your CL is merged into x/crypto/openpgp. The future of x/crypto/openpgp is currently being discussed (see https://groups.google.com/d/topic/golang-openpgp/_P6AmeCmD9w/discussion) and I would prefer not having to maintain a patch on top of it (or one of the fork). |
Interesting PR, thanks @syadav2015! AFAICT, the in-memory/file physical backends of Vault already handle locking, no? If we need locking here, it would be on a higher level, such as adding subkeys around the same time to the same master key. |
@LeSuisse Any updates on this? |
ATM no. I'm guessing this could be revisited now that this repository uses the ProtonMail fork for PGP operations. DataDog fork of this plugin has support of subkeys operations: https://github.com/DataDog/vault-gpg-plugin Maybe we could pull back the changes in here. That said I'm not sure I want to add it because I am not sure to understand what we gain from it. I might be missing something here, if you have a scenario were this could be used which does not have a better alternative than PGP I would be interested to know. |
It's important to know that GPG can operate without the certification key being on file. The keyring I use every day looks like this:
GPG defaults to creating the certification key ( Right now, following the defaults, all keys generated by vault-gpg-plugin look like this:
Without having looked at this PR to see whether it makes that possible, I would say that the option to keep the certification key in Vault with the keyring distributed like the above, would be quite useful. Essentially I'd argue that it's useful to be able to get to this setup:
@syadav2015 is the above possible with the operations defined in this PR by any chance? |
Hey, I'm aware of that but it also means you end up doing key management outside of Vault. If that's fine with you can import your key with
This is what I imported in Vault (this is a throwaway key for the purpose of the test 😃 ):
|
sorry, perhaps I'm missing something obvious. But why would I end up doing key management outside of Vault? Couldn't Vault create a keyring with an "unexportable" certification key, and export me a keyring that only has the subkeys intact? Key operations could then only be done inside of Vault? |
OK you want to use the subkeys outside of Vault. That was not how I understood what you want to achieve. So yes it could likely be done but this PR does do that in its current state. I'm not sure what the advantages are to be honest. In case of a compromise of a subkey you would need to push to the system using the key a revocation certificate. At this point is it not as hard to push a new key and drop the old one? To be frank I'm not sure this is something I want. I created this plugin to have a solution for situations where there is no other ways than to suffer PGP/GPG (e.g. signing packages for Linux distributions). |
Not just, but it would fit my current use case perhaps even a bit better than what I originally was going for when I wrote #10. Using Vault as the access manager and secure store of the certification key sounds appealing to me. As does using the subkeys through the API. :)
That's fair. I have been using this plugin in production for a long time to generate and store keyrings for hosts in distributed clusters and it already solves a lot of those use cases. |
This commit addresses
#10 by introducing
subkey support in the API. The corresponding golang crypto library
changes are in process of being merged upstream at
https://go-review.googlesource.com/c/crypto/+/161817/. Documentation for
the new APIs has been added in the README file. A brief description of
the changes in the commit can be seen below: