diff --git a/docs/release-notes/release-notes-0.17.1.md b/docs/release-notes/release-notes-0.17.1.md index 6627288ed..013538da3 100644 --- a/docs/release-notes/release-notes-0.17.1.md +++ b/docs/release-notes/release-notes-0.17.1.md @@ -31,6 +31,12 @@ -V`](https://github.com/lightninglabs/lightning-terminal/pull/1337): The `-V` flag now prints litd's version instead of the integrated lnd version. +* [Refactor privacy mapper to prevent 32-bit truncation and optimize + allocations](https://github.com/lightninglabs/lightning-terminal/pull/1358): + Refactored the privacy mapper's random number generation to use `int64` + instead of `int` to prevent architecture-dependent truncation on 32-bit + runtimes, and introduced a `sync.Pool` for `*big.Int` to optimize allocations. + ## RPC Updates ## Integrated Binary Updates diff --git a/firewall/privacy_mapper.go b/firewall/privacy_mapper.go index 8049595dc..d7b7bec98 100644 --- a/firewall/privacy_mapper.go +++ b/firewall/privacy_mapper.go @@ -7,9 +7,11 @@ import ( "encoding/json" "errors" "fmt" + "math" "math/big" "strconv" "strings" + "sync" "time" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -61,14 +63,14 @@ var _ mid.RequestInterceptor = (*PrivacyMapper)(nil) // requests to their real values and vice versa for responses. type PrivacyMapper struct { db firewalldb.PrivacyMapper - randIntn func(int) (int, error) + randIntn func(int64) (int64, error) sessionDB firewalldb.SessionDB } // NewPrivacyMapper returns a new instance of PrivacyMapper. The randIntn // function is used to draw randomness for request field obfuscation. func NewPrivacyMapper(newDB firewalldb.PrivacyMapper, - randIntn func(int) (int, error), + randIntn func(int64) (int64, error), sessionDB firewalldb.SessionDB) *PrivacyMapper { return &PrivacyMapper{ @@ -379,7 +381,7 @@ func handleGetInfoResponse(db firewalldb.PrivacyMapDB, func handleFwdHistoryResponse(db firewalldb.PrivacyMapDB, flags session.PrivacyFlags, - randIntn func(int) (int, error)) func(ctx context.Context, + randIntn func(int64) (int64, error)) func(ctx context.Context, r *lnrpc.ForwardingHistoryResponse) (proto.Message, error) { return func(ctx context.Context, r *lnrpc.ForwardingHistoryResponse) ( @@ -582,7 +584,7 @@ func handleListChannelsRequest(db firewalldb.PrivacyMapDB, func handleListChannelsResponse(db firewalldb.PrivacyMapDB, flags session.PrivacyFlags, - randIntn func(int) (int, error)) func(ctx context.Context, + randIntn func(int64) (int64, error)) func(ctx context.Context, r *lnrpc.ListChannelsResponse) (proto.Message, error) { return func(ctx context.Context, r *lnrpc.ListChannelsResponse) ( @@ -866,7 +868,7 @@ func handleUpdatePolicyResponse(db firewalldb.PrivacyMapDB, func handleWalletBalanceResponse(_ firewalldb.PrivacyMapDB, flags session.PrivacyFlags, - randIntn func(int) (int, error)) func(ctx context.Context, + randIntn func(int64) (int64, error)) func(ctx context.Context, r *lnrpc.WalletBalanceResponse) (proto.Message, error) { return func(_ context.Context, r *lnrpc.WalletBalanceResponse) ( @@ -945,7 +947,7 @@ func handleWalletBalanceResponse(_ firewalldb.PrivacyMapDB, func handleClosedChannelsResponse(db firewalldb.PrivacyMapDB, flags session.PrivacyFlags, - randIntn func(int) (int, error)) func(ctx context.Context, + randIntn func(int64) (int64, error)) func(ctx context.Context, r *lnrpc.ClosedChannelsResponse) (proto.Message, error) { return func(ctx context.Context, r *lnrpc.ClosedChannelsResponse) ( @@ -1065,7 +1067,7 @@ func handleClosedChannelsResponse(db firewalldb.PrivacyMapDB, // channel. func obfuscatePendingChannel(ctx context.Context, c *lnrpc.PendingChannelsResponse_PendingChannel, - tx firewalldb.PrivacyMapTx, randIntn func(int) (int, error), + tx firewalldb.PrivacyMapTx, randIntn func(int64) (int64, error), flags session.PrivacyFlags) ( *lnrpc.PendingChannelsResponse_PendingChannel, error) { @@ -1142,7 +1144,7 @@ func obfuscatePendingChannel(ctx context.Context, func handlePendingChannelsResponse(db firewalldb.PrivacyMapDB, flags session.PrivacyFlags, - randIntn func(int) (int, error)) func(ctx context.Context, + randIntn func(int64) (int64, error)) func(ctx context.Context, r *lnrpc.PendingChannelsResponse) (proto.Message, error) { return func(ctx context.Context, r *lnrpc.PendingChannelsResponse) ( @@ -1729,7 +1731,7 @@ func handleConnectPeerRequest(db firewalldb.PrivacyMapDB, } // maybeHideAmount hides an amount if the privacy flag is not set. -func maybeHideAmount(flags session.PrivacyFlags, randIntn func(int) (int, +func maybeHideAmount(flags session.PrivacyFlags, randIntn func(int64) (int64, error), a int64) (int64, error) { if !flags.Contains(session.ClearAmounts) { @@ -1748,8 +1750,8 @@ func maybeHideAmount(flags session.PrivacyFlags, randIntn func(int) (int, // hideAmount symmetrically randomizes an amount around a given relative // variation interval. relativeVariation should be between 0 and 1. -func hideAmount(randIntn func(n int) (int, error), relativeVariation float64, - amount uint64) (uint64, error) { +func hideAmount(randIntn func(n int64) (int64, error), + relativeVariation float64, amount uint64) (uint64, error) { if relativeVariation < 0 || relativeVariation > 1 { return 0, fmt.Errorf("hide amount: relative variation is not "+ @@ -1757,6 +1759,11 @@ func hideAmount(randIntn func(n int) (int, error), relativeVariation float64, relativeVariation) } + if amount > math.MaxInt64 { + return 0, fmt.Errorf("hide amount: amount %d overflows "+ + "int64", amount) + } + if amount == 0 { return 0, nil } @@ -1765,8 +1772,13 @@ func hideAmount(randIntn func(n int) (int, error), relativeVariation float64, // between 0 and 1. fuzzInterval := uint64(float64(amount) * relativeVariation) - amountMin := int(amount - fuzzInterval) - amountMax := int(amount + fuzzInterval) + if amount+fuzzInterval > math.MaxInt64 { + return 0, fmt.Errorf("hide amount: amount %d with variation "+ + "overflows int64", amount) + } + + amountMin := int64(amount - fuzzInterval) + amountMax := int64(amount + fuzzInterval) randAmount, err := randBetween(randIntn, amountMin, amountMax) if err != nil { @@ -1778,7 +1790,7 @@ func hideAmount(randIntn func(n int) (int, error), relativeVariation float64, // hideTimestamp symmetrically randomizes a unix timestamp given an absolute // variation interval. The random input is expected to be rand.Intn. -func hideTimestamp(randIntn func(n int) (int, error), +func hideTimestamp(randIntn func(n int64) (int64, error), absoluteVariation time.Duration, timestamp time.Time) (time.Time, error) { @@ -1802,18 +1814,20 @@ func hideTimestamp(randIntn func(n int) (int, error), timeMax := timestamp.Add(absoluteVariation) timeNs, err := randBetween( - randIntn, int(timeMin.UnixNano()), int(timeMax.UnixNano()), + randIntn, timeMin.UnixNano(), timeMax.UnixNano(), ) if err != nil { return time.Time{}, err } - return time.Unix(0, int64(timeNs)), nil + return time.Unix(0, timeNs), nil } // randBetween generates a random number between [min, max) given a source of // randomness. -func randBetween(randIntn func(int) (int, error), min, max int) (int, error) { +func randBetween(randIntn func(int64) (int64, error), + min, max int64) (int64, error) { + if max < min { return 0, fmt.Errorf("min is not allowed to be greater than "+ "max, (min: %v, max: %v)", min, max) @@ -1833,7 +1847,7 @@ func randBetween(randIntn func(int) (int, error), min, max int) (int, error) { } // hideBool generates a random bool given a random input. -func hideBool(randIntn func(n int) (int, error)) (bool, error) { +func hideBool(randIntn func(n int64) (int64, error)) (bool, error) { random, err := randIntn(2) if err != nil { return false, err @@ -1844,18 +1858,31 @@ func hideBool(randIntn func(n int) (int, error)) (bool, error) { return random >= 1, nil } +// bigIntPool is a pool of *big.Int values to avoid frequent heap allocations in +// CryptoRandIntn. +var bigIntPool = sync.Pool{ + New: func() any { + return new(big.Int) + }, +} + // CryptoRandIntn generates a random number between [0, n). -func CryptoRandIntn(n int) (int, error) { +func CryptoRandIntn(n int64) (int64, error) { if n == 0 { return 0, nil } - nBig, err := rand.Int(rand.Reader, big.NewInt(int64(n))) + nBig := bigIntPool.Get().(*big.Int) + defer bigIntPool.Put(nBig) + + nBig.SetInt64(n) + + randBig, err := rand.Int(rand.Reader, nBig) if err != nil { return 0, err } - return int(nBig.Int64()), nil + return randBig.Int64(), nil } // ObfuscateConfig alters the config string by replacing sensitive data with diff --git a/firewall/privacy_mapper_test.go b/firewall/privacy_mapper_test.go index 53dcf6ebe..060d0efa4 100644 --- a/firewall/privacy_mapper_test.go +++ b/firewall/privacy_mapper_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "math" "testing" "time" @@ -919,7 +920,9 @@ func TestPrivacyMapper(t *testing.T) { require.NoError(t, err) // randIntn is used for deterministic testing. - randIntn := func(n int) (int, error) { return 100, nil } + randIntn := func(n int64) (int64, error) { + return 100, nil + } p := NewPrivacyMapper(db, randIntn, pd) rawMsg, err := proto.Marshal(test.msg) @@ -1188,8 +1191,10 @@ var _ firewalldb.PrivacyMapDB = (*mockPrivacyMapDB)(nil) // TestRandBetween tests random number generation for numbers in an interval. func TestRandBetween(t *testing.T) { - min := 0 - max := 10 + var ( + min int64 + max int64 = 10 + ) for i := 0; i < 100; i++ { val, err := randBetween(CryptoRandIntn, min, max) @@ -1210,38 +1215,40 @@ func TestHideAmount(t *testing.T) { tests := []struct { name string amount uint64 - randIntFn func(int) (int, error) + randIntFn func(int64) (int64, error) expected uint64 }{ { name: "zero test amount", - randIntFn: func(int) (int, error) { return 0, nil }, + randIntFn: func(int64) (int64, error) { return 0, nil }, }, { name: "test small amount", - randIntFn: func(int) (int, error) { return 0, nil }, + randIntFn: func(int64) (int64, error) { return 0, nil }, amount: 1, expected: 1, }, { name: "min value", - randIntFn: func(int) (int, error) { return 0, nil }, + randIntFn: func(int64) (int64, error) { return 0, nil }, amount: testAmount, expected: lowerBound, }, { name: "max value", - randIntFn: func(int) (int, error) { - return int(upperBound - lowerBound), nil + randIntFn: func(n int64) (int64, error) { + return int64(upperBound - lowerBound), nil }, amount: testAmount, expected: upperBound, }, { - name: "some fuzz", - randIntFn: func(int) (int, error) { return 123, nil }, - amount: testAmount, - expected: lowerBound + 123, + name: "some fuzz", + randIntFn: func(int64) (int64, error) { + return 123, nil + }, + amount: testAmount, + expected: lowerBound + 123, }, } @@ -1270,6 +1277,33 @@ func TestHideAmount(t *testing.T) { require.NoError(t, err) } }) + + t.Run("hideAmount overflow validation", func(t *testing.T) { + // amount > math.MaxInt64 should fail. + _, err := hideAmount( + CryptoRandIntn, + relativeVariation, + math.MaxInt64+1, + ) + require.Error(t, err) + + // amount <= math.MaxInt64 but amount + fuzzInterval > + // math.MaxInt64 should fail. + _, err = hideAmount( + CryptoRandIntn, + 0.05, + math.MaxInt64-100, + ) + require.Error(t, err) + + // A value that just fits should succeed. + _, err = hideAmount( + CryptoRandIntn, + 0.1, + 922337203685477580, + ) + require.NoError(t, err) + }) } // TestHideTimestamp test correct timestamp hiding. @@ -1281,31 +1315,33 @@ func TestHideTimestamp(t *testing.T) { tests := []struct { name string - randIntFn func(int) (int, error) + randIntFn func(int64) (int64, error) timestamp time.Time expected time.Time }{ { name: "zero timestamp", - randIntFn: func(int) (int, error) { return 0, nil }, + randIntFn: func(int64) (int64, error) { return 0, nil }, }, { name: "min value", - randIntFn: func(int) (int, error) { return 0, nil }, + randIntFn: func(int64) (int64, error) { return 0, nil }, timestamp: timestamp, expected: lowerBound, }, { name: "max value", - randIntFn: func(int) (int, error) { - return int(upperBound.Sub(lowerBound)), nil + randIntFn: func(n int64) (int64, error) { + return int64(upperBound.Sub(lowerBound)), nil }, timestamp: timestamp, expected: upperBound, }, { - name: "some fuzz", - randIntFn: func(int) (int, error) { return 123, nil }, + name: "some fuzz", + randIntFn: func(int64) (int64, error) { + return 123, nil + }, timestamp: timestamp, expected: lowerBound.Add(time.Duration(123)), }, @@ -1328,17 +1364,37 @@ func TestHideTimestamp(t *testing.T) { // TestHideBool test correct boolean hiding. func TestHideBool(t *testing.T) { - val, err := hideBool(func(int) (int, error) { return 100, nil }) - require.NoError(t, err) - require.True(t, val) - - val, err = hideBool(func(int) (int, error) { return 1, nil }) - require.NoError(t, err) - require.True(t, val) + tests := []struct { + name string + random int64 + expected bool + }{ + { + name: "random value", + random: 100, + expected: true, + }, + { + name: "exact threshold value", + random: 1, + expected: true, + }, + { + name: "below threshold value", + random: 0, + expected: false, + }, + } - val, err = hideBool(func(int) (int, error) { return 0, nil }) - require.NoError(t, err) - require.False(t, val) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + val, err := hideBool(func(n int64) (int64, error) { + return tc.random, nil + }) + require.NoError(t, err) + require.Equal(t, tc.expected, val) + }) + } } // TestObfuscateConfig tests that we substitute substrings in the config @@ -1605,3 +1661,27 @@ func variance(numbers []uint64) uint64 { return uint64(sum) } + +// Test32BitOverflowAndTruncation ensures values > MaxInt32 do not truncate on +// 32-bit runtimes. +func Test32BitOverflowAndTruncation(t *testing.T) { + largeValue := int64(math.MaxInt32) + 1 + + // Ensure randBetween successfully operates on values larger than + // MaxInt32 without any architecture-dependent truncation. + mockRand := func(n int64) (int64, error) { + return n - 1, nil + } + + val, err := randBetween(mockRand, largeValue, largeValue+10) + require.NoError(t, err) + require.Equal(t, largeValue+9, val) + + // Ensure hideTimestamp works with a 64-bit nanosecond timestamp + // representing a real far-future date (> MaxInt32). + farFutureTime := time.Unix(0, 1600000000000000000) // ~Year 2020 in ns + variation := 10 * time.Minute + resTime, err := hideTimestamp(mockRand, variation, farFutureTime) + require.NoError(t, err) + require.NotEmpty(t, resTime) +}