Smartcard menu: drop Common menu, split per-applet; add master fingerprint & Satodime - #442
Open
3rdIteration wants to merge 27 commits into
Open
Smartcard menu: drop Common menu, split per-applet; add master fingerprint & Satodime#4423rdIteration wants to merge 27 commits into
3rdIteration wants to merge 27 commits into
Conversation
…print & Satodime Removes the shared "Common Functions" menu and its Device Filter (fixes #402), which confused users and misbehaved when multiple applets were loaded on one card. The common functions are now duplicated into each applet's own "Card Settings" submenu, with the card filter applied automatically from the launching menu instead of a controller-wide toggle. - Satochip / SeedKeeper / Satodime each get a Card Settings submenu hosting the applicable shared views (Info, Genuine Check, Change PIN/Label/NFC, Configure NDEF, Factory Reset), scoped by an explicit card_filter argument. - Add "View Master Fingerprint" to the Satochip and KeyCard menus (fixes #401): a quick check that reads the master xpub at m without running the full xpub export flow. - Add a Satodime menu + views (deposit addresses, seal/unseal slot, sign tx, transfer ownership), cherry-picked from PR #66 and relocated into smartcard_views.py. Tests: per-applet card-filter unit tests; navigation coverage for the new submenus and fingerprint entries; jcardsim flow coverage for Satodime (real applet); hardware-present keyslot seal/unseal/pubkey tests. Removes the Device Filter flow test.
…p the unlock code A factory-fresh Satodime was unusable: "View Deposit Addresses" showed a parser error on every slot and "Seal Slot" failed. Three separate faults, all confirmed on hardware: - init_satochip() ran the shared PIN-enrolment branch whenever setup_done was False. Satodime has no PIN, so the user was asked to invent one the applet then ignores. - Nothing ever ran INS_SETUP, and the applet answers every state-changing APDU with 0x9C04 until it has. Backing out of the bogus PIN prompt left the card unclaimed, so seal failed. - ec.PublicKey() takes secp256k1's internal 64-byte point, so passing the card's 33-byte SEC pubkey raised "Pubkey should be 64 bytes long" on every sealed slot. ec.PublicKey.parse() is the constructor that reads SEC. Claiming now lives in ToolsSatodimeClaimView behind an explicit confirmation: setup mints a fresh unlock secret, which on a card mid-ownership-transfer would take it from whoever it was being handed to. Read-only views no longer force a claim, since status/keyslot/pubkey all work on an unclaimed card. Consistency with the official Satodime apps (Toporin/Javacryptotools): - Addresses are bech32 P2WPKH, not P2PKH. BaseCoin.pubToAddress() returns segwit whenever the coin supports it and Bitcoin sets segwit_supported = true, so P2PKH printed a different address for the same key and the official app reported a zero balance on anything deposited to it. - Sealing now tags the slot with BTC's slip44 and the deprecated 34-byte contract/tokenid block, byte-identical to NFCCardService.seal(). Untagged slots read back as slip44 0x00000000 and show as an unknown asset. - A slot sealed for another chain renders "Not Bitcoin (<coin>)" rather than a Bitcoin address derived from its key. Unlock-secret handling, for contactless readers: The applet only enforces the unlock code over NFC; a contact interface skips the check entirely. The 20-byte secret is emitted once by INS_SETUP and can never be re-read, so over NFC losing it strands the card -- ownership transfer is gated too. Claiming over contact therefore skips the ceremony entirely, while over NFC it hands off to a backup flow: QR display with a mandatory scan-back, optional MicroSD copy, and a warning that a contact reader can unseal the card without the code at all. Secrets are cached in RAM for the session only, and ToolsSatodimeRestoreUnlockView loads one back from QR or MicroSD. Tests: the jcardsim suite went from 2 tests to 40. The old ones passed while the feature was broken because the fixture monkeypatched init_satochip away (hiding the PIN bug), a fresh applet has no sealed slots (so get_pubkey raised before the views reached the address bug), nothing ever sealed (so 0x9C04 was never hit), and the one view test asserted only on navigation -- a screen whose body was an exception message counted as a pass. A simulated_satodime_raw fixture now patches only PC/SC so the real client runs, and a golden vector pins our derivation to an address the official Android app displayed for a known pubkey. The hardware suite's Satodime _provision() was a no-op; it now claims the card through the app's own helper and asserts the address format and slot tagging. Also adds the 0x9C50/0x9C51/0x9C54 unlock status words, which surfaced as raw hex. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Satodime had no documentation beyond an applet name in a config listing, and the raw-key signing path in particular has non-obvious requirements that cost real debugging time. Covers claiming (and why Satodime never asks for a PIN), the unlock code and the fact that it only matters over NFC, the per-coin address and key formats, and the Electrum procedure for producing a psbt a single raw key can sign. Two things are called out explicitly because they are what broke this in practice and both are now fixed: - SeedSigner used to derive a legacy 1... address where the official apps derive bc1q..., so a watch-only wallet built from the old address was watching a different address than the Satodime app showed. - The seed-selection screen matched on bip32 fingerprints, which an Electrum watch-only export does not carry, so a raw key was silently dropped. States plainly that the unlock code is proximity protection and not theft protection: anyone with the card and a contact reader can unseal it regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The old flat menu (View Deposit Addresses / Seal Slot / Unseal Slot) forced a full card read on every entry and re-read each slot for every action. Reworked around one cached snapshot per session: - ToolsSatodimeView is now a plain menu: Key Slots, Transfer Ownership, Card Settings. It never touches the card. - New ToolsSatodimeSlotsView reads every keyslot once (status + keyslot status + pubkey for live slots) and caches (state, coin, address) per slot on the Controller as satodime_slot_cache. Re-entering it reuses the cache; the cache is dropped when returning Home alongside the unlock secrets. - ToolsSatodimeSlotMenuView offers only the actions valid for a slot's state: [Seal] uninitialized, [View Address, Unseal, Sign] sealed, and [View Address, View Private Key, Sign, Load Key, Reset] unsealed (Bitcoin adds Sign/Load). - Seal/Unseal/Reset/Sign/Load/ViewPrivateKey/ViewAddress read their slot's state from the cache instead of re-querying the card, and seal/unseal/reset update the cached entry in place so returning to the menu reflects the new state immediately. A missing cache fails closed (Unknown State) rather than guessing; SlotMenu redirects to SlotsView to rebuild it. Adds 0x9C52 "Incorrect keyslot state" to the ISO7816 status-word table: the applet answers with it when a slot is not in the state the command accepts (SEAL needs Uninitialized, UNSEAL needs Sealed), which the re-seal refusal surfaces as the no-backup warning. Tests: nav flows walk Key Slots -> slot list -> per-slot menu for both an uninitialized and a sealed slot; the jcardsim suite drives the cached views against the real applet (slot list, seal/unseal/reset, view address/privkey, load key) with the cache pre-built exactly as ToolsSatodimeSlotsView would.
…dIteration/seedsigner into feat/smartcard-menu-split-dev
- Remove Configure NDEF from Satodime Card Settings (NDEF only exists in the unreleased v0.2-beta applet, so offering it just fails on every card). - Rename the slot-menu actions to spell out what each does: Seal Slot (Initialise New Key), View Address (QR), Unseal Slot (View Private Key), View Private Key (QR). - Reword the seal 'no backup' warning into one flowing paragraph and rewrite the reset warning to focus on the unrecoverable loss of the private key; give the re-seal refusal its own constant so it fits the warning screen. - Wipe the cached Satodime slot data when backing out to the smartcard menu (not just Home) so re-entering Key Slots reads fresh state. - init_satochip: never prompt Satodime for a PIN, bind card_pin for the cache step, and stop Satodime from overwriting the cached Satochip PIN (which would otherwise leave set_pin(0, None) for a later Satochip reconnect). - Add a regression test that drives the real init_satochip against a simulated Satodime for genuine check (reproduces the on-device UnboundLocalError without the fix). - jcardsim simulator: refuse to spawn a JVM when free RAM is low (SEEDSIGNER_JCARDSIM_MIN_FREE_RAM_MB, default 3GB); document in AGENTS.md.
ClaimView and ReshowUnlockView now use skip_current_view=True when forwarding to BackupUnlockView, so BackStackView pops straight back to the parent (slot action / card settings) instead of re-running the previous view in a loop. Also fixed MainMenu cache-wipe timing: Satodime_unlock_secrets must be cached after passing through Home via before_run callback. Added two nav regression tests covering both paths.
Direct access to the claim flow from Tools → Smartcard → Satodime, instead of requiring the user to navigate through Key Slots → Seal Slot. Shows 'Already Claimed' warning if the card has an owner.
All user-facing strings now use 'ownership key' terminology: - Card Settings buttons: Back Up/Restore Ownership Key - Screen titles: Ownership Key, No Ownership Key, Ownership Key Set - Warning texts updated accordingly
Contact readers never need the ownership key, so show 'Not Applicable' instead of 'The card only reveals it when first claimed.' NFC cards still get the original message explaining the key was lost.
Claim, Back Up, and Restore ownership key now show 'Not Applicable' when connected via a contact reader, since the applet never uses the ownership key over that interface. Tests updated to simulate NFC.
3rdIteration
force-pushed
the
feat/smartcard-menu-split-dev
branch
from
September 9, 2026 20:11
a331f32 to
f5ed5cd
Compare
3rdIteration
force-pushed
the
feat/smartcard-menu-split-dev
branch
2 times, most recently
from
September 10, 2026 02:15
377c807 to
25cbe59
Compare
The connection medium cannot be detected reliably at the PC/SC layer (dual-interface readers report T=1 for both contact and NFC), so stop guessing. Whatever secret is cached (or the zeroed placeholder) is sent; over contact the applet skips the unlock check entirely, over NFC it rejects with 0x9C50/0x9C51 and the caller now turns that into a 'Key Required -> Restore Key' prompt. - Remove satodime_connection_is_contactless + reader-name heuristics - Remove 'Not Applicable' blocks from Claim/Reshow/Restore views - Claim always offers the backup flow (user can skip; medium unknown) - Wire _satodime_handle_unlock_error into seal/unseal/sign/reset/transfer - Tests: NFC rejection now faked at connector level (jcardsim is contact); drop TestContactlessDetection; fix stale unseal label assertion
SimLauncher gains an optional --protocol argument (e.g. T=CL,TYPE_A,T0) that calls jcardsim's changeProtocol() before serving APDUs, so applets keying security behaviour off APDU.getProtocol() & PROTOCOL_MEDIA_MASK can be exercised over a simulated ISO 14443 Type A card. Without it the launcher command is unchanged (contact default).
_satodime_prepare called the no-argument satodime_set_unlock_counter(), which resets the local counter to zeros. Over a contactless reader the applet checks that 4-byte counter on every state-changing APDU and answers 0x9C50 for a stale or zeroed value -- so after claiming, sealing failed with 'Key Required', and restoring the ownership key did not help: the next prepare zeroed the counter again and dead-ended forever. Sync via satodime_get_status() instead (INS 0x50 returns the card's real counter without an unlock code; pysatochip caches it, and each successful gated APDU advances both sides in lockstep). Read-only views keep the old zeroing -- they send no counter and are never gated. Adds test_nfc_claim_restore_seal_end_to_end: runs claim -> lose key in a new session -> seal refused 0x9C51 -> restore from backup -> seal success against the real applet with jcardsim reporting contactless Type A media, i.e. exactly what a real NFC reader enforces. Verified to fail at the final seal (Key Required instead of Success) when the sync is reverted.
…re flows Claim Ownership on an already-claimed card now confirms once, releases the old owner via ownership transfer, and claims in one flow (no second 'Card Unclaimed' prompt). NFC unlock errors during the transfer route to key restore. BackupUnlockView re-checks the MicroSD each pass: when a matching backup exists the exit button reads 'Finalise Claim' (post-claim) or 'Done' (from Card Settings) and exits without the dire skip warning; otherwise it stays 'Skip Verification'. Save to MicroSD is now the top option. RestoreUnlockView silently checks for this card's backup file: found, it offers [Load Ownership Key from MicroSD, Scan Ownership Key]; not found, scanning starts immediately with no menu.
The backup flow now opens with a third intro screen carrying the 'Skip Backup?' consequence text (contact reader needed to reclaim; NFC-only cards locked), so users who bail before reaching the skip prompt have still seen what losing the key means.
Instead of jumping straight to the QR, the backup flow now opens with a 'Back Up Ownership Key' chooser: Show QR Code (photograph + scan-back verification) or Save to MicroSD. The QR path is an inner loop whose verify menu backs out to the chooser; saving flips the exit button to Finalise Claim/Done on the next pass.
Without is_text=True, DecodeQR runs detect_segment_type() on the payload and classifies it as INVALID (it matches no known format), so the scan-back verification could never succeed on device. Same pattern as the GPG text-QR scans. Adds an end-to-end test that renders the QR exactly like the screenshot generator does and feeds the frame back through the real ScanScreen + DecodeQR path -- every other backup/restore test mocks _satodime_scan_text, so only this one catches encoder/decoder mismatches. Verified to fail without the fix.
conftest and base installed a MagicMock pyzbar unconditionally, which made DecodeQR.is_qr_scanner_available() report True while extract_qr_data() silently decoded nothing (a MagicMock iterates empty). Any test exercising real QR decoding then failed on CI -- where libzbar0 is installed but the OpenCV fallback that masks this locally is not. Mock pyzbar only when it cannot really be imported, matching the existing pyscard pattern. Two follow-on fixes: - The text-QR decode nav test stubs ScanScreen: with real pyzbar the mocked camera's MagicMock frames crash zbar's pixel unpacking inside the real scan loop (it passed before only because the mock decoded nothing). - The Satodime scan-back e2e test skips when pyzbar is a mock, so it runs against a genuine decoder instead of failing on one.
The pinned pyzbar fork ships source only, so on Windows the native ZBar DLLs must be copied from the official PyPI wheel into the installed package. Without them QR scanning is silently disabled at runtime.
The card id (UID_SHA1) is derived from CPLC/IIN/CIN reads that some readers serve only intermittently, so the same chip could derive different ids across connections -- surfacing as a misleading 'Wrong Card' on restore and cache misses ('Key Required') even with the key in memory.
- satodime_card_id() now treats an unset UID_SHA1 or the empty-hash sentinel (sha1 of blank CPLC/IIN/CIN) as unidentified, never a real id
- ClaimView identifies the card before claiming (reconnect retries); it aborts with 'Cannot Identify Card' rather than minting a backup keyed to an empty id that can never be restored
- RestoreUnlockView shows 'Cannot Identify Card' when derivation fails; on a genuine mismatch it warns and offers Load Anyway instead of dead-ending
- ScanScreen.LivePreviewThread no longer dies silently: camera read failures log and exit cleanly, render failures log and continue (a stream stopped by the decode loop used to raise in the preview thread and leave the display undefined)
- jcardsim shim now serves deterministic per-card CPLC/IIN/CIN data so simulated cards derive real ids instead of all sharing the empty-hash sentinel
… checks Readers that serve CPLC/IIN/CIN only intermittently can complete a connection while the derived id is still unset or at the empty-hash sentinel, which misbehaved across several workflows: PIN swap detection could apply a cached PIN to an unconfirmed card (burning one of its limited tries), Satochip_Last_UID_SHA1 got poisoned with sentinels, and Satodime views showed misleading 'Key Required' / 'No Ownership Key' / 'Wrong Card' errors. - add is_usable_uid(): shared predicate for unset/empty/sentinel ids (full or truncated) - init_satochip re-queries the id itself: on a connected-but-unidentified card it rebuilds the connector and reconnects, bounded by MAX_UID_IDENTIFY_ATTEMPTS; plain connect failures keep their single 5-second window - PIN swap check trusts 'same card' only when both ids are usable AND equal; Satochip_Last_UID_SHA1 is never overwritten with a sentinel (recorded as one when previously unset, so continuity cannot be assumed) - Claim/Restore drop their now-redundant view-level retry loops; ReshowUnlock and GenuineCheck guard against unusable ids instead of failing misleadingly; Card Info no longer displays the raw sentinel
…rd id reads blank The ownership key can now be re-shown even when CPLC/IIN/CIN come back empty: the 'Cannot Identify Card' screen offers Retry (re-runs identification) or Use Nickname Only, and a key named earlier in the session is found via a new name->secret reverse lookup. Naming moves to an upfront screen at the top of the backup flow (pre-filled from any existing name), replacing the old chooser button; the name is written to the card label and recorded in the session cache so restore can match it.
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
1 participant
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.
Summary
Closes #402 and #401.
The Smartcard menu had a shared "Common Functions" menu (with a manual Device Filter) for things shared between the Satochip / SeedKeeper applets. That confused users, and worked badly when more than one applet was loaded on the same JavaCard. This removes it and re-homes those functions per-applet.
Menu changes
Common Functionsmenu and theDevice Filterscreen entirely (plus thecontroller.tools_common_card_filterplumbing).card_filter=["satochip"])["seedkeeper"])["satodime"]) — the subset Satodime supportscard_filterargument (intersected with each function's supported card types via a small_applet_card_filterhelper).Master fingerprint (#401)
mand shows the fingerprint directly, instead of only surfacing it inside the full xpub-export flow. Works on both the pysatochip and keycard-compat backends.Satodime menu (cherry-picked from #66)
smartcard_views.py(the PR targeted the pre-splittools_views.py).Testing
pytest tests/test_smartcard_card_filter.py— new unit coverage for_applet_card_filter, shared-view construction, and per-applet Card Settings routing.tests/test_flows_menu_navigation.py).test_real_screen_flows_smartcard*.py); Device Filter flow test removed.tests/test_real_screen_flows_satodime_simulated.pydrives the Satodime status APDUs andToolsSatodimeAddressesViewagainst a real applet (skips without Java).TestSatodimeintest_smartcard_hardware.pywith keyslot seal → pubkey read → unseal (destructive test last).Full smartcard-related suites pass locally; jcardsim/hardware tests skip cleanly where Java / a reader are absent.