From ac372fcdee410e75bab4e9a3979011b3b4a2a70d Mon Sep 17 00:00:00 2001 From: benma's agent Date: Thu, 1 Oct 2026 23:46:51 +0200 Subject: [PATCH 1/2] ci: disable goconst in tests Repeated literals keep test inputs and expected results readable. Exclude goconst from all _test.go files while retaining it for production code. --- .golangci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yml b/.golangci.yml index 332fdec..dc8536b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -81,6 +81,7 @@ linters: rules: - linters: - errcheck + - goconst - gocyclo - godot - gosec From ec282be02565d79285902be6debbf9fe8eb9b75a Mon Sep 17 00:00:00 2001 From: benma's agent Date: Sun, 4 Oct 2026 10:21:54 +0200 Subject: [PATCH 2/2] btc: require verified silent payment outputs BTCSign could return successfully when the device omitted a generated silent payment script or skipped the output exchange entirely. In both cases the host skipped proof verification and left the original PSBT output script unchanged. Require a generated script for each silent payment output exchange, verify it before recording it, and reject completion unless every requested silent payment has a verified output. Reject generated silent payment data on ordinary outputs as well. Add regressions for omitted exchanges, missing or invalid proofs, duplicate output requests, and valid mixed outputs. Keep the shared firmware vectors unchanged and handle older firmware's acceptance of silent payment metadata on owned outputs in the Go runner, expecting the host to reject the missing script. Failed signing must leave PSBT signatures and output scripts unchanged. --- api/firmware/btc.go | 23 ++- api/firmware/btc_silentpayments_test.go | 191 +++++++++++++++++++ api/firmware/btc_transaction_vectors_test.go | 12 +- 3 files changed, 221 insertions(+), 5 deletions(-) create mode 100644 api/firmware/btc_silentpayments_test.go diff --git a/api/firmware/btc.go b/api/firmware/btc.go index a79d429..9faaaec 100644 --- a/api/firmware/btc.go +++ b/api/firmware/btc.go @@ -346,7 +346,7 @@ type BTCTx struct { type BTCSignResult struct { // Signatures contains the input signatures. One 64 byte signature per input. Signatures [][]byte - // GeneratedOutputs contains the outputs generated (silent payments). The map key is the input + // GeneratedOutputs contains the outputs generated (silent payments). The map key is the output // index, the map value is the generated pkScript. GeneratedOutputs map[int][]byte } @@ -523,15 +523,18 @@ func (device *Device) nonAtomicBTCSign( } case messages.BTCSignNextResponse_OUTPUT: outputIndex := next.Index + output := tx.Outputs[outputIndex] next, err = device.nonAtomicQueryBtcSign(&messages.Request{ Request: &messages.Request_BtcSignOutput{ - BtcSignOutput: tx.Outputs[outputIndex], + BtcSignOutput: output, }}) if err != nil { return nil, err } - if next.GeneratedOutputPkscript != nil { - generatedOutputs[int(outputIndex)] = next.GeneratedOutputPkscript + if output.SilentPayment != nil { + if len(next.GeneratedOutputPkscript) == 0 { + return nil, errp.New("missing generated silent payment output") + } err := silentPaymentOutputVerify( tx, int(outputIndex), @@ -541,6 +544,9 @@ func (device *Device) nonAtomicBTCSign( if err != nil { return nil, err } + generatedOutputs[int(outputIndex)] = next.GeneratedOutputPkscript + } else if len(next.GeneratedOutputPkscript) != 0 || len(next.SilentPaymentDleqProof) != 0 { + return nil, errp.New("unexpected silent payment output data") } case messages.BTCSignNextResponse_PAYMENT_REQUEST: paymentRequestIndex := next.Index @@ -559,6 +565,15 @@ func (device *Device) nonAtomicBTCSign( return nil, err } case messages.BTCSignNextResponse_DONE: + // The device may skip output requests entirely, so check completeness against the + // transaction's requested silent payments before returning any signing result. + for i, output := range tx.Outputs { + if output.SilentPayment != nil { + if _, ok := generatedOutputs[i]; !ok { + return nil, errp.New("missing verified silent payment output") + } + } + } return &BTCSignResult{ Signatures: signatures, GeneratedOutputs: generatedOutputs, diff --git a/api/firmware/btc_silentpayments_test.go b/api/firmware/btc_silentpayments_test.go new file mode 100644 index 0000000..f82d8ee --- /dev/null +++ b/api/firmware/btc_silentpayments_test.go @@ -0,0 +1,191 @@ +// SPDX-License-Identifier: Apache-2.0 + +package firmware + +import ( + "bytes" + "testing" + + "github.com/BitBoxSwiss/bitbox02-api-go/api/common" + "github.com/BitBoxSwiss/bitbox02-api-go/api/firmware/messages" + "github.com/BitBoxSwiss/bitbox02-api-go/api/firmware/mocks" + "github.com/BitBoxSwiss/bitbox02-api-go/util/semver" + "github.com/btcsuite/btcd/address/v2/bech32" + "github.com/btcsuite/btcd/btcec/v2" + "github.com/btcsuite/btcd/txscript/v2" + "github.com/stretchr/testify/require" + "google.golang.org/protobuf/proto" +) + +func TestBTCSignSilentPaymentCompleteness(t *testing.T) { + // Reuse the independently generated proof from TestDLEQVerify. The input key is p1, + // the scan key is gen2, and the shared secret included with the proof is p2. + inputKey, err := btcec.ParsePubKey(unhex("021b8594084a12da837a78f9337e3d29169025a8ddbe9f9933a131607c6e62d21e")) + require.NoError(t, err) + scanKey := unhex("0389140f7bb852f020f154e55908fe3699dc9f65153e681527f0d55aabed937f4b") + sharedSecret := unhex("03019807775b92c83d24e3001b55f60373e0bdc7d7cc7bc86befecf5bb987b54ac") + proof := append(bytes.Clone(sharedSecret), unhex("6c885f825f6ce7565bc6d0bfda90506b11e2682dfe943f5a85badf1c8a96edc5f5e03f5ee2c58bf979646fbada920f9f1c5bd92805fb5b01534b42d26a550f79")...) + // Use the input key as the recipient's spend key for this protocol fixture. + addressData, err := bech32.ConvertBits(append(scanKey, inputKey.SerializeCompressed()...), 8, 5, true) + require.NoError(t, err) + address, err := bech32.EncodeM("sp", append([]byte{0}, addressData...)) + require.NoError(t, err) + silentOutput := &messages.BTCSignOutputRequest{ + SilentPayment: &messages.BTCSignOutputRequest_SilentPayment{Address: address}, + Value: 1000, + } + ordinaryOutput := &messages.BTCSignOutputRequest{Value: 2000} + input := &BTCTxInput{ + Input: &messages.BTCSignInputRequest{ + PrevOutHash: bytes.Repeat([]byte{1}, 32), + }, + BIP352Pubkey: inputKey.SerializeCompressed(), + } + fixtureTx := &BTCTx{Inputs: []*BTCTxInput{input}} + cPub, err := btcec.ParsePubKey(sharedSecret) + require.NoError(t, err) + ecdh := scalarMult(bip352SmallestOutpointHash(fixtureTx, inputKey), cPub) + outputKey := pubkeyAdd(inputKey, scalarBaseMult(bip352CalculateTk(ecdh, 0))) + script, err := txscript.PayToTaprootScript(outputKey) + require.NoError(t, err) + invalidProof := bytes.Clone(proof) + invalidProof[len(invalidProof)-1] ^= 1 + wrongScript := bytes.Clone(script) + wrongScript[len(wrongScript)-1] ^= 1 + + type outputExchange struct { + index uint32 + script []byte + proof []byte + } + ordinary := outputExchange{index: 0} + valid := outputExchange{index: 1, script: script, proof: proof} + for _, test := range []struct { + name string + outputs []*messages.BTCSignOutputRequest + exchanges []outputExchange + expectedError string + }{ + { + name: "ordinary output", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput}, + exchanges: []outputExchange{ordinary}, + }, + { + name: "valid mixed outputs", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{ordinary, valid}, + }, + { + name: "valid multiple silent payments", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput, silentOutput}, + exchanges: []outputExchange{ + ordinary, valid, {index: 2, script: script, proof: proof}, + }, + }, + { + name: "immediate completion", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + expectedError: "missing verified silent payment output", + }, + { + name: "silent payment exchange omitted", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{ordinary}, + expectedError: "missing verified silent payment output", + }, + { + name: "duplicate does not cover another silent payment", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput, silentOutput}, + exchanges: []outputExchange{ordinary, valid, valid}, + expectedError: "missing verified silent payment output", + }, + { + name: "generated output and proof omitted", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{{index: 1}}, + expectedError: "missing generated silent payment output", + }, + { + name: "generated output empty", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{{index: 1, script: []byte{}, proof: proof}}, + expectedError: "missing generated silent payment output", + }, + { + name: "proof omitted", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{{index: 1, script: script}}, + expectedError: "wrong DLEQ proof size", + }, + { + name: "proof truncated", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{{index: 1, script: script, proof: proof[:len(proof)-1]}}, + expectedError: "wrong DLEQ proof size", + }, + { + name: "invalid proof", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{{index: 1, script: script, proof: invalidProof}}, + expectedError: "DLEQ proof verification failed", + }, + { + name: "incorrect script", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput, silentOutput}, + exchanges: []outputExchange{{index: 1, script: wrongScript, proof: proof}}, + expectedError: "incorrect silent payment output", + }, + { + name: "generated data for ordinary output", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput}, + exchanges: []outputExchange{{index: 0, script: script, proof: proof}}, + expectedError: "unexpected silent payment output data", + }, + { + name: "proof alone for ordinary output", + outputs: []*messages.BTCSignOutputRequest{ordinaryOutput}, + exchanges: []outputExchange{{index: 0, proof: proof}}, + expectedError: "unexpected silent payment output data", + }, + } { + t.Run(test.name, func(t *testing.T) { + step := 0 + device := newDevice(t, semver.NewSemVer(9, 21, 0), common.ProductBitBox02Multi, + &mocks.Communication{}, func(request *messages.Request) *messages.Response { + next := &messages.BTCSignNextResponse{Type: messages.BTCSignNextResponse_DONE} + if step == 0 { + require.NotNil(t, request.GetBtcSignInit()) + } else { + exchange := test.exchanges[step-1] + require.True(t, proto.Equal(test.outputs[exchange.index], request.GetBtcSignOutput())) + next.GeneratedOutputPkscript = exchange.script + next.SilentPaymentDleqProof = exchange.proof + } + if step < len(test.exchanges) { + next.Type = messages.BTCSignNextResponse_OUTPUT + next.Index = test.exchanges[step].index + } + step++ + return &messages.Response{Response: &messages.Response_BtcSignNext{BtcSignNext: next}} + }) + result, err := device.BTCSign(messages.BTCCoin_BTC, nil, nil, + &BTCTx{Version: 2, Inputs: []*BTCTxInput{input}, Outputs: test.outputs}, + messages.BTCSignInitRequest_DEFAULT) + if test.expectedError != "" { + require.ErrorContains(t, err, test.expectedError) + require.Nil(t, result) + } else { + require.NoError(t, err) + expected := map[int][]byte{} + for i, output := range test.outputs { + if output.SilentPayment != nil { + expected[i] = script + } + } + require.Equal(t, expected, result.GeneratedOutputs) + } + require.Equal(t, len(test.exchanges)+1, step) + }) + } +} diff --git a/api/firmware/btc_transaction_vectors_test.go b/api/firmware/btc_transaction_vectors_test.go index 86cb44b..b0df02a 100644 --- a/api/firmware/btc_transaction_vectors_test.go +++ b/api/firmware/btc_transaction_vectors_test.go @@ -658,11 +658,19 @@ func changedBTCVectorPSBTOutputs(before [][]byte, packet *psbt.Packet) map[int]s func assertBTCVectorSignOutcome( t *testing.T, err error, + vectorID string, expectation *btcTestVectorVersionExpectation, ) bool { t.Helper() switch expectation.Outcome { case btcTestVectorOutcomeSuccess: + if vectorID == "silent-payment-owned-output" { + // The shared vectors describe firmware behavior. Older firmware accepts silent + // payment metadata on owned outputs without returning a generated script, but + // the Go client requires a verified script for every requested silent payment. + require.EqualError(t, err, "missing generated silent payment output") + return false + } require.NoError(t, err) return true case btcTestVectorOutcomeUnsupported: @@ -748,13 +756,15 @@ func runBTCPSBTTestVector( err = device.BTCSignPSBT(coin, packet, options) afterSignatures, signaturesErr := btcTestVectorObservedSignatureSlots(packet) require.NoError(t, signaturesErr) - if !assertBTCVectorSignOutcome(t, err, expectation) { + if !assertBTCVectorSignOutcome(t, err, vector.ID, expectation) { require.ElementsMatch( t, beforeSignatures, afterSignatures, "failed signing changed PSBT signatures", ) + require.Empty(t, changedBTCVectorPSBTOutputs(outputScriptsBefore, packet), + "failed signing changed PSBT outputs") return }