Add smartcard/NFC support - #977
Open
norbusan wants to merge 14 commits into
Open
Conversation
Author
|
I tested this build (with only added changes being image of icon and appId) and importing a key, using it to pull/push, using it for decrypting a secret worked. |
|
Hey! I had a quick look at your changes and they looks great, thank you very much. You saved me a lot of work and time 🫣 |
agrahn
force-pushed
the
develop
branch
4 times, most recently
from
July 21, 2026 21:10
96cc5ae to
f10d4ea
Compare
Smartcard-backed PGP decryption and SSH/commit signing over NFC: OpenPGP card communication, PIN prompt with retry-counter handling, commit signing via OpenPgpCommitSigner, and a modernized PGP key list view. Includes required manifest, settings, and string changes.
Authenticate git SSH sessions with a PGP authentication subkey on an OpenPGP smartcard (PgpCardSshAuthMethod).
Previously the OpenPGP card SSH auth path handled only Ed25519 and ECDSA keys and rejected RSA. Adding RSA required two changes: - Algorithm negotiation. sshj advertises RSA keys under a negotiated algorithm name (rsa-sha2-512, rsa-sha2-256 or ssh-rsa) that differs from the key's base type, and the name in the signature blob must match the one in the request. The old override signed with KeyType.toString(), which is always ssh-rsa (SHA-1) and mismatches what putPubKey advertised. Because the card replaces both putPubKey and putSig, CardSshAuthPublickey now tracks its own copy of sshj's KeyAlgorithm queue (peeked for the current algorithm, dropped in shouldRetry) so the advertised algorithm, the signed algorithm and the retry fallback stay in lockstep. - Card signing input. For RSA the signer builds a PKCS#1 v1.5 DigestInfo (DER hash prefix plus digest, hash chosen from the negotiated algorithm) and hands it to INTERNAL AUTHENTICATE; the card applies the PKCS#1 padding and modular exponentiation and returns the raw modulus-sized signature, which is used directly as the SSH rsa_signature_blob.
verifyPin encoded the PIN with pin.concatToString().toByteArray(), which materializes the PIN as an immutable String. Strings cannot be zeroed and linger on the heap until garbage collection (and may be paged out or captured in a heap dump), defeating the finally-block that wipes the byte copy. Encode the CharArray straight to a wipeable ByteArray through a CharBuffer and zero the encoder's backing array, so no unwipeable copy of the PIN is created.
The public key advertised by the card's URL data object was fetched with URL(url).openStream().readBytes(), accepting any scheme and reading the stream unbounded. The URL originates from the card and can be attacker-influenced, so a plain-HTTP URL is MITM-able, a file:// URL would read local files, and an unbounded read lets a hostile endpoint exhaust memory. Only honor HTTPS URLs and cap the download at 1 MiB (public keys are a few KiB at most). Non-HTTPS or oversized responses fall back to manual import.
The card decryptor only verified the integrity packet when one was present and otherwise returned the plaintext silently. A message with no MDC/SEIPD (legacy SED) is unauthenticated and malleable, and the app's main PGPainless path already refuses such messages by default. Reject messages without integrity protection before decrypting, and document that the in-memory output buffer is discarded by the caller when verify() fails after the streamed plaintext has been written.
Every public-key encrypted session key packet that matches (including every wildcard-recipient packet, which always matches) drives a decipher operation on the card. A crafted message could enqueue arbitrarily many such packets and push a large amount of work onto the card. Bound the attempts to 16.
transceiveExtended was never called (it was annotated @Suppress("unused")); the short-APDU and command-chaining paths cover every card exchange. Drop the dead code.
BasePGPActivity.decrypt() routes any smartcard-backed or stub-decryption-key identifiers straight to decryptWithPassphrase (which handles PIN entry inline) and only falls through to askPassphrase when needsSmartcardPin is false. So the needsSmartcardPin branches inside askPassphrase -- the card-PIN PasswordDialog variant and its dedicated caching block -- can never run. Remove them, which in turn makes the title/hint/error/cache-label parameters added to PasswordDialog unused, so revert PasswordDialog to its single hardcoded passphrase configuration.
Decryption, commit signing and SSH authentication each carried a near-identical ~100-line state machine: create a reader, seed the PIN from cache, then loop prompting for the PIN, running the card operation, and on a rejected PIN wipe it, consult the card's remaining-attempts counter and either re-prompt or report the card blocked, with transient transport errors re-presenting the card. Three copies of this security-sensitive logic meant a fix to one could miss the others. Extract it into a single suspend helper, OpenPgpCardPrompt.runWithPin, returning a CardOutcome (Success/Cancelled/Blocked/Failed) with any open card handed back. Each call site now supplies only the PIN slot (PinMode), the prompt strings, the card operation, and how to map each outcome to its own reader-release and user-facing behaviour (which genuinely differ: decrypt shows a dialog, signing rethrows to its reporting catch, SSH throws SSHException and blocks on card removal). The PIN lifecycle -- seeding, wiping, and cache-only-on-success -- is owned entirely by the helper.
…refixes CardSshSigner carried three hand-written RFC 8017 DigestInfo DER prefixes for SHA-1/256/512, while OpenPgpCommitSigner already builds its DigestInfo through BouncyCastle's DigestInfo/AlgorithmIdentifier. Emit the DER the same way here, mapping each SSH RSA algorithm to its digest OID, and drop the prefix tables.
The decrypt/sign/ssh cache keys deliberately stay separate even though a card's PW1 signing and decryption/auth slots are usually the same physical PIN. Note why, so the apparent duplicate prompting is not "fixed" by assuming the slots share a secret, which is not guaranteed across cards.
Author
|
Rebased after target branch got force-pushed |
agrahn
force-pushed
the
develop
branch
4 times, most recently
from
July 23, 2026 15:30
79f8aaf to
8c172aa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add support for GPG keys on smartcards, both for signing as well as ssh-ing.
This PR started with the relevant code changes from https://github.com/valasiadis/Android-Password-Store who implemented the original changes.
It then added:
The commits are attributed according to the original author: