Android-Password-Store icon indicating copy to clipboard operation
Android-Password-Store copied to clipboard

Tracking issue for RFC 1195: Migrate away from OpenKeychain

Open msfjarvis opened this issue 2 years ago • 45 comments

This is the tracking issue for the implementation of RFC #1195

Steps

  • [x] Introduce alternate PGP backend based on Gopenpgp (#1441)
  • [x] Replace Gopenpgp with PGPainless (#1522)
  • [x] Implement a key management interface
    • [ ] Allow listing keys by type (public/private)
    • [x] Allow deleting keys
  • [x] Implement key import step in onboarding to allow users to pick a key to initialize repositories with
  • [x] Implement passphrase caching (ref OpenKeychain)
  • [x] ~~Offer a migration path to OpenKeychain users to import keys exported from OpenKeychain into APS~~
  • [x] Remove OpenKeychain support completely

Unresolved questions

  • [x] ~~How will we handle migration? Do we attempt to automate this in any fashion or simply write documentation for users to follow?~~ We will not
  • [ ] What constitutes feature parity?
  • [x] ~~Do we release another major version when we drop OpenKeychain?~~ We're dropping it straight away

msfjarvis avatar Oct 23 '21 11:10 msfjarvis

Do we release another major version when we drop OpenKeychain?

I would say yes. This is a fairly major change.

How will we handle migration? Do we attempt to automate this in any fashion or simply write documentation for users to follow?

I would prefer the migration be automatic, but concise documentation on the process would be welcome either way.

daraul avatar Oct 23 '21 14:10 daraul

Do we release another major version when we drop OpenKeychain?

I would say yes. This is a fairly major change.

Agreed.

How will we handle migration? Do we attempt to automate this in any fashion or simply write documentation for users to follow?

I would prefer the migration be automatic, but concise documentation on the process would be welcome either way.

I haven't thought a lot about how this migration would happen, considering the fact that OpenKeychain encrypts the exported keys but I do think it'd be nice to be able to handle it on-device. Will have to investigate this down the line.

msfjarvis avatar Oct 23 '21 15:10 msfjarvis

Passing note, we definitely wanna do passphrase caching. Doing it securely might be a challenge but dear god users will chew me out if I make them enter their passphrase every single time.

msfjarvis avatar Jan 04 '22 20:01 msfjarvis

I assume support for hardware OpenPGP keys needs to be implemented by us, then? It looks like PGPainless doesn't support this kind of functionality.

malte-v avatar Jan 05 '22 14:01 malte-v

I assume support for hardware OpenPGP keys needs to be implemented by us, then? It looks like PGPainless doesn't support this kind of functionality.

That is correct.

msfjarvis avatar Jan 05 '22 14:01 msfjarvis

Thanks all for your work. I rely on this application.

Myridium avatar Jan 08 '22 00:01 Myridium

Status update

  • PGPainless backend now supports importing keys
  • Decryption now takes the password from the user rather than hard-code it into the app
  • You can opt into the PGPainless backend through a settings option

Next steps

  • Make encrypt/decrypt use the key from .gpg-id instead of attempting all keys
  • Reimplement error handling to surface them to the UI rather than crash the app

msfjarvis avatar Jan 09 '22 12:01 msfjarvis

For hardware security key support, I think this might be of use: https://github.com/cotechde/hwsecurity It's GPL3 licensed, pure Java and thoroughly documented at https://hwsecurity.dev/.

malte-v avatar Jan 10 '22 17:01 malte-v

For hardware security key support, I think this might be of use: cotechde/hwsecurity It's GPL3 licensed, pure Java and thoroughly documented at hwsecurity.dev.

We're aware of the Cotech SDK, but we don't have plans to use their OpenPGP library as of now. So far, I can't even get their official sample app to generate an OpenPGP key onto my test Yubikey so it's fairly certain that a lot of pain is headed my way when it eventually comes time to support security keys with the PGPainless backend.

msfjarvis avatar Jan 10 '22 19:01 msfjarvis

Error handling rework has landed in #1672, incorrect passwords will now pop up the password dialog once more with indication that the previously entered password was incorrect. Work on respecting the values from .gpg-id has also begun, with the refactor in #1669 paving the way for more accurately mimicking the OpenKeychain behavior w.r.t. key selection.

msfjarvis avatar Jan 20 '22 19:01 msfjarvis

Sometimes I have to wait a while to view my saved passwords and they don't show up right away, is this because OpenKeychain performs badly?

daiaji avatar Feb 02 '22 05:02 daiaji

Sometimes I have to wait a while to view my saved passwords and they don't show up right away, is this because OpenKeychain performs badly?

Not strictly OpenKeychain's performance, but the transport mechanism it uses to talk to apps. Android's Binder IPC is typically rather constrained and can often be very slow.

msfjarvis avatar Feb 02 '22 05:02 msfjarvis

Android is getting more closed all the time, maybe one day GPG proxy will have to be built into the password-store.

daiaji avatar Feb 03 '22 08:02 daiaji

Status update

  • PGPainless backend now supports importing keys
  • ...

I just installed the most recent snapshot version and tried to import a pgp secret key. I'm getting a (generic?) error message saying "error on pgp key import for input string "<my_key_id>"".

moppman avatar Feb 17 '22 15:02 moppman

Status update

  • PGPainless backend now supports importing keys
  • ...

I just installed the most recent snapshot version and tried to import a pgp secret key. I'm getting a (generic?) error message saying "error on pgp key import for input string "<my_key_id>"".

That's expected, the diagnostics are not in great shape at the moment. If you can provide clear reproduction steps from creating a new key on your PC to the import failing in APS I can try to replicate and debug it.

msfjarvis avatar Feb 17 '22 15:02 msfjarvis

Status update

  • PGPainless backend now supports importing keys
  • ...

I just installed the most recent snapshot version and tried to import a pgp secret key. I'm getting a (generic?) error message saying "error on pgp key import for input string "<my_key_id>"".

That's expected, the diagnostics are not in great shape at the moment. If you can provide clear reproduction steps from creating a new key on your PC to the import failing in APS I can try to replicate and debug it.

Sure. Note that I tried to import my "regular" key, i.e. I didn't create a new one.

  1. gpg --export-secret-keys --output sec.pgp --armor [email protected]
  2. Transfer sec.pgp to phone
  3. Import sec.pgp via pgp settings in APS

The key is a password-protected RSA2048 key with two user IDs. The import error is the same for armored and non-armored keys.

moppman avatar Feb 17 '22 16:02 moppman

Reproduced the issue.

msfjarvis avatar Feb 17 '22 19:02 msfjarvis

Import failure is fixed in snapshot builds with #1741

msfjarvis avatar Feb 21 '22 15:02 msfjarvis

https://github.com/pgpainless/pgpainless/pull/261 should help with improving how errors are surfaced to users

msfjarvis avatar Apr 09 '22 12:04 msfjarvis

How are we meant to make use of the new backend? Later snapshots started to reject OpenKeychain and would pop up an authentication dialog for my repo password (there isn't one). The available auth methods for the git config are SSH key, Password and OpenKeychain. I have been able to successfully import my key but no joy then running a git sync after.

worldofgeese avatar Jun 10 '22 19:06 worldofgeese

How are we meant to make use of the new backend? Later snapshots started to reject OpenKeychain and would pop up an authentication dialog for my repo password (there isn't one). The available auth methods for the git config are SSH key, Password and OpenKeychain. I have been able to successfully import my key but no joy then running a git sync after.

The new backend does not support using SSH through your PGP key, and we have not made any changes to how OpenKeychain handles it. Please file a separate issue and include your device logs and other relevant details.

msfjarvis avatar Jun 10 '22 19:06 msfjarvis

Thinking back and forth about this issue and #1862 is making me realize that there is little to no chance of me ever shipping these changes if I have to retain backwards compatibility. Having to maintain essentially twice the code for both accessing files and cryptography really hobbles my ability to push the project forward, as is evident in the commit activity on the project since May, where the only "useful" changes I've made are adding one small feature and merging a refactor I had finished all the way back in April.

I'm leaning towards not publishing any updates for the 1.13.x release train and instead letting v2 to be a proper break the world update, listed separately under a new package name, which adopters would migrate to themselves rather than Play Store or F-Droid automatically updating the app.

msfjarvis avatar Jul 12 '22 13:07 msfjarvis

#1880 has now been resolved which paves the way forward for a PGPainless-only future. By next week I aim to land the package name change as well as the removal of OpenKeychain, and then we can move on with a singular focus on improving the user story around the PGPainless backend. It's been a while since I've been this pumped about working on APS :)

msfjarvis avatar Jul 13 '22 20:07 msfjarvis

#2003 and #2004 complete the package rename and OpenKeychain removal steps respectively, my plan now is to implement the key management interface and plug it into key selection to restore parity.

msfjarvis avatar Jul 15 '22 09:07 msfjarvis

Looking forward to the updates! I noticed you'd mentioned potentially using age as a backend in this comment: https://github.com/android-password-store/Android-Password-Store/issues/1195#issuecomment-808940598.

Is this still planned/in the works?

raj-magesh avatar Jul 15 '22 09:07 raj-magesh

Looking forward to the updates! I noticed you'd mentioned potentially using age as a backend in this comment: #1195 (comment).

Is this still planned/in the works?

It is! @simao has been doing great work over on the kage library to implement the age specification in Kotlin, you can track the progress of it here. Once the MVP is ready we can start experimenting with it in Password Store.

msfjarvis avatar Jul 15 '22 09:07 msfjarvis

I've started work on key management, beginning with #2014 to kick off building the underlying APIs before the UI can come in.

msfjarvis avatar Jul 17 '22 17:07 msfjarvis

I ran into 2 issues which are probably relevant to a lot of yubikey owners as my setup was created from drduh's Yubikey Guide almost ver batim, but I wanted to confirm this isn't user error first.

here's some problems I noticed while running a free debug build:

  1. ssh doesn't seem to recognize how to handle a gpg based ssh key
  2. After importing my public key(found here), when attempting to access the contents of a password, it asks for the password, despite fact that the encryption subkey doesn't have a password (though the non-encryption masterkey does).

do you think these warrant creating new issues, or am I messing up initial configuration somehow?

skewballfox avatar Jul 18 '22 18:07 skewballfox

I ran into 2 issues which are probably relevant to a lot of yubikey owners as my setup was created from drduh's Yubikey Guide almost ver batim, but I wanted to confirm this isn't user error first.

here's some problems I noticed while running a free debug build:

  1. ssh doesn't seem to recognize how to handle a gpg based ssh key
  2. After importing my public key(found here), when attempting to access the contents of a password, it asks for the password, despite fact that the encryption subkey doesn't have a password (though the non-encryption masterkey does).

do you think these warrant creating new issues, or am I messing up initial configuration somehow?

  1. is a known issue but you can file separate issues for both to ensure they get triaged and resolved before release.

msfjarvis avatar Jul 18 '22 18:07 msfjarvis

Key management UI has landed, and I've started working on restoring support for .gpg-id. I expect it to be done sometime next week after which I'll start exploring passphrase caching.

I'm still on the fence about supporting SSH via PGP keys since firstly I have no idea how to build it, and secondly it doesn't seem any safer than the SSH key generation capabilities we already offer. If anyone has any compelling use cases for the feature feel free to mention them in this thread, otherwise there's a strong chance I simply do not build the feature out.

msfjarvis avatar Jul 24 '22 16:07 msfjarvis