[client, proxy] Harden uspfilter conntrack and share TCP relay

client/firewall/uspfilter/conntrack/cap_test.go

@@ -0,0 +1,125 @@
+package conntrack
+
+import (
+	"net/netip"
+	"testing"
+
+	"github.com/google/gopacket/layers"
+	"github.com/stretchr/testify/require"
+)
+
+func TestTCPCapEvicts(t *testing.T) {
+	t.Setenv(EnvTCPMaxEntries, "4")
+
+	tracker := NewTCPTracker(DefaultTCPTimeout, logger, flowLogger)
+	defer tracker.Close()
+	require.Equal(t, 4, tracker.maxEntries)
+
+	src := netip.MustParseAddr("100.64.0.1")
+	dst := netip.MustParseAddr("100.64.0.2")
+
+	for i := 0; i < 10; i++ {
+		tracker.TrackOutbound(src, dst, uint16(10000+i), 80, TCPSyn, 0)
+	}
+	require.LessOrEqual(t, len(tracker.connections), 4,
+		"TCP table must not exceed the configured cap")
+	require.Greater(t, len(tracker.connections), 0,
+		"some entries must remain after eviction")
+
+	// The most recently admitted flow must be present: eviction must make
+	// room for new entries, not silently drop them.
+	require.Contains(t, tracker.connections,
+		ConnKey{SrcIP: src, DstIP: dst, SrcPort: uint16(10009), DstPort: 80},
+		"newest TCP flow must be admitted after eviction")
+	// A pre-cap flow must have been evicted to fit the last one.
+	require.NotContains(t, tracker.connections,
+		ConnKey{SrcIP: src, DstIP: dst, SrcPort: uint16(10000), DstPort: 80},
+		"oldest TCP flow should have been evicted")
+}
+
+func TestTCPCapPrefersTombstonedForEviction(t *testing.T) {
+	t.Setenv(EnvTCPMaxEntries, "3")
+
+	tracker := NewTCPTracker(DefaultTCPTimeout, logger, flowLogger)
+	defer tracker.Close()
+
+	src := netip.MustParseAddr("100.64.0.1")
+	dst := netip.MustParseAddr("100.64.0.2")
+
+	// Fill to cap with 3 live connections.
+	for i := 0; i < 3; i++ {
+		tracker.TrackOutbound(src, dst, uint16(20000+i), 80, TCPSyn, 0)
+	}
+	require.Len(t, tracker.connections, 3)
+
+	// Tombstone one by sending RST through IsValidInbound.
+	tombstonedKey := ConnKey{SrcIP: src, DstIP: dst, SrcPort: 20001, DstPort: 80}
+	require.True(t, tracker.IsValidInbound(dst, src, 80, 20001, TCPRst|TCPAck, 0))
+	require.True(t, tracker.connections[tombstonedKey].IsTombstone())
+
+	// Another live connection forces eviction. The tombstone must go first.
+	tracker.TrackOutbound(src, dst, uint16(29999), 80, TCPSyn, 0)
+
+	_, tombstonedStillPresent := tracker.connections[tombstonedKey]
+	require.False(t, tombstonedStillPresent,
+		"tombstoned entry should be evicted before live entries")
+	require.LessOrEqual(t, len(tracker.connections), 3)
+
+	// Both live pre-cap entries must survive: eviction must prefer the
+	// tombstone, not just satisfy the size bound by dropping any entry.
+	require.Contains(t, tracker.connections,
+		ConnKey{SrcIP: src, DstIP: dst, SrcPort: uint16(20000), DstPort: 80},
+		"live entries must not be evicted while a tombstone exists")
+	require.Contains(t, tracker.connections,
+		ConnKey{SrcIP: src, DstIP: dst, SrcPort: uint16(20002), DstPort: 80},
+		"live entries must not be evicted while a tombstone exists")
+}
+
+func TestUDPCapEvicts(t *testing.T) {
+	t.Setenv(EnvUDPMaxEntries, "5")
+
+	tracker := NewUDPTracker(DefaultUDPTimeout, logger, flowLogger)
+	defer tracker.Close()
+	require.Equal(t, 5, tracker.maxEntries)
+
+	src := netip.MustParseAddr("100.64.0.1")
+	dst := netip.MustParseAddr("100.64.0.2")
+
+	for i := 0; i < 12; i++ {
+		tracker.TrackOutbound(src, dst, uint16(30000+i), 53, 0)
+	}
+	require.LessOrEqual(t, len(tracker.connections), 5)
+	require.Greater(t, len(tracker.connections), 0)
+
+	require.Contains(t, tracker.connections,
+		ConnKey{SrcIP: src, DstIP: dst, SrcPort: uint16(30011), DstPort: 53},
+		"newest UDP flow must be admitted after eviction")
+	require.NotContains(t, tracker.connections,
+		ConnKey{SrcIP: src, DstIP: dst, SrcPort: uint16(30000), DstPort: 53},
+		"oldest UDP flow should have been evicted")
+}
+
+func TestICMPCapEvicts(t *testing.T) {
+	t.Setenv(EnvICMPMaxEntries, "3")
+
+	tracker := NewICMPTracker(DefaultICMPTimeout, logger, flowLogger)
+	defer tracker.Close()
+	require.Equal(t, 3, tracker.maxEntries)
+
+	src := netip.MustParseAddr("100.64.0.1")
+	dst := netip.MustParseAddr("100.64.0.2")
+
+	echoReq := layers.CreateICMPv4TypeCode(uint8(layers.ICMPv4TypeEchoRequest), 0)
+	for i := 0; i < 8; i++ {
+		tracker.TrackOutbound(src, dst, uint16(i), echoReq, nil, 64)
+	}
+	require.LessOrEqual(t, len(tracker.connections), 3)
+	require.Greater(t, len(tracker.connections), 0)
+
+	require.Contains(t, tracker.connections,
+		ICMPConnKey{SrcIP: src, DstIP: dst, ID: uint16(7)},
+		"newest ICMP flow must be admitted after eviction")
+	require.NotContains(t, tracker.connections,
+		ICMPConnKey{SrcIP: src, DstIP: dst, ID: uint16(0)},
+		"oldest ICMP flow should have been evicted")
+}

client/firewall/uspfilter/conntrack/common.go

@@ -3,14 +3,61 @@ package conntrack
 import (
 	"fmt"
 	"net/netip"
+	"os"
+	"strconv"
 	"sync/atomic"
 	"time"
 
 	"github.com/google/uuid"
 
+	nblog "github.com/netbirdio/netbird/client/firewall/uspfilter/log"
 	nftypes "github.com/netbirdio/netbird/client/internal/netflow/types"
 )
 
+// evictSampleSize bounds how many map entries we scan per eviction call.
+// Keeps eviction O(1) even at cap under sustained load; the sampled-LRU
+// heuristic is good enough for a conntrack table that only overflows under
+// abuse.
+const evictSampleSize = 8
+
+// envDuration parses an os.Getenv(name) as a time.Duration. Falls back to
+// def on empty or invalid; logs a warning on invalid.
+func envDuration(logger *nblog.Logger, name string, def time.Duration) time.Duration {
+	v := os.Getenv(name)
+	if v == "" {
+		return def
+	}
+	d, err := time.ParseDuration(v)
+	if err != nil {
+		logger.Warn3("invalid %s=%q: %v, using default", name, v, err)
+		return def
+	}
+	if d <= 0 {
+		logger.Warn2("invalid %s=%q: must be positive, using default", name, v)
+		return def
+	}
+	return d
+}
+
+// envInt parses an os.Getenv(name) as an int. Falls back to def on empty,
+// invalid, or non-positive. Logs a warning on invalid input.
+func envInt(logger *nblog.Logger, name string, def int) int {
+	v := os.Getenv(name)
+	if v == "" {
+		return def
+	}
+	n, err := strconv.Atoi(v)
+	switch {
+	case err != nil:
+		logger.Warn3("invalid %s=%q: %v, using default", name, v, err)
+		return def
+	case n <= 0:
+		logger.Warn2("invalid %s=%q: must be positive, using default", name, v)
+		return def
+	}
+	return n
+}
+
 // BaseConnTrack provides common fields and locking for all connection types
 type BaseConnTrack struct {
 	FlowId    uuid.UUID

client/firewall/uspfilter/conntrack/defaults_desktop.go

@@ -0,0 +1,11 @@
+//go:build !ios && !android
+
+package conntrack
+
+// Default per-tracker entry caps on desktop/server platforms. These mirror
+// typical Linux netfilter nf_conntrack_max territory with ample headroom.
+const (
+	DefaultMaxTCPEntries  = 65536
+	DefaultMaxUDPEntries  = 16384
+	DefaultMaxICMPEntries = 2048
+)

client/firewall/uspfilter/conntrack/defaults_mobile.go

@@ -0,0 +1,13 @@
+//go:build ios || android
+
+package conntrack
+
+// Default per-tracker entry caps on mobile platforms. iOS network extensions
+// are capped at ~50 MB; Android runs under aggressive memory pressure. These
+// values keep conntrack footprint well under 5 MB worst case (TCPConnTrack
+// is ~200 B plus map overhead).
+const (
+	DefaultMaxTCPEntries  = 4096
+	DefaultMaxUDPEntries  = 2048
+	DefaultMaxICMPEntries = 512
+)

client/firewall/uspfilter/conntrack/icmp.go

@@ -44,6 +44,9 @@ type ICMPConnTrack struct {
 	ICMPCode uint8
 }
 
+// EnvICMPMaxEntries caps the ICMP conntrack table size.
+const EnvICMPMaxEntries = "NB_CONNTRACK_ICMP_MAX"
+
 // ICMPTracker manages ICMP connection states
 type ICMPTracker struct {
 	logger        *nblog.Logger
@@ -52,6 +55,7 @@ type ICMPTracker struct {
 	cleanupTicker *time.Ticker
 	tickerCancel  context.CancelFunc
 	mutex         sync.RWMutex
+	maxEntries    int
 	flowLogger    nftypes.FlowLogger
 }
 
@@ -135,6 +139,7 @@ func NewICMPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nfty
 		timeout:       timeout,
 		cleanupTicker: time.NewTicker(ICMPCleanupInterval),
 		tickerCancel:  cancel,
+		maxEntries:    envInt(logger, EnvICMPMaxEntries, DefaultMaxICMPEntries),
 		flowLogger:    flowLogger,
 	}
 
@@ -221,7 +226,9 @@ func (t *ICMPTracker) track(
 
 	// non echo requests don't need tracking
 	if typ != uint8(layers.ICMPv4TypeEchoRequest) {
-		t.logger.Trace3("New %s ICMP connection %s - %s", direction, key, icmpInfo)
+		if t.logger.Enabled(nblog.LevelTrace) {
+			t.logger.Trace3("New %s ICMP connection %s - %s", direction, key, icmpInfo)
+		}
 		t.sendStartEvent(direction, srcIP, dstIP, typ, code, ruleId, size)
 		return
 	}
@@ -240,10 +247,15 @@ func (t *ICMPTracker) track(
 	conn.UpdateCounters(direction, size)
 
 	t.mutex.Lock()
+	if t.maxEntries > 0 && len(t.connections) >= t.maxEntries {
+		t.evictOneLocked()
+	}
 	t.connections[key] = conn
 	t.mutex.Unlock()
 
-	t.logger.Trace3("New %s ICMP connection %s - %s", direction, key, icmpInfo)
+	if t.logger.Enabled(nblog.LevelTrace) {
+		t.logger.Trace3("New %s ICMP connection %s - %s", direction, key, icmpInfo)
+	}
 	t.sendEvent(nftypes.TypeStart, conn, ruleId)
 }
 
@@ -286,6 +298,34 @@ func (t *ICMPTracker) cleanupRoutine(ctx context.Context) {
 	}
 }
 
+// evictOneLocked removes one entry to make room. Caller must hold t.mutex.
+// Bounded sample scan: picks the oldest among up to evictSampleSize entries.
+func (t *ICMPTracker) evictOneLocked() {
+	var candKey ICMPConnKey
+	var candSeen int64
+	haveCand := false
+	sampled := 0
+
+	for k, c := range t.connections {
+		seen := c.lastSeen.Load()
+		if !haveCand || seen < candSeen {
+			candKey = k
+			candSeen = seen
+			haveCand = true
+		}
+		sampled++
+		if sampled >= evictSampleSize {
+			break
+		}
+	}
+	if haveCand {
+		if evicted := t.connections[candKey]; evicted != nil {
+			t.sendEvent(nftypes.TypeEnd, evicted, nil)
+		}
+		delete(t.connections, candKey)
+	}
+}
+
 func (t *ICMPTracker) cleanup() {
 	t.mutex.Lock()
 	defer t.mutex.Unlock()
@@ -294,8 +334,10 @@ func (t *ICMPTracker) cleanup() {
 		if conn.timeoutExceeded(t.timeout) {
 			delete(t.connections, key)
 
-			t.logger.Trace5("Removed ICMP connection %s (timeout) [in: %d Pkts/%d B out: %d Pkts/%d B]",
-				key, conn.PacketsRx.Load(), conn.BytesRx.Load(), conn.PacketsTx.Load(), conn.BytesTx.Load())
+			if t.logger.Enabled(nblog.LevelTrace) {
+				t.logger.Trace5("Removed ICMP connection %s (timeout) [in: %d Pkts/%d B out: %d Pkts/%d B]",
+					key, conn.PacketsRx.Load(), conn.BytesRx.Load(), conn.PacketsTx.Load(), conn.BytesTx.Load())
+			}
 			t.sendEvent(nftypes.TypeEnd, conn, nil)
 		}
 	}