diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9e0c0314..6ad235d0 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -59,6 +59,10 @@ jobs: with: node-version: ${{ env.NODE_VERSION }} + - name: Install protobuf compiler for zcash-cli (macOS) + if: runner.os == 'macOS' + run: brew install protobuf + - name: Install Yarn run: npm install -g yarn @@ -74,7 +78,7 @@ jobs: path: | modules/hdwallet/node_modules modules/hdwallet/packages/*/dist - key: hdwallet-${{ runner.os }}-${{ hashFiles('modules/hdwallet/yarn.lock') }} + key: hdwallet-${{ runner.os }}-${{ hashFiles('modules/hdwallet/yarn.lock', 'modules/device-protocol/*.proto', 'modules/device-protocol/*.options') }} - name: Cache vault node_modules uses: actions/cache@v4 @@ -92,12 +96,22 @@ jobs: projects/keepkey-vault/zcash-cli/target key: zcash-cli-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('projects/keepkey-vault/zcash-cli/Cargo.lock') }} + - name: Build canonical device protocol + shell: bash + run: | + cd modules/device-protocol + npm ci + test -s lib/messages_pb.js + cd ../.. + node projects/keepkey-vault/scripts/postprocess-device-protocol.mjs modules/device-protocol + node projects/keepkey-vault/scripts/verify-zcash-ironwood-protocol.mjs modules/device-protocol + - name: Build modules (hdwallet + proto-tx-builder) shell: bash run: | cd modules/hdwallet yarn install --frozen-lockfile - yarn tsc --build + yarn build cd ../proto-tx-builder git submodule update --init osmosis-frontend @@ -105,46 +119,10 @@ jobs: npx tsc -p . test -f dist/index.js - - name: Populate device-protocol lib/ + - name: Verify pinned device-protocol lib/ shell: bash run: | - # device-protocol/lib/ is gitignored — protobuf build output. - # Copy pre-built lib/ from hdwallet's resolved npm copy. - # hdwallet's yarn.lock pins the exact device-protocol version it was - # built/tested against, so its lib/ is the correct wire format. - if [ -f modules/device-protocol/lib/messages_pb.js ]; then - echo "device-protocol lib/ already present" - exit 0 - fi - - HDWALLET_DP="modules/hdwallet/node_modules/@keepkey/device-protocol" - - if [ ! -d "$HDWALLET_DP" ] || [ ! -f "$HDWALLET_DP/lib/messages_pb.js" ]; then - echo "FATAL: device-protocol lib/ missing and no hdwallet copy available" - echo "Build on macOS first: cd modules/device-protocol && npm install && npm run build" - exit 1 - fi - - SUBMOD_VER=$(node -p "require('./modules/device-protocol/package.json').version" 2>/dev/null || echo "unknown") - HDWALLET_VER=$(node -p "require('./$HDWALLET_DP/package.json').version" 2>/dev/null || echo "unknown") - echo "Submodule package.json: $SUBMOD_VER" - echo "hdwallet resolved npm: $HDWALLET_VER" - if [ "$SUBMOD_VER" != "$HDWALLET_VER" ]; then - echo "WARNING: version strings differ (git vs npm) — this is expected when" - echo "the npm publish was from a version-bump commit not on the submodule branch." - echo "Using hdwallet's resolved copy (it matches what hdwallet was built against)." - fi - - echo "Copying lib/ from hdwallet's resolved device-protocol..." - rm -rf modules/device-protocol/lib - cp -r "$HDWALLET_DP/lib" modules/device-protocol/lib - JS_COUNT=$(ls modules/device-protocol/lib/*.js 2>/dev/null | wc -l) - echo "Done — $JS_COUNT JS files" - if [ "$JS_COUNT" -lt 1 ]; then - echo "FATAL: cp succeeded but 0 JS files in lib/ — cache may be stale" - echo "Try clearing the hdwallet cache or rebuild locally" - exit 1 - fi + node projects/keepkey-vault/scripts/verify-zcash-ironwood-protocol.mjs modules/device-protocol - name: Install vault dependencies # --frozen-lockfile: install exactly what bun.lock pins. Without it, bun @@ -154,10 +132,6 @@ jobs: run: cd projects/keepkey-vault && bun install --frozen-lockfile shell: bash - - name: Install protoc (macOS) - if: runner.os == 'macOS' - run: brew install protobuf - - name: Install Linux packaging tools if: runner.os == 'Linux' run: sudo apt-get update && sudo apt-get install -y fakeroot lintian dpkg-dev diff --git a/HANDOFF-PASSPHRASE-SIGNING-BUG.md b/HANDOFF-PASSPHRASE-SIGNING-BUG.md new file mode 100644 index 00000000..c55cf4fa --- /dev/null +++ b/HANDOFF-PASSPHRASE-SIGNING-BUG.md @@ -0,0 +1,242 @@ +# HANDOFF — passphrase (hidden) wallet produces invalid Ethereum signatures + +**Severity: P0 / launch-blocking.** A funded production KeepKey **BIP39-passphrase (hidden) wallet cannot produce a valid Ethereum signature**. `GetAddress` under the passphrase returns the correct, stable hidden-wallet address, but every `SignTx` under the same passphrase returns an `(r,s,v)` that recovers to a *different garbage address per transaction digest*. The no-passphrase wallet on the same device signs and broadcasts flawlessly. This blocks a real mainnet token launch: the production passphrase wallet that owns the deployment cannot sign. Address (pubkey) derivation under the passphrase works; **signing under the passphrase is broken**, and the corruption is digest-dependent — which narrows the fault to either the device's passphrase signing path or a host-side preimage/transport divergence that only the passphrase flow exercises. A single already-wired diagnostic (the device echoes the digest it signed in `EthereumTxRequest.hash`) settles host-vs-firmware definitively; instructions are in §8. + +--- + +## 1. Summary + +A KeepKey hidden (BIP39-passphrase) wallet derives the **correct** Ethereum address but emits **cryptographically invalid** Ethereum signatures. The same device, same code path, with no passphrase, signs perfectly and has broadcast 8+ confirmed mainnet/testnet transactions. The defect is isolated to the passphrase wallet's signing operation. Because the recovered signer changes with each transaction digest (rather than being one constant wrong address), this is **not** the textbook "device fell back to the no-passphrase seed" bug. It is one of: (a) a firmware passphrase/seed-selection or digest-handling bug on the SignTx path, or (b) a host preimage/serialization/transport divergence that only the passphrase flow triggers. The evidence below, plus the single decisive test in §8, pin which. + +--- + +## 2. Symptom & evidence + +Running build under test: **Vault REST API on `localhost:1646`**, build = `keepkey-vault-v11/projects/keepkey-vault/_build/dev-macos-arm64`. This is the **Vault host middleware**, not firmware — the firmware runs on the device. + +Path under test: **`m/44'/60'/0'/0/0`** = `addressNList [2147483692, 2147483708, 2147483648, 0, 0]`. + +| Operation | Wallet | Chain / tx | Result | Recovered signer | +|---|---|---|---|---| +| `POST /addresses/eth` | **passphrase** | — | **CORRECT, stable, repeatable** | **`0x21c9a94AF76B59b171b32fD125A4edF0e9A2Ad3e`** | +| `POST /eth/sign-transaction` | passphrase | mainnet (chainId 8453), GOLDToken deploy, 3484B calldata | invalid | `0xef115ddc…02b` (deterministic for that tx, twice) | +| `POST /eth/sign-transaction` | passphrase | Sepolia (84532), same deploy | invalid | `0xa8b3…21a` | +| `POST /eth/sign-transaction` | passphrase | Sepolia (84532), tiny self-send (0 calldata, 21000 gas) | invalid | `0x6d7f…883` | +| `POST /eth/sign-transaction` | **no-passphrase** `0x141D9959…` | Sepolia: GOLDToken deploy, AMMRouter, CharacterSale, addLiquidity, purchaseCharacter, Sablier approve + createWithTimestampsLL (8+ txs) | **valid, broadcast, confirmed on-chain** | `0x141D9959…` every time | + +Observations that constrain the fault: +- The recovered signer **varies with the transaction digest** (different garbage per tx), yet is **deterministic for a given tx**. +- **Tx size and chainId are not the variable.** The no-passphrase wallet signed the **identical 3484-byte GOLDToken deploy** on Sepolia and it recovered correctly. The passphrase fails on both a 3484-byte deploy and a 0-byte 21000-gas self-send. **The passphrase wallet is the only differing variable.** + +--- + +## 3. What it is NOT (ruled out, with forensic basis) + +Forensics performed with **viem** against the bad `(r,s)`: + +1. **NOT a valid signature by `0x21c9…` over our digest** under *either* `yParity`. +2. **NOT a dropped-leading-zero / leading-byte truncation of `r` or `s`** — restoring a dropped leading byte of `r` or `s` does not recover `0x21c9…`. (The mainnet sig's `s` happened to end in `0x00` but the Sepolia sig's `s` ended in `0x6d` — coincidence, not truncation.) +3. **NOT calldata truncation/chunk-boundary corruption** — brute-forced **all 3484 byte positions**: the `(r,s)` is not a valid signature by `0x21c9…` over *any* prefix-truncation, single-byte-drop, or chunk-boundary-drop of the calldata. +4. **NOT a consistent wrong key.** A fixed wrong private key `K` signing the correct digest always recovers to the single fixed `addr(K)` for *every* tx. By ECDSA recovery algebra, `recovered = correctPub + (e_signed − e_recovered)·r⁻¹·G`: **different garbage per digest** means the signed digest differs from the host-recovered digest (or the key varies per call) — it is *inconsistent* with one stable wrong key. +5. **NOT a length/chunking-dependent fault.** It reproduces on a 0-calldata 21000-gas tx (single-frame, no `EthereumTxAck` chunking) as well as the 3484-byte multi-chunk deploy. + +The `(r,s)` simply do not verify against the wallet key over the digest we reconstruct. This leaves two live hypotheses: a **wrong/garbage key inside firmware** (per-tx-varying scratch), or a **digest/preimage mismatch** between what the device signed and what the host recovers against (host serialization, or a firmware preimage divergence specific to the passphrase path). + +--- + +## 4. Key diagnostic + +**Under the same passphrase session: `GetAddress` is CORRECT; `SignTx` is INVALID.** + +On KeepKey, `GetAddress` and `SignTx` derive the signing key through the **identical** code path (see §6) — there is no separate "signing key" derivation. So a correct `GetAddress` mathematically implies the device holds the right passphrase-derived `node->private_key`. The component that behaves differently between the two operations, under the same cached passphrase seed, is the **signing routine / its preimage**, not key derivation per se. Address derivation under passphrase works; signing under passphrase is broken. + +--- + +## 5. Host vs firmware verdict (code evidence) + +### 5a. The host (Vault) sign path is provably passphrase-invariant + +Audited end-to-end in the running build's source tree (`projects/keepkey-vault/src` + vendored `modules/hdwallet`): + +- **REST sign handler** — `projects/keepkey-vault/src/bun/rest-api.ts:2013-2098`. Builds a plain `msg` (`:2041-2049`: `addressNList, to, value, data, nonce, gasLimit, chainId`; EIP-1559 fields `:2052-2057`). **No passphrase / session_id / hidden-wallet field is ever attached.** Optional `txMetadata` clear-sign blob (`:2063-2080`) is driven by calldata decode, not passphrase. Calls `wallet.ethSignTx(msg)` at `:2087`; logs at `:2082` / `:2088`. +- **Delegation** — `modules/hdwallet/packages/hdwallet-keepkey/src/keepkey.ts:1375-1376` → `Eth.ethSignTx(this.transport, msg)`. +- **Proto build + assembly** — `modules/hdwallet/packages/hdwallet-keepkey/src/ethereum.ts:252-408`. `EthereumSignTx` carries only `addressNList, nonce, gasLimit, gasPrice|maxFeePerGas, value, to, dataInitialChunk/dataLength, chainId` (`:301-344`) — **no passphrase/session field exists on the message**. `r/s/v` are read **verbatim** from the device protobuf (`:383-386`: `getSignatureR_asU8()/getSignatureS_asU8()/getSignatureV()`); assembly via `Transaction.fromTxData`/`FeeMarketEIP1559Transaction.fromTxData` (`:390-399`) and `tx.serialize()` (`:405`) is **unconditional and passphrase-agnostic**. +- **Get-address for comparison** — `ethereum.ts:410-426`: `EthereumGetAddress` carries only `addressNList` + `showDisplay`; same transport, same lack of passphrase/session. +- **Transport** — `modules/hdwallet/packages/hdwallet-keepkey/src/transport.ts`. Real I/O via `TransportDelegate.writeChunk/readChunk` (`:18-19`); framing `write/read :66-103`, `toMessageBuffer/fromMessageBuffer :367-396`. Passphrase handling lives in the **shared** read loop `readResponse :154-255` (`MESSAGETYPE_PASSPHRASEREQUEST :218-228`), identical for get-address and sign-tx. +- **Passphrase / session model** — `sendPassphrase` → `PassphraseAck.setPassphrase` → one `transport.call` (`keepkey.ts:1016-1020`); engine wiring `engine-controller.ts:236-246`, `:1929-1948`. **KeepKey has no per-message session_id (unlike Trezor):** the passphrase is entered **once per USB session**; the device caches the passphrase-derived seed; every later op (get-address and sign-tx alike) reads that same cached seed. The host never re-sends or re-derives the passphrase per operation. The only structural host difference is that `/addresses/eth` (`rest-api.ts:1829-1845`) has an in-memory `addressCache` (keyed by deviceId+body, `scopedKey :309-312`) while sign-tx has none — this cannot manufacture a valid-address-but-invalid-signature outcome. + +**Host conclusion:** there is zero passphrase/session branching in the sign path; assembly copies raw protobuf `r/s/v`; and the only host-plausible corruptions (dropped-leading-zero / truncation) are already excluded by the §3 forensics. The host construction, transport, and assembly are **passphrase-invariant**. + +### 5b. The countervailing data point — and how to resolve it + +The host audit concludes **FIRMWARE**: the only differing variable is the passphrase, which influences only the device's once-per-session cached seed; `GetAddress` proves the device holds the right passphrase key; both ops read that same seed; the host reads `r/s/v` verbatim. A signature that is consistent-per-digest yet doesn't verify against the device's own passphrase pubkey — and whose recovered signer changes with the digest — can only be generated where the key is selected and the digest is computed: **inside the firmware**. + +The firmware-locus review adds the necessary caveat for honesty: **"different garbage per digest" is inconsistent with a *stable* wrong key** (§3 item 4). It implies a **digest divergence** — the device signed digest `e_signed` but the host recovers against `e_recovered ≠ e_signed`. Because the digest/RLP/keccak code is *shared* with the no-passphrase path that signs identical-structure txs correctly, a passphrase-only *digest* divergence inside firmware is a priori unlikely — which keeps a **host preimage/serialization mismatch** (legacy-vs-1559 framing, EIP-155 `v`/chainId encoding) in play as well. + +These are not contradictory: both reviews agree the on-device **key derivation** is correct (GetAddress proves it) and the host **byte-handling** is clean (forensics prove it). What remains genuinely undetermined is **whose digest is wrong**. That is exactly what the §8 test measures, because **the firmware returns the digest it signed**. + +--- + +## 6. Most-likely firmware root-cause locus + +Canonical, maintained firmware repo on disk: **`/Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-firmware`** (C, KeepKey fork of trezor-firmware; Trezor crypto vendored under `deps/crypto/trezor-firmware/crypto/`). ~6 sibling checkouts exist; this is the maintained one. + +**Both handlers derive the key identically** — `lib/firmware/fsm_msg_ethereum.h`: `fsm_msgEthereumSignTx` (**line 98**) and `fsm_msgEthereumGetAddress` (**line 123**) both call: +```c +fsm_getDerivedNode(SECP256K1_NAME, msg->address_n, msg->address_n_count, NULL) +``` +GetAddress → `hdnode_get_ethereum_pubkeyhash(node, …)`; SignTx → `ethereum_signing_init(msg, node, …)`. Same `node`. **GetAddress-correct implies `node->private_key` is correct.** + +**Derivation chain (passphrase → seed → node → privkey):** +1. `fsm_getDerivedNode` — `lib/firmware/fsm.c:180` → `storage_getRootNode(curve, /*usePassphrase=*/true, &node)` then `hdnode_private_ckd_cached(...)`. +2. `storage_getRootNode` — `lib/firmware/storage.c:1896`; mnemonic path `:1944-1976` caches `session.seed`, then `hdnode_from_seed(...)`. +3. `storage_getSeed` — `storage.c:1843`: `mnemonic_to_seed(mnemonic, usePassphrase ? session.passphrase : "", session.seed, …)` — where the passphrase enters. +4. `hdnode_private_ckd_cached` — `deps/crypto/trezor-firmware/crypto/bip32.c:547`; cache root compared with full-node `memcmp` at `:566` → **this BIP32 child cache is sound** (passphrase change invalidates it correctly). +5. SignTx copies the key: `lib/firmware/ethereum.c:895` `memcpy(privkey, node->private_key, 32);` +6. ECDSA: `send_signature()` `ethereum.c:266` → `ecdsa_sign_digest(&secp256k1, privkey, hash, sig, &v, ethereum_is_canonic)` at **:282**, with `hash = keccak_Final(&keccak_ctx, …)` at `:281`. + +**Latent firmware bug — the "stale-root / wrong-key" locus (would produce a *stable* wrong address):** +- `storage_getSeed` (`storage.c:1845`) **is** mode-aware: `if (usePassphrase == session.seedUsesPassphrase && session.seedCached) return session.seed;` — re-derives when passphrase mode changes. +- `storage_getRootNode` (the SECP256K1/Ethereum path) is **NOT** mode-aware: at `storage.c:1955` it only checks `if (!session.seedCached)`, then blindly `hdnode_from_seed(session.seed, …)`. It never compares `session.seedUsesPassphrase` to the requested `usePassphrase`. +- `session_cachePassphrase` (`storage.c:1996`) sets `passphraseCached = true` but does **not** reset `session.seedCached`. +- **Combined:** if a no-passphrase (or different-passphrase) seed is already cached and a passphrase is then entered without a session clear, `storage_getRootNode` reuses the **stale** seed → derives with the wrong root. This is the only firmware path that can sign under a different root than intended. **Fix:** mirror `storage_getSeed`'s `seedUsesPassphrase` guard at `storage.c:1955`, and set `session.seedCached = false` in `session_cachePassphrase` (`storage.c:1996`). +- **Caveat:** this bug yields a *stable* wrong address per session, so it explains "wrong key" but **not** the observed per-digest variance. Treat it as a real latent bug to fix regardless, not necessarily *this* symptom's cause. + +**Sharp edge worth auditing for the per-tx-varying case:** `fsm_getDerivedNode` returns a pointer to a `static HDNode`, and `EthereumSignTx` does `ethereum_signing_init(msg, node, ...)` then **`memzero(node, sizeof(*node))` immediately** — so `ethereum_signing_init` (`ethereum.c:618`) must deep-copy the node before the zero, and the streaming Keccak/sign path (calldata arrives as `EthereumTxAck` chunks) must never reuse/clear the key buffer mid-transaction. A missed deep-copy or buffer reuse during streaming would sign against tx-dependent scratch memory → **a recovered signer that varies per tx**, matching the symptom. + +**Files for the firmware team, priority order:** +1. `lib/firmware/storage.c` — `storage_getRootNode` (1896; seedCached gate at **1955**) and `session_cachePassphrase` (**1996**), cross-checked against `storage_getSeed` (1843). +2. `lib/firmware/ethereum.c` — `send_signature` (266; returned `hash` at **313-315**), `ethereum_signing_init` (618; `memcpy privkey` at 895). +3. `lib/firmware/fsm_msg_ethereum.h` — `fsm_msgEthereumSignTx` (98) vs `fsm_msgEthereumGetAddress` (123). +4. `lib/firmware/fsm.c` — `fsm_getDerivedNode` (180). + +--- + +## 7. How to reproduce + +Preconditions: real KeepKey device (not the emulator — see §8 note), the **passphrase/hidden wallet active**, Vault dev build running and serving `localhost:1646`. + +1. `POST /addresses/eth` with `path m/44'/60'/0'/0/0` and the passphrase active → returns `0x21c9a94AF76B59b171b32fD125A4edF0e9A2Ad3e` (correct, stable). +2. `POST /eth/sign-transaction` with the **same path/passphrase** for any tx, e.g.: + - Sepolia (chainId 84532) self-send: `value` to self, `0` calldata, `gasLimit 21000`. + - and/or the GOLDToken deploy (chainId 8453 mainnet or 84532 Sepolia, 3484-byte calldata). +3. Recover the signer from the returned serialized tx (viem `recoverTransactionAddress` / ecrecover). It will be a garbage address that **differs per tx digest** and is **deterministic for a given tx** — never `0x21c9…`. +4. **Control:** repeat step 2 with the **no-passphrase** wallet (`0x141D9959…`) and the *identical* tx → recovers correctly to `0x141D9959…` and broadcasts/confirms. This proves the calldata/clear-sign/chunking/serialization path is good and the passphrase is the only variable. + +--- + +## 8. How to capture the message-by-message proof (the decisive test) + +The firmware **returns the exact digest it signed** in `EthereumTxRequest.hash`, set in `send_signature()` at `lib/firmware/ethereum.c:313-315` (the same `hash` passed to `ecdsa_sign_digest`). Compare it byte-for-byte to the host's locally-computed digest: +- **`EthereumTxRequest.hash` ≠ host digest** → digest/preimage construction mismatch (host serialization or firmware preimage), **not** a key bug. +- **They are equal yet recovery against `0x21c9…` fails** → genuine key bug; locus = the `storage_getRootNode` seed-mode gap (`storage.c:1955`) + `session_cachePassphrase` (`storage.c:1996`), and the streaming/deep-copy edge in `ethereum_signing_init`. + +### 8a. What you already have, no rebuild + +The Bun backend mirrors every `console.*` to a log file at boot (`projects/keepkey-vault/src/bun/index.ts:30-58`): +``` +/Users/highlander/Library/Application Support/com.keepkey.vault/vault-backend.log +``` +It already logs, per `/eth/sign-transaction` (`rest-api.ts:1508, 2082, 2088`): +- `[REST] Signing request /eth/sign-transaction: …` +- `[REST] ethSignTx hdwallet payload: {…}` ← exact host input +- `[REST] ethSignTx result: {"r":"0x…","s":"0x…","v":…,"serialized":"0x…"}` ← device `r/s/v` read straight off the proto (`ethereum.ts:383-386`) + +The `[REST] EVM clear-sign` line tells you whether an `EthereumTxMetadata` blob was sent before `EthereumSignTx` (clear-sign vs blind) — rule this variable in/out. Tail/extract: +```bash +LOG="$HOME/Library/Application Support/com.keepkey.vault/vault-backend.log" +tail -f "$LOG" +grep -nE '\[REST\] (Signing request /eth|ethSignTx (hdwallet payload|result)|EVM clear-sign)' "$LOG" +``` +(A passphrase-wallet `0x21c9…` self-send already appears in this log, e.g. `r=0x7cd512da… s=0x223785cc… v=0` at `04:26:37Z`.) + +### 8b. Full raw wire hook (definitive) + +Every message in both directions funnels through one `eventemitter2` chokepoint in the hdwallet transport (`transport.ts`): outgoing `Transport.call()` emits at `:290` (`from_wallet:false`); incoming `Transport.readResponse()` emits at `:167` (`from_wallet:true`). Add a wildcard `onAny` listener in the Vault's own code — **edit `projects/keepkey-vault/src/bun/engine-controller.ts`, inside `attachTransportListeners()`, right after `const transport = this.wallet.transport` (line 213):** + +```ts +// MSGLOG: dump every protobuf message both directions to vault-backend.log. +transport.onAny((_name: string | string[], ev: any) => { + if (!ev || typeof ev !== 'object' || ev.message_type === undefined) return + const dir = ev.from_wallet ? 'DEV->HOST' : 'HOST->DEV' + let wire = '' + try { if (ev.proto?.serializeBinary) wire = Buffer.from(ev.proto.serializeBinary()).toString('hex') } catch {} + console.log(`[MSGLOG] ${dir} ${ev.message_type}(${ev.message_enum}) ${JSON.stringify(ev.message)}${wire ? ' wire=' + wire : ''}`) +}) +``` +(`ev.message` is jspb `.toObject()`, so `bytes` fields print as base64; the appended `wire=` is the exact serialized protobuf for byte-level forensics. Optional: call `(transport as any).offAny?.()` in `cleanupTransportListeners()` ~`:204` so re-pairs don't stack listeners.) + +Rebuild + restart so the hook takes effect (the dev app loads a pre-bundled backend; per project convention use `make` from the repo root): +```bash +# quit the running keepkey-vault-dev app first, then: +cd /Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-vault-v11 +make dev # bundle-backend → vite build → electrobun build → electrobun dev +# or: make dev-hmr +``` +New `[MSGLOG]` lines land in the same `vault-backend.log`. Inspect: +```bash +LOG="$HOME/Library/Application Support/com.keepkey.vault/vault-backend.log" +grep -nE '\[MSGLOG\]|\[Engine\] (PASSPHRASE_REQUEST|BUTTON_REQUEST)' "$LOG" | tail -60 +``` + +### 8c. What to check (in order) + +1. **Passphrase session parity:** confirm `EthereumSignTx` is preceded by the **same** `PassphraseRequest → HOST->DEV PassphraseAck{passphrase:"…"}` handshake that `EthereumGetAddress` gets. If SignTx runs on a stale/empty/cleared passphrase cache, the device signs under the wrong seed. +2. **Outgoing `HOST->DEV EthereumSignTx`:** verify `addressNList = [2147483692, 2147483708, 2147483648, 0, 0]`, correct `chainId`, `data_length`, `data_initial_chunk` (first ≤1024 bytes), then each `EthereumTxAck.data_chunk` until `data_length` is exhausted — proves the host sent the right tx. +3. **Incoming `DEV->HOST EthereumTxRequest` (terminal):** carries `signatureR/S/V` **and `hash`**. (a) Compare `signatureR/S/V` (and `wire=` hex) byte-for-byte against `[REST] ethSignTx result` — if equal but non-verifying, **host is exonerated**. (b) Compare the device's `hash` against the host's locally-computed digest — this is the §8 decisive test (digest-mismatch vs key-bug). +4. **GetAddress comparison:** `HOST->DEV EthereumGetAddress{address_n}` → `DEV->HOST EthereumAddress{address}` should show `0x21c9a94A…`. Same path, same passphrase handshake — the only delta vs SignTx is the message type, isolating the fault to the device's passphrase-gated signing routine. + +**Note:** recent log sessions show an **emulator** in use (`deviceId 5E4E6B69…`, fw `7.15.0`, `passphraseProtection:false`). Reproduce on the **real device with the passphrase wallet active** (`isPassphraseWallet`) — the emulator's passphrase state differs and will not reproduce the bug faithfully. + +--- + +## 9. Recommended fix + safe interim workaround + +### Interim workaround (unblock the launch now) +**Use the no-passphrase wallet `0x141D9959…` as the mainnet contract owner / deployer.** That path is *proven*: it has signed and broadcast 8+ confirmed transactions (GOLDToken deploy, AMMRouter, CharacterSale, addLiquidity, purchaseCharacter, Sablier approve + createWithTimestampsLL), all recovering correctly. Do **not** ship the passphrase wallet as the funded owner until it produces a signature that passes an on-host `ecrecover` gate. (Alternative if §8 implicates the host: the device key itself is proven correct by GetAddress, so the same tx signed through a known-good reference transport could be broadcast — but the no-passphrase wallet is the cleaner, fully-proven path.) + +### Permanent safety net (host — do this regardless of root cause) +Add a **mandatory post-sign verification gate** in the Vault: after `wallet.ethSignTx`, run on-host `ecrecover(serialize(tx), r, s, v)` and compare to the cached GetAddress for that path. **If it does not equal the expected signer, refuse to return/broadcast and surface an error.** This single guard would have caught this before a mainnet deploy and is the correct permanent safeguard. (Sign path lives at `rest-api.ts:2013-2098`; the address is already derivable via the same handler's GetAddress.) + +### Firmware fix (apply if §8 test (b) shows device `hash` == host digest yet recovery fails) +- Make `storage_getRootNode` mode-aware: compare `session.seedUsesPassphrase` against the requested `usePassphrase` before reusing `session.seed` (`storage.c:1955`), mirroring `storage_getSeed` (`storage.c:1845`). +- Have `session_cachePassphrase` reset `session.seedCached = false` (`storage.c:1996`). +- Ensure `ethereum_signing_init` (`ethereum.c:618`) **deep-copies** the derived node before `EthereumSignTx`'s `memzero(node)`, and that the streaming Keccak/sign path never reuses or clears the key buffer mid-transaction (the per-tx-varying-garbage locus). This is the class of bug Trezor fixed in #1659 / #525. + +### If §8 test (a) shows device `hash` ≠ host digest (preimage/serialization mismatch) +Reconcile the host serialization with the device preimage (legacy RLP vs EIP-1559 `0x02` typed-tx, EIP-155 `v`/chainId encoding) on the passphrase path; the fix is host-side in `ethereum.ts` assembly (`:383-405`) / `rest-api.ts` msg build (`:2041-2057`). + +--- + +## 10. References + +**Running build / host source (Vault, `keepkey-vault-v11`):** +- Log writer + path: `projects/keepkey-vault/src/bun/index.ts:30-58` → `~/Library/Application Support/com.keepkey.vault/vault-backend.log` +- REST sign handler + logs: `projects/keepkey-vault/src/bun/rest-api.ts:2013-2098` (logs `:1508, 2082, 2088`; address handler `:1829-1845`; addressCache `scopedKey :309-312`) +- Hook site: `projects/keepkey-vault/src/bun/engine-controller.ts:207-257` (insert after `:213`; cleanup `:198-205`; PASSPHRASE_REQUEST listener `:236-246`; `sendPassphrase` `:1929-1948`) +- hdwallet delegation: `modules/hdwallet/packages/hdwallet-keepkey/src/keepkey.ts:1375-1376` (passphrase ack `:1016-1020`) +- ETH sign flow + verbatim r/s/v read: `modules/hdwallet/packages/hdwallet-keepkey/src/ethereum.ts:252-408` (assembly `:383-405`; GetAddress `:410-426`) +- Transport chokepoint: `modules/hdwallet/packages/hdwallet-keepkey/src/transport.ts:154-167, 218-228, 257-299, 66-103, 367-396` +- Proto field defs: `modules/device-protocol/messages-ethereum.proto:20-88` +- Build targets: `Makefile:270-276` (`dev` / `dev-hmr`) + +**Firmware source (`keepkey-firmware`, on disk at `/Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-firmware`):** +- `lib/firmware/fsm_msg_ethereum.h` — `fsm_msgEthereumSignTx:98`, `fsm_msgEthereumGetAddress:123` +- `lib/firmware/fsm.c` — `fsm_getDerivedNode:180` +- `lib/firmware/storage.c` — `storage_getSeed:1843`, `storage_getRootNode:1896` (seedCached gate `:1955`), `session_cachePassphrase:1996`, root-seed-cache `:1255/1279` +- `lib/firmware/ethereum.c` — `send_signature:266` (returned `hash` `:313-315`, `ecdsa_sign_digest` `:282`, `keccak_Final` `:281`), `ethereum_signing_init:618`, `memcpy privkey:895` +- `deps/crypto/trezor-firmware/crypto/bip32.c` — `hdnode_private_ckd_cached:547` (cache `memcmp` `:566`) + +**Upstream documentation / known issues (Trezor — KeepKey is a fork of Trezor firmware):** +- Trezor firmware — sessions model + silent fallback ("attempt to resume an unknown session ID will transparently allocate a new session ID"): https://docs.trezor.io/trezor-firmware/common/communication/sessions.html +- Trezor firmware — passphrase / seed caching + session_id guarantee: https://docs.trezor.io/trezor-firmware/common/communication/passphrase.html +- Trezor changelog — passphrase/session caching bugs (#1659 empty passphrase caching; #525 clear_session forgets passphrase state; `state`→`session_id`): https://github.com/trezor/trezor-firmware/blob/main/python/CHANGELOG.md +- Trezor-suite — "duplicated empty passphrase redirects to incorrect wallet" (#3517): https://github.com/trezor/trezor-suite/issues/3517 +- **Trezor Forum — near-identical symptom:** ETH signing for second passphrase wallet returns the correct address but signs to a different address that changes every time; non-passphrase wallets work: https://forum.trezor.io/t/eth-signing-for-second-passphrase-wallet-signs-with-wrong-address-signature/19230 +- Trezor support — "I can't sign my transaction" (key-derivation/passphrase mismatch): https://trezor.io/support/troubleshooting/trezor-suite-issues/i-can-t-sign-my-transaction +- Trezor support — passphrase / hidden wallet issues: https://trezor.io/support/troubleshooting/trezor-suite-issues/passphrase-hidden-wallets-issues +- **Sparrow #219 — KeepKey BIP39 passphrase send fails host-side (host-specific passphrase transmission; FW 7.2.1, wontfix):** https://github.com/sparrowwallet/sparrow/issues/219 +- KeepKey firmware (GitHub mirror): `lib/firmware/fsm.c` https://github.com/keepkey/keepkey-firmware/blob/master/lib/firmware/fsm.c · `lib/firmware/passphrase_sm.c` https://github.com/keepkey/keepkey-firmware/blob/master/lib/firmware/passphrase_sm.c · `lib/firmware/fsm_msg_ethereum.h` https://github.com/keepkey/keepkey-firmware/blob/master/lib/firmware/fsm_msg_ethereum.h +- Passphrase *handling* advisories (related but a different class — unvalidated/ransom passphrases, not invalid signatures): https://benma.github.io/2020/09/02/trezor-keepkey-passphrase.html · https://blog.kraken.com/product/security/flaw-found-in-keepkey-crypto-hardware-wallet-part-2 + +**Note on novelty:** no published advisory was found describing *per-digest-varying, non-verifying* signatures from a KeepKey passphrase wallet (this exact corruption signature). The closest documented case is Trezor forum #19230 — symptomatically identical, but with no posted firmware root-cause. \ No newline at end of file diff --git a/Makefile b/Makefile index 5bfbd915..0bd5cea4 100644 --- a/Makefile +++ b/Makefile @@ -44,8 +44,9 @@ submodules: $(SUBMODULES_STAMP) $(DEVICE_PROTOCOL_BUILD_STAMP): $(DEVICE_PROTOCOL_INPUTS) $(SUBMODULES_STAMP) | $(STAMP_DIR) @echo "=== device-protocol: installing + building ===" - cd modules/device-protocol && npm install - cd modules/device-protocol && npm run build + cd modules/device-protocol && npm ci + node $(PROJECT_DIR)/scripts/postprocess-device-protocol.mjs modules/device-protocol + node $(PROJECT_DIR)/scripts/verify-zcash-ironwood-protocol.mjs modules/device-protocol @test -f modules/device-protocol/lib/messages_pb.js || (echo "ERROR: device-protocol build failed (messages_pb.js missing)"; exit 1) @touch $@ @@ -69,8 +70,8 @@ $(HDWALLET_INSTALL_STAMP): modules/hdwallet/package.json modules/hdwallet/yarn.l modules-install: $(PROTO_INSTALL_STAMP) $(HDWALLET_INSTALL_STAMP) -$(HDWALLET_BUILD_STAMP): modules/hdwallet/tsconfig.json $(HDWALLET_BUILD_INPUTS) $(HDWALLET_INSTALL_STAMP) | $(STAMP_DIR) - cd modules/hdwallet && yarn tsc --build +$(HDWALLET_BUILD_STAMP): modules/hdwallet/tsconfig.json $(HDWALLET_BUILD_INPUTS) $(HDWALLET_INSTALL_STAMP) $(DEVICE_PROTOCOL_BUILD_STAMP) | $(STAMP_DIR) + cd modules/hdwallet && yarn build @touch $@ modules-build: $(HDWALLET_BUILD_STAMP) $(PROTO_BUILD_STAMP) $(DEVICE_PROTOCOL_BUILD_STAMP) diff --git a/docs/RELEASE-CONTROL-7.15.md b/docs/RELEASE-CONTROL-7.15.md new file mode 100644 index 00000000..58ff053d --- /dev/null +++ b/docs/RELEASE-CONTROL-7.15.md @@ -0,0 +1,178 @@ +# Release Control — 7.15 cycle (firmware → vault → bex) + +**Generated 2026-07-16 from live repo state** (gh/git verified, not memory). Single source of truth for the 7.15 firmware release + vault + bex releases. Supersedes the scattered `handoff-*` docs. + +--- + +## State of play (the honest version) + +You are closer to shipping than the handoff pile suggests. Four releases are **chained**, and the long pole is entirely **upstream firmware**. + +- **Firmware 7.15.0** — cut to **rc10** on the fork (`develop == release/7.15.0-rc10 == 183a2b93`, CMakeLists already 7.15.0). No `v7.15.0` tag. Fork develop is **100 commits ahead** of upstream (still 7.14.0). The upstream path is **already live and CI-green**: a clean 5-PR stack **#444–#448** + repro script **#449** on `keepkey/keepkey-firmware`, blocked only on (a) two foundation PRs merging first, (b) upstream human review, (c) Gate-3 OLED. My late fix **#309** (thorchain per-chain clearsign) is on fork develop but **not yet folded into upstream #447**. +- **Vault** — users have **v1.4.10**; **v1.4.11** is an un-promoted pre-release; develop is **105 commits ahead** with one clean open PR (**#364**). One bump + notarization away from a release. **Does not gate on firmware.** +- **BEX / keepkey-client** — at **v0.0.35** with 16 merged-but-unreleased commits (hive + MCP). Its headline features are **hard-gated** on firmware 7.15.0 + a vault release + a Pioneer deploy shipping first. **Must be last.** +- **Already merged** (ignore any handoff saying otherwise): hdwallet #55, pioneer #170, client #108, vault #361 (mcp 401) + #363 (zod fix). + +### Two genuine hard blockers (beyond sequencing) +1. **Clearsign trust anchor** — `METADATA_PUBKEYS` is intentionally all-zero (`signed_metadata.c:45-54`). Warning-free clearsign is impossible without a key ceremony. **Product decision.** Does *not* block a warning-gated 7.15.0. +2. **Upstream firmware is human-review-gated** — the #444–#448 stack is MERGEABLE + green but needs keepkey maintainer sign-off. Budget days. + +--- + +## Critical path (dependency-ordered) + +``` +#309 (+Gate-3 OLED) ──► fork develop ──► fold into upstream #447 + │ + └─ vault #364 (lockstep) LOCAL TEST SWEEP (verify-locally SOP) + │ + device-protocol #111 ─┐ │ + python-keepkey #196 ─┴─► merge to UPSTREAM masters ──►│ + ▼ + re-pin fw deps ► upstream review ► merge #444→#448 in order + │ + [DECISION: trust anchor] [DECISION: upstream this cycle?] + ▼ + cut v7.15.0 FINAL (3/5 airgapped signers, hash-compare) + + ── PARALLEL, does NOT wait on firmware ── + vault #364 merge ► bump 1.4.11→1.4.12 ► notarization creds ► make preflight ► make release + + ── LAST, gated on firmware-on-device + vault release + pioneer deploy ── + bex 0.0.36: release/0.0.36 ► master ► tag ► zip ► GitHub release ► master→develop sync +``` + +--- + +## Phase board + +### Phase 1 — Land the clearsign fix (fw #309 + vault #364) — *in flight* +- [ ] Capture **Gate-3 OLED** of an AVAX THORChain deposit clear-signing (router/amount shown, not a blind-sign warning). SOP: no firmware PR approval without OLED proof. +- [ ] Attach screenshots → merge **#309** to fork develop (all CI green). +- [ ] Merge **#364** to vault develop in lockstep. Keep it single-file (`calldata-decoder.ts`); **do NOT** commit the drifted firmware submodule gitlink (`715c173e`). +- **Exit:** both merged with Gate-3 attached; submodule gitlink not bumped on #364. + +### Phase 2 — Full LOCAL test sweep (verify-locally, not CI) +Run in order (see full command list at bottom). Record pass/fail. +- [ ] firmware docker unit + pyk/OLED +- [ ] vault **`bun test __tests__/`** (whole dir — `make test-unit` silently covers only ~16 of ~35 files, skips `firmware-clearsign-gate.test.ts`) +- [ ] vault emu / rest / sign-gating +- [ ] bex `pnpm vitest run` +- **Exit:** every layer green at the rc10 SHA, recorded. + +### Phase 3 — Firmware 7.15.0 FINAL to UPSTREAM (long pole) +- [ ] **Foundation first (bottom-up SOP):** get **device-protocol #111** + **python-keepkey #196** reviewed + merged to their **upstream masters** (these are the sanctioned upstream exceptions to fork-only). +- [ ] Re-pin firmware `deps/device-protocol` + `deps/python-keepkey` to the merged master SHAs; confirm `check-submodules` stays green. *(Verify pins are actually on master, not just ancestor of a branch.)* +- [ ] Fold **#309** into upstream **#447**; re-verify CI. +- [ ] Upstream review → merge **#444→#445→#446→#447→#448** in order (#448 carries the version bump, lands last). #449 merges independently. +- [ ] Resolve **trust-anchor decision** (see Decisions). +- [ ] Cut **v7.15.0 FINAL** per `docs/Release.md`: local unit+pyk, CMakeLists==7.15.0, tag off `release/7.15.0` (**not** the stale branch), publish GPL source, multi-machine hash compare, **3/5 airgapped signers**, storage-upgrade key-preservation check on a production device. +- **Exit:** #111+#196 on masters; #444–#448 merged with #309 folded; signed `v7.15.0` tag; upstream develop reads 7.15.0. + +### Phase 4 — Vault release v1.4.12 (PARALLEL with Phase 3) +- [ ] Merge #364. +- [ ] Bump `projects/keepkey-vault/package.json` **1.4.11 → 1.4.12** (else `make release` re-cuts the existing pre-release). +- [ ] Decide fork-pin questions (proto-tx-builder on `fix/cosmjs-freegrant-typo-shim`, device-protocol fork pin `98ca1e2`): accept or land on main. +- [ ] Confirm **ELECTROBUN_* creds** + signing cert (`security find-identity -v -p codesigning`). Never read `.env`. +- [ ] `make preflight` → `make release`. Leave the firmware submodule gitlink untouched (excluded from vault gating). +- **Exit:** v1.4.12 draft release with signed+notarized DMG + update.json; users have a path off v1.4.10. + +### Phase 5 — BEX 0.0.36 (LAST — cross-repo gated) +- [ ] Confirm all three gates live: firmware 7.15.0 **on devices** (`requireHiveFirmware` blocks <7.15.0), vault release exposing `/hive/*` + `/bex-bridge`, Pioneer deploy with broadcast + vesting-pool. +- [ ] Decide scope (see Decisions); strip the mis-placed 0.0.36 bump from `feature/hive-network-listing` (commit b49745f bumped all 10 package.jsons on a feature branch). +- [ ] Triage dependabot: merge safe minors #101/#102/#103; **hold** risky majors #104 (TS 5→6) / #105 (vitest 2→4) — they touch CI gates. +- [ ] RELEASE.md 8 steps: type-check/test/lint/build → `release/0.0.36` → bump → PR→master → tag → zip → GitHub release → **PR master→develop (don't skip step 8)**. +- [ ] `make e2e` against device + vault + live Pioneer (only automated hive/mcp check). +- **Exit:** v0.0.36 tagged, zip on a GitHub release, master→develop sync opened, hive/mcp verified on-device. + +### Phase 6 — Cleanup & upstream tidy +- [ ] Close fork rehearsal PRs **#294–#298** once #444–#448 land (superseded staging, no CI). +- [ ] Delete stale branch `keepkey-firmware release/7.15.0` (63 ahead / **133 behind** rc10 — releasing off it ships pre-rc10 firmware). +- [ ] Triage the untracked `docs/handoff-*.md` pile at the repo root — archive the merged/stale ones (cross-check each against `gh` before deleting). +- [ ] Watch fork↔upstream divergence drop toward 0 as the stack lands. + +--- + +## Open PR ledger (all repos) + +| PR | Repo | Status | Next action | +|----|------|--------|-------------| +| **#309** | BitHighlander/keepkey-firmware | OPEN, green | Gate-3 OLED → merge fork develop → fold into upstream #447 | +| **#364** | keepkey/keepkey-vault | OPEN, clean | Merge lockstep w/ #309; single-file; no submodule bump | +| **#111** | keepkey/device-protocol | OPEN, mergeable | **CRITICAL** — review + merge to master (upstream exception) | +| **#196** | keepkey/python-keepkey | OPEN, mergeable | **CRITICAL** — review + merge to master alongside #111 | +| **#444–#448** | keepkey/keepkey-firmware | OPEN, green, review-gated | Solicit upstream review; merge in order after #111/#196 | +| #447 | keepkey/keepkey-firmware | — | Fold #309 in before final merge | +| #449 | keepkey/keepkey-firmware | OPEN, green | Merge independently; run repro-build locally | +| #294–#298 | BitHighlander/keepkey-firmware | OPEN, no CI | **Do not merge** — close after upstream stack lands | +| #101–#103 | keepkey/keepkey-client | Dependabot | Merge if green before 0.0.36 | +| #104–#105 | keepkey/keepkey-client | Dependabot major | Hold/vet — touch CI gate toolchain | + +--- + +## Hard blockers + +| Blocker | Blocks | Unblock | Type | +|---------|--------|---------|------| +| Trust anchor `METADATA_PUBKEYS` all-zero | Warning-free clearsign in 7.15.0 | Key ceremony → slot 0, **or** decide to ship warning-gated | **DECISION** | +| device-protocol #111 + python-keepkey #196 unmerged | Entire upstream fw stack | Review + merge to upstream masters, re-pin | exec (days) | +| Upstream stack review-gated | 7.15.0 reaching upstream + FINAL tag | Request keepkey maintainer review | exec (days) | +| Gate-3 OLED outstanding | Merging #309; cutting FINAL | Run flows on device, screenshot | exec (manual) | +| Vault package.json still 1.4.11 | New vault release | Bump to 1.4.12 | exec | +| ELECTROBUN_* signing creds | Any signed/notarized vault build | Export vars + confirm cert | **DECISION** (human gate) | +| 3/5 airgapped signers + storage-upgrade check | Publishing signed v7.15.0 fw | Schedule airgapped signing session | **DECISION** (logistics) | +| bex deps not shipped | Functioning 0.0.36 | Ship fw→vault→pioneer first, or feature-flag off | exec (sequencing) | + +--- + +## Decisions + +**SETTLED 2026-07-16:** +1. **Trust anchor → SHIP WARNING-GATED in 7.15.0.** Clearsign goes out via the warning-screened LoadClearsignSigner path; production key ceremony deferred to phase-2 / 7.15.1. → Key ceremony is OFF the FINAL critical path; the `METADATA_PUBKEYS` blocker no longer gates 7.15.0. +2. **7.15.0 FINAL → UPSTREAM this cycle** via the #444–#448 stack (after #111/#196 land on masters). Budget days for maintainer review. +3. **Vault → CUT v1.4.12 IN PARALLEL NOW** (does not wait on firmware). v1.4.11 superseded straight to v1.4.12. + +**Still open (lower-stakes, decide before their phase):** +4. **BEX scope:** ship develop as-is, or first fold in the 2 read-only Hive-listing commits? Chrome Web Store publish, or GitHub-release-zip only? +5. **Vault fork pins:** accept proto-tx-builder + device-protocol fork pins for this vault release, or land them on main/master first? +6. **7.15.0 feature set:** confirm the shipped set (EVM clearsign, Hive, Zcash Orchard, Ripple memos, THORChain any-denom, TRON/Solana v0/TON/Maya affiliate) — anything deferred? + +--- + +## Local test sweep — exact commands (Phase 2) + +```bash +FW=/Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-vault-v11/modules/keepkey-firmware +V11=/Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-vault-v11 +BEX=/Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-client + +# 1. firmware unit (docker ONLY — native macOS fails StorageRoundTrip Linux golden) +cd $FW/scripts/emulator && docker compose up --build --exit-code-from firmware-unit firmware-unit +# 2. firmware pyk integration + OLED regression (UDP kkemu = only confirm-flow path) +cd $FW/scripts/emulator && docker compose up --build --exit-code-from python-keepkey python-keepkey +# 3. vault FULL host suite (the true "test everything" — bun test dir, not make test-unit) +cd $V11/projects/keepkey-vault && bun test __tests__/ +# 4. vault emulator smokes (dylib FFI, no device) +make -C $V11 test-emu +# 5. vault aggregate (zcash-cli + curated unit) +make -C $V11 test +# 6. vault REST + sign-gating (needs `make vault` running on :1646) +make -C $V11 test-rest && make -C $V11 test-sign-gating +# 7. vault live Hive sign smoke (device press, after restart) +RUN_LIVE_SIGN=1 make -C $V11 test-sign-gating +# 8. vault preflight (judge typecheck by ~619 baseline, not 0 — minimatch false-green) +make -C $V11 preflight +# 9-10. bex +cd $BEX/chrome-extension && pnpm vitest run +make -C $BEX type-check && make -C $BEX test && make -C $BEX lint && make -C $BEX build +# 11. pioneer +make -C /Users/highlander/WebstormProjects/keepkey-stack/projects/pioneer start && make -C .../pioneer test +# 12. DEVICE Gate-3 (manual): get_features → AVAX clearsign OLED → Hive sign-ops OLED → recovery+wipe (replug!) → swap clearsign — screenshot each +# 13. bex device e2e (device + vault hive endpoints + live pioneer) +make -C $BEX e2e +``` + +## Docs to archive/delete (stop the noise competing with truth) +- `keepkey-client/HANDOFF_vault_mcp_401.md` — fixed by merged vault #361. Delete. +- `keepkey-client/HANDOFF_vault_hive_sign_operations_zod.md` — fixed by merged vault #363 (verified in source). Delete. +- Untracked `docs/handoff-*.md` pile at v11 root (30+) — archive the ones whose PRs are merged; cross-check each against `gh` first. diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-confirm-1.png b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-1.png new file mode 100644 index 00000000..1fa34eee Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-1.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-confirm-2.png b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-2.png new file mode 100644 index 00000000..cb319f96 Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-2.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-confirm-3.png b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-3.png new file mode 100644 index 00000000..a3b141bc Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-3.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-confirm-4.png b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-4.png new file mode 100644 index 00000000..9ba1d0bb Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-4.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-confirm-5.png b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-5.png new file mode 100644 index 00000000..b07335fb Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-confirm-5.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-1.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-1.png new file mode 100644 index 00000000..d38fafd1 Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-1.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-10.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-10.png new file mode 100644 index 00000000..7752485f Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-10.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-2.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-2.png new file mode 100644 index 00000000..bdfaf1f1 Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-2.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-3.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-3.png new file mode 100644 index 00000000..1bf5a7e1 Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-3.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-4.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-4.png new file mode 100644 index 00000000..070483a2 Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-4.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-5.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-5.png new file mode 100644 index 00000000..c740b6c7 Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-5.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-6.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-6.png new file mode 100644 index 00000000..2d35c6ae Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-6.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-7.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-7.png new file mode 100644 index 00000000..5ad92c0a Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-7.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-8.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-8.png new file mode 100644 index 00000000..1a1e6100 Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-8.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-9.png b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-9.png new file mode 100644 index 00000000..01085a5b Binary files /dev/null and b/docs/evidence/clearsign-attestor-gate3/attestor-maxlabels-9.png differ diff --git a/docs/evidence/clearsign-attestor-gate3/attestor_screens.py b/docs/evidence/clearsign-attestor-gate3/attestor_screens.py new file mode 100644 index 00000000..52e4b2f1 --- /dev/null +++ b/docs/evidence/clearsign-attestor-gate3/attestor_screens.py @@ -0,0 +1,142 @@ +#!/usr/bin/env python3 +"""Gate-3 capture: the ClearsignAttestorSign confirm screens (firmware #323). + +Run a kkemu built with -DKK_CLEARSIGN_ATTESTOR=ON -DKK_DEBUG_LINK=ON from an +empty directory (a stale emulator.img makes it exit 1 with no message), then: + + KK_FIRMWARE=/path/to/keepkey-firmware \\ + PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python \\ + python3 attestor_screens.py ./out + +python-keepkey has no generated ClearsignAttestor* classes, so the attestor +exchange is written/read at the wire layer (##, >HL header) instead of through +mapping. Everything else -- wipe/load, DebugLink press, OLED decode -- is the +existing zoo harness. + +Payload = the real production Relay Bridge depositNative schema from +projects/keepkey-vault/src/bun/solana-schemas-local.json. +""" +import base64 +import json +import os +import struct +import sys +import time + +FW = os.environ.get("KK_FIRMWARE", os.path.join( + os.path.dirname(os.path.abspath(__file__)), "..", "..", "..", + "modules", "keepkey-firmware")) +sys.path.insert(0, os.path.join(FW, "deps", "python-keepkey")) +sys.path.insert(0, os.path.join(FW, "scripts", "zoo")) + +from keepkeylib.client import KeepKeyDebuglinkClient +from keepkeylib.transport_udp import UDPTransport +from keepkeylib import messages_pb2 as proto +from screenshot import capture_screenshot + +MSG_SIGN = 1702 +MSG_SIGNATURE = 1703 +MSG_BUTTON_REQUEST = proto.MessageType_ButtonRequest +MSG_BUTTON_ACK = proto.MessageType_ButtonAck +MSG_FAILURE = proto.MessageType_Failure + +OUT = sys.argv[1] if len(sys.argv) > 1 else "." +REGISTRY = os.path.join(os.path.dirname(os.path.abspath(__file__)), "..", "..", + "..", "projects", "keepkey-vault", "src", "bun", + "solana-schemas-local.json") +SCHEMA_KEY = "99vqwtbwytrqqd9ysxbdum3kbdxpavxytaq3cfnjsrn2:0d9e0ddf5fd51c06" + + +def frame(msg_type, body=b""): + return b"##" + struct.pack(">HL", msg_type, len(body)) + body + + +def varint(n): + out = b"" + while n > 0x7F: + out += bytes([(n & 0x7F) | 0x80]) + n >>= 7 + return out + bytes([n]) + + +def max_schema(): + """Worst case for the confirm body: 4 max-length args + 4 max-length + accounts at index 255. 238 bytes, the largest KKSOLSC1 payload there is.""" + p = b"KKSOLSC1" + b"\x01" + bytes(range(32)) + b"\x08" + bytes(8) + p += bytes([20]) + b"P" * 20 + bytes([20]) + b"I" * 20 + p += b"\x04" + b"".join(bytes([1, 16]) + f"Arg{i}".ljust(16, "x").encode() + for i in range(4)) + p += b"\x04" + b"".join(bytes([255, 16]) + f"Acct{i}".ljust(16, "x").encode() + for i in range(4)) + return p + + +def run_flow(client, payload, prefix, expect_shots): + t = client.transport + t._write(frame(MSG_SIGN, b"\x0a" + varint(len(payload)) + payload), None) + + shots = 0 + while True: + msg_type, data = t._read() + if msg_type != MSG_BUTTON_REQUEST: + break + shots += 1 + time.sleep(0.2) + name = os.path.join(OUT, f"{prefix}-{shots}.png") + capture_screenshot(client.debug, name, scale=3) + print(f" captured {name} ({os.path.getsize(name)}B)") + client.debug.press_yes() + t._write(frame(MSG_BUTTON_ACK), None) + + if msg_type == MSG_FAILURE: + f = proto.Failure() + f.ParseFromString(bytes(data)) + raise SystemExit(f"FAILED: {f.message}") + if msg_type != MSG_SIGNATURE: + raise SystemExit(f"unexpected response type {msg_type}") + assert shots == expect_shots, f"expected {expect_shots} screens, got {shots}" + return bytes(data) + + +def main(): + schema = json.load(open(REGISTRY))["schemas"][SCHEMA_KEY] + payload = base64.b64decode(schema["payload"]) + print(f"payload: {schema['program']} / {schema['instruction']}, {len(payload)} bytes") + + client = KeepKeyDebuglinkClient(UDPTransport("127.0.0.1:11044")) + client.set_debuglink(UDPTransport("127.0.0.1:11045")) + + client.auto_button = True + client.wipe_device() + client.load_device_by_mnemonic( + mnemonic="all all all all all all all all all all all all", + pin="", passphrase_protection=False, label="KeepKey Attestor", + language="english", + ) + client.auto_button = False + + data = run_flow(client, payload, "attestor-confirm", 5) + + print("worst-case schema (4 max args + 4 max accounts, 238B):") + mx = max_schema() + assert len(mx) == 238, len(mx) + run_flow(client, mx, "attestor-maxlabels", 10) + + # ClearsignAttestorSignature: field 1 = signature, field 2 = public_key + sig = data[2:2 + data[1]] + pub = data[2 + data[1] + 2:] + print(f"signature: {sig.hex()} ({len(sig)}B)") + print(f"pubkey: {pub.hex()} ({len(pub)}B)") + assert len(sig) == 64 and len(pub) == 33 + + try: + import hashlib + from ecdsa import VerifyingKey, SECP256k1 + vk = VerifyingKey.from_string(pub, curve=SECP256k1) + vk.verify_digest(sig, hashlib.sha256(payload).digest()) + print("signature verifies against returned pubkey over SHA256(payload)") + except ImportError: + print("(ecdsa not installed - skipped host-side verify)") + + +main() diff --git a/docs/evidence/clearsign-builtin-anchor/btn00000.png b/docs/evidence/clearsign-builtin-anchor/btn00000.png new file mode 100644 index 00000000..a1139c6c Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00000.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00001.png b/docs/evidence/clearsign-builtin-anchor/btn00001.png new file mode 100644 index 00000000..54421155 Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00001.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00002.png b/docs/evidence/clearsign-builtin-anchor/btn00002.png new file mode 100644 index 00000000..b98fed87 Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00002.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00003.png b/docs/evidence/clearsign-builtin-anchor/btn00003.png new file mode 100644 index 00000000..946c111b Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00003.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00004.png b/docs/evidence/clearsign-builtin-anchor/btn00004.png new file mode 100644 index 00000000..a28252ef Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00004.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00005.png b/docs/evidence/clearsign-builtin-anchor/btn00005.png new file mode 100644 index 00000000..f6fb4302 Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00005.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00006.png b/docs/evidence/clearsign-builtin-anchor/btn00006.png new file mode 100644 index 00000000..8c02b4b1 Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00006.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00007.png b/docs/evidence/clearsign-builtin-anchor/btn00007.png new file mode 100644 index 00000000..4089aa1f Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00007.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/btn00008.png b/docs/evidence/clearsign-builtin-anchor/btn00008.png new file mode 100644 index 00000000..6571f59e Binary files /dev/null and b/docs/evidence/clearsign-builtin-anchor/btn00008.png differ diff --git a/docs/evidence/clearsign-builtin-anchor/builtin_anchor_e2e.py b/docs/evidence/clearsign-builtin-anchor/builtin_anchor_e2e.py new file mode 100644 index 00000000..de976d2f --- /dev/null +++ b/docs/evidence/clearsign-builtin-anchor/builtin_anchor_e2e.py @@ -0,0 +1,65 @@ +#!/usr/bin/env python3 +"""Gate-3: clear-signing verified by the BUILT-IN anchor, no signer loaded. + +Phase 1 required LoadClearsignSigner before any metadata would verify, and the +device then led with the host-chosen identity ("X (fp) describes this tx"). With +a baked key the same blob verifies with no load at all and presents as +"Insight Verified". This drives exactly that, against a kkemu built with +-DKK_CLEARSIGN_TEST_KEY=ON, and never calls load_clearsign_signer. + + KEEPKEY_SCREENSHOT=1 SCREENSHOT_DIR=out \\ + PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python python3 builtin_anchor_e2e.py +""" +import os +import sys + +FW = os.environ.get("KK_FIRMWARE", os.path.join( + os.path.dirname(os.path.abspath(__file__)), "..", "..", "..", + "modules", "keepkey-firmware")) +sys.path.insert(0, os.path.join(FW, "deps", "python-keepkey")) + +from keepkeylib.client import KeepKeyDebuglinkClient +from keepkeylib.transport_udp import UDPTransport +from keepkeylib.tools import parse_path +from keepkeylib.signed_metadata import (serialize_metadata, sign_metadata, + CLASSIFICATION_VERIFIED) +from keepkeylib.clearsign_catalog import (CLEARSIGN_FLOWS_BY_KEY, FLOW_NONCE, + FLOW_GAS_PRICE, FLOW_GAS_LIMIT, + flow_tx_hash) + +# m/0x4B4B'/0x4353'/0' of the public "all all ... all" seed -- the key the +# attestor holds and KK_CLEARSIGN_TEST_KEY bakes into slot 0. +BUILTIN_PRIV = bytes.fromhex( + "642f523c98dfde47cf6b1c01d08ff8579c2d15a18b8019b53b83a083a2d215ad") +BUILTIN_KEY_ID = 0 + +flow = CLEARSIGN_FLOWS_BY_KEY["aave-v3-supply"] + +client = KeepKeyDebuglinkClient(UDPTransport("127.0.0.1:11044")) +client.set_debuglink(UDPTransport("127.0.0.1:11045")) +client.auto_button = True +client.wipe_device() +client.load_device_by_mnemonic( + mnemonic="all all all all all all all all all all all all", + pin="", passphrase_protection=False, label="Anchor", language="english") + +payload = serialize_metadata( + chain_id=flow["chain_id"], contract_address=flow["to"], + selector=flow["data"][:4], tx_hash=flow_tx_hash(flow), + method_name=flow["method"], args=flow["args"], key_id=BUILTIN_KEY_ID) +blob = sign_metadata(payload, BUILTIN_PRIV) + +resp = client.ethereum_send_tx_metadata(signed_payload=blob, + metadata_version=1, + key_id=BUILTIN_KEY_ID) +print(f"classification: {resp.classification} " + f"(VERIFIED={CLASSIFICATION_VERIFIED})") +assert resp.classification == CLASSIFICATION_VERIFIED, ( + "built-in anchor did not verify the blob") + +v, r, s = client.ethereum_sign_tx( + n=parse_path("44'/60'/0'/0/0"), nonce=FLOW_NONCE, + gas_price=FLOW_GAS_PRICE, gas_limit=FLOW_GAS_LIMIT, to=flow["to"], + value=flow["value"], data=flow["data"], chain_id=flow["chain_id"]) +assert r, "signing failed" +print(f"signed {flow['key']} ({flow['method']}) with NO signer loaded, v={v}") diff --git a/docs/firmware-7.15.0-review-round2-remediation.md b/docs/firmware-7.15.0-review-round2-remediation.md new file mode 100644 index 00000000..85a5ae9d --- /dev/null +++ b/docs/firmware-7.15.0-review-round2-remediation.md @@ -0,0 +1,94 @@ +# 7.15.0 release train — round-2 review remediation ledger + +Stack: `develop ← #444 ← #445 ← #446 ← #447 ← #448`. Fixes land on each +finding's **home** branch, then cascade forward. Because the stack is linear, +a fix on #444 ships in all five. + +Status legend: ☐ open · ⧗ in progress · ☑ fixed · ⊘ deferred (rationale) · +✔ already-correct / not-a-bug (rationale). + +Second review: 40 findings (15 high / 22 medium / 3 low) + 2 CI failures + 1 +bonus. All numbers below reference the reviewer's file:line. + +--- + +## Release blockers (red) — must fix before merge + +| # | Home | Finding | Status | +|---|------|---------|--------| +| B1 | #447 | ARM firmware won't link — `rom` overflowed by 2136 bytes | ☑ root cause: Zcash Pallas curve (~2.4k LOC) unconditionally linked. Fix: bump trezor pin to AES_SMALL_TABLES opt-in + define it (reclaims 15,360 B). **Verified: `firmware.keepkey.elf` links locally (kktech/firmware:v15, MinSizeRel)** | +| B2 | #448 | `unit-tests (zcash-privacy)` segfault — `Storage.BitcoinOnlyBandRefused` feeds 64-byte buffer to V17 reader → 16 KB OOB read (storage.cpp:523) | ☑ test buffer sized to STORAGE_SECTOR_LEN (firmware always reads a full sector) | +| B3 | #444 | Clear-signing silently defeated mid-tx: `EthereumTxMetadata` handler has no "signing in progress" guard; `signed_metadata_process()` clears binding → attacker streams unseen calldata, blind-sign gate stays suppressed (fsm_msg_ethereum.h:24, ethereum.c:304) | ☑ guard added: handler aborts if `ethereum_signing_isInProgress()` | +| B4 | #446 | `recovery_cipher.c:612` guard flipped `!enforce_wordlist` → `enforce_wordlist`, deleting the only wordlist check on the default path → mistyped cipher stored as seed, "Device recovered" | ☑ guard now `if (!auto_completed)` — fails in both modes (cipher recovery is always BIP-39) | +| B5 | #446 | `recovery_cipher.c:483` per-word BIP-39 validation reads `decoded_word` that `recovery_delete_character` never clears → backspace-fix wipes a real recovery | ☑ `recovery_delete_character` resyncs decoded_word (from mnemonic) + coded_word (reverse-cipher) after every edit | +| B6 | #446 | `recovery_cipher.c:379` "previous word" indicator snapshots every keystroke → shows current partial word mislabeled | ☑ snapshot moved to the word-boundary (space) handler; stores the auto-expanded completed word | +| B7 | #446 | `keepkey_main.c:189` replaced `signatures_ok()` (sha256+ecdsa) with spoofable sig-index presence check → forged metadata reads SIG_OK | ☑ (fixed pre-review as task #5) | +| B8 | #445 | `hive.c:194` `ecdsa_sign_digest(..., NULL)` — no canonical callback; Graphene rejects ~50% of sigs. Fix: reuse `eos_is_canonic` (eos.c:488) | ☑ added `hive_is_canonic` (same Graphene rule) and passed it to `ecdsa_sign_digest` | +| B9 | #445 | `fsm_msg_hive.h:147` confirm hardcodes `amount/1000` (3 dp) while serializer signs `msg->decimals` → shows 1000× wrong amount | ☑ display now uses serializer's precision via `bn_format_uint64`; rejects precision > 18 | +| B10 | #444 | `thortx.h:41` `MAYA_ROUTER` pinned to dead `0xd89dce…` (no code on mainnet); real router `0xe3985E6b…` → Maya swaps blocked, deposit-selector tx to code-less addr gets trusted UX, ETH lost | ☑ set to `e3985e6b…46d` (Etherscan-verified Maya ETH Router v4) | + +## Medium — should fix + +| # | Home | Finding | Status | +|---|------|---------|--------| +| M1 | #444 | eip712 cancel propagation inert: `review()` always returns true (confirm_sm.c:443) so `confirmName`/`confirmValue` never see cancel | ☑ `review()` **and** `review_with_icon`/`review_immediate`/`review_without_button_request` now return the real `confirm_helper` result. Adversarial verify caught that the first pass fixed only `review()`, leaving `dsConfirm`'s domain/verifyingContract screen (via `review_with_icon`) still inert — now closed. | +| M2 | #444 | `LAST_ERROR` bump left `failMsgReturn[]` 32 slots / 31 initializers → NULL deref if wired | ☑ added 32nd entry "EIP-712 cancelled"; `failMessage` also short-circuits USER_CANCELLED | +| M3 | #444 | `messages-ripple.options:9` enables `RippleSignTx.memo` but no reader at this head → memo silently dropped, XRP→THOR deposits strand | ✔ resolved-in-stack: pr447 ripple.c:230 serializes `tx->memo`. Artifact of per-PR-head review; release ships the reader | +| M4 | #444 | `ThorchainMsgSend.denom` enabled while `thorchain.c` still hardcodes "rune" | ✔ resolved-in-stack: pr447 THOR/Maya send path consumes `send.denom` | +| M5 | #444 | `signed_metadata.c:370` dup of `bn_from_bytes`; `thortx.c:45` 20× snprintf+strncmp router compare instead of `memcmp` on 20 bytes (mixed-case EIP-55 never matches) | ⊘ quality-only: router constants are lowercase literals + `to.bytes` rendered lowercase, so compare is exact today; no signed-byte/security impact. Next round | +| M6 | #445 | Zcash `fsm_msg_zcash.h:68` ~5.7 KB always-on .bss, no build gate | ⊘ code-size hygiene; the zcash-privacy build variant already gates the feature at CI level, mainstream ROM budget handled by B1. Next round: `#if ZCASH_PRIVACY` the .bss | +| M7 | #445 | Zcash `:91` hardcoded `[16]` instead of `ZCASH_MAX_ACTIONS` → OOB if raised | ☑ `signatures[ZCASH_MAX_ACTIONS][64]` | +| M8 | #445 | Zcash `:646` `SignPCZT` re-inits without `zcash_signing_abort()` → prior session's transparent sigs can leak | ☑ `zcash_signing_abort()` before key derivation / state init | +| M9 | #445 | Zcash `:1208` wire flow diverges from documented proto sequence | ⊘ doc-vs-impl reconciliation, no signed-byte impact; tracked for next round | +| M10 | #445 | Zcash `:756` account-resolution+fingerprint copy-pasted across 3 handlers | ⊘ refactor-only (extract shared `zcash_resolve_account`); no behavior change. Next round | +| M11 | #445 | `fsm_msg_hive.h:45` export label from untrusted display-only role field, not path | ☑ label now derived from `address_n[2]` (path role), not `msg->role` | +| M12 | #447 | `fsm_msg_ton.h:121` TON raw_tx blind-sign skips AdvancedMode gate | ☑ (fixed task #4) | +| M13 | #447 | `fsm_msg_tron.h:392` TRON TIP-712 typed-hash blind-sign skips AdvancedMode gate | ☑ AdvancedMode gate added before the blind-sign confirm (matches TronSignTx/TON) | +| M14 | #447 | `fsm_msg_mayachain.h:166` amount+long-denom overflows `amount_str[32]` → blank amount shown, real value signed | ☑ amount formatted without denom suffix; denom shown on its own "Asset" screen (matches THORChain send) | +| M15 | #447 | `mayachain.c:38/:230` `isValidDenom` + 146-line `parseConfirmMemo` byte-identical copies of THOR versions → shared `tendermint_*` helper | ⊘ (see notes) | +| M16 | #447 | `fsm_msg_solana.h:516/:550` parser-rule re-encoded at call site + verified-path copy-paste SignTx/SignMessage | ⊘ (see notes) | +| M17 | #448 | `storage.c:1240/1252` bitcoin-only seed-lock is exact-match-or-refuse, not via migration chain → next STORAGE_VERSION bump locks out every btc-only wallet | ☑ in-band wallets load via migration chain when underlying ≤ STORAGE_VERSION (migrate), refuse only newer; new `BitcoinOnlyBandMigrates` test | +| M18 | #448 | `fsm_msg_common.h:60` lock state smuggled into variant string as magic `"bitcoin-only-locked"` instead of machine-readable field | ⊘ requires a new Features proto field (device-protocol **fork** + nanopb regen + vault consumer) — multi-repo change out of scope for a stabilization round. Magic string is functional. Next round | +| M19 | #448 | `ci.yml:222` 3-variant matrix copy-pasted 5× across two workflows → drift | ⊘ CI DRY (YAML anchors/reusable workflow); no build/artifact impact. Next round | +| M20 | #447 (THOR/Maya) | deposit asset/signer injected into sign bytes unescaped/unvalidated | ☑ (fixed task #3) | + +## Low + +| # | Home | Finding | Status | +|---|------|---------|--------| +| L1 | #446 | `fsm_msg_bip85.h:5` handler re-validates word_count/index already checked in callee | ✔ keep: handler check rejects before CHECK_PIN/confirm so an invalid request never prompts for a PIN — intentional early defense, not dead duplication | +| L2 | #448 | `storage.c:95` comment says `btc_only_locked` "never set in bitcoin-only builds" but new path sets it | ☑ comment corrected (set in both builds via SUS_BitcoinOnlyLocked) | +| L3 | #448 | `storage.c:1397` locked path returns before `storage_readMeta` → locked device reports empty device_id | ✔ not-reproduced: `meta.uuid`/`uuid_str` are copied from flash at storage.c:1393-1396 **before** the switch and `storage_reset` clears only `.storage`, so `device_id` (= `uuid_str`) is preserved on the locked path | + +## Bonus (trimmed by 8/PR cap) + +| # | Home | Finding | Status | +|---|------|---------|--------| +| X1 | #444 | `storage.c:1970` `storage_getRawSeed()` (pointer to raw 64-byte seed) added with header decl and zero callers → dead sensitive API | ☑ removed function + header decl (zero callers confirmed across stack) | + +--- + +## Adversarial verification (2026-07-08) + +After all fixes were committed, each was independently re-verified by an +adversarial reviewer that read the committed code on the pr448 tip and tried +to refute it. Result: **15 of 16 CONFIRMED**; the one exception was **M1**, +where the first pass fixed only `review()` and left `review_with_icon()` +(used by the EIP-712 domain/verifyingContract `dsConfirm` screen) still +returning `true` — so cancel on that screen still signed. Fixed by making all +`review_*` variants return the real confirm result, then re-cascaded. This is +exactly the class of "compiles + passes CI but is inert" defect that gate G5 +(a fix must fail without itself) and G4 (adversarial review) now target. + +Build/test verification (kktech/firmware:v15, MinSizeRel + emulator): +each stacked branch links the ARM firmware and passes `firmware-unit`; +pr448 verified across all three variants (full / bitcoin-only / zcash-privacy). + +## Deferral notes + +Deferrals are **quality/refactor** items (dedup, code-size hygiene of niche +paths, CI DRY) that do not change signed bytes or on-device security, OR Zcash +items on a feature that is not part of the 7.15.0 shipping surface for the +mainstream device. They are logged here so the next round can pick them up; +none are correctness/security blockers. Each ⊘ will get a one-line reason when +triaged, not silently dropped. diff --git a/docs/firmware/SOLANA-SWAP-METADATA-V1.md b/docs/firmware/SOLANA-SWAP-METADATA-V1.md new file mode 100644 index 00000000..46292a7d --- /dev/null +++ b/docs/firmware/SOLANA-SWAP-METADATA-V1.md @@ -0,0 +1,71 @@ +# Solana transaction-bound swap metadata (`KKSOLSW1`) + +`KKSOLSW1` is the canonical signed descriptor used to ClearSign cross-chain +swaps whose Solana instruction cannot be decoded completely on-device (for +example, a Relay instruction that references address-lookup-table accounts). + +The descriptor does not replace Solana transaction parsing. Firmware still +parses the message structurally, rejects malformed messages, verifies that the +derived key is a required signer, and shows locally decoded priority fees. + +## Trust and signature + +- `payload` is the canonical byte sequence below. +- `signature` is a 64-byte compact secp256k1 ECDSA signature over + `SHA256(payload)`. +- `signer_key_id` selects a ClearSign signer already trusted by the device. +- Firmware rejects partial metadata, invalid signatures, hash mismatches, + program/discriminator mismatches, unsafe display text, and trailing bytes. +- A failed descriptor never downgrades to blind signing. + +The quote/metadata service must resolve all lookup-table accounts and validate +the protocol instruction before signing the descriptor. The private attestation +key must not live in the Vault client. + +## Canonical payload + +All integers are unsigned big-endian. Text fields are one-byte length-prefixed, +printable ASCII, and may not contain `%`. + +| Field | Encoding | +| --- | --- | +| Magic | 8 bytes: ASCII `KKSOLSW1` | +| Solana message hash | 32 bytes: SHA-256 of the exact serialized message signed by the device | +| Program ID | 32-byte Solana program public key | +| Instruction discriminator | First 8 instruction-data bytes | +| Quote expiry | `u64` Unix seconds | +| Source amount | `u64` base units | +| Minimum output | `u64` destination base units | +| Source decimals | `u8`, maximum 18 | +| Destination decimals | `u8`, maximum 18 | +| Protocol | `u8` length + 1–20 bytes | +| Source asset | `u8` length + 1–12 bytes | +| Destination chain | `u8` length + 1–16 bytes | +| Destination asset | `u8` length + 1–12 bytes | +| Destination address | `u8` length + 1–64 bytes | +| Order ID | `u8` length + 1–32 opaque bytes | + +No bytes may follow the order ID. + +## Device display + +After verification, firmware displays: + +1. ClearSign signer alias and fingerprint. +2. Source amount, asset, and protocol. +3. Minimum destination amount, asset, and chain. +4. The complete destination address. +5. Any locally decoded maximum priority fee. + +The exact Solana message hash binds those claims to the transaction signature. +The program ID and discriminator additionally bind the descriptor to the +protocol instruction the metadata signer decoded. + +## Opaque fallback + +`SolanaSignTx.allow_opaque` authorizes blind signing for that protobuf request +only. It does not mutate persistent `AdvancedMode`. Firmware still shows a +dedicated one-time blind-sign warning and requires physical confirmation. + +Hosts should set it only after a route-specific user acknowledgement. It is a +compatibility fallback for missing metadata, not a substitute for `KKSOLSW1`. diff --git a/docs/handoff-715-dress-rehearsal-round2.md b/docs/handoff-715-dress-rehearsal-round2.md new file mode 100644 index 00000000..e297a328 --- /dev/null +++ b/docs/handoff-715-dress-rehearsal-round2.md @@ -0,0 +1,176 @@ +# Handoff — 7.15 Dress Rehearsal, Round 2 (firmware only) + +**Status:** plan, ready to execute from a cold context. Nothing destructive done. Every SHA/count verified live via `git`/`gh` on **2026-07-16** — not from memory (memory has been stale before; re-derive in STEP 0 anyway). + +**Scope:** firmware only. BEX/client owned by another agent. Vault ships in parallel (`RELEASE-CONTROL-7.15.md`). + +--- + +## 0. The SOP (as clarified by the author, 2026-07-16) + +> First we make PRs into the upstream branches that are themselves PR'd to master. We make a branch and PR it **into the branch PR'ing into master**. **The PR into master becomes the canonical pin** for all work done in the fork going forward. The branch-into-branch PR is **for review purposes**. The PR into the PR-into-master **can come from the fork**. + +``` + + │ PR ← review vehicle + ▼ +up/release-protocol ──PR #111──► keepkey/device-protocol : master ─┐ +reconcile/upstream-sync ──PR #196──► keepkey/python-keepkey : master ─┴─► CANONICAL PINS + │ (firmware re-pins to these merged master SHAs) + ▼ +keepkey/keepkey-firmware : develop ◄── the 7.15 stack #444–#448 ◄── round-2 PRs 6..N +``` + +**Bottom-up rule:** proto + tests land on their masters first; firmware pins them after. Nothing in the firmware stack finalizes before that. + +Other binding rules (`firmware-release-sop.md`, `handoff-715-upstream-dress-rehearsal.md`): +- **Mergability is in order, not in isolation** — each PR green *in its turn*, on the full 3-variant matrix. +- **Non-destructive** — never move a shared branch until the new state is proven. +- **Dependency-true order, infra/build-flags LAST** (round 1 replaced "infra first" after a 12-file conflict storm). The SRAM gate must measure the **final** tree. +- **Never pin a fork/feature SHA in an upstreamable PR** (SOP:80-81: "an ancestor-of or equal-to master commit; never a fork/feature SHA"). +- **Gate-3**: no firmware PR approval without on-device OLED proof. Emulator 28/28 is not a substitute. +- pyk pushes go to the **fork** remote (`bithighlander`); `origin`/`upstream` are both keepkey. + +--- + +## 1. THE REFRAME — the proposal is already half-done ⚠️ + +The proposal was: *"checkout step 5 of the 1/5 upstream firmware PRs, bring that into fork develop, and practice our dress rehearsal on the firmware updates after that point."* + +**"Bring #448 into fork develop" is a NO-OP — and as a reset it would be destructive.** + +``` +git merge-base --is-ancestor dada55e9 bithighlander/develop → YES +git rev-list --left-right --count dada55e9...bithighlander/develop → 0 48 +``` + +#448's head is **already an ancestor** of fork develop; the upstream stack was cut *from* fork develop. Fork develop is just **48 commits ahead**. Executing it as `reset --hard` would move develop **backward** and regress `deps/device-protocol` **f0b45498 → 33521a8**, un-defining Hive msgs 1614–1617 while `hive.c`/`fsm_msg_hive.h` remain — the exact failure hit on 2026-07-16 ("Unknown message (code 1)" + wedged transport, which *looks* like a firmware bug but is a pin regression). + +**Round 2 = EXTEND the #448 stack with PRs 6..N. No reset, no rebase of #444–#448.** + +Round 1's "diff-curation, not cherry-pick" lesson was a consequence of rebasing 107 sprawled commits onto a **bare** upstream base lacking context. **That condition is absent here** — the base (dada55e9) already contains every dependency the delta assumes, and 14 of the 33 non-merge commits are already isolated on ready-made fork branches. Round 2 is **thematic-append**, with hand-curation only where shared files actually collide. + +### The real structure: the fork ran **7 parts**; upstream got **5** + +| Fork merge | Upstream | Status | +|---|---|---| +| `bac4cdd4` 7.15 (1/7) EVM clear-signing core | #444 | ✅ open upstream | +| `9d09955a` 7.15 (2/7) New chains — Hive **(phase-1 only)**, Zcash Orchard, Ripple, THOR any-denom | #445 | ✅ open upstream | +| `e28688a7` 7.15 (3/7) Device robustness | #446 | ✅ open upstream | +| `c3c9e4b8` 7.15 (4/7) Per-chain clear/blind-sign | #447 | ✅ open upstream | +| `9844ef4e` 7.15 (5/7) Build variants + seed-lock + version 7.15.0 | #448 | ✅ open upstream | +| `9e9652d5` **7.15 (6/7) Persistent clear-sign identities + icons (STORAGE_VERSION 18)** | — | ❌ **branch exists, not upstream** | +| `110b78ab` **7.15 (7/7) Emulator dylib — Windows cross-compile** | — | ❌ **branch exists, not upstream** | + +Verified linear ancestry: +``` +upstream/develop 1af2ffe7 → bd49ac17(#444) → a53eca0e(#445) → f8fc0a73(#446) + → 8c6c24ff(#447) → dada55e9(#448) → 871505ac(pr6) → e87c2591(pr7) → 183a2b93(fork develop == rc10) +``` + +--- + +## 2. 🔴 LIVE RELEASE BLOCKER FOUND — same-name branch collision + +**There are two different branches named `up/release-protocol`:** + +| Ref | SHA | What | +|---|---|---| +| `BitHighlander:up/release-protocol` | **2ec999a9** | **#111's HEAD** (`gh pr view 111 --json headRefOid`) | +| `keepkey:up/release-protocol` | **33521a8** | `feat(clearsign): identity icon + persist` — **this is what #448 PINS** | + +device-protocol ancestry (linear off keepkey master `f2c3c005`): +``` +f2c3c005(master) → 2ec999a9(#111 head) → 33521a8(icon+persist; #448's pin) + → 9e46aeb(HiveSignMessage 1614/1615) → a793934(docs) → f0b4549(HiveSignOperations 1616/1617; fork develop's pin) +``` + +**So #111 is ONE COMMIT BEHIND what #448 already pins.** Merging #111 as-is produces a master on which **#448's pin is unreachable** — and #448 already ships `LoadClearsignSigner.icon max_size:384` in its `.options`, which would be an **orphan nanopb option** → build break. + +Separately: **fork develop pins `f0b45498`, which exists ONLY on BitHighlander/device-protocol** (`git ls-remote keepkey/device-protocol | grep f0b45498` → no match) — a live violation of SOP:80-81. + +**Both are fixed by PR #36 below.** + +--- + +## 3. ✅ DONE 2026-07-16 (Phase 0 — bottom of the SOP stack) + +Both stack into the branches that PR to master, exactly per the SOP. **Awaiting third-party review.** + +| PR | Base → | Commits | Diff | Fixes | +|---|---|---|---|---| +| **[BitHighlander/device-protocol#36](https://github.com/BitHighlander/device-protocol/pull/36)** | `up/release-protocol` → #111 → master | 4 | **+79 / −0**, 5 files | Advances #111 `2ec999a9 → f0b4549`: `33521a8` icon+persist (**closes the blocker above**) · `9e46aeb` 1614/1615 · `a793934` docs · `f0b4549` 1616/1617 | +| **[BitHighlander/python-keepkey#27](https://github.com/BitHighlander/python-keepkey/pull/27)** | `reconcile/upstream-sync` → #196 → master | 17 | +1429 / −92, 17 files | Advances #196 `e728e311 → 15d95ec`: closes the 10-commit gap to `560b897` (#448's pin) **and** adds hive suites (+465) + clearsign v2 harness (+240) | + +These two PRs collapse what the raw plan listed as four separate foundation steps. Chosen as **PRs** rather than silent fast-forwards of the open PRs' heads — reviewable, and matches the author's SOP. + +--- + +## 4. Round-2 PR groups (dependency-true order) + +Build & prove each **on the fork first** (that *is* the rehearsal); promote to upstream only after green-in-turn. + +| ID | Upstream title | Source | Depends on | Size | +|----|---|---|---|---| +| **R2-A** | `7.15 (6/N)` Persistent clear-sign identities + icons (**STORAGE_VERSION 17→18**) | fork branch `release/7.15.0-pr6-persistent-identity` @ **871505ac** — **already built**, verified ancestor of develop; lift verbatim | base `dada55e9`; **HARD**: dp pinned ≥ `33521a8` (sole consumer of icon/persist fields) | 10 commits / 11 files / **+877 −236** — largest, highest review risk (storage migration). Keep standalone. | +| **R2-B** | `7.15 (7/N)` Emulator dylib (Windows cross-compile, poll-thread confirm gating) | fork branch `release/7.15.0-pr7-emulator-dylib` @ **e87c2591** — already built; lift verbatim | R2-A | 4 commits / 13 files / +590 −88. **DECISION: may be legitimately fork-only** (dev tooling, no upstream device consumer). | +| **R2-C** | `7.15 (8/N)` **Hive phase-2** — SignMessage 1614/1615 + SignOperations 1616/1617 + SLIP-48 hardening | new branch `release/7.15.0-pr8-hive` off e87c2591; cherry-pick linear chain `005cc023 91a63617 f22b56da ddade55d cbf33967 b5ddeb09 767276d2` | R2-B; **HARD-BLOCKED on #36 + #27 reaching master** (needs dp f0b4549 + pyk 15d95ec) | 7 commits / ~6 files / ~+690 −35. Strict **superset** of #445's hive phase-1 (5→7 handlers) — additive, **zero rework of #445**. | +| **R2-D** | `7.15 (9/N)` Zcash shielded-signing progress UX + emulator privacy default | new branch off pr8; `b9a778c4 e50f1f4c 5ce715b9 c81ec389 33c2f379` | R2-C. Pin-neutral. | 5 commits / 5-7 files / ~+174 −8. Display-affecting → **Gate-3 re-capture**. | +| **R2-E** | `7.15 (10/N)` SRAM frame arena + 16KiB reserve gate + CI timeouts | new branch off pr9; `424d9eb4 8a50729e c36488fd da8a23ec` | R2-D. **MUST BE LAST** — the reserve gate must measure the final tree. | 4 commits / ~17 files / ~+590 −115. **Heaviest curation.** | +| **R2-F** | `7.15 (11/N)` THORChain clear-sign on every EVM chain | fork **PR #309** @ `715c173e` (**OPEN, not merged**) | Independent — R2-E or standalone off #448 | 1 commit / 3 files / +236 −14. **Security-adjacent.** | + +### Hand-curation map (only where shared files actually collide) +- **R2-C**: `include/keepkey/firmware/fsm.h` (forward decls ×2), `lib/firmware/messagemap.def` (MSG_IN/OUT 1614–1617), `messages-hive.options`. +- **R2-D**: `lib/board/layout.c/.h` (collide with R2-A's icon render), `CMakeLists.txt` (collides with R2-B/R2-E). Ordering resolves most. +- **R2-E**: heaviest — `signed_metadata.c` (R2-A/#444 territory), `fsm_msg_bip85.h`, `reset.c`, `recovery_cipher.c`, `lib/board/usb.c`, `CMakeLists.txt`, `ci.yml`. +- **R2-A / R2-B**: no collisions with each other. + +--- + +## 5. Execution (cold-start) + +- **STEP 0 — RE-VERIFY.** `git fetch keepkey && git fetch bithighlander`; re-derive every SHA above. Do not trust prose. +- **STEP 1 — PRESERVE.** rc10 is branch-preserved (`refs/heads/release/7.15.0-rc10` == develop == `183a2b93`) but **NOT tag-preserved**. Branches are force-updatable/prunable — weak for a release tip. Also tag the **fork-only** dp pin so it can't be GC'd: + ```bash + git tag -a fork-develop-preround2-183a2b93 183a2b93 -m 'fork develop/rc10 tip entering round-2' && git push bithighlander fork-develop-preround2-183a2b93 + cd deps/device-protocol && git tag -a fw-develop-dp-pin-f0b4549 f0b4549 -m 'dp pin of fw fork develop (Hive 1614-1617)' && git push origin fw-develop-dp-pin-f0b4549 + ``` +- **STEP 2 — DECIDE.** Answer §7 (they gate the rest). +- **STEP 3 — FOUNDATION.** Get **#36** and **#27** reviewed + merged → then **#111** and **#196** → **master**. Record the merged master SHAs = the canonical pins. *(Everything below is hard-blocked on this.)* +- **STEP 4 — RE-PIN #444–#448** from practice-pins (`33521a8`/`560b897`) to the merged **master** SHAs; verify by hand the pins are on master (CI may only check ancestor-of-a-known-branch). Merge the stack **#444→#445→#446→#447→#448** in order; #449 independently. +- **STEPS 5-9 — BUILD R2-A…R2-E on the fork**, in order, each: local Docker verify **first** (SOP: verify-locally-not-CI), then 3-variant CI green **in its turn**. +- **STEP 10 — ACCEPTANCE TEST** (round 1's bijection proof): the R2-E tip's tree must be **identical** to fork develop: + ```bash + git diff --stat release/7.15.0-pr10-sram-ci 183a2b93 # MUST be empty (modulo submodule pins) + ``` +- **STEP 11 — GATE-3.** OLED proof for display-affecting deltas: R2-A identity icons, R2-D zcash progress bar, R2-C hive display budget (`ddade55d`), R2-F (#309). Plus storage-migration proof if R2-A ships (flash v17 → upgrade → keys preserved). +- **STEP 12 — PROMOTE** (needs explicit author OK; pushes to keepkey/keepkey-firmware): push each proven branch upstream with the same name, open PRs bottom-up. Retitle #444–#448 `(N/5)` → `(N/9)` via `gh pr edit` — touches no git, resets no CI. +- **STEP 13 — DOC HYGIENE.** In `handoff-715-upstream-dress-rehearsal.md`, delete the dangling `scratchpad/curation-specs.json` citation at :149 — **that file does not exist anywhere on disk**; nothing should plan around it. + +### 🚫 NEVER +`git push --force bithighlander develop` · `git reset --hard` on develop · rebase/force-push #444–#448 (burns 5 green CI runs + in-flight review on PRs open since 2026-07-08) · `git submodule update --remote/--force` on the vault's firmware submodule (the vault pins `ddade55d`, *inside* the hive stack) · move the dp pin backward. + +--- + +## 6. Risks & guards + +| Risk | Guard | +|---|---| +| Reset regresses dp pin → hive breaks | **Don't reset.** #448 is already an ancestor. | +| #111 merges one commit short → #448's pin unreachable → orphan nanopb option → build break | **PR #36** (merge before #111). | +| Deep stacking (9-10 PRs) on an unreviewed 5-stack; upstream develop moves → full-train rebase | Timing decision in §7. | +| R2-E measured too early | It goes **last**, by construction. | +| Gate-3 captured mid-stack | Capture at the final rc only. | +| #309's memo fix lost | Tracked as R2-F; exploitable on shipped mainnet firmware. | + +--- + +## 7. Decisions needed before executing + +1. **CONFIRM THE REFRAME** — round 2 = extend #448 with PRs 6..N, **no reset**. This contradicts the literal proposal; needs explicit sign-off. *(blocks everything)* +2. **R2-A in 7.15.0?** Persistent identities + STORAGE_VERSION 17→18. You chose **warning-gated clearsign, key ceremony deferred** — if identities are only meaningful with a real trust anchor, R2-A may defer to 7.15.1, which also removes the riskiest storage migration from this release. +3. **R2-B upstream at all?** Emulator dylib is dev tooling with no upstream device consumer. If fork-only, round 2 shrinks by 4 commits and R2-C re-bases on `871505ac`. +4. **R2-F (#309) in or out?** Security-adjacent: non-mainnet EVM THORChain deposits blind-sign today. Fold into #447, or ship standalone (separate may review better)? +5. **Renumber #444–#448** `(N/5)` → `(N/9)`? Free, clearer for reviewers. +6. **Timing** — open round-2 PRs now (stacked deep on an unreviewed 5-stack), or wait for #444–#448 to merge? +7. **Disclosure** — the thortx 64-byte memo read is exploitable on *shipped* firmware. Handle disclosure before a public PR description explains it? diff --git a/docs/handoff-715-upstream-dress-rehearsal.md b/docs/handoff-715-upstream-dress-rehearsal.md new file mode 100644 index 00000000..844d2979 --- /dev/null +++ b/docs/handoff-715-upstream-dress-rehearsal.md @@ -0,0 +1,228 @@ +# Handoff — 7.15 Upstream Dress Rehearsal (reconstitute ~26 PRs → 4) + +**Status:** plan. Nothing reset yet. This defines the next phase per +`firmware-release-sop.md` (upstream-first, bottom-up) and the author's ask: +reset fork develop to upstream develop and re-introduce the 7.15 work as a +*small* number of clean, coherent, reviewable PRs — down from the ~12–26 the +first pass produced. The reconstitution itself is the value: it forces each +feature into a self-contained, human-readable, individually-green diff, which +is what survives upstream peer review. + +## What a dress rehearsal is (definition) + +1. **Reset** fork `BitHighlander/keepkey-firmware:develop` to **upstream** + `keepkey/keepkey-firmware:develop` (the clean base everyone will review against). +2. **Reconstitute** the 7.15 delta as **3–4 thematic feature PRs** into a fresh + fork develop — each self-contained, each green *in its turn* (SOP §"Mergability + is in order, not in isolation"), each pinning the **upstream** proto/test + masters (never fork SHAs). +3. It is a *rehearsal*: fork develop is the practice stage; the production target + is upstream `develop`, gated behind the two upstream-master merges below. + +## Current state (measured 2026-07-07) + +- Fork `develop` is **107 commits / ~26 merged PRs** ahead of upstream + `keepkey/keepkey-firmware:develop`. That sprawl (#262–#293) is the 7.15 line. +- **Foundation already staged upstream** (the bottom of the SOP stack): + - `keepkey/device-protocol:up/release-protocol` @ `33521a8` (PR #111 → master) + - `keepkey/python-keepkey:reconcile/upstream-sync` @ `1674346` (PR #196 → master) + - Both are content-complete + CI-green (rc5 built on them). Master merges are + peer-reviewed + later — they are the **critical path** (SOP §"The rule"). +- **Nothing is lost by the reset:** the current tip is preserved on + `release/7.15.0-rc5` (@ `9372730d`) and every feature branch still exists. + +## DECIDED (2026-07-07) +- **5 PRs** — clear-signing split into **1a core** + **1b per-chain** (below). +- **Non-destructive:** build on a fresh `rehearsal/7.15-upstream` branch off + upstream develop; fork `develop` stays intact until the grouping is proven. +- **Practice-pins:** pin the upstream *branch tips* now (device-protocol + `up/release-protocol` @ `33521a8`, python-keepkey `reconcile/upstream-sync` @ + `1674346`) so the rehearsal isn't blocked on days-long master review; swap to + the real master SHAs for the actual upstream PR. + +## Proposed regrouping — ~26 PRs → **5 feature PRs** + +Each maps a pile of the original stage/** PRs into one coherent, reviewable diff. +The messy bundles get *split by theme* (e.g. #279 mixed zcash + eth-hardening + +insight — that gets torn apart, not preserved). + +### PR 1a — EVM clear-signing CORE +The signed-metadata trust system + the EVM hardening it rides on. The marquee diff. +- signed-metadata v1 + **v2 static schema** (#281 warning/phase-1, #284 v2 + + LoadClearsignSigner + **icon proto**), insight (#257/#258 from #279). +- EVM hardening: eip1559 zero-priority RLP (#275), eip712 security (#263), token + chain-id (#262), ETH RLP length-strip (#255/#260/#261). +- Pins: device-protocol `up/release-protocol`, python-keepkey `reconcile/upstream-sync`. + +### PR 1b — Per-chain clear-sign (rides on 1a) +The application of the metadata system to each non-EVM family. Green only after 1a. +- TRON (#285 clearsign, #266 tip712-gate, #265 blind-sign) +- Solana (#286 v0, #267 token-decimals) +- THOR/Maya (#287 memo-affiliate, #268 maya-evm-display) +- TON (#264 blind-sign) + +### PR 2 — New chain support +Net-new chains / address+memo formats, independent of clear-signing. +- Hive SLIP-0048 (#276), Zcash Orchard (zcash half of #279), Ripple memo (#270), + THORChain any-denom (#269). + +### PR 3 — Device robustness & key features +Non-clearsign firmware features + hardening. +- BIP-85 (#273), BIP-39 recovery (#272), fault-injection hardening (#271). + +### PR 4 — Build / CI / release infrastructure +Already partly on develop; consolidate as one infra PR (or leave as the small +merged set — these are low-review-risk). +- Build-flag variants btc-only / zcash-privacy (#282), CI variant matrix + + canonical names (#290, #293), PDF-report harness pin (#288), submodule + canonical-pin plumbing (#292). + +> Rationale for 4: each is a **reviewable story** with a single theme, self- +> contained enough that a reviewer holds it in their head. PR 1 is unavoidably +> large (it's the marquee feature) — keep it coherent, not artificially split; +> offer the 1a/1b split only if a reviewer asks. + +## Reconstitution findings (2026-07-07, from starting PR 4) + +Attempting PR 4 first surfaced the real structure — worth internalizing before grinding: + +1. **The feature branches are STACKED, not isolated.** Every `feat/**` branch was + authored on top of the accumulated 7.15 develop (clearsign → hive → version-bump + → …), so its commits' diff *context* assumes that stack. Cherry-picking the + "isolated" build-flag commits onto bare upstream develop conflicted in **12 + files** (fsm.c, storage.c, CMakeLists, app_confirm, …) — the surrounding lines + they patch don't exist yet. +2. **Hidden dependency chain:** build-flag variants (bitcoin-only / zcash-privacy) + touch `storage.c`/`fsm.c` and lean on 7.15 storage-version + message context ⇒ + NOT independent. And the **CI variant matrix depends on the build flags** + (bitcoin-only / zcash-privacy CMake flags must exist for those jobs to mean + anything). So the naive "infra PR is green-first" is FALSE: variant-CI → needs + build-flags → needs 7.15 storage/fsm context. +3. **Consequence — reconstitution is diff-curation, not cherry-pick.** Because + final fork develop is known-green, the tractable method is: per PR, take that + feature's **curated diff** and apply it in **true dependency order**, resolving + the shared-file hunks (fsm.c / storage.c / CMakeLists / messagemap.def) by hand. + +### Revised order (dependency-true, replaces the earlier "infra first") +``` +PR 1a clearsign CORE — the storage/fsm/proto foundation most things touch +PR 2 new chains — Hive/Zcash/Ripple/THOR-denom (own files + shared CMake/fsm) +PR 3 robustness — bip85 / bip39-recovery / fault-injection +PR 1b per-chain clearsign — rides 1a +PR 4 build-flags + CI matrix + report — LAST: it wraps everything (variant + builds gate the coins/features the earlier PRs added; CI-only bits + (report harness, release.yml) could split into a tiny green-first PR 0 + if a pure-infra quick win is wanted) +``` +Rationale: build the foundation the shared files accrete onto, THEN the wrapper +that depends on all of it. "Infra first" only works for the *pure-CI* slice +(ci.yml report/release, no C) — carveable as an optional PR 0. + +## PR 1a execution recipe (clearsign core) — the exact delta map + +Base: `pr/rehearsal-1a-clearsign-core` off `rehearsal/7.15-upstream`. Practice-pin +device-protocol `up/release-protocol` (`33521a8`) + python-keepkey +`reconcile/upstream-sync` (`1674346`) first (the proto/tests it needs). + +- **NEW (clean add — take fork develop verbatim):** + `include/keepkey/firmware/signed_metadata.h`, `lib/firmware/signed_metadata.c`, + `unittests/firmware/signed_metadata.cpp`. +- **EVM-exclusive MODIFIED (take fork develop final state — self-contained on the + EVM/thorchain/eip712 base already upstream):** `lib/firmware/ethereum.c`, + `ethereum_contracts.c` + `ethereum_contracts/{saproxy,thortx,zxappliquid, + zxliquidtx,zxswap,zxtransERC20}.c`, `eip712.c`, `include/.../eip712.h`, + `ethereum_tokens.h`, `ethereum_contracts/thortx.h`, + `include/keepkey/transport/messages-ethereum.options` (incl. the `icon` + max_size:384 line + LoadClearsignSigner entries). +- **SHARED dispatchers — CURATE clearsign hunks only (do NOT take final state; they + carry hive/zcash/bip85/etc.):** `lib/firmware/fsm.c` (LoadClearsignSigner + + EthereumTxMetadata dispatch), `lib/firmware/messagemap.def` (msg 115/116/117 + entries), `lib/firmware/CMakeLists.txt` (+signed_metadata.c) and top `CMakeLists.txt`. +- **Verify:** local Docker `make -j kkemu` (init deps/python-keepkey too), then CI. + ⚠ Some of this may already be on upstream develop from a prior cycle (signed_metadata + showed both A and pre-existing in probes) — diff each MODIFIED file against + upstream develop first and take only the *net-new* hunks so the PR is a true + minimal delta. + +## VERIFIED CURATION MAP (workflow wf_b59cd1da, 2026-07-07) + +Full spec: `scratchpad/curation-specs.json`. Adversarial synthesizer verified a bijection over all +89 files. Method insight: **each of the 107 commits is cleanly attributable to ONE PR** — so +reconstitution = cherry-pick PR commit-groups in dependency order (not hunk-surgery). End-state must +equal origin/develop (device-protocol `33521a8`, python-keepkey `1674346`, trezor-firmware `56f404e4`). + +**Fixes baked into the order below:** +1. Orphan `unittests/firmware/coins.cpp` (1838c56c) → **PR1a**. +2. `thortx.c/.h` double-claim → **PR1a wholesale** (Maya-EVM-display folds into 1a); PR1b drops it. +3. Submodule pins device-protocol `33521a8` + python-keepkey `1674346` + `.gitmodules` → **PR0 base** + (needed by 2/3/4, not just 1a); pin the final canonical SUPERSET SHA, don't split ~17 intermediate bumps. +4. `deps/crypto/trezor-firmware`: **PR2 bumps → `0ea97b09`** (Orchard/Pallas — else zcash.c won't build); + **PR4 → `56f404e4`** (AES_SMALL_TABLES superset). +5. `unittests/firmware/CMakeLists.txt` 4-way: PR1a=signed_metadata.cpp, **PR2=thorchain.cpp+zcash.cpp only**, + **PR1b=tron/solana/mayachain.cpp**, PR4=the KK_BITCOIN_ONLY/#if restructure; drop no-op 28c74a0e. +6. Version bump `b3e38b35` (7.14.1→7.15.0) rides **PR4** (or a base commit if per-PR 7.15 identity matters). +7. Drop all merge commits + folded intermediate re-pins (a8e3ab4e/f38a57fb/28c74a0e/565add6c/etc.). + +## Process (per SOP) + +1. Confirm the two upstream-master foundation PRs (#111, #196) are **merged** + (or, for the rehearsal, pin their current branch tips as *practice* pins per + SOP §"rehearse the pin-swap"; swap to master SHAs for the real thing). +2. `git branch rehearsal/7.15-upstream upstream/develop` — the fresh, + non-destructive base. Fork `develop` untouched (promote later once proven). +3. Build **PR 4 (infra)** first → into `rehearsal/7.15-upstream` — green + immediately, unblocks the variant CI everything else runs under. +4. Build **PR 2 (chains)** + **PR 3 (robustness)** next — develop-compatible, + green on their turn. +5. Build **PR 1a (clearsign core)** then **PR 1b (per-chain)** — pin the upstream + proto+test *practice* tips; 1b is green only after 1a. Ordering is the SOP's + green-in-turn pipeline, not standalone-on-bare-develop. +6. Per-PR merge gate: SOP §"Per-firmware-PR merge gate" checklist + (upstream-master pins, proto/test existence, CI green, on-device verify). +7. Reconcile-before-PR for python-keepkey per SOP §"Reconcile-before-PR". + +## Decisions — RESOLVED +1. ~~Split PR 1?~~ **Yes → 1a core + 1b per-chain (5 PRs total).** +2. ~~Rehearse against tips or master?~~ **Practice-pin the upstream branch tips now.** +3. ~~Destructive reset or fresh branch?~~ **Fresh `rehearsal/7.15-upstream` branch.** +4. Still open: **PR 4 scope** — one consolidated infra PR vs leaving the already- + develop-compatible infra commits to ride the base (decide when building PR 4). + +## Safety +- Current 7.15 tip preserved at `release/7.15.0-rc5` (`9372730d`) + all feature + branches. The reset is reversible. +- No upstream **master** merge happens in the rehearsal — that stays peer-reviewed + ([[handoff-upstream-pr-staging-strategy]], [[feedback-device-protocol-fork-only]]). + +## EXECUTED — final fork dress rehearsal (2026-07-13) + +Correction internalized: **dress rehearsals run on the FORK** (SOP step 3); +upstream merge is step 4, LAST. Also measured: upstream **master is stale at +7.14.0** (#421); v7.14.1 tag lives on `release-7141`, fully contained in +upstream develop → **reset base = upstream develop `1af2ffe7`**, never master. + +What was done (worktree, no local checkout disturbed): +1. Canonical stack = upstream PRs **#444–#448** heads (`keepkey/release/7.15.0-pr1…pr5`, + all CI-green, tree ≡ old fork develop mod python-keepkey pin). Old fork + `pr/rehearsal-*` branches are a stale iteration — dead. +2. Built the two missing PRs on the fork (zero cherry-pick conflicts, file sets disjoint): + - `release/7.15.0-pr6-persistent-identity` — 10 identity commits (storage v17→18, icons, compass) + - `release/7.15.0-pr7-emulator-dylib` — 4 emu commits (#249–252) +3. **Fork develop force-reset to `1af2ffe7`** + seven `--no-ff` merges (pr1→pr7), + mimicking the upstream merge train. Old tip preserved on `release/7.15.0-rc7`. +4. **`release/7.15.0-rc8` cut = new develop tip `c36488fd`** (= merge train + `110b78ab` + one ci.yml fix: static-analysis timeout 5m→10m — cppcheck runs + 4m30s+ on the 7.15 tree, the 5m budget flaked two rc8 runs as "cancelled"; + the fix rides pr5/#448 when upstreaming). **CI GREEN on all 4 refs** + (pr6, pr7, develop, rc8 — full 3-variant matrix each). +5. Proven: rc8 tree ≡ rc7 tree **except** `.gitmodules` + python-keepkey pin + (rc8 pins upstream `560b897` — better SOP hygiene than rc7's fork pin). + +Next: fork CI green (develop/rc8/pr6/pr7) → flash rc8, run on-device matrix +(G1–G12 + **7.14.1→v18 storage upgrade preserves wallet** + clearsign/identity +smokes + variant boots) → land #111/#196 upstream masters → pin-swap stack +bottom → upstream PRs 6/7 → merge #444→pr7 → upstream release/7.15.0 → tag. + +Related: `docs/firmware-release-sop.md`, `docs/submodule-pinning-sop.md`, +`docs/handoff-firmware-rc-7x-test-matrix.md` (the first rehearsal's on-device +matrix), [[clearsign-identity-icons]], [[fw-715-rc3-release-cut]]. diff --git a/docs/handoff-audit-uncommon-spend.md b/docs/handoff-audit-uncommon-spend.md new file mode 100644 index 00000000..a2708dc7 --- /dev/null +++ b/docs/handoff-audit-uncommon-spend.md @@ -0,0 +1,106 @@ +# Handoff: "Build TX now" — spend directly from an uncommon-path audit find + +**Date:** 2026-07-02 +**State:** audit discovery of uncommon paths WORKS end-to-end (device-verified +tonight on a real LTC case). What's missing is the last mile: a funded +uncommon-path row should offer a **spend/sweep button right there**, not just +"send to support". + +## What already works (don't rebuild) + +All landed on the working tree 2026-07-02 (rides in v1.4.10): + +- `utxoAccountScriptPaths` (`src/bun/chain-scan.ts`) — per-account xpub set now + includes the chain's own receive convention AND (for LTC) the legacy + **p2wpkh-on-BIP44** branch. Shared by getBalances (bulk), getBalance + (single-chain), buildTx, auditScanUtxoAccounts, addUtxoAccount. Unit tests in + `__tests__/chain-scan.test.ts`. +- **Account-level finds are already spendable**: audit "track" → + `addUtxoAccount` persists all 4 xpubs to `cached_pubkeys` (PK device/chain/ + path; the p2wpkh-on-44 entry is appended LAST so it wins the upsert on the + shared 44' path) → `buildTx` merges cached xpubs into `allXpubs`, and + `txbuilder/utxo.ts` rewrites per-input `addressNList` from the UTXO's source + xpub tag (`_sourceAccountPath`, line ~552). **Device-verified tonight**: + account-1 LTC tracked and merged into the portfolio. +- `AuditKnownPaths.tsx` — "Scan uncommon paths" grid for LTC (scheme list + `LTC_KNOWN_SCHEMES`: uncommon p2wpkh-on-44, standard 44-p2pkh, standard + 84-p2wpkh). Address-level, via `auditScanPaths` (accepts `scriptType`). + Funded rows flow into the support handoff + the `?audit=` GET param + (`docs/handoff-support-audit-get-param.md`). + +## The feature + +On every FUNDED row in `AuditKnownPaths` (and `AuditCustomPath` results), add a +**"Build TX now"** button that sweeps that specific address's UTXOs to the +user's standard receive address (chain default path, account 0). Flow: + +1. Row context: `path: number[]`, `scriptType`, `address`, balance. +2. Fetch UTXOs address-level: `pioneer.ListUnspent({ network, xpub: address })` + — Pioneer's ListUnspent accepts plain addresses too (the sweep-engine + already relies on this; see `src/bun/sweep-engine.ts` line ~207). +3. Build inputs with EXPLICIT `addressNList = [...path]` (full 5-element path + of the found address) and the row's `scriptType`. This bypasses the + xpub/account machinery entirely — no tracking or cache rows needed. +4. One output: the chain's standard receive address (derive fresh at + `chain.defaultPath`), minus fee. It's a sweep: no change output. +5. Fee: reuse `estimateUtxoFee` rates (`txbuilder/utxo.ts`); Zcash ZIP-317 + floor logic if this ever generalizes past LTC. +6. Sign via existing `btcSignTx` path, broadcast via existing broadcast RPC, + then trigger the single-chain refresh (`getBalance`) so the balance lands. + +### Reuse candidates, in order of laziness + +1. **`sweep-engine.ts`** — the audit-adjacent sweep subsystem already builds + single-address sweeps for the seed-recovery flow. Check whether + `buildSweepTx` (or equivalent) takes (address, path, scriptType, dest) — + if yes, this feature is mostly UI + one RPC. + ⚠️ Memory notes say part of the OLD audit sweep (`auditScanBtc`/`auditSweep`) + is dead code scheduled for deletion — verify which half is alive (the + SweepDialog path is the live one). +2. `buildUtxoTx` with `allXpubs: [{ xpub:
, scriptType, accountPath: + path.slice(0,3) }]` — works today IF Pioneer ListUnspent returns per-UTXO + `path`s for a plain address query (it returns the address's own UTXOs; the + input rewrite then rebuilds `addressNList` from `_sourceAccountPath` + the + blockbook path tail). Confirm the tail (change/index) is right for + address-level queries — if blockbook omits `path` for plain addresses, the + `!input.path` fallback in `utxo.ts` (~line 566) uses `[...accountPath, 0, 0]` + which is CORRECT only for 0/0 finds — pass the full found path instead. + +### UI + +- `AuditKnownPaths` row: next to "explorer ↗" on funded rows, add + `Build TX now` (gold). Confirm dialog: from-address, amount (minus est. fee), + destination (default = standard receive, editable), then device confirm. +- Same button on funded `AuditCustomPath` results (`pushCustom` rows). +- Hidden wallets: allowed (it's a spend, not a persistence) — but never write + anything to `cached_pubkeys`. +- After broadcast: show txid + explorer link; push `balance-updated`. + +### Gotchas from tonight's diagnostic session (read before coding) + +- **Device swap mid-session poisons assumptions.** The operator swaps physical + KeepKeys constantly (Testerb2 / Deivce715r2 / main / Zcash2…). Capture + `engine.wallet` at scan time and bail if it changes before sign + (`auditScanUtxoAccounts` has the pattern: `captured !== engine.wallet`). + A "Build TX now" button MUST re-verify the found address derives from the + CURRENTLY connected device before building (derive path → compare address; + it's one device call and prevents signing with the wrong wallet). +- **SLIP-132 version bytes ARE the script type** for blockbook/Pioneer. An + account key queried as `Ltub/xpub` yields p2pkh addresses; as `zpub`, + p2wpkh. That mismatch was tonight's root bug — don't reintroduce it. +- **Honesty rules**: a thrown balance/UTXO lookup is "couldn't verify", never + 0. Broadcast failures must surface verbatim. +- The audit report `?audit=` GET param + robust clipboard copy are in + `AuditDialog.tsx` (`buildHandoff`/`copyHandoff`); a "Build TX now" result + (txid) would be a good addition to that payload (v2 field). + +## Verification (device) + +Live test fixture already on-chain (Testerb2 device, standard wallet): +- `m/44'/2'/0'/0/0` p2wpkh `ltc1q5f772…` ≈ 0.0064 LTC (uncommon branch) +- `m/84'/2'/0'/0/0` p2wpkh `ltc1qqt9x…` ≈ 0.0032 LTC (standard BIP84) +- `m/44'/2'/1'/0/0` p2wpkh `ltc1q8sn04…` ≈ 0.0127 LTC (tracked account 1) + +Acceptance: "Scan uncommon paths" → funded row → Build TX now → device shows +the sweep → broadcast succeeds → funds land on the BIP84 receive address → +LTC balance reflects it after refresh. diff --git a/docs/handoff-balance-server-unavailable-pioneer.md b/docs/handoff-balance-server-unavailable-pioneer.md new file mode 100644 index 00000000..33ee4b5a --- /dev/null +++ b/docs/handoff-balance-server-unavailable-pioneer.md @@ -0,0 +1,81 @@ +# Handoff → pioneer-server agent: investigate "Balance server unavailable" ticket + +**Date raised:** 2026-06-09 +**Ticket symptom window:** ~11:24 AM (user-reported local time — **confirm timezone**, then widen the search to roughly 10:00 AM–12:30 PM around it). +**Server:** `api.keepkey.info` (Cloudflare-fronted: 104.21.13.95 / 172.67.132.200). +**Pioneer monorepo:** `/Users/highlander/WebstormProjects/keepkey-stack/projects/pioneer` + +## What you're investigating + +A Vault user (firmware 7.14.1, default-label device) plugs in, enters PIN, and gets a +**"Balance server unavailable"** banner *instead of* the portfolio. We could not reproduce on a dev +machine. The server is globally healthy right now (swagger `200`), so this is either +network/Cloudflare-specific to that user, a slow/failing upstream for their specific payload, or a +specific pubkey the API rejects. **Server-side logs from the ticket window are the decisive evidence.** + +Important framing: the dev can't reproduce because the dev already has **cached balances**. Vault only +shows the *bare error screen* (suppressing the portfolio) when there is **no cached snapshot** — +i.e. the user's **very first** `GetPortfolioBalances` of the session failed. So you are looking for a +**first-fetch failure**, not a flaky retry. + +## Exactly what the Vault client sends (so you know what to grep) + +All from `projects/keepkey-vault-v11/projects/keepkey-vault/src/bun/`: + +1. **Client init** — `GET /spec/swagger.json` (`pioneer.ts:63-81`), client timeout 60s. +2. **SSE auth registration** — `POST /api/v1/user/register` with + `{ username, queryKey }` (`pioneer.ts:90-97`). Best-effort, non-fatal. +3. **The portfolio call** — `POST GetPortfolioBalances` (`index.ts:1944-1975`): + - Body: `{ pubkeys: [{ caip, pubkey }, ...], extraContracts?: [{...custom tokens}] }` + - **Chunked**: 8 pubkeys per request, up to **4 concurrent** chunks, per-chunk timeout **45s**, + total budget **120s** (single-chain variant uses **60s**). + - On an `extraContracts` schema error the client **retries the same chunk without + `extraContracts`** — so you may see paired requests (with then without custom tokens). + +### Correlation keys — how to isolate THIS user + +Every Vault generates a stable `queryKey` of the form **`vault:`** and registers a +`username` = the first 32 chars of that key (`pioneer.ts:12-24, 87-89`). Both are sent on +`GetPortfolioBalances` (via the client's queryKey auth) and on `/api/v1/user/register`. + +- Grep `user/register` POSTs in the window for `username` starting `vault:` → gives you the + exact `queryKey` for sessions active then. +- Pivot from that `queryKey` to all `GetPortfolioBalances` requests in the window. +- (If you have the user's email/device from the ticket, map it to their queryKey first.) + +## Hypotheses to confirm or refute (in priority order) + +1. **Cloudflare blocked them at the edge** (never reached app). Check Cloudflare/WAF logs for + `403`/`503`/managed-challenge/JS-challenge on `POST GetPortfolioBalances` or `GET /spec/swagger.json` + in the window. Note their country/ASN — region or bot rules can block one user while the service is + globally fine. **If it's Cloudflare, there may be NO app-server log line at all** — absence in app + logs + presence in CF logs is itself the answer. +2. **Timeout under their payload.** Did `GetPortfolioBalances` run long (>45s/chunk, >120s total)? + Look for slow upstream balance providers (which chain/provider was slow?), large pubkey sets, or a + single chunk hanging. A wallet that derives many accounts × EVM chains produces many pubkeys. +3. **A specific pubkey/chain 400s.** Any `400`/validation error tied to a particular `caip` or + malformed `pubkey`? Capture the offending `caip` and pubkey prefix. +4. **`extraContracts` schema rejection loop.** Did the with-custom-tokens call 400 and the retry also + fail? Capture the `body.extraContracts` validation message the server returned. + +## What to report back (so we can close the loop in Vault) + +- The user's `queryKey` and the count/outcome of their `GetPortfolioBalances` calls in the window. +- For each failure: HTTP status, which layer (Cloudflare vs app), latency, the `caip`(s) in the chunk, + and the **exact error body** the server returned (Vault surfaces this verbatim to the user via + `getPioneerPortfolioErrorMessage`, `index.ts:147-157`). +- Whether the failure was edge (CF) or origin (app), and whether it was transient or persistent for + that user. +- Any rate-limit / per-queryKey throttle that could have tripped on the 4 concurrent chunks. + +## Why this matters for the Vault-side fix + +We're about to add a **"copy support handoff" dialog** to Vault that bundles the error + logs so future +occurrences are self-reporting. Knowing the *server-side* failure class (edge block vs timeout vs bad +pubkey vs schema) tells us **which fields to capture in that handoff** and whether Vault should +auto-retry, fall back to the default host, or surface a network-troubleshooting hint. + +## Open question to resolve first + +Confirm the **timezone** of the 11:24 AM ticket timestamp before searching — an offset will put you in +the wrong log window. diff --git a/docs/handoff-cdn-icon-purge.md b/docs/handoff-cdn-icon-purge.md new file mode 100644 index 00000000..29120efd --- /dev/null +++ b/docs/handoff-cdn-icon-purge.md @@ -0,0 +1,118 @@ +# Handoff: OP/ARB Icon CDN Update + +**Context**: PR #181 (Vault dashboard redesign by sktbrd/xvlad) is ready to merge. +Vlad's PR description says he uploaded new brand-pack versions of the Optimism and +Arbitrum chain icons to the DigitalOcean Spaces bucket "out of band". The CDN edge +may have cached the old versions. + +--- + +## Icon System Architecture + +``` +Vault app (AssetIcon component) + → caipToIcon('eip155:10') + → https://api.keepkey.info/coins/{base64(caip)}.png ← Express on Cloudflare + → 302 redirect → + https://keepkey.sfo3.cdn.digitaloceanspaces.com/coins/{base64}.png +``` + +- **Bucket**: `keepkey` in DigitalOcean Spaces, region `sfo3` +- **Key pattern**: `coins/{base64url-no-padding(caip)}.png` +- **CDN endpoint**: `keepkey.sfo3.cdn.digitaloceanspaces.com` + +The two files in question: + +| Chain | CAIP | Spaces object key | +|-----------|---------------|-----------------------------| +| Optimism | `eip155:10` | `coins/ZWlwMTU1OjEw.png` | +| Arbitrum | `eip155:42161`| `coins/ZWlwMTU1OjQyMTYx.png`| + +--- + +## What Needs Doing + +### Step 1 — Verify the new icons are in the bucket + +Using the DO console or `doctl`: + +```bash +doctl storage object list keepkey --region sfo3 | grep ZWlwMTU1Oj +``` + +Or via the DO web console: Spaces → keepkey → coins/ folder → check the two files +above exist and were recently modified (should show Vlad's upload timestamp). + +If they're missing, go to Step 2a. If they exist, skip to Step 2b. + +### Step 2a — Upload if missing + +Get the official brand-pack icons: +- Optimism: https://cryptologos.cc/logos/optimism-ethereum-op-logo.png + (or official https://www.optimism.io/brand-kit — use the circular logo variant) +- Arbitrum: https://cryptologos.cc/logos/arbitrum-arb-logo.png + (or official https://arbitrum.io/logo — use the circular logo variant) + +Upload with public-read ACL: + +```bash +doctl storage object put keepkey \ + --region sfo3 \ + --acl public-read \ + --remote-path coins/ZWlwMTU1OjEw.png \ + optimism-logo.png + +doctl storage object put keepkey \ + --region sfo3 \ + --acl public-read \ + --remote-path coins/ZWlwMTU1OjQyMTYx.png \ + arbitrum-logo.png +``` + +### Step 2b — Purge CDN cache + +In the DO console: **Spaces → keepkey → CDN → Purge Cache** + +Enter these paths (one per line): +``` +coins/ZWlwMTU1OjEw.png +coins/ZWlwMTU1OjQyMTYx.png +``` + +Or via API: +```bash +# Get the CDN endpoint ID first +doctl cdn list + +# Then purge +doctl cdn flush --files "coins/ZWlwMTU1OjEw.png,coins/ZWlwMTU1OjQyMTYx.png" +``` + +### Step 3 — Verify + +```bash +# Should return a PNG (not an AccessDenied XML error) +curl -I -L -A "Mozilla/5.0" \ + "https://api.keepkey.info/coins/ZWlwMTU1OjEw.png" + +curl -I -L -A "Mozilla/5.0" \ + "https://api.keepkey.info/coins/ZWlwMTU1OjQyMTYx.png" +``` + +Look for `content-type: image/png` and `200 OK` at the final redirect destination. + +--- + +## Impact if Skipped + +PR #181 can be merged without this. If the CDN still serves old icons, users see +the previous OP/ARB logos until TTL expires naturally. Not a crash, just stale icons +for two chains. The `AssetIcon` component has a letter-bubble fallback if the image +fails entirely. + +--- + +## Credentials + +You need DO account access with Spaces write permissions for the `keepkey` bucket. +Check with whoever manages the keepkey DigitalOcean account (bithighlander@gmail.com). diff --git a/docs/handoff-clearsign-attestor-and-trust-model.md b/docs/handoff-clearsign-attestor-and-trust-model.md new file mode 100644 index 00000000..1b045468 --- /dev/null +++ b/docs/handoff-clearsign-attestor-and-trust-model.md @@ -0,0 +1,323 @@ +# Clear-sign: what shipped, what was rejected, and what's next (Handoff, 2026-07-29) + +> **Release-direction update (later 2026-07-29; supersedes the packaging and +> presentation recommendations below).** The attestor is now part of the +> regular firmware rather than a separate build variant. This release still +> bakes no production ClearSign key: all attestor operations, runtime signer +> loading, and runtime metadata consumption require Advanced Mode; loaded keys +> remain RAM-only and are cleared when Advanced Mode is disabled. Runtime +> metadata is additive—it no longer suppresses EVM raw calldata or Solana's +> unverified-transaction warning. Vault exposes the workflow through the +> Advanced-only ClearSign Studio and exports a reproducible JSON evidence log. +> See `projects/keepkey-vault/docs/CLEARSIGN-STUDIO.md` for the current release +> boundary and test matrix. + +Relay swaps now clear-sign on device in both directions, verified on real hardware. +This handoff covers what landed, one design that was **rejected after review** (and why +the reasoning matters), and the two pieces of remaining work. + +Repo root assumed: `/Users/highlander/WebstormProjects/keepkey-stack` + +--- + +## 0. TL;DR + +- **Shipped and live.** ETH→SOL and SOL→ETH Relay swaps clear-sign. No per-transaction + signing service anywhere in the path. +- **Rejected.** Persisting clear-sign signers to public flash (firmware PR #322, closed). + The justification was wrong in a way worth internalising — see §3. +- **Next.** (a) a KeepKey that *issues* clear-sign signatures, (b) certificate chains so + onboarding a provider doesn't need a firmware release. (b) needs a spec before code. + +**Update 2026-07-29.** (a) is built: firmware PR **#323** (`feat/clearsign-attestor-v2`, +fork develop) + device-protocol `feat/clearsign-attestor-v2`. Builds clean and 379/379 unit +tests pass in both the flag-on and default-off configurations. **Gate 3 is done** — +emulator OLED captures and the harness that produced them are in +`docs/evidence/clearsign-attestor-gate3/`. It caught two real truncation bugs (the +discriminator rendered off the screen entirely; batched labels scrolled off at max +length), both fixed in #323; confirm screens are now one label each. + +**Phase 2 of the trust model is now built too:** firmware PR **#324** +(`feat/clearsign-builtin-anchor`, stacked on #323) fills the built-in key table that +phase 1 deliberately left empty. A blob or schema signed by a baked anchor verifies +with no `LoadClearsignSigner` and no per-boot prompt, presenting as *Insight Verified* +rather than a host-chosen identity; `LoadClearsignSigner` is refused on a slot that has +an anchor. **No production key is baked** — that is a release step, not a code step. +Emulator proof and the harness are in `docs/evidence/clearsign-builtin-anchor/`. + +**The bootstrap is not circular.** The issuing firmware bakes no key, so it ships first; +the key is read off the issuing device afterwards and baked into the verifying release. +The only ordering rule that bites is the bootloader's: `should_restore()` +(`tools/bootloader/usb_flash.c:135`) restores the storage sector only when the old and +new images have **matching signedness**. Signed→signed preserves the seed, so the +existing signing device keeps its seed when the attestor image is flashed onto it — +which is why `release.yml` now builds `attestor` as a third signed variant beside +`full` and `bitcoin-only` rather than leaving the issuer on a local unsigned build +(a one-way door: it could never move to signed without losing the key). The signing +step must set `SIG_FLAG != 0` and use non-expired keys, or install wipes the device. (b) now has a +written proposal at `docs/spec-clearsign-certificate-chains.md` — still unimplemented and +still needs security-model review, per §5. + +--- + +## 1. What shipped + +| Repo | Change | State | +|---|---|---| +| `modules/keepkey-firmware` | PR **#321** → `develop` = `b649ba8b` | merged | +| `projects/keepkey-vault` | PR **#380** → `develop` = `98d08014` | merged | +| `projects/pioneer` | **v1.3.149** = `807c16c42` | **live on green** | + +### The idea + +A **schema** describes how to *read* one instruction or contract method — program/contract, +discriminator/selector, and the labelled args. It carries **no amounts and no transaction +hash**, so one signature covers every future call to that method and the device decodes the +values out of the bytes it is about to sign. + +Safety comes from **structural completeness**, not transaction binding: +discriminator + declared arg widths must equal the instruction data length *exactly*; every +displayed account index must exist; lookup-table-backed instructions are never eligible; +and every other instruction must be one firmware already decodes. + +### Firmware (#321) + +- `KKSOLSC1` Solana instruction schemas — `solana_parseInstrSchema()` / + `solana_schemaApplies()` in `lib/firmware/solana.c` +- **Payable EVM v2 calls** now clear-sign. Previously *any* native value was refused, which + forced blind-signing on exactly the routes most worth reviewing. Refusing was never the + safety property; **showing the amount is** — `signed_metadata_schema_moves_value()` makes + `ethereum.c` keep the amount screen. +- **`ARG_FORMAT_BYTES`** accepted in v2 args (Relay's order id is an opaque word; without + this the call was inexpressible) +- **`CreateAssociatedTokenAccountIdempotent`** (ATA data `[1]`) accepted. This was a *wide* + bug: one unrecognised instruction forces the whole tx opaque, so **any** SPL transfer to + an address without a token account blind-signed. + +### Vault (#380) + +- `src/bun/evm-schema-registry.ts`, `src/bun/solana-schema-registry.ts` + their + `*-local.json` registries; `swap.ts` attaches a matching schema. **No match = today's + behaviour**, so it can never block a swap. +- `fix(assets)`: Pioneer lowercases the network part of token CAIPs, but Solana/Tron network + ids are base58 and **case-sensitive**. The icon URL is `base64(caip)`, so USDT-on-Solana + 404'd. Fixed where the CAIP enters, not at the URL. + +### Pioneer (v1.3.149) + +- **Inlines address-lookup-table accounts** when the tx still fits (`compileToV0Message([])`, + measured **261 → 320 bytes** against a 1232 limit). A hardware wallet has no network, so + ALT accounts are absent from the bytes it signs — it cannot show where funds go. Falls back + to the ALT form if inlining would overflow. +- Registers the Relay bridge program in `pioneer-discovery`. + +### Real Relay data (captured from `api.relay.link`, 2026-07-27) + +``` +Solana program 99vQwtBwYtrqqD9YSXbdum3KBdxPAVxYTaQ3cfnJSrN2 + 0d9e0ddf5fd51c06 depositNative (native SOL, 5 accounts) + 0b9c60da27a3b413 depositToken (SPL, 10 accounts) + both 48 bytes = 8 disc + u64 LE amount @8 + 32-byte order id + +EVM router 0x4cd00e387622c35bddb9b4c962c136462338bc31 + selector 0x49290c1c, calldata 68 bytes = 4 + address(depositor) + bytes32(orderId) + PAYABLE (0.00798 ETH on the sample quote) +``` + +Router addresses are stable per route but **differ between routes**. + +--- + +## 2. How to test it + +```bash +# Vault against staging (v1.3.149 is on green now, so this is only for pre-release checks) +PIONEER_API_BASE=https://api-blue.keepkey.info make vault +``` + +**The DB setting `pioneer_api_base` WINS over the env var** — check Settings first or you +will silently stay on green. + +**Load the CI signer before testing.** Schemas in the registries are signed with the CI test +key (slot 3), and loaded signers are **RAM-only** — every reboot or flash wipes them: + +```bash +cd /Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-vault-v11/projects/keepkey-sdk +node tests/solana-clearsign/schema-sign.js # loads signer + 4 on-device assertions +node tests/solana-clearsign/offline-schema.js # 14 offline checks, no device +``` + +Success looks like a decoded `Relay Bridge / depositNative` review, and for ETH→SOL a +decoded `bridgeDeposit` review **plus** the native ETH amount screen (two screens — the +amount screen is deliberately kept because a schema cannot bind `msg->value`). + +If it blind-signs, check the vault log for `clear-sign schema attached:` — **absent** means +the lookup missed (Pioneer returned ALTs, or a new router address); **present** means the +device rejected it (usually slot 3 is empty). + +--- + +## 3. REJECTED: persisting signers to public flash (firmware #322, closed) + +Read this before proposing it again. + +**The problem it tried to solve is real.** With no built-in key, clear-signing only works if +a user loads a signer, and loaded signers are RAM-only — so users must reload on **every +boot**. That trains them to approve a trust anchor routinely, and a malicious host then only +has to ask. + +**The proposed fix was worse than the disease.** The argument was: "public storage lacks +integrity, but `AdvancedMode` lives there too, so persisting a trust anchor gives a flash +attacker nothing new." **That is false**, and the code says so: + +- `lib/firmware/ethereum.c:867-879` — **AdvancedMode** leaves `data_needs_confirm` true and + still calls `layoutEthereumData()`. Blind signing is permitted, but the user **still sees + the raw calldata**. +- `lib/firmware/ethereum.c:795-807` — a **matched signed-metadata blob** sets + `data_needs_confirm = false`. The raw-data screen is **replaced** by whatever the metadata + claims. + +So a persisted rogue signer is **strictly more powerful** than flipping AdvancedMode: the +attacker signs v1 display metadata for the real malicious transaction and the user never sees +the bytes. Attack A is loud and honest; attack B is quiet and lies. + +Additional findings from review, all valid: the feature was **unreachable** +(`fsm_msgLoadClearsignSigner` rejects `persist=true` via `CHECK_PARAM` at +`lib/firmware/fsm_msg_ethereum.h:119`); restored records skipped +`signed_metadata_signer_valid()`, icon validation and slot-consistency; the consent screen +says "for this session"; and `storage_clearClearsignIdentity()` had no production caller. + +**V18 storage records stay scrubbed.** Do not re-enable without authenticated storage. + +### Testing lesson that let it through + +Three of those tests **crashed** inside `storage_commit()` (no `storage_location`) and exited +1 — but they were reported as passing because the check counted `[ OK ]` lines instead of the +**exit code**. A crash read as a pass. Always assert the exit status. + +--- + +## 4. NEXT: KeepKey as the signature issuer + +**Why this replaces persistence.** A KeepKey-as-issuer needs no persistence at all: the +*verifying* devices get the production key baked into firmware (signature-protected), and the +*issuing* KeepKey holds the private key in its seed, where it already belongs. No per-boot +prompts, no trust anchors in writable flash. + +### Does firmware support it today? No — one message short. + +`SignIdentity` is close: it derives a deterministic secp256k1 key and returns the 33-byte +compressed pubkey (this is how the CI test key was derived). But its **signature will never +verify**: + +- `SignIdentity` → `cryptoMessageSign` → `cryptoMessageHash`, which prepends the **Bitcoin + message header** + varint length and double-hashes; returns **65 bytes** +- the verifier (`signed_metadata_verify_attestation`) does plain `sha256_Raw(payload)` + + `ecdsa_verify_digest`, **64-byte compact** + +Different digest construction, different length. + +### What to build + +A message that signs `SHA256(payload)` directly with secp256k1, 64-byte compact. + +**It must not be a raw signing oracle.** The device should **parse and validate the payload +before signing**, using the same parsers a verifying device runs +(`solana_parseInstrSchema`, the metadata parsers). A fully compromised host could then only +obtain signatures over well-formed descriptors — never arbitrary bytes. This is the single +most important property of the design. + +**Prior art in this session** (rebase, don't restart): + +- Firmware handler: `lib/firmware/fsm_msg_clearsign_attestor.h` — copied onto worktree + branch `feat/clearsign-attestor-v2` (off `b649ba8b`). Derives the key at a dedicated + hardened path `m/0x4B4B'/0x4353'/0'` ("KK"/"CS"), validates KKSOLSC1 before signing, + requires an on-device confirm. +- device-protocol messages **1700-1703** on branch `feat/clearsign-attestor` (`328c6bc`) in + `https://github.com/BitHighlander/device-protocol`. + +**Gate it behind a CMake flag** (`KK_CLEARSIGN_ATTESTOR`, OFF for device builds) — the 7.15 +line is 4-10KB from the ROM wall. Wire IDs stay reserved either way, so promoting the +physical-device tier later is a flag flip. + +### Checklist for the new message + +Follow `firmware-new-message-checklist`: `.options` caps in **BOTH** device-protocol and +`include/keepkey/transport/messages-*.options`, forward-declare the handler in +`include/keepkey/firmware/fsm.h`, add `messagemap.def` rows, and verify +`grep -c pb_callback_t` on the generated header is **0**. + +--- + +## 5. NEXT: certificate chains (needs a spec first) + +**The problem.** Baking provider keys into firmware works — `METADATA_MAX_KEYS = 4`, direct +slot lookup — but it means **a firmware release per provider**, with a ceiling of four. That +is not a workable B2B onboarding path. + +**There is no chain verification anywhere in the metadata code today.** Grep for +`certificate|delegat|issuer` in `lib/firmware/signed_metadata.c` returns nothing. + +**The shape.** One KeepKey **root key** baked into firmware signs a small certificate per +provider ("key X belongs to Acme Trust"); the device verifies root → provider → metadata. +Onboarding becomes *issuing a certificate*, not shipping firmware. This composes with §4: the +issuing KeepKey holds the root. + +**Design questions to settle before code:** + +1. Certificate format and size (ROM budget) +2. **Key-usage scoping** — today *any* trusted key can attest *any* metadata type. A provider + certificate authorised for Solana schemas must not attest EVM ones. This was flagged in + independent research of Ledger's implementation and is cheap now, painful later. +3. Revocation — no mechanism exists +4. Whether the root can be rotated, and how + +**Do not implement this without a written spec reviewed by whoever owns the firmware security +model.** The persistence PR (§3) is a direct demonstration of the cost of skipping that step +on a trust-model change. + +--- + +## 6. Smaller open items + +**SDK signing timeouts — systemic.** Nearly every signing endpoint in +`projects/keepkey-sdk/src/index.ts` calls `client.post(path, params)` with **no timeout**, so +the 30s default aborts while the user reads the device screen and surfaces as a device +failure. Only `solanaSignTransaction` and `loadClearsignSigner` pass `signingTimeoutMs`. +Still affected: `/eth/sign-transaction`, `/eth/sign`, `/eth/sign-typed-data`, nine +`/cosmos/sign-amino-*`, `/hive/sign-operations`, and others. + +**Dead KKSOLSW1 code.** The abandoned per-transaction descriptor format left fixtures behind: +`projects/keepkey-sdk/tests/fixtures/solana-clearsign.js` and +`tests/solana-clearsign/descriptor-sign.js`. That device test cannot pass (the published +`@keepkey/device-protocol` has no `setSwapMetadataPayload`). Delete when convenient. + +**Uncommitted work on the vault tree** (from the same session, unrelated to clear-signing): +Zcash privacy UI, address book picker, `src/bun/solana-outflow.*`, activity panel, +`src/bun/solana-programs-local.json`. These want their own branches. + +**CircleCI is red on pioneer `master`** — pre-existing and unrelated, but a permanently-red +check means the next genuine failure will not stand out. + +--- + +## 7. Gotchas worth keeping + +- **Loaded signers are RAM-only.** Reload slot 3 after every reboot/flash. This looks exactly + like a regression when you forget. +- **Schemas in the registries use the CI TEST key.** Production needs the slot-0 key — a + custody decision, not code. Until then this is inert for real users. +- **`pioneer_api_base` DB setting beats the env var.** +- **An untracked file that a committed file imports** passes every local check and only fails + in a built image. This broke every quote on blue (`Cannot find module './solana-clearsign'`) + and was caught only by the post-deploy smoke test. +- **`Storage.StorageRoundTrip` fails on macOS** — Linux-padding golden. Trust CI; never + regenerate locally. +- **`Authenticator.WipeCancellationFailsClosed` hangs** the full unit suite locally. + Pre-existing; filter around it. +- **clang-format version matters.** CI pins **20.1.8**; a newer local build disagrees even on + untouched files. `pip install clang-format==20.1.8` in a venv. +- **`gh pr edit` fails** on this repo with a Projects-classic GraphQL error — use + `gh api -X PATCH repos/.../pulls/N`. +- **`git submodule update --init --recursive` silently resets** submodule branches you have + checked out. It wiped a device-protocol branch mid-session. diff --git a/docs/handoff-clearsign-identity-icons.md b/docs/handoff-clearsign-identity-icons.md new file mode 100644 index 00000000..f1800a9e --- /dev/null +++ b/docs/handoff-clearsign-identity-icons.md @@ -0,0 +1,194 @@ +# Handoff — Clearsign Identity Icons (persistent) + Protocol Icons (transient) + +**Status:** design + feasibility verified against `BitHighlander/keepkey-firmware@origin/develop` +(== rc4, 7.15.0). Not yet implemented. This is the spec to build against. Post-7.15.0-release, +opt-in / feature-gated. Pioneer catalog signing is a *separate*, later track (see +`handoff-pioneer-server-clearsign-metadata.md`). + +**Author intent (verbatim):** the device is *now a "KeepKey + identity" device*. A clearsign +**identity** (the entity whose key vouches for clearsign metadata) must be a first-class, +**persistent** trust anchor: users need the assurance that their identity provider is still the +one they approved/trusted. That assurance is delivered by showing the loaded identities **on boot** +(click-through) and by leading **every** clearsign with the identity's logo + name instead of a +scary "NOT verified by KeepKey" warning. + +--- + +## 1. Two distinct concepts — do not conflate + +| | **Identity** (clearsign signer / provider) | **Protocol** (relay, Aave, …) | +|---|---|---| +| What | The secp256k1 signer whose attestation vouches for clearsign metadata | The dApp/contract being clear-signed in a given tx | +| Trust | The trust anchor — user approved it once, must stay stable | Display aid, under the identity's attestation umbrella | +| Persistence | **PERSISTENT** — survives reboot, stored in flash | **TRANSIENT** — supplied by the Vault per-clearsign | +| Icon source | Loaded once via `LoadClearsignSigner` (+ icon), kept in flash | Sent by the Vault app at clearsign time, not persisted | +| Shown | On boot (click-through) + as the header of every clearsign it vouches for | Beside the method/protocol during the clearsign pages | +| Count | up to `METADATA_MAX_KEYS = 4` slots | 1 per tx | + +The identity is the security-relevant piece. The protocol icon is cosmetic — its trust derives from +the identity that attested the metadata, which is why the identity is shown **first**. + +--- + +## 2. Requirements + +1. A clearsign **identity** has a loaded **icon** (logo), loaded together with its pubkey+alias. +2. Loaded identities display **on boot**, with a **click-through** (carousel) so the user can review + each trusted provider before use — the "is my provider still the one I trusted?" check. +3. **Remove** the "Signer '…' NOT verified by KeepKey" warning. **Replace** it with the identity's + **logo + name** shown at the **start of every clearsign** it vouches for. +4. **Multiple identities:** handle N loaded identities — cascade/click-through the boot review; each + individual clearsign is vouched by exactly ONE identity (the `key_id` that signed the blob), so + the clearsign header shows THAT identity. +5. **Protocol icons** (e.g. relay) are loaded from the **Vault app** per-clearsign and rendered + alongside the method — transient, never persisted. +6. Verify **storage space** + **icon format** fit the device (done below). + +--- + +## 3. Feasibility — verified numbers (origin/develop) + +**Display.** OLED is `256 × 64`, 1bpp mono (`KEEPKEY_DISPLAY_{WIDTH,HEIGHT}`). The layout engine +**already** supports a left icon column: `layout.h` has `TITLE_WIDTH_WITH_ICON`, +`BODY_WIDTH_WITH_ICON`, `LEFT_MARGIN_WITH_ICON`, and `review_with_icon()` is already used for the +built-in "Verified" trust indicator. Image format is 1bpp mono, RLE-compressed +(`draw_bitmap_mono_rle(Canvas*, AnimationFrame*, …)`, `draw.h`), frames carry `uint16 width/height`. + +**Icon size budget.** Recommend a fixed **48 × 48** identity glyph: +- 48×48 mono **raw** = 288 B; 64×64 = 512 B; 32×32 = 128 B. +- Stored RLE-compressed with a hard cap (reject larger at load). Cap suggestion: **384 B** per icon + (comfortably fits a 48×48, most logos compress well below raw). + +**Flash storage.** Config lives in a **16 KiB** sector (`FLASH_STORAGE_LEN = 0x4000`), wear-leveled +across sectors 1–3. `storage.c` has a **compile-time guard**: +`_Static_assert(sizeof(ConfigFlash) <= FLASH_STORAGE_LEN, …)` — so any addition that overflows 16 KiB +**fails the build**, never bricks. Current large consumers: `V17_ENCSEC_SIZE = 1024` (encrypted +secret), `mnemonic[241]`, `authBlock[512]`, policies, cache — total is well under 16 KiB with several +KiB headroom (exact figure prints at build; confirm with the assert). + +**Persistent-identity cost** (worst case, 4 slots, 384 B icon cap): +`4 × (33 pubkey + 32 alias + 384 icon + ~4 len/flags) ≈ 4 × 453 ≈ 1.8 KiB` added to `ConfigFlash`. +Very likely fits; the `_Static_assert` is the authoritative gate. If tight, drop persistent slots to +2–3 (RAM slots can stay 4) or cap icons at 32×32. + +**Conclusion: feasible.** Icons = 1bpp mono RLE, ≤384 B; persistent identity store ≈1.8 KiB fits the +16 KiB sector; the build-time assert guarantees we never overflow. + +--- + +## 4. Current state (what exists today, origin/develop) + +- Signers are **RAM-only**: `loaded_pubkeys[METADATA_MAX_KEYS][33]`, + `loaded_aliases[METADATA_MAX_KEYS][31+1]` in `lib/firmware/signed_metadata.c` — **cleared on reboot + and on WipeDevice**. No icon, no persistence. +- The warning to replace: `signed_metadata_confirm()` (`signed_metadata.c:565`) prints + `"Signer '%s' (%s) … NOT verified by KeepKey."` then a plain "Call: " screen. The built-in + (phase-2) path uses `review_with_icon()` with a trust indicator. +- `LoadClearsignSigner` = msg **117**. NOTE: device-protocol submodule does **not** contain msg 117 + as a real `.proto` — hdwallet uses a **hand-written jspb** class. Adding an icon field means adding + 117 (with the icon field) to the **device-protocol fork** properly, then regenerating. +- Boot/home screen: `lib/firmware/home_sm.c` (`layoutHome`, `layoutHomeForced`, screensaver states). + +--- + +## 5. Changes required (staged) + +### Stage 0 — device-protocol (fork only; NEVER upstream keepkey) +- Add `LoadClearsignSigner` (msg 117) to the fork `.proto` with fields: + `key_id=1 (uint32)`, `pubkey=2 (bytes,33)`, `alias=3 (string,≤31)`, **`icon=4 (bytes, ≤384, mono + RLE 48×48)`**, optional `icon_w=5`, `icon_h=6`. Regenerate; replace hdwallet's hand-written jspb. + +### Stage 1 — firmware storage (the migration; highest care) +- Move identities RAM → flash: add a `ClearsignIdentity identities[N]` array to `Storage.Public` + (`storagepb.h`): `{ bool present; uint8_t pubkey[33]; char alias[32]; uint16_t icon_len; + uint8_t icon[384]; }`. Pick `N` (start with 2–3 persistent; keep 4 RAM slots for ephemeral). +- **Bump `STORAGE_VERSION`** and add a migration in `storage_versions.inc` / `storage.c` that + zero-initializes the new field for existing devices (no data loss). Verify the `_Static_assert` + still passes (this is the space gate). +- Load path: `signed_metadata_store_signer()` writes to flash (persistent slot) vs RAM (ephemeral) — + decide policy (e.g. a `persist` flag on msg 117, or all loads persist). WipeDevice clears them. + +### Stage 2 — firmware rendering + messaging +- Store/validate icon at load (`signed_metadata_signer_valid` + a new icon validator: length ≤ cap, + decodes as valid mono RLE at the fixed dims). Load-confirm screen shows the **icon + alias + + fingerprint** ("Trust identity ''?"). +- **Replace** the `signed_metadata_confirm()` warning branch: instead of "NOT verified by KeepKey", + lead with `review_with_icon(identity.icon, "Identity: ")` as the FIRST clearsign page, then + the method/protocol pages (protocol icon beside the method — see Stage 3). Keep the fingerprint + reachable (e.g. on the identity page) so a swapped provider is detectable. + +### Stage 3 — boot click-through (home_sm) +- On boot / at home, if ≥1 persistent identity is present, add a reviewable panel: "Trusted clearsign + identities (N)" → click-through each `icon + alias + fingerprint`. This is the trust-anchor check. + Gate behind the feature flag. Screensaver/idle interactions per existing `home_sm` states. + +### Stage 4 — Vault (protocol icons + identity icon upload) +- **Identity icon:** when the user loads an identity, the Vault sends the icon bytes in msg 117 + (`POST /eth/clearsign/load-signer` grows an `icon` field → hdwallet `ethLoadClearsignSigner`). +- **Protocol icons:** per clearsign, the Vault supplies the protocol glyph (relay/Aave/…) for the + method screen — transient. Wire via a companion field on the sign request (display-only; NOT + persisted, NOT part of the signed blob). Source glyphs from the existing coin/asset icon pipeline. + ⚠ A protocol icon is unsigned display — safe only because the **identity** (shown first) attests the + metadata; do not present a protocol icon as a trust signal on its own. + +--- + +## 5b. LOCKED DECISIONS + progress (2026-07-07) + +**Decisions (author):** **2 persistent** identity slots (RAM ephemeral stays 4); icons **48×48** +1bpp mono, RLE-stored, **384 B cap**. Persistent-store cost ≈ **2 × 454 ≈ 0.9 KiB** — comfortable in +the 16 KiB sector. + +**Stage 0 — DONE.** device-protocol fork branch `feat/clearsign-signer-icon` (commit `33521a8`): +`LoadClearsignSigner` (msg 117) gained `icon` (bytes, `max_size:384`), `icon_width`, `icon_height`, +`persist`. Firmware re-pins to this in Stage 1. + +**Stage 1 — storage migration recipe (reverse-engineered from `storage.c`, ready to implement):** +1. `include/keepkey/firmware/storage.h` — add to `struct Public`: + ```c + typedef struct { + bool present; + uint8_t pubkey[33]; + char alias[32]; + uint8_t icon_w, icon_h; // <= 64 (48 today) + uint16_t icon_len; // RLE bytes, <= 384 + uint8_t icon[384]; // 1bpp mono RLE + } ClearsignIdentity; /* ~454 B */ + #define PERSISTENT_IDENTITY_COUNT 2 + // in struct Public: ClearsignIdentity clearsign_identities[PERSISTENT_IDENTITY_COUNT]; + ``` +2. Bump `STORAGE_VERSION` 17 → **18** (storage.h) + `storage_versions.inc`: change `LAST(17)` to + `ENTRY(17)` + `LAST(18)`. +3. `storage_fromFlash()` (storage.c ~1184): the migration is **free** for existing devices — + `memzero(dst, sizeof(*dst))` at the top already zero-inits the new array (⇒ present=false ⇒ no + identities ⇒ no data loss). Add `case StorageVersion_17:` fallthrough into the current reader so a + v17 device loads + is stamped v18 (`SUS_Updated`). Add a `case StorageVersion_18` that reads the + new persistent field via a new `storage_readV18` (extends `storage_readV17`, then reads the + identities block); add matching `storage_writeV18` to persist it. Mind the Public serialization is + offset-based (`storage_readV17`/write pair) — append the identities block at the end, never reorder + existing fields. +4. The existing `_Static_assert(sizeof(ConfigFlash) <= FLASH_STORAGE_LEN)` (storage.c:90) is the + space gate — if 0.9 KiB overflowed, build fails (it won't). +5. **TEST (mandatory before merge):** storage upgrade matrix — seed a device on v17, flash v18, assert + keys/label/policies intact + identities empty; then load an identity, reboot, assert it persists; + WipeDevice clears it. Unit-test the reader/writer round-trip in `unittests/firmware/storage*`. + +**Stages 2–4** unchanged (handler/render/messaging, boot click-through, vault) — see §5. + +## 6. Open questions (need author decision) +1. Persistent identity slot count `N` (2? 3? 4?) — trades flash for how many providers persist. +2. Icon dims: 48×48 (recommended) vs 32×32 (cheaper) vs 64×64 (crisper, 512 B). +3. Do ALL loaded identities persist, or is persistence opt-in per load (a `persist` flag on msg 117)? +4. Protocol-icon trust: leave display-only, or eventually fold the protocol icon into the v2 schema + so the identity attests it too? (Cleaner but bloats the signed blob.) +5. Feature flag name + default (off for 7.15.0; enable post-release). + +## 7. Risks +- **Storage migration is brick-adjacent.** A bad `STORAGE_VERSION` migration can lose keys on upgrade. + Stage 1 needs the full storage upgrade-path test matrix (old→new on a seeded device) before merge. +- device-protocol msg 117 must be added **fork-only** (never PR to keepkey/device-protocol). +- The `_Static_assert` is the space gate — if it fails, shrink icon cap or slot count, don't raise the + sector size. + +Related: [[clearsign-v2-static-schema]], [[clearsign-phase1-and-pdf]], [[keepkey-sdk-clearsign-coverage]], +`docs/relay-v2-schema-payload/` (the on-device v2 demo this builds on). diff --git a/docs/handoff-clearsign-live-signer-build.md b/docs/handoff-clearsign-live-signer-build.md new file mode 100644 index 00000000..766d2b4f --- /dev/null +++ b/docs/handoff-clearsign-live-signer-build.md @@ -0,0 +1,163 @@ +# EVM Clear-Signing — Live Per-Tx Signer: Build / Sign / Do (Handoff, 2026-07-02) + +**Picks up from** `handoff-pioneer-server-clearsign-metadata.md` and the rc3 firmware/SDK handoffs. +This is the **actionable build plan**: what to build, in what order, what NOT to merge, how to +sign, how to test on-device, and the decisions that gate the live path. + +> One-line: prod **blind-signs every EVM contract tx today**. rc3 fail-closes on per-tx `tx_hash` +> binding, so the 2933 static zero-hash blobs are all refused. The fix is a **live per-tx signer** +> on an **operator-controlled isolated box** (not the public prod pod, not the user's machine). + +--- + +## 0. TL;DR / current runtime truth (verified on `develop`) + +- **Prod blind-signs EVM contract txs.** Vault `calldata-decoder.ts:fetchPioneerSignedBlob` (~:453) + requests a blob with only `{chainId, contractAddress, data}` (3s race). On `develop` the + `/api/v1/descriptors/sign` route is **unregistered (404)**; the only built artifact + (`dist/controllers/descriptors.controller.js`) has `/sign` as a **hardcoded `{success:false}` stub**. + 404 / `success:false` / timeout → `fetchPioneerSignedBlob` returns `null` → `rest-api.ts` ethSignTx + logs `no metadata blob — device will show raw hex` → **blind-sign**. Silent. +- **The 3-field request can never produce an rc3-valid blob anyway** — it omits `nonce/gas/value`, + so no sighash is computable. +- **Serializer is behind rc3.** `pioneer-insight/lib/serializer.js` (no `src/` on disk) has only + ARG formats 0–3 and a hard **32-byte** value cap. rc3 needs **STRING(4)** + **TOKEN_AMOUNT(5)** + and a **44-byte** cap — so real token amounts/strings can't even be encoded. +- **Key handling is correct and must stay so.** key_id=0, mnemonic in `pioneer-insight/.env` + `INSIGHT_MNEMONIC` (0600), used only by the offline `sign-all.mjs` batch job, **never** loaded in + the server. (Note: prior `.keys` were reportedly lost; a new keypair is being minted regardless.) + +## 1. The trust model that shipped (rc3 / PR #281) — the stale 2026-03-17 doc is obsolete + +| | OLD doc (2026-03-17) | **rc3 (shipped, emulator-verified)** | +|---|---|---| +| Trust root | firmware ships embedded prod key | `METADATA_PUBKEYS` **all-zero**; host loads key at runtime via `LoadClearsignSigner` (msg 117), RAM-only | +| Bad/mismatched metadata | non-fatal, warns | **fail-closed**: contract data w/o VERIFIED metadata is hard-rejected ("Blocked") | +| Arg formats | 0–3, cap 256 | adds STRING(4)+TOKEN_AMOUNT(5), cap **44** | +| Signing | static pre-signed catalog | **per-tx live signing** (real sighash+values); zero-hash blobs refused | +| UI | "Insight Verified" | **"CLEARSIGN WARNING — Signer '' … NOT verified by KeepKey"** every page | + +## 2. The irreducible tension (state this to anyone who proposes a catalog) + +rc3 requires `tx_hash == keccak(exact unsigned tx)` — nonce/gas/value/data. That sighash **only +exists at request time**. Therefore: **you cannot have BOTH a fully air-gapped key AND per-tx +clear-signing for arbitrary user txs.** The signing key MUST be reachable per request. A static +offline catalog (sign once, upload JSON) is exactly today's broken zero-hash state — **do not +pursue it.** + +## 3. "Sign locally, only upload the payloads" — the correct reading + +Three referents for "local"; only one is both safe and functional: + +1. **User's vault machine — NO.** The user must never hold KeepKey's key_id=0 attestation key; a + leak lets anyone forge VERIFIED metadata that makes a drain read as a benign transfer. +2. **Shared internet-facing prod pod — NO** (this is what the directive is rejecting): a + brand-forgery key in a 3-replica multi-tenant pod's env/memory is the wrong blast radius. +3. **Operator-controlled isolated signer (HSM/KMS ideal) — YES.** "sign locally" = the key stays on + KeepKey-operated hardware and never enters the public cluster or the vault; "upload only the + payloads" = the server/CDN and vault only ever see finished signed blobs, never the key. + +**Why an openly-reachable signer is still safe:** it only attests facts it **independently derives +from public calldata** (tx_hash binding stops *replay*, not *mis-signing*); it authorizes nothing +and only ever makes true statements about public calldata. Unrecognized calldata → classify +**OPAQUE / decline**, never blind-stamp VERIFIED. + +## 4. Target architecture + +Vault sends the **full unsigned tx** → **isolated signer** (holds key_id=0, ideally YubiHSM2 / cloud +KMS doing non-extractable raw secp256k1+SHA256 digest signing) **independently** decodes calldata, +classifies against the descriptor catalog, serializes canonical binary metadata with real `tx_hash` ++ typed args (ADDRESS/STRING/TOKEN_AMOUNT; RAW banned), signs, returns **only the blob**. +`pioneer-server` exposes a **thin proxy** route to the signer; the production mnemonic is **never** in +the public prod pod env. Round-trip (decode+hash+sign+return) must fit the vault's **~3s** budget or +it silently blind-signs — **make that fallback explicit/visible.** + +- **Approach A (recommended):** signer computes the sighash from the full tx it receives — one source + of truth for all clients. +- **Approach B:** vault sends `tx_hash`+args, signer stamps — smaller, but lets the caller dictate the + attested "what" (forgery vector). Avoid. + +## 5. Branch strategy — do NOT merge the kitchen-sink + +- **Do NOT merge `feat/clearsign-local`** (316 files, +209k/-67k). It re-touches node-failover/swap + files already on `develop` and **drags in a regression**: `insight.controller.ts` reverts + `eth/bsc/polygon.drpc.org` back to the dead `*.llamarpc.com` URLs killed in **#141/#142**. +- The `feature/evm-clear-signing`, `feature/pioneer-insight-clear-signing`, + `origin/feat/discovery-descriptors-signed` branches are **stale** — they carry discovery JSONs + already on develop and would REVERT them; ignore. +- **Cut a fresh branch off `develop`** (e.g. `feat/clearsign-live-signer`) and bring over ONLY the + clear-sign files via read-only `git show feat/clearsign-local: > ` — never a + merge/checkout. **EXCLUDE** the `insight.controller.ts` llamarpc reversion, all `dist/` artifacts, + and the discovery JSON deletions. + +## 6. The focused PR contents (~4200 LOC), ordered for independent review + +1. **pioneer-insight `src/` restore + serializer fix** (self-contained, **zero device**): + restore `src/serializer.ts`, `src/signer.ts`, `src/index.ts`, `src/keys/keygen.ts`, + `src/cli/keygen.ts`, `package.json`, `tsconfig`, `__tests__` from `feat/clearsign-local` — but + **fix the serializer**: add `ARG_FORMAT_STRING=4`, `ARG_FORMAT_TOKEN_AMOUNT=5`, raise the value + cap **32→44** (legacy formats keep 32), export the new formats from `index.ts`. The branch's + `src/serializer.ts` has the **same gaps** as the compiled `lib/` — correct it, don't copy as-is. +2. **Offline parity gate** (zero device, do FIRST): reproduce python-keepkey + `REFERENCE_BLOB_SNAPSHOTS` (sha256_hex + byte_len per blob; slot-3 test key, alias "CI Test", + test-seed idx 0, `REFERENCE_TIMESTAMP=1700000000`) from the JS signer, and byte-match `tx_hash` + against python's `--flows` dump. Catches serializer drift cheaply. +3. **pioneer-server read-only controllers** (low-risk, no key): `clearsign.controller.ts` (ClearSign + Explorer catalog views) + the **`/descriptors/decode`** half of `descriptors.controller.ts`. + tsoa `@Route` auto-wires. EXCLUDE the `/sign` stub for now. +4. **[gated on §8 answers] the real per-tx `/descriptors/sign`**: replace the stub — accept the full + unsigned tx, decode→attest, compute the correct sighash (legacy vs EIP-1559; **chainId 0→1 + normalization** as the vault does at `rest-api.ts:2039`), call the isolated signer/HSM, return the + blob. Widen `SignRequest` to `{chainId, to, data, nonce, gasLimit, value, gasPrice | (maxFeePerGas + + maxPriorityFeePerGas)}`. **Feature-flag it so it ships dark until device-verified.** +5. **Vault (separate repo/PR):** `calldata-decoder.ts` sends the full unsigned tx; widen the 3s race + or make the blind-sign fallback explicit/visible. +6. **[follow-up] SDK/Vault `LoadClearsignSigner` train:** regenerate hdwallet JS proto bindings + (msg 117), `hdwallet.loadClearsignSigner`, vault `POST /eth/clearsign/load-signer`, + `sdk.eth.loadClearsignSigner`, typed `txMetadata` on `/eth/sign-transaction`. + +## 7. BUILD → SIGN → TEST → DEPLOY workflow + +- **BUILD:** fresh branch off develop; land §6.1–6.3 first (no key, no device). `make` builds the + workspace; pioneer-insight/lib is turbo-built from the restored src. +- **SIGN:** the key lives on the isolated signer only. For the offline parity gate use the **test** + key (slot 3). The production key_id=0 signer is stood up per §8's answer. +- **TEST (device), in order of readiness:** + 1. **python-keepkey path (ready now, fastest):** flash rc3 DEBUG_LINK build, run + `test_msg_ethereum_clear_signing.py`. **WARNING: wipes the device, loads the public test seed — + never a device with real funds.** Eyeball the OLED for **no hex anywhere** across flows; confirm + signer == device address. + 2. **SDK/Vault path (after §6.6):** rebuild the vault on :1646 (`make vault` — restarts the live + vault, ~minutes) to pick up the new hdwallet method + route, then drive a real EVM approval and + **physically confirm** the trust-warning screen + decoded clear-sign pages on the KeepKey. +- **DEPLOY:** ship §6.1–6.3 anytime (dark/no-op for signing). Flip the `/sign` feature flag only + after on-device verification; wire the vault to send the full tx in the same coordinated release. + +## 8. Open decisions that GATE the live `/sign` path (need answers before §6.4) + +1. **WHERE does the key_id=0 signer physically run?** HSM/KMS (recommended, non-extractable) vs a + self-hosted isolated VM vs (reject) the shared prod pod. This one answer sets the attestation + key's blast radius. +2. **Confirm intent:** "sign locally, only upload payloads" = key OFF the internet-facing cluster and + out of the vault, only signed blobs crossing back — **not** a static offline catalog (broken). +3. **Approach A vs B** (§4). Recommend A. +4. **Signer auth:** open to any vault (OK only if it independently decodes-and-attests and authorizes + nothing) vs auth'd/allowlisted. +5. **Key custody / backup / rotation / incident:** the pubkey is loaded at runtime (rc3) — but a + leaked signing key still lets anyone forge VERIFIED metadata until the signer key is rotated and + old signers de-authorized. What's the backup/rotation/incident plan before it goes live? +6. **Availability/degradation:** signer down → vault currently **silently** blind-signs. Acceptable, + or surface an explicit "unverified — verification service unavailable" state? +7. **Scope:** build the focused PR (§6.1–6.5) and abandon the kitchen-sink merge? Is the + `LoadClearsignSigner` SDK/Vault train (§6.6) same-landing or follow-up? +8. **Device-test:** start with the python path (wipes device)? Confirm an rc3 DEBUG_LINK build and a + throwaway device are available. + +## 9. What can start NOW (no key, no device, no gating decision) + +- §6.1 serializer fix + pioneer-insight `src/` restore +- §6.2 offline parity gate (test key) +- §6.3 read-only ClearSign Explorer + `/descriptors/decode` controllers + +These are safe, independently reviewable, improve host-side decode UX, and touch **no key material +and no device**. The live `/sign` handler (§6.4) waits on §8. diff --git a/docs/handoff-clearsign-pdf-and-device-testing.md b/docs/handoff-clearsign-pdf-and-device-testing.md new file mode 100644 index 00000000..ea508503 --- /dev/null +++ b/docs/handoff-clearsign-pdf-and-device-testing.md @@ -0,0 +1,164 @@ +# Clear-Signing Phase 1 — Final PDF Report + Real-Device Testing (Handoff, 2026-07-02) + +**Where this picks up:** supersedes the V/clearsign portions of +`handoff-firmware-715-pdf-coverage.md` (whose Hive/zcash/bip85/frame-picker items are now +DONE). Firmware PR **#281** (`BitHighlander/keepkey-firmware`, `feat/clearsign-signer-warning` +→ develop) is the merge candidate. Everything below is emulator-verified; the next gate is +**real-device testing**, then merge → rc3. + +--- + +## What ships in PR #281 (one PR, stacked on the 7.15.0 version bump #280) + +**Phase-1 trust model — no hardcoded "KeepKey says this is safe":** +- `METADATA_PUBKEYS` all-zero (including the old DEBUG_LINK CI slot). +- `LoadClearsignSigner` (device-protocol msg **117**): host loads a 33-byte compressed + secp256k1 pubkey + alias into a key slot. Mandatory on-device confirm (alias + sha256[:4] + fingerprint). RAM-only; dropped on reboot AND WipeDevice. Alias is a strict + `[A-Za-z0-9 _-]` allowlist (adversarial review found a quoted-region breakout: + alias `x' verified by KeepKey. Safe (` — fixed). +- Every tx verified by a loaded signer shows **CLEARSIGN WARNING — Signer '' () + you loaded describes this tx. NOT verified by KeepKey.** BEFORE any clearsign page. + The "Insight Verified" icon presentation is reserved for the future built-in key, which a + loaded signer can never shadow. +- With AdvancedMode OFF, contract data without VERIFIED metadata is hard-rejected + ("Blocked") — so the raw-hex "Confirm Ethereum Data" screen is **structurally + unreachable**: if a contract tx signs at all, it clearsigned. tx-hash binding + (`signed_metadata_enforce`) is fail-closed at send_signature. + +**Human-readable WHAT (the who/what/why):** +- New attested arg formats: `ARG_FORMAT_STRING` (4) — printable label, e.g. + `protocol: Aave V3`; `ARG_FORMAT_TOKEN_AMOUNT` (5) — decimals+symbol+amount, device + renders `10.5 DAI` / `UNLIMITED USDC` (all-0xFF 32-byte amount). Both fail-closed + validated at parse (`arg_value_ok`). Max arg value 32→44 bytes (legacy formats keep 32). +- Screens per flow: warning → `Call: ` → `Contract: 0x…` (full, never truncated) + → each decoded arg (ADDRESS full / TOKEN_AMOUNT scaled / STRING label) → tx/gas confirm + (nonzero ETH value IS shown: "Send 0.01 ETH from your wallet…"). + +**The 51-flow reference catalog** (`keepkeylib/clearsign_catalog.py`, python-keepkey +branch `feat/clearsign-load-signer`): +- 51 real mainnet tx types across: DEX swaps (Uniswap V2/V3/**V4 Universal Router**, Curve + 3pool), lending (Aave V3 borrow/repay/withdraw/supply, Compound V3, Spark), liquid + staking/restaking (Lido, Rocket Pool, ether.fi, EigenLayer ×2), approvals/permits + (approve/unlimited/increase/decrease, **EIP-2612 permit**, **Permit2 approve + + permitTransferFrom**, **DAI's non-standard permit**, USDT, ERC-721/1155 + setApprovalForAll), NFTs (721/1155 single + batch transfer), governance/ENS, bridges + (Hop, Wormhole, **Across depositV3 = ERC-7683-style intent**), ERC-4626 vaults + (MetaMorpho, Yearn V2/V3), WETH wrap/unwrap, transferFrom, and the newest tx shapes: + **ERC-4337 EntryPoint v0.7 handleOps**, **EIP-7702 set-code authorization**, + **Safe execTransaction**. +- Calldata is never hand-typed: `keepkeylib/clearsign_abi.py` derives selectors via + keccak256(signature) and ABI-encodes static types; the 8 genuinely-dynamic layouts + (nested dynamic tuples) were each verified by offline round-trip decode. Every contract + address web-sourced; 5 research transcription errors (off-by-one hex chars, incl. a wrong + Permit2 address) were caught by length-checks before reaching a device — **always + len-check hex from any external source**. +- Tests are GENERATED from the catalog (one device test per flow + a batch test that + device-validates every blob and rejects a 1-byte tamper of each). Adding flow #52 = + editing the catalog only. +- Offline reference vectors: `sign_metadata` is RFC 6979 deterministic; sha256+length + snapshots for all 51 blobs frozen in `REFERENCE_BLOB_SNAPSHOTS`. + `python3 tests/test_msg_ethereum_clear_signing.py --flows` dumps + to/value/calldata/tx_hash/blob hex per flow — **the external contract for any signer + implementation** (pioneer-insight, keepkey-sdk). +- Standards alignment: Ledger + Trezor (as of May 2026) both converge on **ERC-7730** + descriptors (intent + typed fields: addressName/tokenAmount, "Unlimited" threshold, + field hiding). Our STRING/ADDRESS/TOKEN_AMOUNT + curated-display-subset model maps + 1:1 onto it — an ERC-7730→metadata compiler is a natural later step for the signer + service. ERC-8176 (descriptor integrity attestations) is the trust layer to watch. + +**Verified state (emulator):** full compose run **491 passed / 0 failed / 27 skipped** +(all documented). PDF: **fw 7.15.0, 216 tests, 211 passed, 0 failed, 5 pending**. Report +V-section entries are generated from the catalog (fixed a drift bug where hand-typed +entries pointed at renamed tests) — verified 100% name-match against pytest collection. +Frames visually inspected for: Aave supply full sequence, UNLIMITED USDC approve, +ETH-value swap, EIP-7702 ("delegate: 0x4Cd2…"), handleOps ("sender: 0x9406…"), +Safe execTransaction, Permit2 permitTransferFrom. Zero calldata hex anywhere. + +--- + +## Final-PDF remaining items (5 pendings, all enumerated — no silent skips) + +| item | why pending | action | +|---|---|---| +| Z5–Z7 zcash Orchard legacy-sighash signing | real firmware capability gap (needs header/orchard digests) | DECISION: implement for 7.15 or ship as the documented post-7.15 gap (section text already states it) | +| V8 `test_ethereum_blind_sign_allowed` | test itself gates on 7.15.1 | leave; or retarget the gate to 7.15.0 if it's meant to run now | +| C31 bip39 invalid-word rejection | pending per original audit | verify it runs on rc3; it's the #272 feature | +| ~2 misc | see junit skips | `grep skipped junit.xml` on the CI artifact | + +PDF hygiene criteria from the original audit are otherwise met: header 7.15.0, no +setup/blank frames (capture-time reset + density picker), every [NEW] section has real +feature screenshots, Hive G 5/5, Zcash 15/18 with real UA/QR screens, BIP-85 6/6 +(fixed a false-skip: `requires_message` probe couldn't serialize required-field protos). + +## The iteration loop (unchanged, for reference) + +```bash +# local fast loop (arm64 Mac: DOCKER_DEFAULT_PLATFORM=linux/amd64) +cd /scripts/emulator +docker compose down -v +docker compose up --build --exit-code-from python-keepkey python-keepkey +# artifacts in the emulator_test-reports volume; PDF also generatable locally: +python3 scripts/generate-test-report.py --junit junit.xml --fw-version 7.15.0 \ + --screenshots screenshots/ --output test-report.pdf +``` +Pin train: edit python-keepkey `feat/clearsign-load-signer` → bump `deps/python-keepkey` +in the firmware branch → push (CI runs on PR #281 pushes) → download `test-report` / +`oled-screenshots` artifacts → judge the PDF. + +## Real-device testing runbook (the next step after merge) + +The python-keepkey suite runs unmodified against a physical device — this is the fastest +"run all the clearsign txs" path (vault/SDK path is NOT ready, see blockers below). + +1. **Flash** the rc3 build (must be a DEBUG_LINK build for the automated suite; for a + production-signed build, drive manually via keepkeyctl — no debuglink auto-confirm). +2. **⚠️ The device tests WIPE the device** (`common.KeepKeyTest.setUp`) and load the + public mnemonic12 test seed. Never run against a device holding real funds. +3. Run: `cd python-keepkey/tests && python3 -m pytest test_msg_ethereum_clear_signing.py -v` + with the USB transport configured in `tests/config.py` (device + debuglink interface). +4. What to verify by eye on the OLED (the point of doing it on hardware): + - Load confirm: "Trust signer 'CI Test' () to describe transactions? NOT verified + by KeepKey." — fingerprint should match the one shown later on warnings. + - Per tx: CLEARSIGN WARNING → Call → Contract (full addr) → args ("10.5 DAI", + "UNLIMITED USDC", protocol labels) → tx/gas confirm. **No hex anywhere.** + - Reject paths: cancel at the warning cancels the tx; wipe drops the signer + (re-load required). +5. Known gotchas (from prior device sessions): USB transport can wedge after + recovery/wipe → replug; unsigned-RC reboots hit the bootloader gate. + +## Merge order + companion branches + +1. Merge **#281** → develop (contains the #280 version bump; close #280 or merge first). +2. device-protocol: merge `feat/load-clearsign-signer` (2ec999a9, msg 117) into + `up/release-protocol` so upstream PR #111 carries it (fork-only until release SOP). +3. python-keepkey: fold `feat/clearsign-load-signer` (tip `1545299`) into + `reconcile/upstream-sync` — **user-gated** (that branch feeds upstream PR #196); + the firmware pin references the SHA directly, so merge order is not blocking. +4. Cut `release/rc3` off develop per the release SOP; the CI PDF from that build is the + test-plan artifact. + +## Production blockers (host side — independent of rc3, but gate REAL-WORLD clearsign) + +1. **Pioneer signer is incompatible with the firmware today**: the shipped + `descriptor-signing.service` pre-signs blobs with **txHash zeroed + empty arg values + + classification always VERIFIED**. This firmware refuses them at `signed_metadata_enforce` + (fail-closed). Pioneer must sign **per-tx** (real tx_hash, real values, STRING/ + TOKEN_AMOUNT formats) — the catalog's `--flows` dump + snapshots are the contract to + build against. +2. **Vault/SDK load-signer train**: proto → hdwallet-keepkey → vault REST → keepkey-sdk + need `LoadClearsignSigner` support; the SDK's `tests/evm-clearsign` static fixtures + (key_id 0, zero hash, protocol-as-RAW) must be replaced with runtime signing (the JS + signer in pioneer-insight lib can do this in-test, like python does). +3. Signer discipline: RAW/BYTES args still render hex by design (honest fallback) — + production blobs should use ADDRESS/STRING/TOKEN_AMOUNT only (the catalog's offline + test enforces this for the reference set). + +## Key files +- firmware: `lib/firmware/signed_metadata.c`, `fsm_msg_ethereum.h` (LoadClearsignSigner), + `unittests/firmware/signed_metadata.cpp` (59 tests) +- python-keepkey: `keepkeylib/clearsign_catalog.py` (THE catalog), + `keepkeylib/clearsign_abi.py`, `keepkeylib/signed_metadata.py` (RFC 6979), + `tests/test_msg_ethereum_clear_signing.py`, `scripts/generate-test-report.py` +- specs: vault repo `docs/firmware/SIGNED-METADATA-CLEAR-SIGNING-PLAN.md` (who/what/why, + "Amount: 1,000 USDC" mandate), ERC-7730 registry `github.com/ethereum/clear-signing-erc7730-registry` diff --git a/docs/handoff-emulator-rc-abi-backport-and-dialog.md b/docs/handoff-emulator-rc-abi-backport-and-dialog.md new file mode 100644 index 00000000..cc5a612e --- /dev/null +++ b/docs/handoff-emulator-rc-abi-backport-and-dialog.md @@ -0,0 +1,77 @@ +# Handoff — Emulator RC ABI gap, firmware backport, and Vault version/seed dialog + +**Prepared:** 2026-07-08 · **For:** whoever picks up rc7 cut + the firmware PR +**Repos:** `keepkey-vault` (worktree: `keepkey-vault-v11-release-149`, branch `develop`) · `keepkey-firmware` (worktree: `/private/tmp/.../scratchpad/kkfw-emu-backport`, branch `feat/emulator-poll-thread-backport`, fork `BitHighlander/keepkey-firmware`) +**Status:** Vault-side dialog shipped and running locally. Firmware backport built + ABI-verified locally, **committed but not yet pushed / PR'd / merged**. rc7 not yet cut. + +--- + +## TL;DR + +1. `release/7.15.0-rc5` and `rc6` emulator dylibs (and `develop` itself) are **missing the poll-thread FFI API** (`kkemu_start/stop/lock/unlock/trylock`) the Vault's `develop` branch requires to load the dylib at all. Symptom: "Start" does nothing — `kkemu_init` throws `Symbol "kkemu_start" not found`. +2. Root cause: `develop`'s `lib/emulator/libkkemu.c` and firmware's `alpha` branch have **two independently-written implementations** sharing the same file paths (322 vs 573 lines — real `add/add` conflicts on cherry-pick, not a clean history). +3. Fix: wholesale-adopted alpha's version of the emulator subsystem onto a new firmware branch off `develop`. Built and verified locally — all 12 Vault-required symbols present. **Not yet pushed or PR'd.** +4. Added a CI step that would have caught this (asserts the full symbol set after the dylib build) — dry-run confirmed it fails against the old rc6 artifact and passes against the new build. +5. Vault side: added a version/seed control dialog to the existing (already `deviceState.isEmulator`-gated) Emulator section in `DeviceSettingsDrawer.tsx`, plus one new hard-gated RPC (`emulatorRevealSeed`) for emulator-only seed backup. + +--- + +## 1. The ABI gap (firmware) + +- Confirmed via `nm -gU`: the `release/7.15.0-rc6` CI dylib artifact (`libkkemu-4504c9d...`, run `28956306972`) exports `kkemu_init/shutdown/write/read/poll/is_running/pop_frame/get_display` but **not** `kkemu_start/stop/lock/unlock/trylock`. +- `git log origin/develop..origin/alpha -- lib/emulator include/keepkey/emulator` shows 7 alpha-only commits (`f75dd420` → `f322fc40`) that never landed on `develop`, hence never on any `release/7.15.0-rc*` branch (all cut from `develop`). +- `release/7.15.0-rc6`'s `lib/emulator/libkkemu.c` is **byte-identical** to `develop`'s — confirms this isn't an rc-specific regression, `develop` itself has always lacked the poll-thread API. +- Diffing `develop` vs `alpha`'s current `libkkemu.c` (369 changed lines against a 322-line file) plus `add/add` conflicts on cherry-pick attempt confirmed these are **not the same file lineage** — a genuine independent rewrite, not a simple divergence. + +## 2. The backport (firmware, `feat/emulator-poll-thread-backport`, commit `3461c688`) + +Wholesale-replaced (byte-identical to `origin/alpha` tip) rather than cherry-picked, since cherry-picking `add/add` conflicts in threading/memory-safety C code is not something to resolve blindly: + +- `lib/emulator/{libkkemu.c,setup.c,udp.c,CMakeLists.txt}` +- `include/keepkey/emulator/libkkemu.h` +- `tools/emulator/CMakeLists.txt` +- **New file** `include/keepkey/board/bsd_compat.h` (strlcpy/strlcat prototypes for glibc/MinGW; force-included on non-Apple emulator builds only — hardware build untouched) + +Top-level `CMakeLists.txt` was **surgically** patched (NOT wholesale-replaced) — alpha's version predates and is missing `develop`'s `KK_BITCOIN_ONLY` / `KK_ZCASH_PRIVACY` variant-build flags (PR #282). Only added: `NANOPB_PLUGIN` cache var + the non-Apple `bsd_compat.h` force-include. + +`ringbuf.c`/`ringbuf.h` were checked and found **byte-identical** between `develop` and `alpha` already — no change needed there. + +**CI hardening** (`.github/workflows/ci.yml`, `python-dylib-tests` job): added a "Verify Vault ABI (exported symbols)" step right after the dylib build, asserting all 12 symbols the Vault's `src/bun/emulator.ts` `dlopen()`s. This is why the gap shipped silently — the existing python-keepkey tests only exercise caller-driven `kkemu_poll()`, never `kkemu_start()`, so a dylib missing the whole thread API still passed CI. Dry-ran the check locally against both the broken rc6 dylib (fails, reports the exact 5 missing symbols) and the new build (passes). + +**Local verification:** `cmake -DKK_EMULATOR=ON -DKK_DEBUG_LINK=ON -DKK_BUILD_DYLIB=ON ... && make kkemu kkemulator_dylib` — clean build, macOS arm64. `nm -gU` on the output confirms all 12 symbols. Installed at `~/.keepkey/emulator/libkkemu.dylib`, ad-hoc codesigned, and the Vault was rebuilt (`make vault`) and run against it — user confirmed the emulator now starts. + +### Not done yet +- [ ] Push `feat/emulator-poll-thread-backport` to `BitHighlander/keepkey-firmware` +- [ ] Open PR → `develop` (**fork only** — never upstream keepkey/keepkey-firmware, per [[feedback-near-firmware-pr-target]]) +- [ ] Get CI green (watch the new ABI-check step specifically) +- [ ] Merge +- [ ] Cut next rc branch — **note: `release/7.15.0-rc6` already exists and was cut before this fix**, so the natural next cut is `release/7.15.0-rc7` unless you want to re-cut rc6. Not decided yet — ask before cutting. +- [ ] Re-download the new rc's CI dylib artifact, re-verify against the Vault dialog +- [ ] Later, on explicit request only: upstream this to `keepkey/keepkey-firmware` (this was called out as a deliberate "step 6," after testing — not now) + +## 3. Vault-side: emulator version/seed dialog + +File: `projects/keepkey-vault/src/mainview/components/DeviceSettingsDrawer.tsx`, inside the existing `deviceState.isEmulator`-gated "Emulator" `Section`. All new controls reuse existing backend RPCs except one: + +- **Firmware version display** (`deviceState.firmwareVersion`, already available — no new plumbing) +- **"Change Version…"** — reuses the existing dylib file-picker install path (`emulatorInstallDylib`); the hidden `` was hoisted to the drawer's top-level return so both the Settings-panel install button and this one share it regardless of which section is open +- **Wallet/seed switcher** — ` setDraftField("programId", event.target.value)} size="sm" fontFamily="mono" bg="rgba(0,0,0,0.18)" /> + + Discriminator setDraftField("discriminator", event.target.value)} size="sm" fontFamily="mono" bg="rgba(0,0,0,0.18)" /> + Program label setDraftField("programName", event.target.value)} maxLength={20} size="sm" bg="rgba(0,0,0,0.18)" /> + Instruction label setDraftField("instructionName", event.target.value)} maxLength={20} size="sm" bg="rgba(0,0,0,0.18)" /> + + + + Arguments + + {draft.args.length === 0 && No arguments. The discriminator must then cover the entire instruction data.} + {draft.args.map((arg, index) => ( + + + setDraftField("args", draft.args.map((item, i) => i === index ? { ...item, label: event.target.value } : item))} maxLength={16} size="sm" placeholder="Display label" bg="rgba(0,0,0,0.18)" /> + + + ))} + + + + + Displayed accounts + + {draft.accounts.length === 0 && No accounts selected for labelled display.} + {draft.accounts.map((account, index) => ( + + setDraftField("accounts", draft.accounts.map((item, i) => i === index ? { ...item, index: Number(event.target.value) } : item))} size="sm" w="90px" bg="rgba(0,0,0,0.18)" /> + setDraftField("accounts", draft.accounts.map((item, i) => i === index ? { ...item, label: event.target.value } : item))} maxLength={16} size="sm" placeholder="Display label" bg="rgba(0,0,0,0.18)" /> + + + ))} + + + + + + + + + + + 2 · Review canonical bytesRaw hex is always visible and remains editable for negative tests. + + +