rehearsal: roll audited develop and remaining 7.16 alpha PRs together - #625
Conversation
Everything 7.14.3 sits on top of: the 7.14.2 disclosure and EVM work, its release documentation, and the accompanying unit tests. Separated from the 7.14.3 commits below so the new work can be read on its own.
Schnorr signing, the taproot sighash and witness serialisation, and BIP-86 path validation in coins.c, with 18 BIP-340/341 vectors. Proven on hardware: a P2TR spend has been executed on a real device on MAINNET, so the sighash, signature and witness serialisation were all accepted by the network.
KK_BITCOIN_ONLY gates the non-Bitcoin coin families out of the image, and
release.yml builds `full` and `bitcoin-only` from one matrix.
Three collisions had to be fixed for two variants to coexist, each of which
would have shipped wrong artifacts from a GREEN job: ${SUFFIX} was referenced
but never assigned, so both variants wrote the same filename; both legs
uploaded the same artifact name, a hard failure on upload-artifact@v4+; and
both wrote HASHES.txt, so one manifest silently overwrote the other under
merge-multiple.
STORAGE_VERSION_LAST_SHIPPED records the highest version actually shipped, with a static assert so lowering STORAGE_VERSION below it fails the build instead of silently wiping every device that upgrades. A seed CREATED under bitcoin-only firmware is stamped 10000 + STORAGE_VERSION so multi-chain firmware refuses it -- without this a bitcoin-only seed was indistinguishable from a multi-chain one. Only creation paths stamp, so a pre-existing multi-chain wallet migrated under bitcoin-only firmware keeps its portable version. Multi-chain firmware REFUSES rather than wipes; bitcoin-only firmware recovers the underlying version and loads through the normal migration chain. The lock is enforced at CHECK_NOT_INITIALIZED, not only at the write. A locked device looks uninitialized (the RAM shadow was reset), so without this guard it would accept a full ResetDevice ceremony, write nothing, and report Success -- leaving a funded seed in RAM only.
BNB, CMT, GTO, IOST, MCO, ODEM and QTUM each migrated off Ethereum or died. The vetted source now gives three competing addresses for six of them and none at all for ODEM, so the generator refuses them -- it takes a symbol only when the source gives exactly one address, so a scam token cannot inherit a real one's label. The device was advertising seven tokens it cannot identify. Coins resolve by name, never by table index, and no index is persisted, so removing entries cannot shift the meaning of stored data.
device-protocol is pinned to the MINIMAL taproot change (Features.supports_taproot), not the full 7.15 proto branch: that one additionally carries structured EIP-712 and Solana LUT fields with no .options entries in this tree, so nanopb emits pb_callback_t and both the ARM and emulator builds fail. No source file here references any of them. Both pins are heads of open PRs into master and become plain master pins when those merge.
Brings the bitcoin-only variant, taproot, the storage version band and the
token-table cleanup onto the 7.14.2 line, so 7.15 has a single current base to
stage against.
develop already carried the 7.14.2 work, and 7.14.3 was cut from upstream
develop (7.14.1) with that work cherry-picked, so the two arrived at the same
content by different routes: 86 of the 97 shared files were already byte
identical, and no file existed on develop that 7.14.3 lacked.
Seven conflicts, all resolved to 7.14.3:
CMakeLists.txt version 7.14.2 -> 7.14.3
release.yml per-variant HASHES/artifact names (two variants
cannot share one filename)
storage.c, transaction.c additions only; the HEAD side was empty
unittests/.../CMakeLists same suites, with the altcoin ones moved behind
KK_BITCOIN_ONLY -- a superset, plus signing.cpp
deps/* the reviewed pins
Audited every line the merge drops: all 95 are supersessions (HASHES.txt ->
HASHES${SUFFIX}.txt, coin sources moved into the conditional, the seven dead
ERC-20s). Verified the 7.14.2 security work survives -- thorchain.c is byte
identical on both sides, so the NUL and memo fixes are present.
BIP-340/341/86 hold and spend; the shared frame arena replacing a 12 KB stack frame that faulted at boot; whole addresses on screen instead of the first 42 characters; and confirm bodies paged rather than silently truncated.
The 7.14.2 disclosure work on the EVM paths -- every byte covered by the signature reachable on screen -- plus SLIP-0048 hive keys and operation signing, TRON and TON sign-message with honest TRON fees, and Tendermint-family signing policy with custom denoms.
Orchard engine, shielded handlers, constant-time Pallas paths, NU6.3 Ironwood. ZCASH_PRIVACY=1 makes Orchard reachable in the regular image, and redpallas_sign_with_rsk() draws its spend-authorization nonce from unchecked random_buffer() -- a REPEATED nonce discloses the spend authorization key. The draw is in vendored crypto, so the gate sits at the call site and fails CLOSED before every spend: the latched boot verdict plus a live checked draw folded into the continuous SP 800-90B state. NOT equivalent to the nonce itself being checked; the complete fix needs a change to the vendored crypto.
EVM and Solana clear-signing with reusable instruction schemas, signed transaction metadata verified on device, KeepKey as the schema attestor, and EIP-712 overflow-width and array-size rejections.
One checked-RNG verdict consumed by every key-material draw, on-device dice entropy in the ResetDevice flow, and no display of internal entropy. The gate is opt-in BY NAME: adding a new key-material draw does not inherit it.
…ning STORAGE_VERSION_LAST_SHIPPED with a static assert, so lowering STORAGE_VERSION below a shipped version fails the build instead of silently wiping every device that upgrades. A seed CREATED under bitcoin-only firmware is stamped 10000 + STORAGE_VERSION so multi-chain firmware refuses it. Only creation paths stamp. Multi-chain REFUSES rather than wipes, and the refusal is enforced at CHECK_NOT_INITIALIZED, not only at the write: a locked device looks uninitialized, so without that guard it would accept a whole ResetDevice ceremony, write nothing, and report Success. Also versioned PIN KDF, and seven dead ERC-20s dropped -- each migrated off Ethereum, and the vetted source now gives three competing addresses for six of them and none for ODEM, so the device was advertising tokens it cannot identify. BIP-85 derives mnemonics for OTHER wallets, so it is gated to multi-chain across messagemap.def, fsm.c and the source list; all three must agree.
KK_BITCOIN_ONLY gates the coin engines out of the image, and release.yml builds
both variants from one matrix. Three collisions had to be fixed for that to be
possible, each of which would have shipped wrong artifacts from a GREEN job:
${SUFFIX} referenced but never assigned, duplicate artifact names, and two legs
writing one HASHES.txt.
Two defects found in review and fixed here:
- the verified gitleaks binary was replaced by an unverified one. A clean
merge unioned both parents: 8.30.1 was checksummed, then 8.30.0 was fetched
through `curl | tar` with no checksum, over the top, before the scan ran.
One version, one archive, one verified digest.
- the generated release checklist omitted signature verification.
hash-manifest.sh --require-signed is STRUCTURAL ONLY -- a signature region
holding one non-zero byte passes it -- so the real 3-of-5 ECDSA check is now
required for every variant, before upload.
The SRS, the defect register, the storage version gate, the clear-sign decisions with what each cost, and the RNG coverage scope -- which states plainly which draws are covered and that everything else, including the Orchard nonce, is not.
device-protocol is deliberately NOT device-protocol#112's head: that adds thirteen structured EIP-712 fields this tree has no .options entries for, so nanopb emits pb_callback_t and every build fails. Pinned to what 7.15 builds against, verified by building both variants.
Drop RC-numbered process/handoff docs, historical evidence screenshots,
and 7.16+/7.17+ scope docs that don't describe what this PR ships:
- docs/release/rc30-evidence/* (13 files) — superseded by this PR's own
verification section
- docs/release/{7.14.2-rc30-hardware-plan,7.14.2,ALPHA-MERGE-HANDOFF,
DEFECTS-2026-08,TOKEN-TABLE-BUDGET}.md — one-time process/handoff notes
- docs/release/{SRS-7.16,SRS-7.17,ROADMAP-715-717}.md — future scope,
explicitly excluded by this PR
- docs/security/evidence/* (4 dirs, ~25 files) — per-finding screenshot
proof for individual past RCs
- docs/security/{7.14.2-signing-hardening,7.15.0-dependency-review-map,
7.15.0-rc17-hardening,7.15.0-rc18-release-shape,
7.15.0-rc21-clearsign-release-control,next-wave-hardening,
rc28-open-findings-handoff,token-table-retirement}.md — RC-dated
process notes
- docs/security/DESIGN-716-reductive.md — 7.16 scope
- docs/zoo/reports/zcash-report.md — one-off report
Folded the durable content that was still cited from kept docs/code:
- release-shape's two-product table -> docs/Build.md
- RC21's human-attestation gate + Solana tag reservation policy ->
docs/security/clearsign-provider-tier.md
- code comments pointing at rc28-open-findings-handoff.md now state the
reason (degraded-RNG recovery) inline instead of citing a deleted doc
Left in place as still-active design notes, not history:
anti-rollback-security-epoch-rfc.md, clearsign-key-delegation-roadmap.md,
BRANCHING-SOP.md (dangling ALPHA-MERGE-HANDOFF reference removed).
docs: trim release/7.15 docs to what's current
- docs/coin-integration/zcash-clearsign-handoff.md — dated one-time PR-prep notes; the durable spec already lives in zcash-pczt-clearsign.md - docs/release/BRANCHING-SOP.md — contributor git-workflow process, not a statement of what this release ships
The Orchard spend-authorization signer did not implement the spec's nonce
construction. This branch pinned a crypto revision whose signer drew its own
randomness with plain random_buffer() and reduced it straight to the nonce
scalar. The Zcash protocol specification draws an 80-byte T and computes
r = H*(T || vk || M)
hashing the randomness together with the verification key and the message.
That binding is the point. With M inside the hash, two signatures over
different messages get different nonces even when T repeats. Without it, one
repeated draw across two messages discloses the spend authorization key by
arithmetic any observer can do from public data:
ask + alpha = (s1 - s2) / (c1 - c2)
Two further defects rode along.
BIAS. Reducing 32 bytes mod the Pallas scalar order leaves each residue with 3
or 4 preimages -- a 33% relative bias, which is exactly the input a lattice or
hidden-number attack wants. The digest is now reduced wide, where the bias is
negligible. (The same narrow reduction is safe in BIP-340 only because
secp256k1's order sits within 2^-128 of 2^256; Pallas is ~2^254.)
NORMALISATION. pallas_ct_scalar_replace_zero_with_one() converted a dead
entropy source into the CONSTANT nonce 1 on every signature -- reused AND
publicly known, which discloses the key from a single signature rather than
two. It is deleted, not merely unused: a zero scalar now fails the signature.
Refusing to sign is always safe; signing with a predictable nonce is not.
CALLER. fsm_msg_zcash.h draws T through random_buffer_checked() behind
rng_health_check(), so a degraded generator yields no signature instead of a
predictable one. rng_health.h listed this draw as a known exception; it is
covered now, so the exception is removed rather than left to rot, and the
stale example in storage.c's scope note goes with it.
GATE. tools/check_pallas_api_boundary.py REQUIRED the normaliser in the
signing core -- the defect was written down as an invariant, and the gate
would have rejected this fix. It now forbids the normaliser, forbids the
signing core drawing its own randomness, requires the spec hash, and requires
the nonce hash to wipe its BLAKE2b context (blake2b_Final leaves the digest in
ctx.h and the last input block, containing T, in ctx.buf).
TESTS. Six cases, and they have teeth: with the pre-fix derivation restored by
mutation, RepeatedT_DifferentMessage_DifferentR and
RepeatedT_DifferentKey_DifferentR both fail. The suite that shipped alongside
the defect did not -- RedPallasSign_DifferentSighash passes under the broken
code, because different messages produce different CHALLENGES whatever the
nonce does. It watched the wrong half of the signature; the new tests assert
on R, the half that discloses the key.
The constant-time test for the deleted normaliser is removed with it.
Sibling audit: ECDSA is RFC 6979, BIP-340 does the spec construction binding
key and message, generate_k_random() is rejection sampling used for coordinate
blinding, and bip39's self-draw, cosi and Monero are unreachable from firmware.
RedPallas was the only member of this class.
Upstream fixes: keepkey/trezor-firmware#10 and #11.
Full detail: #509.
An adversarial audit of this release produced 33 findings, 27 of which survived independent refutation. This lands the fixes for the exploitable and high-severity ones. All are verified green on the full CI graph. ZCASH: A v6 TRANSACTION NO LONGER SIGNS AN UNVERIFIED BUNDLE. The device streams and verifies only the ACTIVE pool's actions, but orchard_digest was mandatory on the Ironwood path, folded into the signed sighash, and never checked against anything. That is a theft path: stream honest Ironwood actions, point orchard_digest at a real Orchard bundle spending one of this seed's notes, and reuse the alpha of an approved action so rk is byte identical. RedPallas verifies as [s]G = R + [H(R||rk||M)]rk, so with rk and M shared the single signature the device emits verifies in BOTH bundles, and the malicious bundle's valueBalance never reaches the fee arithmetic. The user approves a small send and an arbitrary note moves with it. EMPTY_ORCHARD_DIGEST pins the inactive pool exactly as EMPTY_SAPLING_DIGEST and EMPTY_TRANSPARENT_DIGEST already did -- Orchard was the one unused pool taken on trust. The ZIP-244 value was validated by reproducing both existing constants with the same method before being used. ZCASH: rk IS CHECKED AGAINST THE DEVICE'S OWN KEY. Production signed through redpallas_sign_digest_for_rk(), which feeds the host's rk into the nonce and challenge hashes without ever verifying it describes this device's key. redpallas_sign_digest_with_ak() derives rk from the cached ak and alpha, refuses on mismatch, and signs with the derived value. It already existed and was already tested. Cost: one scalar multiplication per action. ZCASH: THE SIGNING SESSION IS TORN DOWN. zcash.h documents zcash_signing_abort() as existing so a host cannot resume streaming against a previously-approved session after Initialize, Cancel or ClearSession, and fsm.c repeats the claim, but nothing called it. The Orchard spend authorizing key survived all three. ZCASH: SEEDS AND KEYS NO LONGER SURVIVE IN HASH CONTEXTS. blake2b_Final() leaves the digest in ctx.h and the last input block in ctx.buf. zip32_orchard_master() hashes the BIP-39 SEED, prf_expand() hashes the Orchard spending key, and storage_getRootNode()'s PBKDF2 context is passphrase-keyed and derives the AES key that decrypts the private key. A sweep found 17 unwiped contexts; the other 14 hash public data and are left alone. CLEAR-SIGN AND ETH: AN AMOUNT NEVER RENDERS BLANK. bn_format() answers overflow with memset(output, 0, len); return 0, and all three call sites ignored the return, so an oversized amount displayed as an EMPTY string -- the one rendering a user cannot read as wrong. 256-bit at 18 decimals needs ~80 characters against a 32-byte buffer. THE BOUNDARY GATE STATED THE WEAKER PROPERTY. It REQUIRED _for_rk and explicitly FORBADE _with_ak, so it would have rejected this fix -- the third time this file was found pinning the weaker of two available implementations, after it required the nonce normaliser. It now requires the validating path, forbids the unvalidated one, and requires the nonce hash to wipe its context. Every rule was mutation-checked in both directions. TESTS. The v6 fixture passed filler bytes as orchard_digest with a comment describing the vulnerability as the design; it now uses the ZIP-244 value, and Z26 asserts a non-empty Orchard bundle is refused. The display-disclosure suite was passing vacuously against an uninitialized device -- four tests reported PASS having exercised zero display logic and the fifth skipped itself. With a seed loaded they now run and pass for real. Findings: #509 #511 #513 #514 #517 #527.
MultisigRedeemScriptType.signatures is declared max_size:73, so pb_decode accepts 73 bytes -- but a DER-encoded ECDSA signature is at most 72. Nothing checked the runtime size, and the witness serializer appended the sighash byte AT signatures[i].size, so 73 wrote one past the end of bytes[73]. For i < 14 that lands on signatures[i+1].size: a host can flip a later slot from 0 to nonzero, the `size == 0` skip stops firing, and the device emits an extra witness element built from a slot the host left empty -- the witness stack no longer matches what the user reviewed or what was hashed. At i == 14 it lands on has_m. signing_validate_input() now rejects anything over 72 bytes before that size indexes anything, and the witness element is built in a local so the out-of-bounds write no longer exists to be gated. A declared max_size is a DECODER bound and must never be relied on as the runtime one. The device's own signature append is unaffected: same 73-byte field, but the device produces the value so it is at most 72 and the write is in bounds. Test Z27 asserts an oversized signature is refused, registered in SECTIONS so it is in the CI filter and actually runs.
…RNG latch Second batch from the 7.15 audit. Rebased onto a3da828 -- that commit's files are untouched here. SEED BACKUP: 1.7% OF 24-WORD BACKUPS WERE CLIPPED. layout_constant_power_- notification() draws from x = 128 + LEFT_MARGIN, so only KEEPKEY_DISPLAY_WIDTH - (128 + LEFT_MARGIN) = 124 px exists past that origin, while BODY_WIDTH (225) is passed as the wrap width. The wrap never fires before the canvas edge does: draw_char_impl() rejects the first glyph crossing 256, draw_string_walk() stops, and every character after it is dropped -- including whole later lines. No ellipsis, no warning, no page indicator. confirm()'s completeness check was hard-gated to layout_standard_notification, with a comment noting that measuring a constant-power body against BODY_WIDTH would be wrong. It would have been. But excluding it left the backup pages with no check at all. Measured with the real font tables and the real placement rules over 200,000 random 24-word mnemonics: 1.712% a backup page is clipped (1 in 58) 0.646% a whole word is never shown (1 in 155) 1.075% only the final word truncates, e.g. "24.observ" A missing word is not recoverable from the backup. The fix measures at the right origin instead of skipping the measurement, and the probe is selected by layout and threaded through page_take(), so an over-full page is PAGED by machinery that already exists. No new buffers, MAX_PAGES untouched -- widening the packing width in reset.c instead would have needed MAX_PAGES 6 -> 12, about 3.7 KB more static SRAM, and would have hard-failed wallet creation with "Too many pages of mnemonic words". ZCASH SESSION: cleared wherever the device session is torn down, not at three named handlers. The screensaver auto-lock (home_sm.c) and the PIN-failure path (pin_sm.c) both tear it down without going through Initialize or ClearSession, and both left the Orchard spend AUTHORIZING key live in RAM. session_clear() already revokes ClearSign signer slots for exactly this reason; the Orchard key belongs to the unlocked session on the same grounds. BITCOIN-ONLY LOCK: a locked wallet resets the RAM shadow, so handlers that persist settings ran normally, storage_commit() declined to write, and the handler answered Success -- the change survived the session and vanished at the next boot. CHECK_NOT_BITCOIN_ONLY_LOCKED now guards ChangePin, ChangeWipeCode, ApplySettings and ApplyPolicies. ChangeWipeCode was found by sweeping every storage_commit() caller rather than fixing only what was reported. WipeDevice is deliberately NOT gated: wiping is the documented way out. RNG: rng_source_live() tested RNG_SR_SEIS claiming "SEIS latches, so a transient fault is still visible here". It does not -- random32() clears SEIS/CEIS as soon as the underlying condition goes, as it must to keep drawing, and it runs constantly. A fault it saw, recovered from and erased was invisible to the boot health verdict that every checked key draw fails closed behind. Now mirrored in a boot-lifetime software latch set where the hardware latch is cleared. CI: firmware-unit failures now fail their job. The entrypoint ended with `cp`, so the container's exit status was the copy's, and CI consumes that status directly. Verified both shapes rather than assumed. The separate unit-tests job does gate the same binary, so this was not a total blind spot. Tests: Board.ConstantPowerBodyFitsMeasuresFromItsOwnOrigin pins a real clipped page and asserts the left-margin probe still calls it fine, so it keeps demonstrating the blind spot. Findings: #519 #513 #526 #528 #533.
Paging a constant-power body changes the HOST protocol. page_body_confirm()
emits one ButtonRequest per page -- "Every page after the first writes its own
request" -- and the seed-backup flow is driven by a host that reads one word
group per ButtonRequest. A split backup page therefore made the host read that
group twice, and test_reset_device reconstructed a mnemonic with duplicated
words:
...mushroom valid chronic category mushroom valid way suit parent
...mushroom valid way suit parent (expected)
Any host driving ResetDevice counts ButtonRequests, the vault included, and the
corruption lands in the words the user is copying down. Worse than the clipping
it was meant to fix, so it is reverted.
Kept: confirm_body_fits_constant_power(), which measures at the origin that
layout actually draws from, and the test pinning a real clipped backup page.
The measurement is correct and now inert -- the blind spot stays documented
instead of being forgotten.
The clipping is still real: 1.712% of 24-word backups clip and 0.646% never
show one of the words, measured over 200k mnemonics with the real font tables.
The fix is to pack reset.c's pages against the width they are drawn at, which
needs MAX_PAGES raised (~3.7 KB static SRAM) and OLED verification on hardware.
Tracked in #519.
Backported from release/7.15, where the collection UI and the physical
short-press/long-hold classification are hardware-verified. Eight files
arrive byte-identical to that tree (dice_input.{c,h}, rng_health.{c,h},
rng.{c,h}, u2f.c, keepkey_flash.c) along with both unit-test suites; the
reset.c / fsm_msg_common.h / fsm_msg_debug.h integration is ported
surgically and diffs clean against 7.15 except for the one deliberate
divergence below.
ResetDevice.dice_entropy collects 50/75/99 rolls on the device (short press
advances 1-6/undo, 800 ms hold commits) and folds
SHA256(int_entropy || rolls) into the internal entropy BEFORE
EntropyRequest, so the host contribution arrives strictly after the device
has committed to its own. The completion screen shows the first 8 bytes of
SHA256(rolls) so the user can recompute it from their written rolls; the
same digest is served over DebugLink for test parity. docs/DiceEntropy.md
covers what the digest proves and what it deliberately does not.
Divergence from 7.15, on purpose: display_random is RETAINED, because
already-shipped 7.14-line hosts request the Internal Entropy screen and the
pinned python suite asserts it below 7.15.0. The two are mutually exclusive
by refusal -- ResetDevice with both flags fails with a SyntaxError, since
the screen shows the POST-mix value, which is the seed pre-image once
ext_entropy is known.
The seed-time RNG gate covers every call site on rng_health.h's documented
list, so the header's COMPLETE LIST claim is true in this tree: reset_init
(fail with FirmwareError), the storage encryption and wipe-code keys and
the V1-upgrade PIN-KDF salt (storage_drawKeyMaterial, halt), U2F key-handle
derivation (refuse registration), and the one-shot OTP randomness block
(skip the write, leave the block claimable by a healthy boot).
Pins: deps/device-protocol advances to 8545cd5b -- the exact pin
release/7.15 builds against; every bytes/string field new to the compiled
protos verified to carry nanopb sizing (the options files here were already
pre-staged for it, plus three Solana LUT lines). deps/python-keepkey
advances to 9aaaa84, which moves the dice end-to-end test, the ceremony
re-entry regression, and report Section K from a 7.15.0 gate to 7.14.3 --
exact for the fleet, since no firmware between 7.14.3 and 7.15.0 exists
without dice. Those tests previously SKIPPED against this branch and CI
went green with zero device-level dice coverage.
ci: build-arm-firmware compiles both release variants on every PR, so a
change that only breaks the bitcoin-only image can no longer go green.
Verified in CI-parity docker (same base image digest): bitcoin-only ARM
links at .text 372,928 and full at 523,600; firmware-unit 103/103 including
Dice.* (5) and RngHealth.* (15); the full python-integration compose suite
passes 449/449 selected with test_reset_device_dice and
test_reset_reentry_disarms_entropy_ack RUNNING and green; clang-format-20
clean on every touched file.
…20260826 release candidate: stage upstream 7.15 onto fork develop
|
Integration accounting for exact head
The merge was conflict-free and |
|
Exact-head update: companion run |
|
Final pin-alignment update: required CircleCI on companion #63 was still testing old firmware |
|
Final exact-head disposition for |
Integrates the exact audited 7.16 develop rehearsal with the remaining Solana disclosure and structured EIP-712 alpha work.
This PR is the single coherent alpha candidate: it preserves the reviewed #621 ancestry, includes #611 and #619, and uses one exact python-keepkey companion pin. The companion workflow will be cross-pinned to this candidate and this PR will then be updated to that exact companion head.
No signing or release is authorized by this PR. Merge is gated on the complete firmware and companion matrices and exact-head review.