Add SeedKeeper keystore support#21
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80c0fdcbbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.keystore_type == "seedkeeper": | ||
| self._load_seedkeeper() |
There was a problem hiding this comment.
_detect_keystore() returns the firmware-reported class name (e.g., SeedKeeper), but this branch compares against lowercase "seedkeeper", so SeedKeeper detection falls through to _load_internal_flash(). In that scenario the test harness configures the wrong unlock flow while still marking keystore_type as SeedKeeper (via .lower() in has_seedkeeper()), which makes SeedKeeper tests run against an internally loaded mnemonic path instead of the card path.
Useful? React with 👍 / 👎.
| await self.load_mnemonic() | ||
| await self.show( | ||
| Alert("Success!", "Your key is loaded from SeedKeeper.", button_text="OK") | ||
| ) | ||
| return True |
There was a problem hiding this comment.
load_mnemonic() explicitly returns False on card/app errors, but this menu path ignores the return value and always shows a success alert and returns True. If loading fails (or secret selection is canceled), the UI still reports success and the caller may reinitialize apps as if a new key was loaded, leading to misleading state and stale key usage.
Useful? React with 👍 / 👎.
| card_iv = encrypted_response[:16] | ||
| data_size = int.from_bytes(encrypted_response[16:18], 'big') | ||
| ciphertext = encrypted_response[18:18 + data_size] | ||
|
|
||
| cipher = aes(self.aes_key, 2, card_iv) |
There was a problem hiding this comment.
Response handling decrypts ciphertext immediately and never validates any message authentication tag, even though request wrapping includes a MAC. With no integrity check, a faulty or malicious card/link can alter encrypted response bytes and, if padding remains syntactically valid, the device may accept corrupted secret material (including mnemonic data) without detection.
Useful? React with 👍 / 👎.
When MemoryCard.is_available() fails (wrong card type, card removed during probe, etc), the connection is left open. This blocks all subsequent keystores from detecting because connect() fails when already connected. Add disconnect() in the except block so the connection is always released, allowing the next keystore in the polling list to probe. This is a latent upstream bug that becomes visible when multiple smartcard-based keystores are listed (e.g. MemoryCard + SeedKeeper).
Adds SeedKeeper as a smartcard-based keystore option. SeedKeeper stores encrypted BIP39 mnemonics on JavaCard and communicates via the Satochip secure channel protocol (ECDH + AES-CBC + HMAC-SHA1). Key implementation details: - HMAC-SHA1 via hashlib.sha1() (MicroPython hmac module lacks SHA1 support) - PKCS#7 padding for Satochip protocol compliance - 20ms stabilization delay in is_available() for USART settling - Conditional debug traces guarded by hil_test_mode
- Add ping() for connection health monitoring via get_card_status - Reset secure channel and disconnect on lock() - Incorporate card pubkey into userkey for per-card isolation - Add hexid property for card fingerprint display - Add 'Use a different card' to storage menu - Fix pin_attempts_left to return 0 instead of None on error - Fix Makefile echo quoting for POSIX shell compatibility
- Add import_secret (INS 0xA1) to seedkeeper_applet.py with multi-step INIT/PROCESS/FINALIZE protocol for BIP39 entropy import - Add TEST_IMPORT_SECRET, TEST_DELETE_SECRET, TEST_CARD_RESET HIL commands - Fix PIN error handling: use ISO 7816 0x63cX format (not 0x9c02) - Fix controller keystore detection: case-insensitive comparison - Handle blank SeedKeeper (no BIP39 secrets) in firmware unlock flow - Add SeedKeeper storage button to init menu - Add card management helpers to HardwareController: card_import_bip39(), card_delete_secret(), card_delete_all_secrets() - Add lifecycle HIL tests: delete/restore, import bacon, delete all - Fix _card_reset to re-init secure channel after GPIO power cycle - Add hexid property and userkey with card pubkey binding - Fix Makefile echo quoting for POSIX shell compatibility
1 participant
Summary
Adds SeedKeeper — a smartcard-based keystore that stores encrypted BIP39 mnemonics on JavaCard — as a new keystore option in Specter-DIY.
New files
src/keystore/seedkeeper.pysrc/keystore/javacard/applets/satochip_securechannel.pysrc/keystore/javacard/applets/seedkeeper_applet.pysrc/keystore/javacard/card_scanner.pysrc/gui/screens/debug_info.pytest/integration/tests/test_seedkeeper.pyModified files
src/keystore/memorycard.pydisconnect()in except block ofis_available()src/main.pysrc/specter.pysrc/hil.pytest/hil/controller.py_detect_keystore(),_load_seedkeeper(),_select_secret_by_label()Key design decisions
hmacmodule lacks SHA1; implemented viahashlib.sha1()(C-accelerated fromuhashlibusermod). See MicroPython hmac module lacks SHA1 support #20get_word=Nonefor SeedKeeper PIN screen — no anti-phishing words (card doesn't support them)init_apps()after unlock — USB commands need keystore-loaded apps. See init_apps() must be called after keystore unlock for USB commands #18hil_test_mode, no impact on production firmwareTesting
All tests use the
abandon * 11 + aboutmnemonic (fingerprint73c5da0a). The SeedKeeper card must be provisioned with this mnemonic under labelabandon.Related