Skip to content

Commit ac50dea

Browse files
committed
wallet: Enforce security boundary in ScriptForOutput
The ScriptForOutput function previously returned a `waddrmgr.ManagedPubKeyAddress`. This interface includes a `PrivKey()` method, which meant that a component designed for managing public address information (the AddressManager) was leaking an object that could be used to access private keys. This is a leaky abstraction and violates the principle of separation of concerns. This commit refactors ScriptForOutput to return the more general `waddrmgr.ManagedAddress` interface instead. This interface does not provide access to private key material. This change enforces a clean architectural boundary between the AddressManager (responsible for public data) and the Signer (responsible for private key operations). The Signer is now the sole component responsible for retrieving and using private keys, which is its intended role. This improves the security posture and clarity of the API.
1 parent 42a4716 commit ac50dea

File tree

2 files changed

+32
-12
lines changed

2 files changed

+32
-12
lines changed

wallet/address_manager.go

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ var (
4747
"address is not a p2wkh or np2wkh address",
4848
)
4949

50+
// ErrUnableToExtractAddress is returned when an address cannot be
51+
// extracted from a pkscript.
52+
ErrUnableToExtractAddress = errors.New("unable to extract address")
53+
5054
// errStopIteration is a special error used to stop the iteration in
5155
// ForEachAccountAddress.
5256
errStopIteration = errors.New("stop iteration")
@@ -65,7 +69,7 @@ type AddressProperty struct {
6569
// Script represents the script information required to spend a UTXO.
6670
type Script struct {
6771
// Addr is the managed address of the UTXO.
68-
Addr waddrmgr.ManagedPubKeyAddress
72+
Addr waddrmgr.ManagedAddress
6973

7074
// WitnessProgram is the witness program of the UTXO.
7175
WitnessProgram []byte
@@ -691,20 +695,28 @@ func (w *Wallet) ImportTaprootScript(_ context.Context,
691695
// - The operation is dominated by the database lookup for the address, which
692696
// is typically fast (O(log N) or O(1) with indexing). The script
693697
// generation is a constant-time operation.
694-
func (w *Wallet) ScriptForOutput(_ context.Context, output wire.TxOut) (
698+
func (w *Wallet) ScriptForOutput(ctx context.Context, output wire.TxOut) (
695699
Script, error) {
696700

697-
// First make sure we can sign for the input by making sure the script
698-
// in the UTXO belongs to our wallet and we have the private key for it.
699-
walletAddr, err := w.fetchOutputAddr(output.PkScript)
701+
// First, we'll extract the address from the output's pkScript.
702+
addr := extractAddrFromPKScript(output.PkScript, w.chainParams)
703+
if addr == nil {
704+
return Script{}, fmt.Errorf("%w: from pkscript %x",
705+
ErrUnableToExtractAddress, output.PkScript)
706+
}
707+
708+
// We'll then use the address to look up the managed address from the
709+
// database.
710+
managedAddr, err := w.AddressInfo(ctx, addr)
700711
if err != nil {
701-
return Script{}, err
712+
return Script{}, fmt.Errorf("unable to get address info "+
713+
"for %s: %w", addr.String(), err)
702714
}
703715

704-
pubKeyAddr, ok := walletAddr.(waddrmgr.ManagedPubKeyAddress)
716+
pubKeyAddr, ok := managedAddr.(waddrmgr.ManagedPubKeyAddress)
705717
if !ok {
706-
return Script{}, fmt.Errorf("%w: %s", ErrNotPubKeyAddress,
707-
walletAddr.Address())
718+
return Script{}, fmt.Errorf("%w: addr %s",
719+
ErrNotPubKeyAddress, managedAddr.Address())
708720
}
709721

710722
var (
@@ -715,7 +727,7 @@ func (w *Wallet) ScriptForOutput(_ context.Context, output wire.TxOut) (
715727
switch {
716728
// If we're spending p2wkh output nested within a p2sh output, then
717729
// we'll need to attach a sigScript in addition to witness data.
718-
case walletAddr.AddrType() == waddrmgr.NestedWitnessPubKey:
730+
case managedAddr.AddrType() == waddrmgr.NestedWitnessPubKey:
719731
pubKey := pubKeyAddr.PubKey()
720732
pubKeyHash := btcutil.Hash160(pubKey.SerializeCompressed())
721733

@@ -753,7 +765,7 @@ func (w *Wallet) ScriptForOutput(_ context.Context, output wire.TxOut) (
753765
}
754766

755767
return Script{
756-
Addr: pubKeyAddr,
768+
Addr: managedAddr,
757769
WitnessProgram: witnessProgram,
758770
RedeemScript: sigScript,
759771
}, nil

wallet/deprecated.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package wallet
33

44
import (
55
"context"
6+
"fmt"
67

78
"github.com/btcsuite/btcd/btcutil"
89
"github.com/btcsuite/btcd/chaincfg/chainhash"
@@ -167,7 +168,14 @@ func (w *Wallet) ScriptForOutputDeprecated(output *wire.TxOut) (
167168
return nil, nil, nil, err
168169
}
169170

170-
return script.Addr, script.WitnessProgram, script.RedeemScript, nil
171+
addr := script.Addr
172+
pubKeyAddr, ok := addr.(waddrmgr.ManagedPubKeyAddress)
173+
if !ok {
174+
return nil, nil, nil, fmt.Errorf("%w: addr %s",
175+
ErrNotPubKeyAddress, addr.Address())
176+
}
177+
178+
return pubKeyAddr, script.WitnessProgram, script.RedeemScript, nil
171179
}
172180

173181
// ComputeInputScript generates a complete InputScript for the passed

0 commit comments

Comments
 (0)