[BUG] Authentication fails with curve25519 subkey on security token

[MoritzMaxeiner]

Describe the bug
Trying to clone a remote repository with OpenKeychain-provided curve25519 authentication subkey located on security token falls back on password authentication due null object reference in signing operation.

To Reproduce
Steps to reproduce the behavior:

1. Register a security token with an curve25519 authentication subkey in OpenKeychain
2. Try to clone a remote repository (where the key is authorized)
3. See (fallback) password prompt instead of (security token) pin prompt

Expected behavior
The pin prompt for the security token should have popped up.

Screenshots

Device information (please complete the following information):

• Device: Samsung SM-G960F
• OS: LineageOS 17.1-20201026-NIGHTLY-starlte
• App version 1.13.2

Additional context
logcat.txt

[msfjarvis]

@fmeum the exception is java.lang.NullPointerException: Attempt to invoke virtual method 'long org.bouncycastle.openpgp.PGPPrivateKey.getKeyID()', from within the guts of the OpenKeychain sshauth library.

[MoritzMaxeiner]

Should I report this to OpenKeychain itself instead, then?

[msfjarvis]

Should I report this to OpenKeychain itself instead, then?

No it's more likely that we regressed this on our end, once Fabian has had a chance to review they'll let you know if OpenKeychain needs to be notified.

[fmeum]

@MoritzMaxeiner The bug you are seeing has already been reported upstream as open-keychain/open-keychain#2538 and open-keychain/open-keychain#2589. I will try to fix it myself when I have time, but it is not a high priority as our new Keystore-backed SSH authentication serves more or less the same purpose (albeit it is slightly less convenient to set up as you need to add a new public key to your server).

[fmeum]

@msfjarvis I felt like we needed a blocked-on-upstream label, so I went ahead and created one.

[MoritzMaxeiner]

@fmeum I see, thanks. FWIW: The key is shown as healthy and capable of signing (In contrast to open-keychain/open-keychain#2589).

[clumbo]

Just to add to this, using a yubikey is completely broken atm, to keep things secure and not storing ssh private keys on my phone goes against the convention.

[sevmonster]

I don't know if this is related or not, but I feel like it might be.

Just to add to this, using a yubikey is completely broken atm, to keep things secure and not storing ssh private keys on my phone goes against the convention.

Mine works fine when I attempt to decrypt pass entries created or last edited from terminal. However, when I create a new entry or edit an existing entry and then later try to decrypt it, I am asked for PIN as normal, but then told the PIN is incorrect when I activate NFC. I try again, and OpenKeychain crashes with this error:

Attempt to invoke virtual method 'org.bouncycastle.math.ec.ECCurve org.bouncycastle.asn1.x9.X9ECParameters.getCurve()' on a null object reference

If I push, edit in terminal, and pull, I am able to open it again.

I know the PIN is correct because the counter does not decrement, and I can use the same PIN with other entries and *nix gpg just fine. Relevant info from GPG:

Application type .: OpenPGP
Version ..........: 3.4
Key attributes ...: ed25519 cv25519 ed25519
PIN retry counter : 3 0 3

Traceback:

FATAL EXCEPTION: AsyncTask #3
Process: org.sufficientlysecure.keychain, PID: 24474
java.lang.RuntimeException: An error occurred while executing doInBackground()
        at android.os.AsyncTask$3.done(AsyncTask.java:354)
        at java.util.concurrent.FutureTask.finishCompletion(FutureTask.java:383)
        at java.util.concurrent.FutureTask.setException(FutureTask.java:252)
        at java.util.concurrent.FutureTask.run(FutureTask.java:271)
        at android.os.AsyncTask$SerialExecutor$1.run(AsyncTask.java:245)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1167)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:641)
        at java.lang.Thread.run(Thread.java:764)
Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'org.bouncycastle.math.ec.ECCurve org.bouncycastle.asn1.x9.X9ECParameters.getCurve()' on a null object reference
        at org.sufficientlysecure.keychain.securitytoken.operations.PsoDecryptTokenOp.getEcDecipherPayload(PsoDecryptTokenOp.java:203)
        at org.sufficientlysecure.keychain.securitytoken.operations.PsoDecryptTokenOp.decryptSessionKeyEcdh(PsoDecryptTokenOp.java:119)
        at org.sufficientlysecure.keychain.securitytoken.operations.PsoDecryptTokenOp.verifyAndDecryptSessionKey(PsoDecryptTokenOp.java:80)
        at org.sufficientlysecure.keychain.ui.SecurityTokenOperationActivity.doSecurityTokenInBackground(SecurityTokenOperationActivity.java:214)
        at org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity.handleSecurityToken(BaseSecurityTokenActivity.java:439)
        at org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity$1.doInBackground(BaseSecurityTokenActivity.java:149)
        at org.sufficientlysecure.keychain.ui.base.BaseSecurityTokenActivity$1.doInBackground(BaseSecurityTokenActivity.java:137)
        at android.os.AsyncTask$2.call(AsyncTask.java:333)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        ... 4 more

Some bugs with the same exception:

• Null object reference when ECDSA key open-keychain/open-keychain#1933
• Attempt to invoke virtual method 'org.bouncycastle.math.ec.ECCurve org.bouncycastle.asn1.x9.X9ECParameters.getCurve()' on a null object reference bcgit/bc-java#425

Unfortunately I am no guru so leafing through the packets does not reveal much about why this might be happening.

Android-Password-Store packets:

% gpg --list-packets example.com.gpg
gpg: encrypted with 256-bit ECDH key, ID 0xABCDEF1234567890, created 2020-09-20
      "test <test@key>"
# off=0 ctb=84 tag=1 hlen=2 plen=94
:pubkey enc packet: version 3, algo 18, keyid ABCDEF1234567890
        data: [263 bits]
        data: [392 bits]
# off=96 ctb=d2 tag=18 hlen=2 plen=148 new-ctb
:encrypted data packet:
        length: 148
        mdc_method: 2
# off=117 ctb=ac tag=11 hlen=2 plen=105
:literal data packet:
        mode b (62), created 1611380040, name="GckbLi-example.com.txt",
        raw data: 69 bytes

GnuPG on Linux packets:

% gpg --list-packets example.com.gpg
gpg: encrypted with 256-bit ECDH key, ID 0xABCDEF1234567890, created 2020-09-20
      "testkey <test@key>"
# off=0 ctb=84 tag=1 hlen=2 plen=94
:pubkey enc packet: version 3, algo 18, keyid ABCDEF1234567890
        data: [263 bits]
        data: [392 bits]
# off=96 ctb=d2 tag=18 hlen=2 plen=118 new-ctb
:encrypted data packet:
        length: 118
        mdc_method: 2
# off=117 ctb=a3 tag=8 hlen=1 plen=0 indeterminate
:compressed packet: algo=1
# off=119 ctb=cb tag=11 hlen=2 plen=74 new-ctb
:literal data packet:
        mode b (62), created 1611380080, name="",
        raw data: 68 bytes

Relevant prefs from gnupg.conf on Linux:

personal-cipher-preferences AES256 AES192 AES
personal-digest-preferences SHA512 SHA384 SHA256
personal-compress-preferences ZLIB BZIP2 ZIP Uncompressed
default-preference-list SHA512 SHA384 SHA256 AES256 AES192 AES ZLIB BZIP2 ZIP Uncompressed
cert-digest-algo SHA512
s2k-digest-algo SHA512
s2k-cipher-algo AES256
charset utf-8

The only change made was a newline removed from the end of the file, and the only visible differences in the packets to my untrained eyes is that I use compression on Linux, and the file has a name on Android. Otherwise they appear identical.

[msfjarvis]

This is definitely an OpenKeychain bug, but their development has been too unreliable to rely on them for fixing these things. Forking introduces a rather large maintenance burden which I am not entirely confident that I want to take on, but sooner than later it might become a necessity. Increasing priority in the mean time.

[sevmonster]

Ultimately it seems their support for ed25519/cv25519 is just lacking in general... Which is a shame, as they are still very secure despite having smaller keys and operations being soo much faster on my devices.

[msfjarvis]

It looks like there is a pull request for fixing this specific bug, I've generated a release build for people willing to test it (source for it is available at https://github.com/android-password-store/open-keychain). You'll need to uninstall the Play Store/F-Droid version of OpenKeychain before installing this one.

[fmeum]

@DS6 Are you using a YubiKey 5 with firmware version 5.2.6 by any chance?

This is not a bug in OpenKeychain, but rather a hardware bug in the YubiKey 5 series before firmware version 5.2.8 (not yet released). I informed Yubico about this issue in September 2020 (see https://bugs.chromium.org/p/chromium/issues/detail?id=1120933#c10), but they have not released a fix yet. Once a fixed firmware version is available, you should be able to get a replacement from Yubico.

That said, the fix is simply to retry the OID lookup with the last byte stripped off if it fails the first time. I am running this locally and have had no issues with my YubiKey 5 NFC since.

[MoritzMaxeiner]

It looks like there is a pull request for fixing this specific bug, I've generated a release build for people willing to test it (source for it is available at https://github.com/android-password-store/open-keychain). You'll need to uninstall the Play Store/F-Droid version of OpenKeychain before installing this one.

Thank you, but both your linked APK, as well as building OpenKeychain myself from master branch and applying the linked fix to it yield a different error:

[msfjarvis]

It looks like there is a pull request for fixing this specific bug, I've generated a release build for people willing to test it (source for it is available at https://github.com/android-password-store/open-keychain). You'll need to uninstall the Play Store/F-Droid version of OpenKeychain before installing this one.

Thank you, but both your linked APK, as well as building OpenKeychain myself from master branch and applying the linked fix to it yield a different error:

That's weird... Can you also try out the development branch of APS? If it still fails, please try to get a logcat.

[codewiz]

Not sure if this is the same bug, but today I created an EdDSA subkey for authentication in OpenKeychain, exported it to my ssh server, then tried to use it in PasswordStore, and authentication fails.

No security token is necessary to trigger the bug. Authentication works when using an RSA 3072 bit subkey.

[msfjarvis]

OpenKeychain v5.6 is out, can the people who've reported bugs here check if any of their problems are fixed?

[MoritzMaxeiner]

OpenKeychain v5.6 is out, can the people who've reported bugs here check if any of their problems are fixed?

Using:

• OpenKeychain 5.6 from google playstore (not available yet on fdroid)
• Password Store develop branch, commit 051d455, built as freeDebug

I also get the unknown key format error mentioned here, logcat captured using Android Studio default settings here:
yubikey_5_curve25519_password_store_logcat.txt

So, the initial error relating to the yubikey's hardware bug is solved, but the other one remains.

[msfjarvis]

OpenKeychain v5.6 is out, can the people who've reported bugs here check if any of their problems are fixed?

Using:

• OpenKeychain 5.6 from google playstore (not available yet on fdroid)
• Password Store develop branch, commit 051d455, built as freeDebug

I also get the unknown key format error mentioned here, logcat captured using Android Studio default settings here:
yubikey_5_curve25519_password_store_logcat.txt

So, the initial error relating to the yubikey's hardware bug is solved, but the other one remains.

Thanks for the log, I'll take a look.

[msfjarvis]

https://github.com/open-keychain/open-keychain/blob/0ec0c34cb996e3bd848f0db111dca7da0db98c54/OpenKeychain/src/main/java/org/sufficientlysecure/keychain/pgp/SshPublicKey.java#L41-L57

yeah we can't do much here :(

[msfjarvis]

New APK with this PR included, source pushed to android-password-store/open-keychain, and the signature is the same as the APS binaries from Google Play and GitHub Releases.

Should fix ED25519 support, please let me know here and drop a comment in the linked PR if it solves things for you.

[tulir]

Added the PR to my own build (I had already built open-keychain/open-keychain#2631 myself earlier), can confirm it works with Yubikey 5C NFC

[MoritzMaxeiner]

Should fix ED25519 support, please let me know here and drop a comment in the linked PR if it solves things for you.

Works for me (also Yubikey 5C NFC). Thank you :)

[sevmonster]

Yep, everything is resolved with these series of patches. I finally feel that I can safely use my YubiKey on all my devices thanks to this setup solving it in Termux w/ OkcAgent for ssh/gpg and APS for pass.

Thank you everyone for your hard work.

[msfjarvis]

The fix for the Yubikey bug has landed upstream 🎉

[sevmonster]

The fix for the Yubikey bug has landed upstream 🎉

Excellent. But open-keychain/open-keychain#2662 is still not merged, so I think I will be sticking to the fork for a while longer.

[msfjarvis]

open-keychain/open-keychain#2662 has now been merged so this issue can be closed as soon as a new release is published for OpenKeychain.

[msfjarvis]

OpenKeychain v5.7 has been tagged.