-
-
Notifications
You must be signed in to change notification settings - Fork 274
Support OpenPGP hardware keys #2170
base: develop
Are you sure you want to change the base?
Conversation
val incomingKeyRing = tryParseKeyring(key) | ||
|
||
if (incomingKeyRing is PGPPublicKeyRing) { | ||
throw NoSecretKeyException(tryGetId(key)?.toString() ?: "Failed to retrieve key ID") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method intentionally did not forbid public keys because there are use cases where folks encrypt their store to an additional public key to grant read-only access to the store for the owner of that key without having to disclose their own.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this should be a TODO then.
Thanks for pushing this up! Realistically I will only be able to review it in a few days, but I've left some notes about your suggestions for future improvements.
I don't think I'd want that, keyservers are a fairly cursed domain that I don't wish to include in the app.
I'll see if I can do something about that
Let's avoid additional maintenance burden if it's not strictly necessary. |
This is a bigger issue now for some Android users since OpenKeychain has stopped working on some versions of Android 13. pgpainless/pgpainless#323 has been merged, does that hopefully unblock some things relevant to this PR? |
It was never really blocking the PR since we've been using a source dependency for the relevant PGPainless branch, the larger body of work here is to refine the user experience and ensure all edge cases are properly handled. This branch is also fairly out of date so I'm going to spend some time today rebasing it, but there are much more important things for me to work on so this PR is still on the back burner. |
4de5e9d
to
247670e
Compare
I've rebased the branch and preserved the previous state here in case there are any newly introduced errors in the rebase. |
Great to see all the progress :) |
Is that stale? |
Are there any plans to continue this draft or to replace it with a new MR? Currently the new v2 has a regression, since the old OpenKeychain integration supported hardware keys. |
As of right now, no. There is an upper limit on how much I can do alone and this does not currently fit into my availability. I do intend for this to be merged at some point but cannot promise any timeline for it. |
Integrate hwsecurity to delegate decryption operations to OpenPGP security keys. This has been tested with NFC and USB hardware.
The integration point is a new callback to
CryptoHandler.decrypt
that provides the encrypted session key and expects the decryptedPGPSessionKey
in return. The implementation then delegates as much as possible to hwsecurity'sOpenPGPSecurityKeyDialogFragment
.Importing is handled by creating GnuPG-compatible stub keys, so no change to key storage is required, and it should be possible to import a secret keychain with stub keys that was exported from GnuPG. The import activity also prompts to "pair" a hardware key when importing a public key, so one could download their key from a keyserver and have hardware support without too much work.
Future improvements:
TODOs: