LeSuisse / vault-gpg-plugin

"Transit like" secret backend plugin for PGP/GPG in Hashicorp Vault
MIT License
89 stars 20 forks source link

Feature Suggestion: Expose subkeys #10

Open jdelic opened 6 years ago

jdelic commented 6 years ago

I am currently implementing this plugin to distribute GPG keys in my cluster to be used with duplicity. While doing that I came up with the following idea. Unfortunately, I don't speak Go yet, so I'll settle for filing an issue here.

The basics

GPG keyrings consist of a certification key (C) which can be equivalent to the signing key (S), which is the default in GnuPG and an encryption subkey (E). For obvious reasons, the signing key and encryption key shouldn't be the same. For the certification and signing keys that is debatable and great things can be accomplished when they're not the same. Unfortunately the library underlying this plugin seems to only create this type of keyring and doesn't expose an API to create a certification-only key, which I think is a bug, that I'll report over there.

Still, it's perfectly viable for a keyring to have multiple signing subkeys and encryption subkeys, which is especially useful when combined with expiry dates.

The request

So it would be fantastic if the plugin would expose additional API endpoints to manage subkeys for the keyrings stored in Vault, allowing users to create new ones, perhaps even with permissions scoped to types of subkeys.

Future outlook

Even better would be, when crypto.openpgp starts supporting it, to have an API model that supports the keyring-structure described in the article linked above, where only Vault holds the certification key and can create new subkeys, but an API exists to export the keyring with the certification private key removed, leaving only the private keys for the subkeys in there.

Thank you for writing this plugin in the first place 👍

LeSuisse commented 6 years ago

Hi,

Thank you for your interest and this proposal!

So yeah subkeys is something that I had in mind when I started the plugin. I did not implemented it for a few reasons:

I'm not really against it as I perfectly understand it can be useful. It is however unlikely that implement it myself in the near future.

Quick note on your usage with duplicity: I'm not intimately familiar with it but I'm guessing you end up with large encrypted files. Vault limits the size of the requests si you won't be able to use the decrypt endpoint. You will probably need to write a small wrapper to extract the encrypted session key of your file and call the show-session-key endpoint to get the session key and use it to decrypt the rest of the file.

jdelic commented 6 years ago

I wish I was more famliar with golang already, then I'd take a stab at implementing it myself, but right now I am at the mercy of others :). Though I can live with the "basic" generated keyrings as well for this particular use-case.

Quick note on your usage with duplicity: I'm not intimately familiar with it but I'm guessing you end up with large encrypted files. Vault limits the size of the requests si you won't be able to use the decrypt endpoint. You will probably need to write a small wrapper to extract the encrypted session key of your file and call the show-session-key endpoint to get the session key and use it to decrypt the rest of the file.

So duplicity sometimes needs access to the decrypted metadata when it verifies backups or manages server-side incremental backups. As such, it actually must have access to the private key. So right now I'm planning on setting exportable: true and importing the full secret keyring into the local gpg keyring used by duplicity. So I kind of "sidestep" that particular issue.

But this also brings me to the point where subkey management would be useful, in my opinion. Right now I can only allow the whole keyring to be exported or not. I was envisioning a version where Vault allows the export of secret subkeys, but not the main certification key. This way, only Vault can certify and revoke subkeys, but clients can gain access to the secret keys of the subkeys for their operations. In theory Vault could limit a token's access even to only certain subkeys of a keyring.

Anyway, thank you for your response. As I understand the upstream PR into the Vault Transit backend, I have some time to learn golang before this is being merged into Vault proper ;)

erahn commented 5 years ago

Hello LeSuisse. I'm interested in this feature as well. I am thinking of implementing it myself. I did see in your merge request to Vault upstream you discussed how an API for subkeys could work. As a result I want to ask before I get started - are you working on this presently?

LeSuisse commented 5 years ago

Hi!

You are right there has been some discussion in hashicorp/vault#4308 but there has not been a lot of progress on my side on this.

In the meantime I got a few more encounters with PGP (outside of this plugin/Vault) and let just say the more it goes the less I'm convinced that I can propose something that works with all the things people do with PGP. Merging it upstream will be a painful process for everyone and after that it will not be a walk in park to maintain. My take on PGP these days is avoid it if you can.

That's said, I still use the plugin in production, I will keep maintaining it and even if subkeys are not really needed for my use cases I will be happy to review a PR. SO @erahn if you want to give it a try, please help yourself ;) .

syadav2015 commented 5 years ago

@erahn @LeSuisse I've created a pull request with changes supporting subkeys with the plugin, plus support for revoking keys and subkeys as well as signing identities of keys. It would be great to this merged into the vault-pgp-plugin repository. The upstream golang changes that this PR uses are also in review here before they merge into crypto/openpgp package. Till then, the crypto package changes are vendored in using the replace directive in go modules. Once the upstream changes merge, this directive can be removed from the go.mod file. I would be happy to discuss/address any concerns with the PR needed to get this to merge.