feat(abstract-utxo): add safe signing helpers for external PSBTs - #9856
ralph-bitgo[bot] wants to merge 2 commits into
Conversation
Add signExternalPsbt and assertExternalPsbtSighashPolicy to the recovery module. signExternalPsbt signs an externally supplied (untrusted) PSBT with a single key under the SIGHASH_ALL-only policy: it enforces the wasm-utxo sighash policy before signing, verifies the output set is byte-identical to a pre-signing snapshot, re-checks the policy on the re-parsed serialized result, and determines the signed inputs by verifying the signer's signature on each input (BitGoPsbt.sign reports attempted inputs, so a key matching no input still returns indexes). assertExternalPsbtSighashPolicy runs the policy check alone, for callers that hand the PSBT to a different signer (e.g. the SDK signer). Why: wallet recovery tooling signs externally pasted PSBTs with user keys, and a foreign PSBT can request SIGHASH_NONE/SINGLE/ANYONECANPAY per input so the user signature does not bind to the recipient output - the output-swap drain from WCN-1994. Consumers (wallet-recovery-wizard) get the complete policy from the SDK instead of reimplementing it, with network-aware rules (BCH FORKID, Taproot SIGHASH_DEFAULT) owned by wasm-utxo. Requires the wasm-utxo sighash policy primitives; the yarn.lock bump to the published wasm-utxo release follows after the BitGoWASM PR lands. Ticket: WCN-1994 Session-Id: 5c05e047-31d0-43fa-91ab-f1c595b37850 Task-Id: f7cdbdd7-f9f0-4164-a69b-0c7e58e609a8
e20f799 to
6c4ab26
Compare
|
CI status: the failing checks fail only on the documented stack dependency, not on this diff's logic. Root cause of every red check: CI installs That method is added by the stack's part 1: BitGo/BitGoWASM#416 (CI fully green there, including the wasm-pack node/chrome legs). Evidence the code itself is sound: the same 14 unit tests ( To green: merge BitGoWASM#416 → let release publish Ticket: WCN-1994 |
|
|
|
Update on the failing tests: resolved — BitGoWASM#416 merged and published CI on 9aa397a is now fully green: Merge order remainder: merge this PR → release publishes Ticket: WCN-1994 |
Update the lockfile to the wasm-utxo release that publishes the sighash policy primitives (BitGoPsbt.assertSighashAllPolicy / getInputSighashTypes, BitGoWASM#416). Raise the @bitgo/wasm-utxo dependency range to ^5.6.0 in every module that declares it (abstract-utxo, utxo-bin, utxo-core, utxo-descriptors, utxo-ord, utxo-staking): signExternalPsbt requires the new primitives, so the previous ^5.5.1 range would still install a release without them, and one consistent minimum avoids mixing the old and new ranges across the monorepo. Why: the CI failures on this branch came from the lockfile resolving wasm-utxo 5.5.1, which predates the primitives; with 5.6.0 published, the full suite can build and test against the real release. Ticket: WCN-1994 Session-Id: 5c05e047-31d0-43fa-91ab-f1c595b37850 Task-Id: f7cdbdd7-f9f0-4164-a69b-0c7e58e609a8 Requested-By: Otto Allmendinger <otto@bitgo.com>
9aa397a to
7d14e06
Compare
What
signExternalPsbt(psbtHex, coinName, signer)to@bitgo/abstract-utxo(recovery module): signs an externally supplied (untrusted) PSBT with a single key under the SIGHASH_ALL-only policy. It enforces the sighash policy before signing, verifies the output set is byte-identical to a pre-signing snapshot, re-checks the policy on the re-parsed serialized result, and determines the signed inputs by verifying the signer's signature on every input (BitGoPsbt.signreports attempted inputs — a key matching no input still returns indexes — so a key that signs nothing is rejected explicitly).assertExternalPsbtSighashPolicy(psbtHex, coinName)for callers that hand the PSBT to a different signer (e.g. the SDK signer): runs the policy check alone.UtxoCoinNames (normalized via the existingtoWasmUtxoCoinName) and are exported from the package root.test/unit/recovery/signExternalPsbt.ts: signed happy path (single + multi input, xprv string and BIP32 instance signers), all unsafe modes rejected before signing, unsafe type on a later input named in the error, omitted type accepted, non-matching signer rejected, BCHSIGHASH_ALL|FORKIDaccepted, and a PSBT already carrying a non-SIGHASH_ALL signature rejected.Why
Wallet-recovery tooling signs externally pasted PSBTs with user keys. A foreign PSBT can request
SIGHASH_NONE/SINGLE/ANYONECANPAYper input so the user signature does not bind to the recipient output — the output-swap drain demonstrated in AnchorWatch finding SIG-001 (WCN-1994). Consumers (wallet-recovery-wizard) should get the complete policy from the SDK instead of reimplementing it per app, with the network-aware rules (BCH FORKID, Taproot SIGHASH_DEFAULT) owned by wasm-utxo.Test plan
tsc --noEmitclean (module + tests)yarn.lockbumped to the published@bitgo/wasm-utxo@5.6.0(BitGoWASM#416, merged and released); abstract-utxo dependency range raised to^5.6.0becausesignExternalPsbtrequires the new primitives — 9aa397aBitGo SDK(all unit-test/browser-test/docker-build/all-checks jobs) andAudit API Specboth passStack
This PR is part 2 of 3 in the WCN-1994 stack. Review and merge in order:
@bitgo/wasm-utxo— sighash policy primitives (BitGoPsbt.assertSighashAllPolicy/getInputSighashTypes) — feat(wasm-utxo): add sighash policy primitives to BitGoPsbt BitGoWASM#416@bitgo/abstract-utxo≥ 13.3.0)Step 1 is merged and published
@bitgo/wasm-utxo@5.6.0; the lockfile bump is included here (9aa397a). After this PR merges and publishes@bitgo/abstract-utxo≥ 13.3.0, refreshpackage-lock.jsonin part 3.Ticket: WCN-1994