Skip to content

Commit 0f526b5

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 492b2a2 commit 0f526b5

File tree

2 files changed

+31
-17
lines changed

2 files changed

+31
-17
lines changed

wallet/address_manager.go

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,9 @@ var (
4141
ErrImportedAccountNoAddrGen = errors.New("addresses cannot be " +
4242
"generated for the default imported account")
4343

44-
// ErrNotPubKeyAddress is an error returned when a function requires a
45-
// public key address, but a different type of address is provided.
46-
ErrNotPubKeyAddress = errors.New(
47-
"address is not a p2wkh or np2wkh address",
48-
)
44+
// ErrUnableToExtractAddress is returned when an address cannot be
45+
// extracted from a pkscript.
46+
ErrUnableToExtractAddress = errors.New("unable to extract address")
4947

5048
// errStopIteration is a special error used to stop the iteration in
5149
// ForEachAccountAddress.
@@ -65,7 +63,7 @@ type AddressProperty struct {
6563
// Script represents the script information required to spend a UTXO.
6664
type Script struct {
6765
// Addr is the managed address of the UTXO.
68-
Addr waddrmgr.ManagedPubKeyAddress
66+
Addr waddrmgr.ManagedAddress
6967

7068
// WitnessProgram is the witness program of the UTXO.
7169
WitnessProgram []byte
@@ -691,20 +689,28 @@ func (w *Wallet) ImportTaprootScript(_ context.Context,
691689
// - The operation is dominated by the database lookup for the address, which
692690
// is typically fast (O(log N) or O(1) with indexing). The script
693691
// generation is a constant-time operation.
694-
func (w *Wallet) ScriptForOutput(_ context.Context, output wire.TxOut) (
692+
func (w *Wallet) ScriptForOutput(ctx context.Context, output wire.TxOut) (
695693
Script, error) {
696694

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)
695+
// First, we'll extract the address from the output's pkScript.
696+
addr := extractAddrFromPKScript(output.PkScript, w.chainParams)
697+
if addr == nil {
698+
return Script{}, fmt.Errorf("%w: from pkscript %x",
699+
ErrUnableToExtractAddress, output.PkScript)
700+
}
701+
702+
// We'll then use the address to look up the managed address from the
703+
// database.
704+
managedAddr, err := w.AddressInfo(ctx, addr)
700705
if err != nil {
701-
return Script{}, err
706+
return Script{}, fmt.Errorf("unable to get address info "+
707+
"for %s: %w", addr.String(), err)
702708
}
703709

704-
pubKeyAddr, ok := walletAddr.(waddrmgr.ManagedPubKeyAddress)
710+
pubKeyAddr, ok := managedAddr.(waddrmgr.ManagedPubKeyAddress)
705711
if !ok {
706-
return Script{}, fmt.Errorf("%w: %s", ErrNotPubKeyAddress,
707-
walletAddr.Address())
712+
return Script{}, fmt.Errorf("%w: addr %s",
713+
ErrNotPubKeyAddress, managedAddr.Address())
708714
}
709715

710716
var (
@@ -715,7 +721,7 @@ func (w *Wallet) ScriptForOutput(_ context.Context, output wire.TxOut) (
715721
switch {
716722
// If we're spending p2wkh output nested within a p2sh output, then
717723
// we'll need to attach a sigScript in addition to witness data.
718-
case walletAddr.AddrType() == waddrmgr.NestedWitnessPubKey:
724+
case managedAddr.AddrType() == waddrmgr.NestedWitnessPubKey:
719725
pubKey := pubKeyAddr.PubKey()
720726
pubKeyHash := btcutil.Hash160(pubKey.SerializeCompressed())
721727

@@ -753,7 +759,7 @@ func (w *Wallet) ScriptForOutput(_ context.Context, output wire.TxOut) (
753759
}
754760

755761
return Script{
756-
Addr: pubKeyAddr,
762+
Addr: managedAddr,
757763
WitnessProgram: witnessProgram,
758764
RedeemScript: sigScript,
759765
}, 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)