feat: BTC-only firmware build support#388
Open
BitHighlander wants to merge 26 commits intodevelopfrom
Open
Conversation
- Point device-protocol and python-keepkey submodules to upstream master (includes BIP-85, Solana, Tron, TON wire IDs and proto definitions) - Add nanopb .options files for Solana, Tron, TON (field size constraints) - Add Bip85Mnemonic.mnemonic max_size:241 to messages.options - Update lib/transport/CMakeLists.txt with new proto sources, options, headers, and protoc compilation commands - Fix CI: use pre-installed clang-format instead of apt-get install (eliminates 3-minute timeout on GitHub runners) - Update Zcash transparent branch ID from Sapling to NU6
397323f to
47f49f1
Compare
…double-hash (F3) Replace early-return ECDSA verify chain with bitwise-OR accumulation and sentinel counter. A single voltage glitch can no longer skip one branch to fall through to SIG_OK — attacker must now corrupt all three verify results AND the sentinel. Add double SHA-256 computation with constant-time memcmp_s comparison to detect transient faults during hash computation. References: VULN-21020, fault-injection-assessment.md finding F3
…eck (F5) Remove redundant ~1 second full ECDSA re-verification in firmware main. The bootloader already performed authoritative signature verification before jumping here. Replace with fast metadata presence check that validates signature indices are present and distinct. Eliminates the wide timing window (VULN-21020 class) where a voltage glitch during the long-running crypto re-check could bypass protections. References: VULN-21020, fault-injection-assessment.md finding F5
Moves which_field tag assignment after the conditional memset and only resets when the oneof variant changes. Prevents allocated memory from being zeroed before release when PB_ENABLE_MALLOC is active. Ref: #361 Upstream: nanopb 4fe23595732b6f1254cfc11a9b8d6da900b55b0c
The condition on line 578 was inverted — `!enforce_wordlist` meant the wordlist check was skipped when enforce_wordlist was true (the default). Invalid words were silently accepted and only caught later as a generic "Invalid mnemonic" checksum error, giving users no indication which word was wrong. Flip the condition so that when enforce_wordlist is set, words that fail auto-complete against the BIP39 list are rejected immediately with a clear error before the mnemonic is finalized. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, wordlist validation only happened at finalization after all words were entered. Invalid words were silently accepted and the user had to complete the entire 12/24-word entry before getting a vague "Invalid mnemonic" error with no indication which word was wrong. Add a BIP39 check in the space handler of recovery_character() that rejects immediately with "Word not found in BIP39 wordlist" when the decoded characters don't match any entry. Combined with the enforce_wordlist condition fix, both per-word and finalization validation now work correctly.
Add Lynx cryptocurrency support to the firmware coins table. Parameters sourced from Lynx Core (Bitcoin v26 fork with PoS): - SLIP-44 coin type: 191 (0x800000BF) - Address type: 45 (pubkey hash), 22 (script hash) - SegWit enabled with bech32 prefix "lynx" - Taproot enabled - Standard Bitcoin xpub/ypub/zpub magic numbers - Signed message header: "Bitcoin Signed Message:\n" (matches upstream) - 8 decimal places, secp256k1 curve
Add BIP-85 support for deriving child mnemonics from the device seed. Supports 12, 18, and 24 word mnemonics via HMAC-SHA512 derivation on path m/83696968'/39'/0'/<word_count>'/<index>'. New files: - bip85.h/bip85.c: Core derivation using trezor-crypto primitives - fsm_msg_bip85.h: FSM handler for GetBip85Mnemonic message Integration: fsm.h, fsm.c, messagemap.def, firmware CMakeLists.txt device-protocol bumped to 17b38803e (includes BIP-85 message defs)
- Reject index >= 0x80000000 to prevent hardened-bit derivation collision - Add has_word_count/has_index field presence checks - Change confirmation text from "Derive" to "Export" to convey secret export - Add second confirmation before sending child mnemonic to host
Required proto fields don't generate has_ members in nanopb. word_count and index are guaranteed present by the proto schema. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
47f49f1 to
0b9c84f
Compare
Add Ed25519-based Solana chain support using existing trezor-crypto primitives (ed25519, base58). - SolanaGetAddress: derive Ed25519 key, Base58-encode pubkey - SolanaSignTx: parse transaction, confirm on device, Ed25519 sign - SolanaSignMessage: off-chain message signing with device confirm - Transaction parser: System Transfer, SPL Token, Stake instructions
…,M6) - Fix compact-u16 decode: use correct LE varint (bit 7 continuation), not big-endian ranges - Remove duplicate compact-u16 in solana.c, share via read_compact_u16() in solana_tx.h - Change solana_signTx from void to bool, propagate errors to FSM handler - Default show_display to true when field not set (hardware wallet must confirm) - Fix token transfer: check TransferChecked (len>=10) with instruction type, parse amount for both - Reduce stack: account_keys 64→16, instructions 32→8 (saves ~3KB) - Zero public_key after use in solana.c and solana_msg.c - Remove dead read_u64_le function - Fix formatLamports buffer guard: 20→30 bytes - Guard MIN macro with #ifndef - Add const to SolanaSignTx and SolanaSignMessage FSM params
- Fix NULL pointer dereference: remove fsm_getCoin("Solana") call (not in coin table),
use bip32_path_to_string() directly for address display
- Add BIP44 path validation (m/44'/501'/...) to all 3 handlers
- Zero public_key stack variables on all exit paths
TRON: secp256k1 + Keccak256 address derivation, SHA256 tx signing TON: Ed25519 address derivation with CRC16 + Base64url, Ed25519 tx signing Both use existing trezor-crypto primitives only.
- Fix bounceable flag no-op: use ternary (0x11 vs 0x51) instead of OR - Check ecdsa_uncompress_pubkey return value in tron_getAddress - Add BIP44 path validation: m/44'/195'/... for Tron, m/44'/607'/... for TON - Change signTx functions to return bool (was void, silent failures) - Check signTx return in FSM handlers, send proper failure on error Note: TON address derivation uses SHA-256(pubkey) which produces non-standard addresses. Standard TON addresses require SHA-256(StateInit cell) which includes wallet contract code — too large for firmware flash. This is a known limitation documented for future resolution.
ton_get_address() was computing sha256(pubkey) directly, which does not correspond to any deployable TON wallet contract. The correct TON v4r2 address is sha256(StateInit(code_cell, data_cell)) where the data cell contains seqno=0, walletId=698983191, and the ed25519 public key. The fix hardcodes the well-known v4r2 code cell hash and depth, computes the data cell representation hash from the public key, then derives the StateInit hash. This produces addresses compatible with the standard v4r2 wallet contract used by TonKeeper, MyTonWallet, and @ton/ton SDK.
0b9c84f to
0b9b319
Compare
# Conflicts: # include/keepkey/firmware/fsm.h # lib/firmware/fsm.c # lib/firmware/messagemap.def
# Conflicts: # include/keepkey/firmware/fsm.h # include/keepkey/transport/interface.h # lib/firmware/fsm.c # lib/firmware/messagemap.def
c666b82 to
2db352b
Compare
ff9f789 to
781aa84
Compare
Both standard and BTC-only firmware are now built on every PR/push, not just on release tags.
… options BIP-85 is chain-agnostic (BIP-32 derived mnemonics) and should be available in BTC-only builds. Move bip85.c, fsm_msg_bip85.h, and message map entries out of #ifndef BITCOIN_ONLY guards. Also remove duplicate/conflicting entries in messages-tron.options and messages-ton.options.
177f9bb to
ccf3947
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
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.
Motivation
A BTC-only firmware variant reduces attack surface for users who only hold Bitcoin. Every alt-chain message handler, signing implementation, and proto parser is code that doesn't need to exist on a Bitcoin-only device. Removing it:
This follows the pattern established by Trezor (which ships separate BTC-only firmware) and Coldcard (BTC-only by design).
Changes
Build system (
CMakeLists.txtat multiple levels):COIN_SUPPORT— set toBTCfor Bitcoin-only, unset for full multichainlib/transport/CMakeLists.txt: conditional proto compilation — BTC-only builds only compiletypes.proto+messages.proto, skipping all alt-chain protoslib/firmware/CMakeLists.txt: alt-chain source files added vialist(APPEND)insideif(NOT COIN_SUPPORT BTC)guardtools/firmware/CMakeLists.txt,unittests/: matching conditional compilationSource guards:
#ifndef BITCOIN_ONLY/#endifaround alt-chain includes infsm.c,interface.hmessagemap.deffsm_msg_common.h: token enumeration guarded (BTC-only firmware has no ERC-20 tokens)fsm.h: alt-chain function declarations guardedBTC variant:
lib/variant/keepkey/logobtc.c— Bitcoin-branded boot logoNo changes to the full multichain build path. When
COIN_SUPPORTis not set toBTC, all existing behavior is preserved — the guards compile out completely.Risk assessment
Moderate complexity but low risk to existing builds. The
#ifndef BITCOIN_ONLYpattern is purely additive to the full build (the define is never set in the normal build). The conditional CMakeLists uselist(APPEND)which doesn't modify the base lists.The main risk is missing a guard somewhere, causing a BTC-only build to reference an alt-chain symbol. This manifests as a link error (fail-fast, not silent). The CI runs both full and BTC-only builds to catch this.
Testing