diff --git a/staticaddr/address/manager.go b/staticaddr/address/manager.go index e96d362bf..3382210d2 100644 --- a/staticaddr/address/manager.go +++ b/staticaddr/address/manager.go @@ -11,6 +11,7 @@ import ( "github.com/btcsuite/btcd/btcec/v2/schnorr" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" + "github.com/btcsuite/btcd/wire" "github.com/lightninglabs/lndclient" "github.com/lightninglabs/loop/staticaddr/script" "github.com/lightninglabs/loop/staticaddr/version" @@ -21,6 +22,13 @@ import ( "github.com/lightningnetwork/lnd/lnwallet" ) +const ( + // maxStaticAddressCSVExpiry is the maximum CSV delay that we accept + // from the server for a static address timeout path: 200 days at 144 + // blocks per day. + maxStaticAddressCSVExpiry = uint32(200 * 144) +) + // ManagerConfig holds the configuration for the address manager. type ManagerConfig struct { // AddressClient is the client that communicates with the loop server @@ -158,9 +166,16 @@ func (m *Manager) NewAddress(ctx context.Context) (*btcutil.AddressTaproot, return nil, 0, err } + if resp == nil { + return nil, 0, fmt.Errorf("missing server new address response") + } + serverParams := resp.GetParams() + if err := validateServerAddressParams(serverParams); err != nil { + return nil, 0, err + } - serverPubKey, err := btcec.ParsePubKey(serverParams.ServerKey) + serverPubKey, err := btcec.ParsePubKey(serverParams.GetServerKey()) if err != nil { return nil, 0, err } @@ -222,6 +237,41 @@ func (m *Manager) NewAddress(ctx context.Context) (*btcutil.AddressTaproot, return address, int64(serverParams.Expiry), nil } +// validateServerAddressParams validates the server-controlled static address +// parameters before they are committed into the address script or database. +func validateServerAddressParams( + params *staticaddressrpc.ServerAddressParameters) error { + + if params == nil { + return fmt.Errorf("missing server address parameters") + } + + serverKey := params.GetServerKey() + if len(serverKey) == 0 { + return fmt.Errorf("missing server public key") + } + if !btcec.IsCompressedPubKey(serverKey) { + return fmt.Errorf("server public key is not a compressed " + + "secp256k1 public key") + } + + expiry := params.GetExpiry() + switch { + case expiry == 0: + return fmt.Errorf("static address CSV expiry must be non-zero") + + case expiry&^wire.SequenceLockTimeMask != 0: + return fmt.Errorf("static address expiry does not fit into "+ + "CSV: %x", expiry) + + case expiry > maxStaticAddressCSVExpiry: + return fmt.Errorf("static address CSV expiry %v exceeds "+ + "maximum %v", expiry, maxStaticAddressCSVExpiry) + } + + return nil +} + // GetTaprootAddress returns a taproot address for the given client and server // public keys and expiry. func (m *Manager) GetTaprootAddress(clientPubkey, serverPubkey *btcec.PublicKey, diff --git a/staticaddr/address/manager_test.go b/staticaddr/address/manager_test.go index c23f246cd..5881bf848 100644 --- a/staticaddr/address/manager_test.go +++ b/staticaddr/address/manager_test.go @@ -8,6 +8,7 @@ import ( "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcec/v2/schnorr" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/wire" "github.com/lightninglabs/loop/loopdb" "github.com/lightninglabs/loop/staticaddr/script" "github.com/lightninglabs/loop/swap" @@ -93,8 +94,9 @@ func (m *mockStaticAddressClient) ServerNewAddress(ctx context.Context, args := m.Called(ctx, in, opts) - return args.Get(0).(*swapserverrpc.ServerNewAddressResponse), - args.Error(1) + resp, _ := args.Get(0).(*swapserverrpc.ServerNewAddressResponse) + + return resp, args.Error(1) } // TestManager tests the static address manager generates the corerct static @@ -128,6 +130,100 @@ func TestManager(t *testing.T) { require.EqualValues(t, defaultExpiry, expiry) } +// TestNewAddressValidatesServerResponse tests that the untrusted +// ServerNewAddress response is validated before the address script is created. +func TestNewAddressValidatesServerResponse(t *testing.T) { + tests := []struct { + name string + resp *swapserverrpc.ServerNewAddressResponse + expected string + }{ + { + name: "nil response", + expected: "missing server new address response", + }, + { + name: "nil params", + resp: &swapserverrpc.ServerNewAddressResponse{}, + expected: "missing server address parameters", + }, + { + name: "missing server key", + resp: &swapserverrpc.ServerNewAddressResponse{ + Params: &swapserverrpc.ServerAddressParameters{ + Expiry: defaultExpiry, + }, + }, + expected: "missing server public key", + }, + { + name: "uncompressed server key", + resp: &swapserverrpc.ServerNewAddressResponse{ + Params: &swapserverrpc.ServerAddressParameters{ + ServerKey: []byte{0x04}, + Expiry: defaultExpiry, + }, + }, + expected: "server public key is not a compressed", + }, + { + name: "zero expiry", + resp: newServerNewAddressResponse(0), + expected: "static address CSV expiry must be non-zero", + }, + { + name: "seconds flag", + resp: newServerNewAddressResponse( + wire.SequenceLockTimeIsSeconds | 1, + ), + expected: "static address expiry does not fit into CSV", + }, + { + name: "disabled flag", + resp: newServerNewAddressResponse( + wire.SequenceLockTimeDisabled | 1, + ), + expected: "static address expiry does not fit into CSV", + }, + { + name: "reserved flag", + resp: newServerNewAddressResponse( + wire.SequenceLockTimeMask + 1, + ), + expected: "static address expiry does not fit into CSV", + }, + { + name: "too large", + resp: newServerNewAddressResponse( + maxStaticAddressCSVExpiry + 1, + ), + expected: "exceeds maximum", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + testContext := NewAddressManagerTestContextWithResponse( + t, test.resp, + ) + + _, _, err := testContext.manager.NewAddress(t.Context()) + require.ErrorContains(t, err, test.expected) + }) + } +} + +// TestNewAddressAcceptsMaxCSVExpiry tests the upper valid CSV boundary. +func TestNewAddressAcceptsMaxCSVExpiry(t *testing.T) { + testContext := NewAddressManagerTestContextWithResponse( + t, newServerNewAddressResponse(maxStaticAddressCSVExpiry), + ) + + _, expiry, err := testContext.manager.NewAddress(t.Context()) + require.NoError(t, err) + require.EqualValues(t, maxStaticAddressCSVExpiry, expiry) +} + // GenerateExpectedTaprootAddress generates the expected taproot address that // the predefined parameters are supposed to generate. func GenerateExpectedTaprootAddress(t *ManagerTestContext) ( @@ -170,6 +266,16 @@ type ManagerTestContext struct { // NewAddressManagerTestContext creates a new test context for the static // address manager. func NewAddressManagerTestContext(t *testing.T) *ManagerTestContext { + return NewAddressManagerTestContextWithResponse( + t, newServerNewAddressResponse(defaultExpiry), + ) +} + +// NewAddressManagerTestContextWithResponse creates a new test context with a +// custom ServerNewAddress response. +func NewAddressManagerTestContextWithResponse(t *testing.T, + resp *swapserverrpc.ServerNewAddressResponse) *ManagerTestContext { + ctxb, cancel := context.WithCancel(context.Background()) defer cancel() @@ -184,14 +290,7 @@ func NewAddressManagerTestContext(t *testing.T) *ManagerTestContext { mockStaticAddressClient.On( "ServerNewAddress", mock.Anything, mock.Anything, mock.Anything, - ).Return( - &swapserverrpc.ServerNewAddressResponse{ - Params: &swapserverrpc.ServerAddressParameters{ - ServerKey: defaultServerPubkeyBytes, - Expiry: defaultExpiry, - }, - }, nil, - ) + ).Return(resp, nil) cfg := &ManagerConfig{ Store: store, @@ -215,3 +314,14 @@ func NewAddressManagerTestContext(t *testing.T) *ManagerTestContext { mockStaticAddressClient: mockStaticAddressClient, } } + +// newServerNewAddressResponse returns a valid server response with the given +// CSV expiry. +func newServerNewAddressResponse(expiry uint32) *swapserverrpc.ServerNewAddressResponse { + return &swapserverrpc.ServerNewAddressResponse{ + Params: &swapserverrpc.ServerAddressParameters{ + ServerKey: defaultServerPubkeyBytes, + Expiry: expiry, + }, + } +}