diff --git a/deps/python-keepkey b/deps/python-keepkey index 84b4a2503..1f2eecd22 160000 --- a/deps/python-keepkey +++ b/deps/python-keepkey @@ -1 +1 @@ -Subproject commit 84b4a25030763bbe8f84175b12e7a8a245bbb6d2 +Subproject commit 1f2eecd227f73996b1d60af3af1b2972430527ad diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/01-p2tr-address-full-62-chars.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/01-p2tr-address-full-62-chars.png new file mode 100644 index 000000000..e4b9629d5 Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/01-p2tr-address-full-62-chars.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/02-p2wsh-address-full.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/02-p2wsh-address-full.png new file mode 100644 index 000000000..cb81e3ff6 Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/02-p2wsh-address-full.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/03-p2tr-spend-recipient.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/03-p2tr-spend-recipient.png new file mode 100644 index 000000000..a5e653075 Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/03-p2tr-spend-recipient.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/04-p2tr-spend-fee.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/04-p2tr-spend-fee.png new file mode 100644 index 000000000..6af0534f8 Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/04-p2tr-spend-fee.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/05-p2tr-change-recipient.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/05-p2tr-change-recipient.png new file mode 100644 index 000000000..c3ee4d3d6 Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/05-p2tr-change-recipient.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/06-p2tr-change-fee.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/06-p2tr-change-fee.png new file mode 100644 index 000000000..ad719d2cb Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/06-p2tr-change-fee.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/07-mixed-p2tr-legacy-recipient.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/07-mixed-p2tr-legacy-recipient.png new file mode 100644 index 000000000..07e407309 Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/07-mixed-p2tr-legacy-recipient.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/08-mixed-p2tr-legacy-fee.png b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/08-mixed-p2tr-legacy-fee.png new file mode 100644 index 000000000..a97901939 Binary files /dev/null and b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/08-mixed-p2tr-legacy-fee.png differ diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/README.md b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/README.md new file mode 100644 index 000000000..fbec1f746 --- /dev/null +++ b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/README.md @@ -0,0 +1,69 @@ +# 7.15.0-rc26 — Taproot witness serialization and long-address display + +Gate-3 OLED evidence for the two defects rc25 shipped, both fixed in rc26. + +Captured from the emulator built at this branch, driven by python-keepkey with +`KEEPKEY_SCREENSHOT=1`. Seed throughout is the standard BIP-39 test vector +`abandon abandon ... about`, so every address below is a published BIP-86 +value and can be checked independently. + +> Emulator captures. Per the release SOP these do **not** substitute for +> on-device Gate-3 proof; they establish that the rendering logic is correct +> before anyone picks up a device. + +## What rc25 got wrong + +**1. Addresses longer than one line were silently truncated.** +`draw_string()` stopped at the bottom of the canvas and dropped the rest, so the +user verified a 42-character prefix while the QR code beside it encoded the whole +address — the two disagreed with no indication. This became urgent the moment +rc25 shipped Taproot, because every `bc1p` address is 62 characters. + +`01` shows `m/86'/0'/0'/0/0` rendering the complete +`bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr` +across two lines, matching the QR. `02` shows the same fix for P2WSH multisig, +which had the identical defect before Taproot existed. + +**2. The witness and locktime never reached the host.** +The Taproot signing branch set `has_signature` but not `has_serialized_tx`, so +nanopb omitted `serialized_tx` entirely and 70 bytes — the 66-byte witness plus +the 4-byte locktime footer — were dropped on the wire. The signature itself was +always correct, which is why every test passed. + +`03`–`08` are the confirm screens for the three signing flows that now also +assert the full BIP-144 serialization. They prove the amount, destination and +fee shown to the user are unchanged by that fix. + +## Frames + +| File | Screen | +|---|---| +| `01-p2tr-address-full-62-chars.png` | BIP-86 receive address, complete, with QR | +| `02-p2wsh-address-full.png` | P2WSH multisig address, complete | +| `03-p2tr-spend-recipient.png` | `Send 0.0009 BTC to 1BitcoinEater…` | +| `04-p2tr-spend-fee.png` | Total `0.001 BTC`, fee `0.0001 BTC` | +| `05-p2tr-change-recipient.png` | Recipient, device-derived P2TR change omitted | +| `06-p2tr-change-fee.png` | Fee confirmation, change flow | +| `07-mixed-p2tr-legacy-recipient.png` | Recipient, mixed Taproot + legacy inputs | +| `08-mixed-p2tr-legacy-fee.png` | Fee confirmation, mixed inputs | + +## Reproducing + +```sh +cd scripts/emulator +docker compose build kkemu && docker compose up -d kkemu +docker compose run --rm -v "$PWD/out:/out" --entrypoint /bin/sh python-keepkey -c ' + cd /kkemu/deps/python-keepkey/tests && export PYTHONPATH=".." + KEEPKEY_SCREENSHOT=1 SCREENSHOT_DIR=/out \ + KK_TRANSPORT_MAIN=kkemu:11044 KK_TRANSPORT_DEBUG=kkemu:11045 \ + python3 -m pytest test_msg_signtx_taproot.py test_msg_getaddress_taproot.py \ + test_taproot_screens.py -q' +``` + +Frames land in `$SCREENSHOT_DIR///btnNNNNN.png`. + +## Still owed + +On-device Gate-3 for both flows: display a `bc1p` receive address and compare it +character-for-character against the QR, then spend a P2TR input and confirm the +amount, destination and fee screens appear and require a press. diff --git a/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/manifest.json b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/manifest.json new file mode 100644 index 000000000..a7e0d082f --- /dev/null +++ b/docs/security/evidence/7.15.0-rc26-taproot-witness-and-address/manifest.json @@ -0,0 +1,92 @@ +{ + "candidate": "7.15.0-rc26", + "seed": "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about", + "capture": { + "source": "emulator", + "method": "python-keepkey with KEEPKEY_SCREENSHOT=1", + "note": "Emulator captures. Per the release SOP these do not substitute for on-device Gate-3 proof." + }, + "defects_covered": [ + { + "pr": 331, + "title": "addresses longer than one line were silently truncated", + "detail": "draw_string() stopped at the bottom of the canvas and dropped the remainder, so the rendered text and the QR code disagreed with no indication.", + "frames": [ + "01-p2tr-address-full-62-chars.png", + "02-p2wsh-address-full.png" + ] + }, + { + "pr": 337, + "title": "taproot witness and locktime footer never reached the host", + "detail": "The taproot signing branch set has_signature but not has_serialized_tx, so nanopb omitted serialized_tx and 70 bytes (66-byte witness + 4-byte locktime) were dropped on the wire.", + "frames": [ + "03-p2tr-spend-recipient.png", + "04-p2tr-spend-fee.png", + "05-p2tr-change-recipient.png", + "06-p2tr-change-fee.png", + "07-mixed-p2tr-legacy-recipient.png", + "08-mixed-p2tr-legacy-fee.png" + ] + } + ], + "verified_values": { + "bip86_receive_address_m_86h_0h_0h_0_0": "bc1p5cyxnuxmeuwuvkwfem96lqzszd02n6xdcjrs20cac6yqjjwudpxqkedrcr", + "bip86_address_characters": 62, + "serialized_tx_bytes": { + "p2tr": 153, + "p2tr_with_change": 196, + "mixed_p2tr_and_legacy": 301 + } + }, + "tests": { + "files": [ + "test_msg_signtx_taproot.py", + "test_msg_getaddress_taproot.py", + "test_taproot_screens.py" + ], + "result": "9 passed" + }, + "frames": [ + { + "file": "01-p2tr-address-full-62-chars.png", + "sha256": "b1a4174f8e4c88b30a41ce337d0e45ded840c104f6637dcc98a7dfc89d27e073", + "bytes": 996 + }, + { + "file": "02-p2wsh-address-full.png", + "sha256": "7cf807b25d8a2a6a67bc83f887184b89221a0bfd9d8a2a24872d3bc954669b17", + "bytes": 1057 + }, + { + "file": "03-p2tr-spend-recipient.png", + "sha256": "91390aec2d647e5dcd5c9cfc767e72fcc62a41e710aae9d8d42abe4af9451fd5", + "bytes": 574 + }, + { + "file": "04-p2tr-spend-fee.png", + "sha256": "c635bd2cb4c52b4553e4ad0ccbf53ae0e6fa3c61f9dbe60bfbf43d18692228fb", + "bytes": 793 + }, + { + "file": "05-p2tr-change-recipient.png", + "sha256": "fb1e137d2463b5ba44decd151f5380f649b3e884323c7c54f6f66e87cc3288a0", + "bytes": 579 + }, + { + "file": "06-p2tr-change-fee.png", + "sha256": "7260c9c0ee5cc85c61356b41b91aa4d30a091bf1be60b7516bff0e4b25b225aa", + "bytes": 790 + }, + { + "file": "07-mixed-p2tr-legacy-recipient.png", + "sha256": "6964d237e5208ec5cf2ca03ca35a3515cbd522eb4d0799e372285b5262cf1f6c", + "bytes": 587 + }, + { + "file": "08-mixed-p2tr-legacy-fee.png", + "sha256": "8ff0582a2bb578485b1c4973f042f8f7e1454473b7995174afdfb4e72cb56f1e", + "bytes": 796 + } + ] +} diff --git a/lib/firmware/signing.c b/lib/firmware/signing.c index ec8dfdd67..bc66580ed 100644 --- a/lib/firmware/signing.c +++ b/lib/firmware/signing.c @@ -1437,6 +1437,11 @@ static bool signing_sign_segwit_input(TxInputType* txinput) { resp.serialized.has_signature_index = true; resp.serialized.signature_index = idx1; resp.serialized.has_signature = true; + /* signing_txack() memsets resp, and this branch bypasses + signing_sign_hash(), which is where every other input type sets this. + Without it nanopb omits serialized_tx and the host loses the witness + and the tx footer. */ + resp.serialized.has_serialized_tx = true; resp.serialized.signature.size = 64; memcpy(resp.serialized.signature.bytes, sig, 64);