From 419ed275faff4db5f34878aedf191f31e024caec Mon Sep 17 00:00:00 2001 From: hakansa <43675540+hakansa@users.noreply.github.com> Date: Wed, 5 Mar 2025 20:25:42 +0300 Subject: [PATCH 01/11] Handle TCP RST flag to transition connection state to closed (#3432) --- client/firewall/uspfilter/conntrack/tcp.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/client/firewall/uspfilter/conntrack/tcp.go b/client/firewall/uspfilter/conntrack/tcp.go index acf34c49d..1e6364f68 100644 --- a/client/firewall/uspfilter/conntrack/tcp.go +++ b/client/firewall/uspfilter/conntrack/tcp.go @@ -287,17 +287,24 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i conn.State = TCPStateCloseWait } conn.SetEstablished(false) + } else if flags&TCPRst != 0 { + conn.State = TCPStateClosed + conn.SetTombstone() + t.sendEvent(nftypes.TypeEnd, key, conn) } case TCPStateFinWait1: switch { case flags&TCPFin != 0 && flags&TCPAck != 0: - // Simultaneous close - both sides sent FIN conn.State = TCPStateClosing case flags&TCPFin != 0: conn.State = TCPStateFinWait2 case flags&TCPAck != 0: conn.State = TCPStateFinWait2 + case flags&TCPRst != 0: + conn.State = TCPStateClosed + conn.SetTombstone() + t.sendEvent(nftypes.TypeEnd, key, conn) } case TCPStateFinWait2: From e9f11fb11b0c12be3deef4a0ae7cd9dbf08bcbe0 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Wed, 5 Mar 2025 18:28:05 +0100 Subject: [PATCH 02/11] Replace net.IP with netip.Addr (#3425) --- client/firewall/uspfilter/allow_netbird.go | 9 +- .../uspfilter/allow_netbird_windows.go | 9 +- client/firewall/uspfilter/conntrack/common.go | 14 -- .../uspfilter/conntrack/common_test.go | 18 +- client/firewall/uspfilter/conntrack/icmp.go | 59 +++-- .../firewall/uspfilter/conntrack/icmp_test.go | 10 +- client/firewall/uspfilter/conntrack/tcp.go | 57 ++--- .../firewall/uspfilter/conntrack/tcp_test.go | 35 +-- client/firewall/uspfilter/conntrack/udp.go | 53 +++-- .../firewall/uspfilter/conntrack/udp_test.go | 42 ++-- client/firewall/uspfilter/localip.go | 17 +- client/firewall/uspfilter/localip_test.go | 17 +- client/firewall/uspfilter/rule.go | 3 +- client/firewall/uspfilter/tracer.go | 29 ++- client/firewall/uspfilter/tracer_test.go | 0 client/firewall/uspfilter/uspfilter.go | 202 +++++++++--------- .../uspfilter/uspfilter_bench_test.go | 4 +- .../uspfilter/uspfilter_filter_test.go | 12 +- client/firewall/uspfilter/uspfilter_test.go | 30 +-- client/iface/device/device_filter.go | 3 +- client/iface/mocks/filter.go | 3 +- client/internal/dns/service_memory.go | 9 +- client/server/trace.go | 15 +- 23 files changed, 341 insertions(+), 309 deletions(-) create mode 100644 client/firewall/uspfilter/tracer_test.go diff --git a/client/firewall/uspfilter/allow_netbird.go b/client/firewall/uspfilter/allow_netbird.go index 1982e4e7e..4808c3090 100644 --- a/client/firewall/uspfilter/allow_netbird.go +++ b/client/firewall/uspfilter/allow_netbird.go @@ -4,6 +4,7 @@ package uspfilter import ( "context" + "net/netip" "time" log "github.com/sirupsen/logrus" @@ -17,8 +18,8 @@ func (m *Manager) Reset(stateManager *statemanager.Manager) error { m.mutex.Lock() defer m.mutex.Unlock() - m.outgoingRules = make(map[string]RuleSet) - m.incomingRules = make(map[string]RuleSet) + m.outgoingRules = make(map[netip.Addr]RuleSet) + m.incomingRules = make(map[netip.Addr]RuleSet) if m.udpTracker != nil { m.udpTracker.Close() @@ -35,8 +36,8 @@ func (m *Manager) Reset(stateManager *statemanager.Manager) error { m.tcpTracker = conntrack.NewTCPTracker(conntrack.DefaultTCPTimeout, m.logger, m.flowLogger) } - if m.forwarder != nil { - m.forwarder.Stop() + if fwder := m.forwarder.Load(); fwder != nil { + fwder.Stop() } if m.logger != nil { diff --git a/client/firewall/uspfilter/allow_netbird_windows.go b/client/firewall/uspfilter/allow_netbird_windows.go index cacabe1b3..ff80fec41 100644 --- a/client/firewall/uspfilter/allow_netbird_windows.go +++ b/client/firewall/uspfilter/allow_netbird_windows.go @@ -3,6 +3,7 @@ package uspfilter import ( "context" "fmt" + "net/netip" "os/exec" "syscall" "time" @@ -26,8 +27,8 @@ func (m *Manager) Reset(*statemanager.Manager) error { m.mutex.Lock() defer m.mutex.Unlock() - m.outgoingRules = make(map[string]RuleSet) - m.incomingRules = make(map[string]RuleSet) + m.outgoingRules = make(map[netip.Addr]RuleSet) + m.incomingRules = make(map[netip.Addr]RuleSet) if m.udpTracker != nil { m.udpTracker.Close() @@ -44,8 +45,8 @@ func (m *Manager) Reset(*statemanager.Manager) error { m.tcpTracker = conntrack.NewTCPTracker(conntrack.DefaultTCPTimeout, m.logger, m.flowLogger) } - if m.forwarder != nil { - m.forwarder.Stop() + if fwder := m.forwarder.Load(); fwder != nil { + fwder.Stop() } if m.logger != nil { diff --git a/client/firewall/uspfilter/conntrack/common.go b/client/firewall/uspfilter/conntrack/common.go index 915b57549..b87ec6fd2 100644 --- a/client/firewall/uspfilter/conntrack/common.go +++ b/client/firewall/uspfilter/conntrack/common.go @@ -2,7 +2,6 @@ package conntrack import ( "fmt" - "net" "net/netip" "sync/atomic" "time" @@ -52,16 +51,3 @@ type ConnKey struct { func (c ConnKey) String() string { return fmt.Sprintf("%s:%d -> %s:%d", c.SrcIP.Unmap(), c.SrcPort, c.DstIP.Unmap(), c.DstPort) } - -// makeConnKey creates a connection key -func makeConnKey(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16) ConnKey { - srcAddr, _ := netip.AddrFromSlice(srcIP) - dstAddr, _ := netip.AddrFromSlice(dstIP) - - return ConnKey{ - SrcIP: srcAddr, - DstIP: dstAddr, - SrcPort: srcPort, - DstPort: dstPort, - } -} diff --git a/client/firewall/uspfilter/conntrack/common_test.go b/client/firewall/uspfilter/conntrack/common_test.go index 6d1ed5890..e83b6aa85 100644 --- a/client/firewall/uspfilter/conntrack/common_test.go +++ b/client/firewall/uspfilter/conntrack/common_test.go @@ -2,7 +2,7 @@ package conntrack import ( "context" - "net" + "net/netip" "testing" "github.com/sirupsen/logrus" @@ -21,11 +21,11 @@ func BenchmarkMemoryPressure(b *testing.B) { defer tracker.Close() // Generate different IPs - srcIPs := make([]net.IP, 100) - dstIPs := make([]net.IP, 100) + srcIPs := make([]netip.Addr, 100) + dstIPs := make([]netip.Addr, 100) for i := 0; i < 100; i++ { - srcIPs[i] = net.IPv4(192, 168, byte(i/256), byte(i%256)) - dstIPs[i] = net.IPv4(10, 0, byte(i/256), byte(i%256)) + srcIPs[i] = netip.AddrFrom4([4]byte{192, 168, byte(i / 256), byte(i % 256)}) + dstIPs[i] = netip.AddrFrom4([4]byte{10, 0, byte(i / 256), byte(i % 256)}) } b.ResetTimer() @@ -46,11 +46,11 @@ func BenchmarkMemoryPressure(b *testing.B) { defer tracker.Close() // Generate different IPs - srcIPs := make([]net.IP, 100) - dstIPs := make([]net.IP, 100) + srcIPs := make([]netip.Addr, 100) + dstIPs := make([]netip.Addr, 100) for i := 0; i < 100; i++ { - srcIPs[i] = net.IPv4(192, 168, byte(i/256), byte(i%256)) - dstIPs[i] = net.IPv4(10, 0, byte(i/256), byte(i%256)) + srcIPs[i] = netip.AddrFrom4([4]byte{192, 168, byte(i / 256), byte(i % 256)}) + dstIPs[i] = netip.AddrFrom4([4]byte{10, 0, byte(i / 256), byte(i % 256)}) } b.ResetTimer() diff --git a/client/firewall/uspfilter/conntrack/icmp.go b/client/firewall/uspfilter/conntrack/icmp.go index fda08154c..97617a52d 100644 --- a/client/firewall/uspfilter/conntrack/icmp.go +++ b/client/firewall/uspfilter/conntrack/icmp.go @@ -2,7 +2,6 @@ package conntrack import ( "fmt" - "net" "net/netip" "sync" "time" @@ -70,8 +69,13 @@ func NewICMPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nfty return tracker } -func (t *ICMPTracker) updateIfExists(srcIP net.IP, dstIP net.IP, id uint16, seq uint16) (ICMPConnKey, bool) { - key := makeICMPKey(srcIP, dstIP, id, seq) +func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16) (ICMPConnKey, bool) { + key := ICMPConnKey{ + SrcIP: srcIP, + DstIP: dstIP, + ID: id, + Sequence: seq, + } t.mutex.RLock() conn, exists := t.connections[key] @@ -87,7 +91,7 @@ func (t *ICMPTracker) updateIfExists(srcIP net.IP, dstIP net.IP, id uint16, seq } // TrackOutbound records an outbound ICMP connection -func (t *ICMPTracker) TrackOutbound(srcIP net.IP, dstIP net.IP, id uint16, seq uint16, typecode layers.ICMPv4TypeCode) { +func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode) { if _, exists := t.updateIfExists(dstIP, srcIP, id, seq); !exists { // if (inverted direction) conn is not tracked, track this direction t.track(srcIP, dstIP, id, seq, typecode, nftypes.Egress) @@ -95,12 +99,12 @@ func (t *ICMPTracker) TrackOutbound(srcIP net.IP, dstIP net.IP, id uint16, seq u } // TrackInbound records an inbound ICMP Echo Request -func (t *ICMPTracker) TrackInbound(srcIP net.IP, dstIP net.IP, id uint16, seq uint16, typecode layers.ICMPv4TypeCode) { +func (t *ICMPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode) { t.track(srcIP, dstIP, id, seq, typecode, nftypes.Ingress) } // track is the common implementation for tracking both inbound and outbound ICMP connections -func (t *ICMPTracker) track(srcIP net.IP, dstIP net.IP, id uint16, seq uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction) { +func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction) { // TODO: icmp doesn't need to extend the timeout key, exists := t.updateIfExists(srcIP, dstIP, id, seq) if exists { @@ -112,7 +116,7 @@ func (t *ICMPTracker) track(srcIP net.IP, dstIP net.IP, id uint16, seq uint16, t // non echo requests don't need tracking if typ != uint8(layers.ICMPv4TypeEchoRequest) { t.logger.Trace("New %s ICMP connection %s type %d code %d", direction, key, typ, code) - t.sendStartEvent(direction, key, typ, code) + t.sendStartEvent(direction, srcIP, dstIP, typ, code) return } @@ -120,8 +124,8 @@ func (t *ICMPTracker) track(srcIP net.IP, dstIP net.IP, id uint16, seq uint16, t BaseConnTrack: BaseConnTrack{ FlowId: uuid.New(), Direction: direction, - SourceIP: key.SrcIP, - DestIP: key.DstIP, + SourceIP: srcIP, + DestIP: dstIP, }, ICMPType: typ, ICMPCode: code, @@ -133,16 +137,21 @@ func (t *ICMPTracker) track(srcIP net.IP, dstIP net.IP, id uint16, seq uint16, t t.mutex.Unlock() t.logger.Trace("New %s ICMP connection %s type %d code %d", direction, key, typ, code) - t.sendEvent(nftypes.TypeStart, key, conn) + t.sendEvent(nftypes.TypeStart, conn) } // IsValidInbound checks if an inbound ICMP Echo Reply matches a tracked request -func (t *ICMPTracker) IsValidInbound(srcIP net.IP, dstIP net.IP, id uint16, seq uint16, icmpType uint8) bool { +func (t *ICMPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, icmpType uint8) bool { if icmpType != uint8(layers.ICMPv4TypeEchoReply) { return false } - key := makeICMPKey(dstIP, srcIP, id, seq) + key := ICMPConnKey{ + SrcIP: dstIP, + DstIP: srcIP, + ID: id, + Sequence: seq, + } t.mutex.RLock() conn, exists := t.connections[key] @@ -177,7 +186,7 @@ func (t *ICMPTracker) cleanup() { delete(t.connections, key) t.logger.Debug("Removed ICMP connection %s (timeout)", &key) - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) } } } @@ -192,40 +201,28 @@ func (t *ICMPTracker) Close() { t.mutex.Unlock() } -func (t *ICMPTracker) sendEvent(typ nftypes.Type, key ICMPConnKey, conn *ICMPConnTrack) { +func (t *ICMPTracker) sendEvent(typ nftypes.Type, conn *ICMPConnTrack) { t.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: conn.FlowId, Type: typ, Direction: conn.Direction, Protocol: nftypes.ICMP, // TODO: adjust for IPv6/icmpv6 - SourceIP: key.SrcIP, - DestIP: key.DstIP, + SourceIP: conn.SourceIP, + DestIP: conn.DestIP, ICMPType: conn.ICMPType, ICMPCode: conn.ICMPCode, }) } -func (t *ICMPTracker) sendStartEvent(direction nftypes.Direction, key ICMPConnKey, typ, code uint8) { +func (t *ICMPTracker) sendStartEvent(direction nftypes.Direction, srcIP netip.Addr, dstIP netip.Addr, typ uint8, code uint8) { t.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: uuid.New(), Type: nftypes.TypeStart, Direction: direction, Protocol: nftypes.ICMP, - SourceIP: key.SrcIP, - DestIP: key.DstIP, + SourceIP: srcIP, + DestIP: dstIP, ICMPType: typ, ICMPCode: code, }) } - -// makeICMPKey creates an ICMP connection key -func makeICMPKey(srcIP net.IP, dstIP net.IP, id uint16, seq uint16) ICMPConnKey { - srcAddr, _ := netip.AddrFromSlice(srcIP) - dstAddr, _ := netip.AddrFromSlice(dstIP) - return ICMPConnKey{ - SrcIP: srcAddr, - DstIP: dstAddr, - ID: id, - Sequence: seq, - } -} diff --git a/client/firewall/uspfilter/conntrack/icmp_test.go b/client/firewall/uspfilter/conntrack/icmp_test.go index b8328ae94..259cc21a4 100644 --- a/client/firewall/uspfilter/conntrack/icmp_test.go +++ b/client/firewall/uspfilter/conntrack/icmp_test.go @@ -1,7 +1,7 @@ package conntrack import ( - "net" + "net/netip" "testing" ) @@ -10,8 +10,8 @@ func BenchmarkICMPTracker(b *testing.B) { tracker := NewICMPTracker(DefaultICMPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") b.ResetTimer() for i := 0; i < b.N; i++ { @@ -23,8 +23,8 @@ func BenchmarkICMPTracker(b *testing.B) { tracker := NewICMPTracker(DefaultICMPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") // Pre-populate some connections for i := 0; i < 1000; i++ { diff --git a/client/firewall/uspfilter/conntrack/tcp.go b/client/firewall/uspfilter/conntrack/tcp.go index 1e6364f68..0c0e7bd99 100644 --- a/client/firewall/uspfilter/conntrack/tcp.go +++ b/client/firewall/uspfilter/conntrack/tcp.go @@ -3,7 +3,7 @@ package conntrack // TODO: Send RST packets for invalid/timed-out connections import ( - "net" + "net/netip" "sync" "sync/atomic" "time" @@ -144,8 +144,13 @@ func NewTCPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nftyp return tracker } -func (t *TCPTracker) updateIfExists(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16, flags uint8) (ConnKey, bool) { - key := makeConnKey(srcIP, dstIP, srcPort, dstPort) +func (t *TCPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) (ConnKey, bool) { + key := ConnKey{ + SrcIP: srcIP, + DstIP: dstIP, + SrcPort: srcPort, + DstPort: dstPort, + } t.mutex.RLock() conn, exists := t.connections[key] @@ -154,7 +159,6 @@ func (t *TCPTracker) updateIfExists(srcIP net.IP, dstIP net.IP, srcPort uint16, if exists { conn.Lock() t.updateState(key, conn, flags, conn.Direction == nftypes.Egress) - conn.UpdateLastSeen() conn.Unlock() return key, true @@ -164,7 +168,7 @@ func (t *TCPTracker) updateIfExists(srcIP net.IP, dstIP net.IP, srcPort uint16, } // TrackOutbound records an outbound TCP connection -func (t *TCPTracker) TrackOutbound(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16, flags uint8) { +func (t *TCPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) { if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort, flags); !exists { // if (inverted direction) conn is not tracked, track this direction t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Egress) @@ -172,12 +176,12 @@ func (t *TCPTracker) TrackOutbound(srcIP net.IP, dstIP net.IP, srcPort uint16, d } // TrackInbound processes an inbound TCP packet and updates connection state -func (t *TCPTracker) TrackInbound(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16, flags uint8) { +func (t *TCPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) { t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Ingress) } // track is the common implementation for tracking both inbound and outbound connections -func (t *TCPTracker) track(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16, flags uint8, direction nftypes.Direction) { +func (t *TCPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, direction nftypes.Direction) { key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort, flags) if exists { return @@ -187,14 +191,13 @@ func (t *TCPTracker) track(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort u BaseConnTrack: BaseConnTrack{ FlowId: uuid.New(), Direction: direction, - SourceIP: key.SrcIP, - DestIP: key.DstIP, + SourceIP: srcIP, + DestIP: dstIP, SourcePort: srcPort, DestPort: dstPort, }, } - conn.UpdateLastSeen() conn.established.Store(false) conn.tombstone.Store(false) @@ -205,12 +208,17 @@ func (t *TCPTracker) track(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort u t.connections[key] = conn t.mutex.Unlock() - t.sendEvent(nftypes.TypeStart, key, conn) + t.sendEvent(nftypes.TypeStart, conn) } // IsValidInbound checks if an inbound TCP packet matches a tracked connection -func (t *TCPTracker) IsValidInbound(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16, flags uint8) bool { - key := makeConnKey(dstIP, srcIP, dstPort, srcPort) +func (t *TCPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) bool { + key := ConnKey{ + SrcIP: dstIP, + DstIP: srcIP, + SrcPort: dstPort, + DstPort: srcPort, + } t.mutex.RLock() conn, exists := t.connections[key] @@ -233,13 +241,12 @@ func (t *TCPTracker) IsValidInbound(srcIP net.IP, dstIP net.IP, srcPort uint16, conn.Unlock() t.logger.Trace("TCP connection reset: %s", key) - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) return true } conn.Lock() t.updateState(key, conn, flags, false) - conn.UpdateLastSeen() isEstablished := conn.IsEstablished() isValidState := t.isValidStateForFlags(conn.State, flags) conn.Unlock() @@ -249,6 +256,8 @@ func (t *TCPTracker) IsValidInbound(srcIP net.IP, dstIP net.IP, srcPort uint16, // updateState updates the TCP connection state based on flags func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, isOutbound bool) { + conn.UpdateLastSeen() + state := conn.State defer func() { if state != conn.State { @@ -312,7 +321,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i conn.State = TCPStateTimeWait t.logger.Trace("TCP connection %s completed", key) - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) } case TCPStateClosing: @@ -321,7 +330,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i // Keep established = false from previous state t.logger.Trace("TCP connection %s closed (simultaneous)", key) - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) } case TCPStateCloseWait: @@ -335,7 +344,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i conn.SetTombstone() // Send close event for gracefully closed connections - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) t.logger.Trace("TCP connection %s closed gracefully", key) } } @@ -422,7 +431,7 @@ func (t *TCPTracker) cleanup() { // event already handled by state change if conn.State != TCPStateTimeWait { - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) } } } @@ -453,15 +462,15 @@ func isValidFlagCombination(flags uint8) bool { return true } -func (t *TCPTracker) sendEvent(typ nftypes.Type, key ConnKey, conn *TCPConnTrack) { +func (t *TCPTracker) sendEvent(typ nftypes.Type, conn *TCPConnTrack) { t.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: conn.FlowId, Type: typ, Direction: conn.Direction, Protocol: nftypes.TCP, - SourceIP: key.SrcIP, - DestIP: key.DstIP, - SourcePort: key.SrcPort, - DestPort: key.DstPort, + SourceIP: conn.SourceIP, + DestIP: conn.DestIP, + SourcePort: conn.SourcePort, + DestPort: conn.DestPort, }) } diff --git a/client/firewall/uspfilter/conntrack/tcp_test.go b/client/firewall/uspfilter/conntrack/tcp_test.go index 200d77501..122deae1e 100644 --- a/client/firewall/uspfilter/conntrack/tcp_test.go +++ b/client/firewall/uspfilter/conntrack/tcp_test.go @@ -1,7 +1,7 @@ package conntrack import ( - "net" + "net/netip" "testing" "time" @@ -12,8 +12,8 @@ func TestTCPStateMachine(t *testing.T) { tracker := NewTCPTracker(DefaultTCPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("100.64.0.1") - dstIP := net.ParseIP("100.64.0.2") + srcIP := netip.MustParseAddr("100.64.0.1") + dstIP := netip.MustParseAddr("100.64.0.2") srcPort := uint16(12345) dstPort := uint16(80) @@ -165,8 +165,8 @@ func TestRSTHandling(t *testing.T) { tracker := NewTCPTracker(DefaultTCPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("100.64.0.1") - dstIP := net.ParseIP("100.64.0.2") + srcIP := netip.MustParseAddr("100.64.0.1") + dstIP := netip.MustParseAddr("100.64.0.2") srcPort := uint16(12345) dstPort := uint16(80) @@ -208,7 +208,12 @@ func TestRSTHandling(t *testing.T) { tt.sendRST() // Verify connection state is as expected - key := makeConnKey(srcIP, dstIP, srcPort, dstPort) + key := ConnKey{ + SrcIP: srcIP, + DstIP: dstIP, + SrcPort: srcPort, + DstPort: dstPort, + } conn := tracker.connections[key] if tt.wantValid { require.NotNil(t, conn) @@ -220,7 +225,7 @@ func TestRSTHandling(t *testing.T) { } // Helper to establish a TCP connection -func establishConnection(t *testing.T, tracker *TCPTracker, srcIP, dstIP net.IP, srcPort, dstPort uint16) { +func establishConnection(t *testing.T, tracker *TCPTracker, srcIP, dstIP netip.Addr, srcPort, dstPort uint16) { t.Helper() tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPSyn) @@ -236,8 +241,8 @@ func BenchmarkTCPTracker(b *testing.B) { tracker := NewTCPTracker(DefaultTCPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") b.ResetTimer() for i := 0; i < b.N; i++ { @@ -249,8 +254,8 @@ func BenchmarkTCPTracker(b *testing.B) { tracker := NewTCPTracker(DefaultTCPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") // Pre-populate some connections for i := 0; i < 1000; i++ { @@ -267,8 +272,8 @@ func BenchmarkTCPTracker(b *testing.B) { tracker := NewTCPTracker(DefaultTCPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") b.RunParallel(func(pb *testing.PB) { i := 0 @@ -291,8 +296,8 @@ func BenchmarkCleanup(b *testing.B) { defer tracker.Close() // Pre-populate with expired connections - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") for i := 0; i < 10000; i++ { tracker.TrackOutbound(srcIP, dstIP, uint16(i), 80, TCPSyn) } diff --git a/client/firewall/uspfilter/conntrack/udp.go b/client/firewall/uspfilter/conntrack/udp.go index 922db371d..465e400be 100644 --- a/client/firewall/uspfilter/conntrack/udp.go +++ b/client/firewall/uspfilter/conntrack/udp.go @@ -1,7 +1,7 @@ package conntrack import ( - "net" + "net/netip" "sync" "time" @@ -54,7 +54,7 @@ func NewUDPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nftyp } // TrackOutbound records an outbound UDP connection -func (t *UDPTracker) TrackOutbound(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16) { +func (t *UDPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) { if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort); !exists { // if (inverted direction) conn is not tracked, track this direction t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Egress) @@ -62,12 +62,17 @@ func (t *UDPTracker) TrackOutbound(srcIP net.IP, dstIP net.IP, srcPort uint16, d } // TrackInbound records an inbound UDP connection -func (t *UDPTracker) TrackInbound(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16) { +func (t *UDPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) { t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Ingress) } -func (t *UDPTracker) updateIfExists(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16) (ConnKey, bool) { - key := makeConnKey(srcIP, dstIP, srcPort, dstPort) +func (t *UDPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) (ConnKey, bool) { + key := ConnKey{ + SrcIP: srcIP, + DstIP: dstIP, + SrcPort: srcPort, + DstPort: dstPort, + } t.mutex.RLock() conn, exists := t.connections[key] @@ -82,7 +87,7 @@ func (t *UDPTracker) updateIfExists(srcIP net.IP, dstIP net.IP, srcPort uint16, } // track is the common implementation for tracking both inbound and outbound connections -func (t *UDPTracker) track(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16, direction nftypes.Direction) { +func (t *UDPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, direction nftypes.Direction) { key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort) if exists { return @@ -92,8 +97,8 @@ func (t *UDPTracker) track(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort u BaseConnTrack: BaseConnTrack{ FlowId: uuid.New(), Direction: direction, - SourceIP: key.SrcIP, - DestIP: key.DstIP, + SourceIP: srcIP, + DestIP: dstIP, SourcePort: srcPort, DestPort: dstPort, }, @@ -105,12 +110,17 @@ func (t *UDPTracker) track(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort u t.mutex.Unlock() t.logger.Trace("New %s UDP connection: %s", direction, key) - t.sendEvent(nftypes.TypeStart, key, conn) + t.sendEvent(nftypes.TypeStart, conn) } // IsValidInbound checks if an inbound packet matches a tracked connection -func (t *UDPTracker) IsValidInbound(srcIP net.IP, dstIP net.IP, srcPort uint16, dstPort uint16) bool { - key := makeConnKey(dstIP, srcIP, dstPort, srcPort) +func (t *UDPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) bool { + key := ConnKey{ + SrcIP: dstIP, + DstIP: srcIP, + SrcPort: dstPort, + DstPort: srcPort, + } t.mutex.RLock() conn, exists := t.connections[key] @@ -146,7 +156,7 @@ func (t *UDPTracker) cleanup() { delete(t.connections, key) t.logger.Trace("Removed UDP connection %s (timeout)", key) - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) } } } @@ -162,11 +172,16 @@ func (t *UDPTracker) Close() { } // GetConnection safely retrieves a connection state -func (t *UDPTracker) GetConnection(srcIP net.IP, srcPort uint16, dstIP net.IP, dstPort uint16) (*UDPConnTrack, bool) { +func (t *UDPTracker) GetConnection(srcIP netip.Addr, srcPort uint16, dstIP netip.Addr, dstPort uint16) (*UDPConnTrack, bool) { t.mutex.RLock() defer t.mutex.RUnlock() - key := makeConnKey(srcIP, dstIP, srcPort, dstPort) + key := ConnKey{ + SrcIP: srcIP, + DstIP: dstIP, + SrcPort: srcPort, + DstPort: dstPort, + } conn, exists := t.connections[key] return conn, exists } @@ -176,15 +191,15 @@ func (t *UDPTracker) Timeout() time.Duration { return t.timeout } -func (t *UDPTracker) sendEvent(typ nftypes.Type, key ConnKey, conn *UDPConnTrack) { +func (t *UDPTracker) sendEvent(typ nftypes.Type, conn *UDPConnTrack) { t.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: conn.FlowId, Type: typ, Direction: conn.Direction, Protocol: nftypes.UDP, - SourceIP: key.SrcIP, - DestIP: key.DstIP, - SourcePort: key.SrcPort, - DestPort: key.DstPort, + SourceIP: conn.SourceIP, + DestIP: conn.DestIP, + SourcePort: conn.SourcePort, + DestPort: conn.DestPort, }) } diff --git a/client/firewall/uspfilter/conntrack/udp_test.go b/client/firewall/uspfilter/conntrack/udp_test.go index 29c7111fd..db7fa0f51 100644 --- a/client/firewall/uspfilter/conntrack/udp_test.go +++ b/client/firewall/uspfilter/conntrack/udp_test.go @@ -1,7 +1,6 @@ package conntrack import ( - "net" "net/netip" "testing" "time" @@ -49,10 +48,15 @@ func TestUDPTracker_TrackOutbound(t *testing.T) { srcPort := uint16(12345) dstPort := uint16(53) - tracker.TrackOutbound(srcIP.AsSlice(), dstIP.AsSlice(), srcPort, dstPort) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort) // Verify connection was tracked - key := makeConnKey(srcIP.AsSlice(), dstIP.AsSlice(), srcPort, dstPort) + key := ConnKey{ + SrcIP: srcIP, + DstIP: dstIP, + SrcPort: srcPort, + DstPort: dstPort, + } conn, exists := tracker.connections[key] require.True(t, exists) assert.True(t, conn.SourceIP.Compare(srcIP) == 0) @@ -66,8 +70,8 @@ func TestUDPTracker_IsValidInbound(t *testing.T) { tracker := NewUDPTracker(1*time.Second, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.2") - dstIP := net.ParseIP("192.168.1.3") + srcIP := netip.MustParseAddr("192.168.1.2") + dstIP := netip.MustParseAddr("192.168.1.3") srcPort := uint16(12345) dstPort := uint16(53) @@ -76,8 +80,8 @@ func TestUDPTracker_IsValidInbound(t *testing.T) { tests := []struct { name string - srcIP net.IP - dstIP net.IP + srcIP netip.Addr + dstIP netip.Addr srcPort uint16 dstPort uint16 sleep time.Duration @@ -94,7 +98,7 @@ func TestUDPTracker_IsValidInbound(t *testing.T) { }, { name: "invalid source IP", - srcIP: net.ParseIP("192.168.1.4"), + srcIP: netip.MustParseAddr("192.168.1.4"), dstIP: srcIP, srcPort: dstPort, dstPort: srcPort, @@ -104,7 +108,7 @@ func TestUDPTracker_IsValidInbound(t *testing.T) { { name: "invalid destination IP", srcIP: dstIP, - dstIP: net.ParseIP("192.168.1.4"), + dstIP: netip.MustParseAddr("192.168.1.4"), srcPort: dstPort, dstPort: srcPort, sleep: 0, @@ -170,20 +174,20 @@ func TestUDPTracker_Cleanup(t *testing.T) { // Add some connections connections := []struct { - srcIP net.IP - dstIP net.IP + srcIP netip.Addr + dstIP netip.Addr srcPort uint16 dstPort uint16 }{ { - srcIP: net.ParseIP("192.168.1.2"), - dstIP: net.ParseIP("192.168.1.3"), + srcIP: netip.MustParseAddr("192.168.1.2"), + dstIP: netip.MustParseAddr("192.168.1.3"), srcPort: 12345, dstPort: 53, }, { - srcIP: net.ParseIP("192.168.1.4"), - dstIP: net.ParseIP("192.168.1.5"), + srcIP: netip.MustParseAddr("192.168.1.4"), + dstIP: netip.MustParseAddr("192.168.1.5"), srcPort: 12346, dstPort: 53, }, @@ -215,8 +219,8 @@ func BenchmarkUDPTracker(b *testing.B) { tracker := NewUDPTracker(DefaultUDPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") b.ResetTimer() for i := 0; i < b.N; i++ { @@ -228,8 +232,8 @@ func BenchmarkUDPTracker(b *testing.B) { tracker := NewUDPTracker(DefaultUDPTimeout, logger, flowLogger) defer tracker.Close() - srcIP := net.ParseIP("192.168.1.1") - dstIP := net.ParseIP("192.168.1.2") + srcIP := netip.MustParseAddr("192.168.1.1") + dstIP := netip.MustParseAddr("192.168.1.2") // Pre-populate some connections for i := 0; i < 1000; i++ { diff --git a/client/firewall/uspfilter/localip.go b/client/firewall/uspfilter/localip.go index 7664b65d5..b86d16043 100644 --- a/client/firewall/uspfilter/localip.go +++ b/client/firewall/uspfilter/localip.go @@ -3,6 +3,7 @@ package uspfilter import ( "fmt" "net" + "net/netip" "sync" log "github.com/sirupsen/logrus" @@ -31,13 +32,9 @@ func (m *localIPManager) setBitmapBit(ip net.IP) { m.ipv4Bitmap[high] |= 1 << (low % 32) } -func (m *localIPManager) checkBitmapBit(ip net.IP) bool { - ipv4 := ip.To4() - if ipv4 == nil { - return false - } - high := (uint16(ipv4[0]) << 8) | uint16(ipv4[1]) - low := (uint16(ipv4[2]) << 8) | uint16(ipv4[3]) +func (m *localIPManager) checkBitmapBit(ip []byte) bool { + high := (uint16(ip[0]) << 8) | uint16(ip[1]) + low := (uint16(ip[2]) << 8) | uint16(ip[3]) return (m.ipv4Bitmap[high] & (1 << (low % 32))) != 0 } @@ -122,12 +119,12 @@ func (m *localIPManager) UpdateLocalIPs(iface common.IFaceMapper) (err error) { return nil } -func (m *localIPManager) IsLocalIP(ip net.IP) bool { +func (m *localIPManager) IsLocalIP(ip netip.Addr) bool { m.mu.RLock() defer m.mu.RUnlock() - if ipv4 := ip.To4(); ipv4 != nil { - return m.checkBitmapBit(ipv4) + if ip.Is4() { + return m.checkBitmapBit(ip.AsSlice()) } return false diff --git a/client/firewall/uspfilter/localip_test.go b/client/firewall/uspfilter/localip_test.go index 02f41bf4f..890b7a30d 100644 --- a/client/firewall/uspfilter/localip_test.go +++ b/client/firewall/uspfilter/localip_test.go @@ -2,6 +2,7 @@ package uspfilter import ( "net" + "net/netip" "testing" "github.com/stretchr/testify/require" @@ -13,7 +14,7 @@ func TestLocalIPManager(t *testing.T) { tests := []struct { name string setupAddr iface.WGAddress - testIP net.IP + testIP netip.Addr expected bool }{ { @@ -25,7 +26,7 @@ func TestLocalIPManager(t *testing.T) { Mask: net.CIDRMask(24, 32), }, }, - testIP: net.ParseIP("127.0.0.2"), + testIP: netip.MustParseAddr("127.0.0.2"), expected: true, }, { @@ -37,7 +38,7 @@ func TestLocalIPManager(t *testing.T) { Mask: net.CIDRMask(24, 32), }, }, - testIP: net.ParseIP("127.0.0.1"), + testIP: netip.MustParseAddr("127.0.0.1"), expected: true, }, { @@ -49,7 +50,7 @@ func TestLocalIPManager(t *testing.T) { Mask: net.CIDRMask(24, 32), }, }, - testIP: net.ParseIP("127.255.255.255"), + testIP: netip.MustParseAddr("127.255.255.255"), expected: true, }, { @@ -61,7 +62,7 @@ func TestLocalIPManager(t *testing.T) { Mask: net.CIDRMask(24, 32), }, }, - testIP: net.ParseIP("192.168.1.1"), + testIP: netip.MustParseAddr("192.168.1.1"), expected: true, }, { @@ -73,7 +74,7 @@ func TestLocalIPManager(t *testing.T) { Mask: net.CIDRMask(24, 32), }, }, - testIP: net.ParseIP("192.168.1.2"), + testIP: netip.MustParseAddr("192.168.1.2"), expected: false, }, { @@ -85,7 +86,7 @@ func TestLocalIPManager(t *testing.T) { Mask: net.CIDRMask(64, 128), }, }, - testIP: net.ParseIP("fe80::1"), + testIP: netip.MustParseAddr("fe80::1"), expected: false, }, } @@ -174,7 +175,7 @@ func TestLocalIPManager_AllInterfaces(t *testing.T) { t.Logf("Testing %d IPs", len(tests)) for _, tt := range tests { t.Run(tt.ip, func(t *testing.T) { - result := manager.IsLocalIP(net.ParseIP(tt.ip)) + result := manager.IsLocalIP(netip.MustParseAddr(tt.ip)) require.Equal(t, tt.expected, result, "IP: %s", tt.ip) }) } diff --git a/client/firewall/uspfilter/rule.go b/client/firewall/uspfilter/rule.go index 7a587c832..a23d2011b 100644 --- a/client/firewall/uspfilter/rule.go +++ b/client/firewall/uspfilter/rule.go @@ -1,7 +1,6 @@ package uspfilter import ( - "net" "net/netip" "github.com/google/gopacket" @@ -13,7 +12,7 @@ import ( type PeerRule struct { id string mgmtId []byte - ip net.IP + ip netip.Addr ipLayer gopacket.LayerType matchByIP bool protoLayer gopacket.LayerType diff --git a/client/firewall/uspfilter/tracer.go b/client/firewall/uspfilter/tracer.go index aff886b58..a980cda29 100644 --- a/client/firewall/uspfilter/tracer.go +++ b/client/firewall/uspfilter/tracer.go @@ -2,7 +2,7 @@ package uspfilter import ( "fmt" - "net" + "net/netip" "time" "github.com/google/gopacket" @@ -53,8 +53,8 @@ type TraceResult struct { } type PacketTrace struct { - SourceIP net.IP - DestinationIP net.IP + SourceIP netip.Addr + DestinationIP netip.Addr Protocol string SourcePort uint16 DestinationPort uint16 @@ -72,8 +72,8 @@ type TCPState struct { } type PacketBuilder struct { - SrcIP net.IP - DstIP net.IP + SrcIP netip.Addr + DstIP netip.Addr Protocol fw.Protocol SrcPort uint16 DstPort uint16 @@ -126,8 +126,8 @@ func (p *PacketBuilder) buildIPLayer() *layers.IPv4 { Version: 4, TTL: 64, Protocol: layers.IPProtocol(getIPProtocolNumber(p.Protocol)), - SrcIP: p.SrcIP, - DstIP: p.DstIP, + SrcIP: p.SrcIP.AsSlice(), + DstIP: p.DstIP.AsSlice(), } } @@ -260,7 +260,7 @@ func (m *Manager) TracePacket(packetData []byte, direction fw.RuleDirection) *Pa return m.traceInbound(packetData, trace, d, srcIP, dstIP) } -func (m *Manager) traceInbound(packetData []byte, trace *PacketTrace, d *decoder, srcIP net.IP, dstIP net.IP) *PacketTrace { +func (m *Manager) traceInbound(packetData []byte, trace *PacketTrace, d *decoder, srcIP netip.Addr, dstIP netip.Addr) *PacketTrace { if m.stateful && m.handleConntrackState(trace, d, srcIP, dstIP) { return trace } @@ -273,14 +273,14 @@ func (m *Manager) traceInbound(packetData []byte, trace *PacketTrace, d *decoder return trace } - if m.nativeRouter { + if m.nativeRouter.Load() { return m.handleNativeRouter(trace) } return m.handleRouteACLs(trace, d, srcIP, dstIP) } -func (m *Manager) handleConntrackState(trace *PacketTrace, d *decoder, srcIP, dstIP net.IP) bool { +func (m *Manager) handleConntrackState(trace *PacketTrace, d *decoder, srcIP, dstIP netip.Addr) bool { allowed := m.isValidTrackedConnection(d, srcIP, dstIP) msg := "No existing connection found" if allowed { @@ -309,13 +309,12 @@ func (m *Manager) buildConntrackStateMessage(d *decoder) string { return msg } -func (m *Manager) handleLocalDelivery(trace *PacketTrace, packetData []byte, d *decoder, srcIP, dstIP net.IP) bool { +func (m *Manager) handleLocalDelivery(trace *PacketTrace, packetData []byte, d *decoder, srcIP, dstIP netip.Addr) bool { if !m.localForwarding { trace.AddResult(StageRouting, "Local forwarding disabled", false) trace.AddResult(StageCompleted, "Packet dropped - local forwarding disabled", false) return true } - trace.AddResult(StageRouting, "Packet destined for local delivery", true) ruleId, blocked := m.peerACLsBlock(srcIP, packetData, m.incomingRules, d) @@ -341,7 +340,7 @@ func (m *Manager) handleLocalDelivery(trace *PacketTrace, packetData []byte, d * } func (m *Manager) handleRouting(trace *PacketTrace) bool { - if !m.routingEnabled { + if !m.routingEnabled.Load() { trace.AddResult(StageRouting, "Routing disabled", false) trace.AddResult(StageCompleted, "Packet dropped - routing disabled", false) return false @@ -357,7 +356,7 @@ func (m *Manager) handleNativeRouter(trace *PacketTrace) *PacketTrace { return trace } -func (m *Manager) handleRouteACLs(trace *PacketTrace, d *decoder, srcIP, dstIP net.IP) *PacketTrace { +func (m *Manager) handleRouteACLs(trace *PacketTrace, d *decoder, srcIP, dstIP netip.Addr) *PacketTrace { proto, _ := getProtocolFromPacket(d) srcPort, dstPort := getPortsFromPacket(d) id, allowed := m.routeACLsPass(srcIP, dstIP, proto, srcPort, dstPort) @@ -373,7 +372,7 @@ func (m *Manager) handleRouteACLs(trace *PacketTrace, d *decoder, srcIP, dstIP n } trace.AddResult(StageRouteACL, msg, allowed) - if allowed && m.forwarder != nil { + if allowed && m.forwarder.Load() != nil { m.addForwardingResult(trace, "proxy-remote", fmt.Sprintf("%s:%d", dstIP, dstPort), true) } diff --git a/client/firewall/uspfilter/tracer_test.go b/client/firewall/uspfilter/tracer_test.go new file mode 100644 index 000000000..e69de29bb diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go index 01a3976b4..05dd9ff06 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/uspfilter.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "github.com/google/gopacket" "github.com/google/gopacket/layers" @@ -66,9 +67,9 @@ func (r RouteRules) Sort() { // Manager userspace firewall manager type Manager struct { // outgoingRules is used for hooks only - outgoingRules map[string]RuleSet + outgoingRules map[netip.Addr]RuleSet // incomingRules is used for filtering and hooks - incomingRules map[string]RuleSet + incomingRules map[netip.Addr]RuleSet routeRules RouteRules wgNetwork *net.IPNet decoders sync.Pool @@ -80,9 +81,9 @@ type Manager struct { // indicates whether server routes are disabled disableServerRoutes bool // indicates whether we forward packets not destined for ourselves - routingEnabled bool + routingEnabled atomic.Bool // indicates whether we leave forwarding and filtering to the native firewall - nativeRouter bool + nativeRouter atomic.Bool // indicates whether we track outbound connections stateful bool // indicates whether wireguards runs in netstack mode @@ -95,7 +96,7 @@ type Manager struct { udpTracker *conntrack.UDPTracker icmpTracker *conntrack.ICMPTracker tcpTracker *conntrack.TCPTracker - forwarder *forwarder.Forwarder + forwarder atomic.Pointer[forwarder.Forwarder] logger *nblog.Logger flowLogger nftypes.FlowLogger } @@ -168,18 +169,18 @@ func create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableSe }, }, nativeFirewall: nativeFirewall, - outgoingRules: make(map[string]RuleSet), - incomingRules: make(map[string]RuleSet), + outgoingRules: make(map[netip.Addr]RuleSet), + incomingRules: make(map[netip.Addr]RuleSet), wgIface: iface, localipmanager: newLocalIPManager(), disableServerRoutes: disableServerRoutes, - routingEnabled: false, stateful: !disableConntrack, logger: nblog.NewFromLogrus(log.StandardLogger()), flowLogger: flowLogger, netstack: netstack.IsEnabled(), localForwarding: enableLocalForwarding, } + m.routingEnabled.Store(false) if err := m.localipmanager.UpdateLocalIPs(iface); err != nil { return nil, fmt.Errorf("update local IPs: %w", err) @@ -211,7 +212,7 @@ func create(iface common.IFaceMapper, nativeFirewall firewall.Manager, disableSe } func (m *Manager) blockInvalidRouted(iface common.IFaceMapper) error { - if m.forwarder == nil { + if m.forwarder.Load() == nil { return nil } wgPrefix, err := netip.ParsePrefix(iface.Address().Network.String()) @@ -255,20 +256,20 @@ func (m *Manager) determineRouting() error { switch { case disableUspRouting: - m.routingEnabled = false - m.nativeRouter = false + m.routingEnabled.Store(false) + m.nativeRouter.Store(false) log.Info("userspace routing is disabled") case m.disableServerRoutes: // if server routes are disabled we will let packets pass to the native stack - m.routingEnabled = true - m.nativeRouter = true + m.routingEnabled.Store(true) + m.nativeRouter.Store(true) log.Info("server routes are disabled") case forceUserspaceRouter: - m.routingEnabled = true - m.nativeRouter = false + m.routingEnabled.Store(true) + m.nativeRouter.Store(false) log.Info("userspace routing is forced") @@ -276,19 +277,19 @@ func (m *Manager) determineRouting() error { // if the OS supports routing natively, then we don't need to filter/route ourselves // netstack mode won't support native routing as there is no interface - m.routingEnabled = true - m.nativeRouter = true + m.routingEnabled.Store(true) + m.nativeRouter.Store(true) log.Info("native routing is enabled") default: - m.routingEnabled = true - m.nativeRouter = false + m.routingEnabled.Store(true) + m.nativeRouter.Store(false) log.Info("userspace routing enabled by default") } - if m.routingEnabled && !m.nativeRouter { + if m.routingEnabled.Load() && !m.nativeRouter.Load() { return m.initForwarder() } @@ -297,24 +298,24 @@ func (m *Manager) determineRouting() error { // initForwarder initializes the forwarder, it disables routing on errors func (m *Manager) initForwarder() error { - if m.forwarder != nil { + if m.forwarder.Load() != nil { return nil } // Only supported in userspace mode as we need to inject packets back into wireguard directly intf := m.wgIface.GetWGDevice() if intf == nil { - m.routingEnabled = false + m.routingEnabled.Store(false) return errors.New("forwarding not supported") } forwarder, err := forwarder.New(m.wgIface, m.logger, m.flowLogger, m.netstack) if err != nil { - m.routingEnabled = false + m.routingEnabled.Store(false) return fmt.Errorf("create forwarder: %w", err) } - m.forwarder = forwarder + m.forwarder.Store(forwarder) log.Debug("forwarder initialized") @@ -330,7 +331,7 @@ func (m *Manager) IsServerRouteSupported() bool { } func (m *Manager) AddNatRule(pair firewall.RouterPair) error { - if m.nativeRouter && m.nativeFirewall != nil { + if m.nativeRouter.Load() && m.nativeFirewall != nil { return m.nativeFirewall.AddNatRule(pair) } @@ -341,7 +342,7 @@ func (m *Manager) AddNatRule(pair firewall.RouterPair) error { // RemoveNatRule removes a routing firewall rule func (m *Manager) RemoveNatRule(pair firewall.RouterPair) error { - if m.nativeRouter && m.nativeFirewall != nil { + if m.nativeRouter.Load() && m.nativeFirewall != nil { return m.nativeFirewall.RemoveNatRule(pair) } return nil @@ -360,17 +361,23 @@ func (m *Manager) AddPeerFiltering( action firewall.Action, _ string, ) ([]firewall.Rule, error) { + // TODO: fix in upper layers + i, ok := netip.AddrFromSlice(ip) + if !ok { + return nil, fmt.Errorf("invalid IP: %s", ip) + } + + i = i.Unmap() r := PeerRule{ id: uuid.New().String(), mgmtId: id, - ip: ip, + ip: i, ipLayer: layers.LayerTypeIPv6, matchByIP: true, drop: action == firewall.ActionDrop, } - if ipNormalized := ip.To4(); ipNormalized != nil { + if i.Is4() { r.ipLayer = layers.LayerTypeIPv4 - r.ip = ipNormalized } if s := r.ip.String(); s == "0.0.0.0" || s == "::" { @@ -395,10 +402,10 @@ func (m *Manager) AddPeerFiltering( } m.mutex.Lock() - if _, ok := m.incomingRules[r.ip.String()]; !ok { - m.incomingRules[r.ip.String()] = make(RuleSet) + if _, ok := m.incomingRules[r.ip]; !ok { + m.incomingRules[r.ip] = make(RuleSet) } - m.incomingRules[r.ip.String()][r.id] = r + m.incomingRules[r.ip][r.id] = r m.mutex.Unlock() return []firewall.Rule{&r}, nil } @@ -412,13 +419,10 @@ func (m *Manager) AddRouteFiltering( dPort *firewall.Port, action firewall.Action, ) (firewall.Rule, error) { - if m.nativeRouter && m.nativeFirewall != nil { + if m.nativeRouter.Load() && m.nativeFirewall != nil { return m.nativeFirewall.AddRouteFiltering(id, sources, destination, proto, sPort, dPort, action) } - m.mutex.Lock() - defer m.mutex.Unlock() - ruleID := uuid.New().String() rule := RouteRule{ // TODO: consolidate these IDs @@ -432,14 +436,16 @@ func (m *Manager) AddRouteFiltering( action: action, } + m.mutex.Lock() m.routeRules = append(m.routeRules, rule) m.routeRules.Sort() + m.mutex.Unlock() return &rule, nil } func (m *Manager) DeleteRouteRule(rule firewall.Rule) error { - if m.nativeRouter && m.nativeFirewall != nil { + if m.nativeRouter.Load() && m.nativeFirewall != nil { return m.nativeFirewall.DeleteRouteRule(rule) } @@ -468,10 +474,10 @@ func (m *Manager) DeletePeerRule(rule firewall.Rule) error { return fmt.Errorf("delete rule: invalid rule type: %T", rule) } - if _, ok := m.incomingRules[r.ip.String()][r.id]; !ok { + if _, ok := m.incomingRules[r.ip][r.id]; !ok { return fmt.Errorf("delete rule: no rule with such id: %v", r.id) } - delete(m.incomingRules[r.ip.String()], r.id) + delete(m.incomingRules[r.ip], r.id) return nil } @@ -519,9 +525,6 @@ func (m *Manager) UpdateLocalIPs() error { } func (m *Manager) processOutgoingHooks(packetData []byte) bool { - m.mutex.RLock() - defer m.mutex.RUnlock() - d := m.decoders.Get().(*decoder) defer m.decoders.Put(d) @@ -534,7 +537,8 @@ func (m *Manager) processOutgoingHooks(packetData []byte) bool { } srcIP, dstIP := m.extractIPs(d) - if srcIP == nil { + if !srcIP.IsValid() { + m.logger.Error("Unknown network layer: %v", d.decoded[0]) return false } @@ -551,14 +555,18 @@ func (m *Manager) processOutgoingHooks(packetData []byte) bool { return false } -func (m *Manager) extractIPs(d *decoder) (srcIP, dstIP net.IP) { +func (m *Manager) extractIPs(d *decoder) (srcIP, dstIP netip.Addr) { switch d.decoded[0] { case layers.LayerTypeIPv4: - return d.ip4.SrcIP, d.ip4.DstIP + src, _ := netip.AddrFromSlice(d.ip4.SrcIP) + dst, _ := netip.AddrFromSlice(d.ip4.DstIP) + return src, dst case layers.LayerTypeIPv6: - return d.ip6.SrcIP, d.ip6.DstIP + src, _ := netip.AddrFromSlice(d.ip6.SrcIP) + dst, _ := netip.AddrFromSlice(d.ip6.DstIP) + return src, dst default: - return nil, nil + return netip.Addr{}, netip.Addr{} } } @@ -585,7 +593,7 @@ func getTCPFlags(tcp *layers.TCP) uint8 { return flags } -func (m *Manager) trackOutbound(d *decoder, srcIP, dstIP net.IP) { +func (m *Manager) trackOutbound(d *decoder, srcIP, dstIP netip.Addr) { transport := d.decoded[1] switch transport { case layers.LayerTypeUDP: @@ -598,7 +606,7 @@ func (m *Manager) trackOutbound(d *decoder, srcIP, dstIP net.IP) { } } -func (m *Manager) trackInbound(d *decoder, srcIP, dstIP net.IP) { +func (m *Manager) trackInbound(d *decoder, srcIP, dstIP netip.Addr) { transport := d.decoded[1] switch transport { case layers.LayerTypeUDP: @@ -611,8 +619,11 @@ func (m *Manager) trackInbound(d *decoder, srcIP, dstIP net.IP) { } } -func (m *Manager) checkUDPHooks(d *decoder, dstIP net.IP, packetData []byte) bool { - for _, ipKey := range []string{dstIP.String(), "0.0.0.0", "::"} { +func (m *Manager) checkUDPHooks(d *decoder, dstIP netip.Addr, packetData []byte) bool { + m.mutex.RLock() + defer m.mutex.RUnlock() + + for _, ipKey := range []netip.Addr{dstIP, netip.IPv4Unspecified(), netip.IPv6Unspecified()} { if rules, exists := m.outgoingRules[ipKey]; exists { for _, rule := range rules { if rule.udpHook != nil && portsMatch(rule.dPort, uint16(d.udp.DstPort)) { @@ -627,9 +638,6 @@ func (m *Manager) checkUDPHooks(d *decoder, dstIP net.IP, packetData []byte) boo // dropFilter implements filtering logic for incoming packets. // If it returns true, the packet should be dropped. func (m *Manager) dropFilter(packetData []byte) bool { - m.mutex.RLock() - defer m.mutex.RUnlock() - d := m.decoders.Get().(*decoder) defer m.decoders.Put(d) @@ -638,7 +646,7 @@ func (m *Manager) dropFilter(packetData []byte) bool { } srcIP, dstIP := m.extractIPs(d) - if srcIP == nil { + if !srcIP.IsValid() { m.logger.Error("Unknown network layer: %v", d.decoded[0]) return true } @@ -658,15 +666,13 @@ func (m *Manager) dropFilter(packetData []byte) bool { // handleLocalTraffic handles local traffic. // If it returns true, the packet should be dropped. -func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP net.IP, packetData []byte) bool { +func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP netip.Addr, packetData []byte) bool { if ruleId, blocked := m.peerACLsBlock(srcIP, packetData, m.incomingRules, d); blocked { - srcAddr, _ := netip.AddrFromSlice(srcIP) - dstAddr, _ := netip.AddrFromSlice(dstIP) _, pnum := getProtocolFromPacket(d) srcPort, dstPort := getPortsFromPacket(d) m.logger.Trace("Dropping local packet (ACL denied): rule_id=%s proto=%v src=%s:%d dst=%s:%d", - ruleId, pnum, srcAddr, srcPort, dstAddr, dstPort) + ruleId, pnum, srcIP, srcPort, dstIP, dstPort) m.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: uuid.New(), @@ -674,8 +680,8 @@ func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP net.IP, packetData RuleID: ruleId, Direction: nftypes.Ingress, Protocol: pnum, - SourceIP: srcAddr, - DestIP: dstAddr, + SourceIP: srcIP, + DestIP: dstIP, SourcePort: srcPort, DestPort: dstPort, // TODO: icmp type/code @@ -700,12 +706,12 @@ func (m *Manager) handleNetstackLocalTraffic(packetData []byte) bool { return false } - if m.forwarder == nil { + if m.forwarder.Load() == nil { m.logger.Trace("Dropping local packet (forwarder not initialized)") return true } - if err := m.forwarder.InjectIncomingPacket(packetData); err != nil { + if err := m.forwarder.Load().InjectIncomingPacket(packetData); err != nil { m.logger.Error("Failed to inject local packet: %v", err) } @@ -715,16 +721,16 @@ func (m *Manager) handleNetstackLocalTraffic(packetData []byte) bool { // handleRoutedTraffic handles routed traffic. // If it returns true, the packet should be dropped. -func (m *Manager) handleRoutedTraffic(d *decoder, srcIP, dstIP net.IP, packetData []byte) bool { +func (m *Manager) handleRoutedTraffic(d *decoder, srcIP, dstIP netip.Addr, packetData []byte) bool { // Drop if routing is disabled - if !m.routingEnabled { + if !m.routingEnabled.Load() { m.logger.Trace("Dropping routed packet (routing disabled): src=%s dst=%s", srcIP, dstIP) return true } // Pass to native stack if native router is enabled or forced - if m.nativeRouter { + if m.nativeRouter.Load() { return false } @@ -732,9 +738,6 @@ func (m *Manager) handleRoutedTraffic(d *decoder, srcIP, dstIP net.IP, packetDat srcPort, dstPort := getPortsFromPacket(d) if id, pass := m.routeACLsPass(srcIP, dstIP, proto, srcPort, dstPort); !pass { - srcAddr, _ := netip.AddrFromSlice(srcIP) - dstAddr, _ := netip.AddrFromSlice(dstIP) - m.logger.Trace("Dropping routed packet (ACL denied): rule_id=%s proto=%v src=%s:%d dst=%s:%d", id, pnum, srcIP, srcPort, dstIP, dstPort) @@ -744,8 +747,8 @@ func (m *Manager) handleRoutedTraffic(d *decoder, srcIP, dstIP net.IP, packetDat RuleID: id, Direction: nftypes.Ingress, Protocol: pnum, - SourceIP: srcAddr, - DestIP: dstAddr, + SourceIP: srcIP, + DestIP: dstIP, SourcePort: srcPort, DestPort: dstPort, // TODO: icmp type/code @@ -754,7 +757,7 @@ func (m *Manager) handleRoutedTraffic(d *decoder, srcIP, dstIP net.IP, packetDat } // Let forwarder handle the packet if it passed route ACLs - if err := m.forwarder.InjectIncomingPacket(packetData); err != nil { + if err := m.forwarder.Load().InjectIncomingPacket(packetData); err != nil { m.logger.Error("Failed to inject incoming packet: %v", err) } @@ -799,7 +802,7 @@ func (m *Manager) isValidPacket(d *decoder, packetData []byte) bool { return true } -func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP net.IP) bool { +func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP netip.Addr) bool { switch d.decoded[1] { case layers.LayerTypeTCP: return m.tcpTracker.IsValidInbound( @@ -844,20 +847,22 @@ func (m *Manager) isSpecialICMP(d *decoder) bool { icmpType == layers.ICMPv4TypeTimeExceeded } -func (m *Manager) peerACLsBlock(srcIP net.IP, packetData []byte, rules map[string]RuleSet, d *decoder) ([]byte, bool) { +func (m *Manager) peerACLsBlock(srcIP netip.Addr, packetData []byte, rules map[netip.Addr]RuleSet, d *decoder) ([]byte, bool) { + m.mutex.RLock() + defer m.mutex.RUnlock() if m.isSpecialICMP(d) { return nil, false } - if mgmtId, filter, ok := validateRule(srcIP, packetData, rules[srcIP.String()], d); ok { + if mgmtId, filter, ok := validateRule(srcIP, packetData, rules[srcIP], d); ok { return mgmtId, filter } - if mgmtId, filter, ok := validateRule(srcIP, packetData, rules["0.0.0.0"], d); ok { + if mgmtId, filter, ok := validateRule(srcIP, packetData, rules[netip.IPv4Unspecified()], d); ok { return mgmtId, filter } - if mgmtId, filter, ok := validateRule(srcIP, packetData, rules["::"], d); ok { + if mgmtId, filter, ok := validateRule(srcIP, packetData, rules[netip.IPv6Unspecified()], d); ok { return mgmtId, filter } @@ -882,10 +887,10 @@ func portsMatch(rulePort *firewall.Port, packetPort uint16) bool { return false } -func validateRule(ip net.IP, packetData []byte, rules map[string]PeerRule, d *decoder) ([]byte, bool, bool) { +func validateRule(ip netip.Addr, packetData []byte, rules map[string]PeerRule, d *decoder) ([]byte, bool, bool) { payloadLayer := d.decoded[1] for _, rule := range rules { - if rule.matchByIP && !ip.Equal(rule.ip) { + if rule.matchByIP && ip.Compare(rule.ip) != 0 { continue } @@ -919,16 +924,13 @@ func validateRule(ip net.IP, packetData []byte, rules map[string]PeerRule, d *de return nil, false, false } -// routeACLsPass returns treu if the packet is allowed by the route ACLs -func (m *Manager) routeACLsPass(srcIP, dstIP net.IP, proto firewall.Protocol, srcPort, dstPort uint16) ([]byte, bool) { +// routeACLsPass returns true if the packet is allowed by the route ACLs +func (m *Manager) routeACLsPass(srcIP, dstIP netip.Addr, proto firewall.Protocol, srcPort, dstPort uint16) ([]byte, bool) { m.mutex.RLock() defer m.mutex.RUnlock() - srcAddr := netip.AddrFrom4([4]byte(srcIP.To4())) - dstAddr := netip.AddrFrom4([4]byte(dstIP.To4())) - for _, rule := range m.routeRules { - if m.ruleMatches(rule, srcAddr, dstAddr, proto, srcPort, dstPort) { + if matches := m.ruleMatches(rule, srcIP, dstIP, proto, srcPort, dstPort); matches { return rule.mgmtId, rule.action == firewall.ActionAccept } } @@ -972,9 +974,7 @@ func (m *Manager) SetNetwork(network *net.IPNet) { // AddUDPPacketHook calls hook when UDP packet from given direction matched // // Hook function returns flag which indicates should be the matched package dropped or not -func (m *Manager) AddUDPPacketHook( - in bool, ip net.IP, dPort uint16, hook func([]byte) bool, -) string { +func (m *Manager) AddUDPPacketHook(in bool, ip netip.Addr, dPort uint16, hook func(packet []byte) bool) string { r := PeerRule{ id: uuid.New().String(), ip: ip, @@ -984,23 +984,22 @@ func (m *Manager) AddUDPPacketHook( udpHook: hook, } - if ip.To4() != nil { + if ip.Is4() { r.ipLayer = layers.LayerTypeIPv4 } m.mutex.Lock() if in { - if _, ok := m.incomingRules[r.ip.String()]; !ok { - m.incomingRules[r.ip.String()] = make(map[string]PeerRule) + if _, ok := m.incomingRules[r.ip]; !ok { + m.incomingRules[r.ip] = make(map[string]PeerRule) } - m.incomingRules[r.ip.String()][r.id] = r + m.incomingRules[r.ip][r.id] = r } else { - if _, ok := m.outgoingRules[r.ip.String()]; !ok { - m.outgoingRules[r.ip.String()] = make(map[string]PeerRule) + if _, ok := m.outgoingRules[r.ip]; !ok { + m.outgoingRules[r.ip] = make(map[string]PeerRule) } - m.outgoingRules[r.ip.String()][r.id] = r + m.outgoingRules[r.ip][r.id] = r } - m.mutex.Unlock() return r.id @@ -1048,20 +1047,21 @@ func (m *Manager) DisableRouting() error { m.mutex.Lock() defer m.mutex.Unlock() - if m.forwarder == nil { + fwder := m.forwarder.Load() + if fwder == nil { return nil } - m.routingEnabled = false - m.nativeRouter = false + m.routingEnabled.Store(false) + m.nativeRouter.Store(false) // don't stop forwarder if in use by netstack if m.netstack && m.localForwarding { return nil } - m.forwarder.Stop() - m.forwarder = nil + fwder.Stop() + m.forwarder.Store(nil) log.Debug("forwarder stopped") diff --git a/client/firewall/uspfilter/uspfilter_bench_test.go b/client/firewall/uspfilter/uspfilter_bench_test.go index ea9a4285a..9b06b2803 100644 --- a/client/firewall/uspfilter/uspfilter_bench_test.go +++ b/client/firewall/uspfilter/uspfilter_bench_test.go @@ -1054,8 +1054,8 @@ func BenchmarkRouteACLs(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { for _, tc := range cases { - srcIP := net.ParseIP(tc.srcIP) - dstIP := net.ParseIP(tc.dstIP) + srcIP := netip.MustParseAddr(tc.srcIP) + dstIP := netip.MustParseAddr(tc.dstIP) manager.routeACLsPass(srcIP, dstIP, tc.proto, 0, tc.dstPort) } } diff --git a/client/firewall/uspfilter/uspfilter_filter_test.go b/client/firewall/uspfilter/uspfilter_filter_test.go index d3dbef126..7005d501c 100644 --- a/client/firewall/uspfilter/uspfilter_filter_test.go +++ b/client/firewall/uspfilter/uspfilter_filter_test.go @@ -306,8 +306,8 @@ func setupRoutedManager(tb testing.TB, network string) *Manager { require.NoError(tb, manager.EnableRouting()) require.NoError(tb, err) require.NotNil(tb, manager) - require.True(tb, manager.routingEnabled) - require.False(tb, manager.nativeRouter) + require.True(tb, manager.routingEnabled.Load()) + require.False(tb, manager.nativeRouter.Load()) tb.Cleanup(func() { require.NoError(tb, manager.Reset(nil)) @@ -818,8 +818,8 @@ func TestRouteACLFiltering(t *testing.T) { require.NoError(t, manager.DeleteRouteRule(rule)) }) - srcIP := net.ParseIP(tc.srcIP) - dstIP := net.ParseIP(tc.dstIP) + srcIP := netip.MustParseAddr(tc.srcIP) + dstIP := netip.MustParseAddr(tc.dstIP) // testing routeACLsPass only and not DropIncoming, as routed packets are dropped after being passed // to the forwarder @@ -1006,8 +1006,8 @@ func TestRouteACLOrder(t *testing.T) { }) for i, p := range tc.packets { - srcIP := net.ParseIP(p.srcIP) - dstIP := net.ParseIP(p.dstIP) + srcIP := netip.MustParseAddr(p.srcIP) + dstIP := netip.MustParseAddr(p.dstIP) _, isAllowed := manager.routeACLsPass(srcIP, dstIP, p.proto, p.srcPort, p.dstPort) require.Equal(t, p.shouldPass, isAllowed, "packet %d failed", i) diff --git a/client/firewall/uspfilter/uspfilter_test.go b/client/firewall/uspfilter/uspfilter_test.go index e0e2b86c7..429265794 100644 --- a/client/firewall/uspfilter/uspfilter_test.go +++ b/client/firewall/uspfilter/uspfilter_test.go @@ -125,19 +125,19 @@ func TestManagerDeleteRule(t *testing.T) { return } - ip := net.ParseIP("192.168.1.1") + ip := netip.MustParseAddr("192.168.1.1") proto := fw.ProtocolTCP port := &fw.Port{Values: []uint16{80}} action := fw.ActionDrop - rule2, err := m.AddPeerFiltering(nil, ip, proto, nil, port, action, "") + rule2, err := m.AddPeerFiltering(nil, ip.AsSlice(), proto, nil, port, action, "") if err != nil { t.Errorf("failed to add filtering: %v", err) return } for _, r := range rule2 { - if _, ok := m.incomingRules[ip.String()][r.ID()]; !ok { + if _, ok := m.incomingRules[ip][r.ID()]; !ok { t.Errorf("rule2 is not in the incomingRules") } } @@ -151,7 +151,7 @@ func TestManagerDeleteRule(t *testing.T) { } for _, r := range rule2 { - if _, ok := m.incomingRules[ip.String()][r.ID()]; ok { + if _, ok := m.incomingRules[ip][r.ID()]; ok { t.Errorf("rule2 is not in the incomingRules") } } @@ -162,7 +162,7 @@ func TestAddUDPPacketHook(t *testing.T) { name string in bool expDir fw.RuleDirection - ip net.IP + ip netip.Addr dPort uint16 hook func([]byte) bool expectedID string @@ -171,7 +171,7 @@ func TestAddUDPPacketHook(t *testing.T) { name: "Test Outgoing UDP Packet Hook", in: false, expDir: fw.RuleDirectionOUT, - ip: net.IPv4(10, 168, 0, 1), + ip: netip.MustParseAddr("10.168.0.1"), dPort: 8000, hook: func([]byte) bool { return true }, }, @@ -179,7 +179,7 @@ func TestAddUDPPacketHook(t *testing.T) { name: "Test Incoming UDP Packet Hook", in: true, expDir: fw.RuleDirectionIN, - ip: net.IPv6loopback, + ip: netip.MustParseAddr("::1"), dPort: 9000, hook: func([]byte) bool { return false }, }, @@ -196,11 +196,11 @@ func TestAddUDPPacketHook(t *testing.T) { var addedRule PeerRule if tt.in { - if len(manager.incomingRules[tt.ip.String()]) != 1 { + if len(manager.incomingRules[tt.ip]) != 1 { t.Errorf("expected 1 incoming rule, got %d", len(manager.incomingRules)) return } - for _, rule := range manager.incomingRules[tt.ip.String()] { + for _, rule := range manager.incomingRules[tt.ip] { addedRule = rule } } else { @@ -208,12 +208,12 @@ func TestAddUDPPacketHook(t *testing.T) { t.Errorf("expected 1 outgoing rule, got %d", len(manager.outgoingRules)) return } - for _, rule := range manager.outgoingRules[tt.ip.String()] { + for _, rule := range manager.outgoingRules[tt.ip] { addedRule = rule } } - if !tt.ip.Equal(addedRule.ip) { + if tt.ip.Compare(addedRule.ip) != 0 { t.Errorf("expected ip %s, got %s", tt.ip, addedRule.ip) return } @@ -357,7 +357,7 @@ func TestRemovePacketHook(t *testing.T) { // Add a UDP packet hook hookFunc := func(data []byte) bool { return true } - hookID := manager.AddUDPPacketHook(false, net.IPv4(192, 168, 0, 1), 8080, hookFunc) + hookID := manager.AddUDPPacketHook(false, netip.MustParseAddr("192.168.0.1"), 8080, hookFunc) // Assert the hook is added by finding it in the manager's outgoing rules found := false @@ -423,7 +423,7 @@ func TestProcessOutgoingHooks(t *testing.T) { hookCalled := false hookID := manager.AddUDPPacketHook( false, - net.ParseIP("100.10.0.100"), + netip.MustParseAddr("100.10.0.100"), 53, func([]byte) bool { hookCalled = true @@ -573,7 +573,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { require.False(t, drop, "Initial outbound packet should not be dropped") // Verify connection was tracked - conn, exists := manager.udpTracker.GetConnection(srcIP.AsSlice(), srcPort, dstIP.AsSlice(), dstPort) + conn, exists := manager.udpTracker.GetConnection(srcIP, srcPort, dstIP, dstPort) require.True(t, exists, "Connection should be tracked after outbound packet") require.True(t, srcIP.Compare(conn.SourceIP) == 0, "Source IP should match") @@ -641,7 +641,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { // If the connection should still be valid, verify it exists if cp.shouldAllow { - conn, exists := manager.udpTracker.GetConnection(srcIP.AsSlice(), srcPort, dstIP.AsSlice(), dstPort) + conn, exists := manager.udpTracker.GetConnection(srcIP, srcPort, dstIP, dstPort) require.True(t, exists, "Connection should still exist during valid window") require.True(t, time.Since(conn.GetLastSeen()) < manager.udpTracker.Timeout(), "LastSeen should be updated for valid responses") diff --git a/client/iface/device/device_filter.go b/client/iface/device/device_filter.go index f87f10429..f21804683 100644 --- a/client/iface/device/device_filter.go +++ b/client/iface/device/device_filter.go @@ -2,6 +2,7 @@ package device import ( "net" + "net/netip" "sync" "golang.zx2c4.com/wireguard/tun" @@ -19,7 +20,7 @@ type PacketFilter interface { // // Hook function returns flag which indicates should be the matched package dropped or not. // Hook function receives raw network packet data as argument. - AddUDPPacketHook(in bool, ip net.IP, dPort uint16, hook func(packet []byte) bool) string + AddUDPPacketHook(in bool, ip netip.Addr, dPort uint16, hook func(packet []byte) bool) string // RemovePacketHook removes hook by ID RemovePacketHook(hookID string) error diff --git a/client/iface/mocks/filter.go b/client/iface/mocks/filter.go index 6348e0e77..f00024e38 100644 --- a/client/iface/mocks/filter.go +++ b/client/iface/mocks/filter.go @@ -6,6 +6,7 @@ package mocks import ( net "net" + "net/netip" reflect "reflect" gomock "github.com/golang/mock/gomock" @@ -35,7 +36,7 @@ func (m *MockPacketFilter) EXPECT() *MockPacketFilterMockRecorder { } // AddUDPPacketHook mocks base method. -func (m *MockPacketFilter) AddUDPPacketHook(arg0 bool, arg1 net.IP, arg2 uint16, arg3 func([]byte) bool) string { +func (m *MockPacketFilter) AddUDPPacketHook(arg0 bool, arg1 netip.Addr, arg2 uint16, arg3 func([]byte) bool) string { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "AddUDPPacketHook", arg0, arg1, arg2, arg3) ret0, _ := ret[0].(string) diff --git a/client/internal/dns/service_memory.go b/client/internal/dns/service_memory.go index 250f3ab2e..34c563757 100644 --- a/client/internal/dns/service_memory.go +++ b/client/internal/dns/service_memory.go @@ -2,7 +2,7 @@ package dns import ( "fmt" - "net" + "net/netip" "sync" "github.com/google/gopacket" @@ -117,5 +117,10 @@ func (s *ServiceViaMemory) filterDNSTraffic() (string, error) { return true } - return filter.AddUDPPacketHook(false, net.ParseIP(s.runtimeIP), uint16(s.runtimePort), hook), nil + ip, err := netip.ParseAddr(s.runtimeIP) + if err != nil { + return "", fmt.Errorf("parse runtime ip: %w", err) + } + + return filter.AddUDPPacketHook(false, ip, uint16(s.runtimePort), hook), nil } diff --git a/client/server/trace.go b/client/server/trace.go index 66b83d8cf..8b9d375f3 100644 --- a/client/server/trace.go +++ b/client/server/trace.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net" + "net/netip" fw "github.com/netbirdio/netbird/client/firewall/manager" "github.com/netbirdio/netbird/client/firewall/uspfilter" @@ -41,11 +42,21 @@ func (s *Server) TracePacket(_ context.Context, req *proto.TracePacketRequest) ( srcIP = engine.GetWgAddr() } + srcAddr, ok := netip.AddrFromSlice(srcIP) + if !ok { + return nil, fmt.Errorf("invalid source IP address") + } + dstIP := net.ParseIP(req.GetDestinationIp()) if req.GetDestinationIp() == "self" { dstIP = engine.GetWgAddr() } + dstAddr, ok := netip.AddrFromSlice(dstIP) + if !ok { + return nil, fmt.Errorf("invalid source IP address") + } + if srcIP == nil || dstIP == nil { return nil, fmt.Errorf("invalid IP address") } @@ -85,8 +96,8 @@ func (s *Server) TracePacket(_ context.Context, req *proto.TracePacketRequest) ( } builder := &uspfilter.PacketBuilder{ - SrcIP: srcIP, - DstIP: dstIP, + SrcIP: srcAddr, + DstIP: dstAddr, Protocol: protocol, SrcPort: uint16(req.GetSourcePort()), DstPort: uint16(req.GetDestinationPort()), From 0a042ac36de383013f716b95c0affde7880156df Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Wed, 5 Mar 2025 19:11:20 +0100 Subject: [PATCH 03/11] Fix merge conflict --- client/firewall/uspfilter/conntrack/tcp.go | 4 ++-- client/firewall/uspfilter/tracer_test.go | 0 2 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 client/firewall/uspfilter/tracer_test.go diff --git a/client/firewall/uspfilter/conntrack/tcp.go b/client/firewall/uspfilter/conntrack/tcp.go index 0c0e7bd99..892ae6bcb 100644 --- a/client/firewall/uspfilter/conntrack/tcp.go +++ b/client/firewall/uspfilter/conntrack/tcp.go @@ -299,7 +299,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i } else if flags&TCPRst != 0 { conn.State = TCPStateClosed conn.SetTombstone() - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) } case TCPStateFinWait1: @@ -313,7 +313,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i case flags&TCPRst != 0: conn.State = TCPStateClosed conn.SetTombstone() - t.sendEvent(nftypes.TypeEnd, key, conn) + t.sendEvent(nftypes.TypeEnd, conn) } case TCPStateFinWait2: diff --git a/client/firewall/uspfilter/tracer_test.go b/client/firewall/uspfilter/tracer_test.go deleted file mode 100644 index e69de29bb..000000000 From b180edbe5c110bc8a43cb79ea80ee6057fd495ec Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Thu, 6 Mar 2025 14:51:23 +0100 Subject: [PATCH 04/11] Track icmp with id only (#3447) --- client/firewall/uspfilter/conntrack/icmp.go | 42 +++++++++---------- .../firewall/uspfilter/conntrack/icmp_test.go | 6 +-- client/firewall/uspfilter/uspfilter.go | 5 +-- 3 files changed, 24 insertions(+), 29 deletions(-) diff --git a/client/firewall/uspfilter/conntrack/icmp.go b/client/firewall/uspfilter/conntrack/icmp.go index 97617a52d..7837c1a23 100644 --- a/client/firewall/uspfilter/conntrack/icmp.go +++ b/client/firewall/uspfilter/conntrack/icmp.go @@ -22,14 +22,13 @@ const ( // ICMPConnKey uniquely identifies an ICMP connection type ICMPConnKey struct { - SrcIP netip.Addr - DstIP netip.Addr - Sequence uint16 - ID uint16 + SrcIP netip.Addr + DstIP netip.Addr + ID uint16 } func (i ICMPConnKey) String() string { - return fmt.Sprintf("%s -> %s (%d/%d)", i.SrcIP, i.DstIP, i.ID, i.Sequence) + return fmt.Sprintf("%s -> %s (id %d)", i.SrcIP, i.DstIP, i.ID) } // ICMPConnTrack represents an ICMP connection state @@ -69,12 +68,11 @@ func NewICMPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nfty return tracker } -func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16) (ICMPConnKey, bool) { +func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint16) (ICMPConnKey, bool) { key := ICMPConnKey{ - SrcIP: srcIP, - DstIP: dstIP, - ID: id, - Sequence: seq, + SrcIP: srcIP, + DstIP: dstIP, + ID: id, } t.mutex.RLock() @@ -91,22 +89,21 @@ func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint } // TrackOutbound records an outbound ICMP connection -func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode) { - if _, exists := t.updateIfExists(dstIP, srcIP, id, seq); !exists { +func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode) { + if _, exists := t.updateIfExists(dstIP, srcIP, id); !exists { // if (inverted direction) conn is not tracked, track this direction - t.track(srcIP, dstIP, id, seq, typecode, nftypes.Egress) + t.track(srcIP, dstIP, id, typecode, nftypes.Egress) } } // TrackInbound records an inbound ICMP Echo Request -func (t *ICMPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode) { - t.track(srcIP, dstIP, id, seq, typecode, nftypes.Ingress) +func (t *ICMPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode) { + t.track(srcIP, dstIP, id, typecode, nftypes.Ingress) } // track is the common implementation for tracking both inbound and outbound ICMP connections -func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction) { - // TODO: icmp doesn't need to extend the timeout - key, exists := t.updateIfExists(srcIP, dstIP, id, seq) +func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction) { + key, exists := t.updateIfExists(srcIP, dstIP, id) if exists { return } @@ -141,16 +138,15 @@ func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq u } // IsValidInbound checks if an inbound ICMP Echo Reply matches a tracked request -func (t *ICMPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, icmpType uint8) bool { +func (t *ICMPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, icmpType uint8) bool { if icmpType != uint8(layers.ICMPv4TypeEchoReply) { return false } key := ICMPConnKey{ - SrcIP: dstIP, - DstIP: srcIP, - ID: id, - Sequence: seq, + SrcIP: dstIP, + DstIP: srcIP, + ID: id, } t.mutex.RLock() diff --git a/client/firewall/uspfilter/conntrack/icmp_test.go b/client/firewall/uspfilter/conntrack/icmp_test.go index 259cc21a4..01040feb2 100644 --- a/client/firewall/uspfilter/conntrack/icmp_test.go +++ b/client/firewall/uspfilter/conntrack/icmp_test.go @@ -15,7 +15,7 @@ func BenchmarkICMPTracker(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), uint16(i%65535), 0) + tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 0) } }) @@ -28,12 +28,12 @@ func BenchmarkICMPTracker(b *testing.B) { // Pre-populate some connections for i := 0; i < 1000; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i), uint16(i), 0) + tracker.TrackOutbound(srcIP, dstIP, uint16(i), 0) } b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.IsValidInbound(dstIP, srcIP, uint16(i%1000), uint16(i%1000), 0) + tracker.IsValidInbound(dstIP, srcIP, uint16(i%1000), 0) } }) } diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go index 05dd9ff06..e663966ab 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/uspfilter.go @@ -602,7 +602,7 @@ func (m *Manager) trackOutbound(d *decoder, srcIP, dstIP netip.Addr) { flags := getTCPFlags(&d.tcp) m.tcpTracker.TrackOutbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags) case layers.LayerTypeICMPv4: - m.icmpTracker.TrackOutbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.Seq, d.icmp4.TypeCode) + m.icmpTracker.TrackOutbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode) } } @@ -615,7 +615,7 @@ func (m *Manager) trackInbound(d *decoder, srcIP, dstIP netip.Addr) { flags := getTCPFlags(&d.tcp) m.tcpTracker.TrackInbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags) case layers.LayerTypeICMPv4: - m.icmpTracker.TrackInbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.Seq, d.icmp4.TypeCode) + m.icmpTracker.TrackInbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode) } } @@ -826,7 +826,6 @@ func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP netip.Addr) srcIP, dstIP, d.icmp4.Id, - d.icmp4.Seq, d.icmp4.TypeCode.Type(), ) From 5ff77b3595c58809c6ee3342daa49c44eefc7b90 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Thu, 6 Mar 2025 16:52:56 +0100 Subject: [PATCH 05/11] Add flow userspace counters (#3438) --- client/firewall/uspfilter/conntrack/common.go | 27 +++++-- .../uspfilter/conntrack/common_test.go | 8 +- client/firewall/uspfilter/conntrack/icmp.go | 44 ++++++---- .../firewall/uspfilter/conntrack/icmp_test.go | 6 +- client/firewall/uspfilter/conntrack/tcp.go | 41 ++++++---- .../firewall/uspfilter/conntrack/tcp_test.go | 56 ++++++------- client/firewall/uspfilter/conntrack/udp.go | 41 ++++++---- .../firewall/uspfilter/conntrack/udp_test.go | 14 ++-- client/firewall/uspfilter/forwarder/icmp.go | 55 +++++-------- client/firewall/uspfilter/forwarder/tcp.go | 32 ++++++-- client/firewall/uspfilter/forwarder/udp.go | 33 ++++++-- client/firewall/uspfilter/tracer.go | 4 +- client/firewall/uspfilter/uspfilter.go | 45 ++++++----- .../uspfilter/uspfilter_bench_test.go | 80 +++++++++---------- .../uspfilter/uspfilter_filter_test.go | 4 +- client/firewall/uspfilter/uspfilter_test.go | 14 ++-- client/iface/device/device_filter.go | 8 +- client/iface/device/device_filter_test.go | 4 +- client/iface/mocks/filter.go | 16 ++-- client/internal/dns/server_test.go | 2 +- client/internal/netflow/types/types.go | 5 +- 21 files changed, 307 insertions(+), 232 deletions(-) diff --git a/client/firewall/uspfilter/conntrack/common.go b/client/firewall/uspfilter/conntrack/common.go index b87ec6fd2..3de0bb3f4 100644 --- a/client/firewall/uspfilter/conntrack/common.go +++ b/client/firewall/uspfilter/conntrack/common.go @@ -13,13 +13,15 @@ import ( // BaseConnTrack provides common fields and locking for all connection types type BaseConnTrack struct { - FlowId uuid.UUID - Direction nftypes.Direction - SourceIP netip.Addr - DestIP netip.Addr - SourcePort uint16 - DestPort uint16 - lastSeen atomic.Int64 + FlowId uuid.UUID + Direction nftypes.Direction + SourceIP netip.Addr + DestIP netip.Addr + lastSeen atomic.Int64 + PacketsTx atomic.Uint64 + PacketsRx atomic.Uint64 + BytesTx atomic.Uint64 + BytesRx atomic.Uint64 } // these small methods will be inlined by the compiler @@ -29,6 +31,17 @@ func (b *BaseConnTrack) UpdateLastSeen() { b.lastSeen.Store(time.Now().UnixNano()) } +// UpdateCounters safely updates the packet and byte counters +func (b *BaseConnTrack) UpdateCounters(direction nftypes.Direction, bytes int) { + if direction == nftypes.Egress { + b.PacketsTx.Add(1) + b.BytesTx.Add(uint64(bytes)) + } else { + b.PacketsRx.Add(1) + b.BytesRx.Add(uint64(bytes)) + } +} + // GetLastSeen safely gets the last seen timestamp func (b *BaseConnTrack) GetLastSeen() time.Time { return time.Unix(0, b.lastSeen.Load()) diff --git a/client/firewall/uspfilter/conntrack/common_test.go b/client/firewall/uspfilter/conntrack/common_test.go index e83b6aa85..ca1136e6f 100644 --- a/client/firewall/uspfilter/conntrack/common_test.go +++ b/client/firewall/uspfilter/conntrack/common_test.go @@ -32,11 +32,11 @@ func BenchmarkMemoryPressure(b *testing.B) { for i := 0; i < b.N; i++ { srcIdx := i % len(srcIPs) dstIdx := (i + 1) % len(dstIPs) - tracker.TrackOutbound(srcIPs[srcIdx], dstIPs[dstIdx], uint16(i%65535), 80, TCPSyn) + tracker.TrackOutbound(srcIPs[srcIdx], dstIPs[dstIdx], uint16(i%65535), 80, TCPSyn, 0) // Simulate some valid inbound packets if i%3 == 0 { - tracker.IsValidInbound(dstIPs[dstIdx], srcIPs[srcIdx], 80, uint16(i%65535), TCPAck) + tracker.IsValidInbound(dstIPs[dstIdx], srcIPs[srcIdx], 80, uint16(i%65535), TCPAck, 0) } } }) @@ -57,11 +57,11 @@ func BenchmarkMemoryPressure(b *testing.B) { for i := 0; i < b.N; i++ { srcIdx := i % len(srcIPs) dstIdx := (i + 1) % len(dstIPs) - tracker.TrackOutbound(srcIPs[srcIdx], dstIPs[dstIdx], uint16(i%65535), 80) + tracker.TrackOutbound(srcIPs[srcIdx], dstIPs[dstIdx], uint16(i%65535), 80, 0) // Simulate some valid inbound packets if i%3 == 0 { - tracker.IsValidInbound(dstIPs[dstIdx], srcIPs[srcIdx], 80, uint16(i%65535)) + tracker.IsValidInbound(dstIPs[dstIdx], srcIPs[srcIdx], 80, uint16(i%65535), 0) } } }) diff --git a/client/firewall/uspfilter/conntrack/icmp.go b/client/firewall/uspfilter/conntrack/icmp.go index 7837c1a23..730edd6ce 100644 --- a/client/firewall/uspfilter/conntrack/icmp.go +++ b/client/firewall/uspfilter/conntrack/icmp.go @@ -68,7 +68,7 @@ func NewICMPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nfty return tracker } -func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint16) (ICMPConnKey, bool) { +func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint16, direction nftypes.Direction, size int) (ICMPConnKey, bool) { key := ICMPConnKey{ SrcIP: srcIP, DstIP: dstIP, @@ -81,6 +81,7 @@ func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint if exists { conn.UpdateLastSeen() + conn.UpdateCounters(direction, size) return key, true } @@ -89,21 +90,22 @@ func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint } // TrackOutbound records an outbound ICMP connection -func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode) { - if _, exists := t.updateIfExists(dstIP, srcIP, id); !exists { +func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode, size int) { + if _, exists := t.updateIfExists(dstIP, srcIP, id, nftypes.Egress, size); !exists { // if (inverted direction) conn is not tracked, track this direction - t.track(srcIP, dstIP, id, typecode, nftypes.Egress) + t.track(srcIP, dstIP, id, typecode, nftypes.Egress, size) } } // TrackInbound records an inbound ICMP Echo Request -func (t *ICMPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode) { - t.track(srcIP, dstIP, id, typecode, nftypes.Ingress) +func (t *ICMPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, size int) { + t.track(srcIP, dstIP, id, typecode, nftypes.Ingress, size) } // track is the common implementation for tracking both inbound and outbound ICMP connections -func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction) { - key, exists := t.updateIfExists(srcIP, dstIP, id) +func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction, size int) { + // TODO: icmp doesn't need to extend the timeout + key, exists := t.updateIfExists(srcIP, dstIP, id, direction, size) if exists { return } @@ -113,7 +115,7 @@ func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typec // non echo requests don't need tracking if typ != uint8(layers.ICMPv4TypeEchoRequest) { t.logger.Trace("New %s ICMP connection %s type %d code %d", direction, key, typ, code) - t.sendStartEvent(direction, srcIP, dstIP, typ, code) + t.sendStartEvent(direction, srcIP, dstIP, typ, code, size) return } @@ -138,7 +140,7 @@ func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typec } // IsValidInbound checks if an inbound ICMP Echo Reply matches a tracked request -func (t *ICMPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, icmpType uint8) bool { +func (t *ICMPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, icmpType uint8, size int) bool { if icmpType != uint8(layers.ICMPv4TypeEchoReply) { return false } @@ -158,6 +160,7 @@ func (t *ICMPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, id uint } conn.UpdateLastSeen() + conn.UpdateCounters(nftypes.Ingress, size) return true } @@ -181,7 +184,8 @@ func (t *ICMPTracker) cleanup() { if conn.timeoutExceeded(t.timeout) { delete(t.connections, key) - t.logger.Debug("Removed ICMP connection %s (timeout)", &key) + t.logger.Debug("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) } } @@ -207,11 +211,15 @@ func (t *ICMPTracker) sendEvent(typ nftypes.Type, conn *ICMPConnTrack) { DestIP: conn.DestIP, ICMPType: conn.ICMPType, ICMPCode: conn.ICMPCode, + RxPackets: conn.PacketsRx.Load(), + TxPackets: conn.PacketsTx.Load(), + RxBytes: conn.BytesRx.Load(), + TxBytes: conn.BytesTx.Load(), }) } -func (t *ICMPTracker) sendStartEvent(direction nftypes.Direction, srcIP netip.Addr, dstIP netip.Addr, typ uint8, code uint8) { - t.flowLogger.StoreEvent(nftypes.EventFields{ +func (t *ICMPTracker) sendStartEvent(direction nftypes.Direction, srcIP netip.Addr, dstIP netip.Addr, typ uint8, code uint8, size int) { + fields := nftypes.EventFields{ FlowID: uuid.New(), Type: nftypes.TypeStart, Direction: direction, @@ -220,5 +228,13 @@ func (t *ICMPTracker) sendStartEvent(direction nftypes.Direction, srcIP netip.Ad DestIP: dstIP, ICMPType: typ, ICMPCode: code, - }) + } + if direction == nftypes.Ingress { + fields.RxPackets = 1 + fields.RxBytes = uint64(size) + } else { + fields.TxPackets = 1 + fields.TxBytes = uint64(size) + } + t.flowLogger.StoreEvent(fields) } diff --git a/client/firewall/uspfilter/conntrack/icmp_test.go b/client/firewall/uspfilter/conntrack/icmp_test.go index 01040feb2..5a7b36a36 100644 --- a/client/firewall/uspfilter/conntrack/icmp_test.go +++ b/client/firewall/uspfilter/conntrack/icmp_test.go @@ -15,7 +15,7 @@ func BenchmarkICMPTracker(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 0) + tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 0, 0) } }) @@ -28,12 +28,12 @@ func BenchmarkICMPTracker(b *testing.B) { // Pre-populate some connections for i := 0; i < 1000; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i), 0) + tracker.TrackOutbound(srcIP, dstIP, uint16(i), 0, 0) } b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.IsValidInbound(dstIP, srcIP, uint16(i%1000), 0) + tracker.IsValidInbound(dstIP, srcIP, uint16(i%1000), 0, 0) } }) } diff --git a/client/firewall/uspfilter/conntrack/tcp.go b/client/firewall/uspfilter/conntrack/tcp.go index 892ae6bcb..96bbf0220 100644 --- a/client/firewall/uspfilter/conntrack/tcp.go +++ b/client/firewall/uspfilter/conntrack/tcp.go @@ -88,6 +88,8 @@ const ( // TCPConnTrack represents a TCP connection state type TCPConnTrack struct { BaseConnTrack + SourcePort uint16 + DestPort uint16 State TCPState established atomic.Bool tombstone atomic.Bool @@ -144,7 +146,7 @@ func NewTCPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nftyp return tracker } -func (t *TCPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) (ConnKey, bool) { +func (t *TCPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, direction nftypes.Direction, size int) (ConnKey, bool) { key := ConnKey{ SrcIP: srcIP, DstIP: dstIP, @@ -161,6 +163,8 @@ func (t *TCPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort t.updateState(key, conn, flags, conn.Direction == nftypes.Egress) conn.Unlock() + conn.UpdateCounters(direction, size) + return key, true } @@ -168,34 +172,34 @@ func (t *TCPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort } // TrackOutbound records an outbound TCP connection -func (t *TCPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) { - if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort, flags); !exists { +func (t *TCPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, size int) { + if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort, flags, 0, 0); !exists { // if (inverted direction) conn is not tracked, track this direction - t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Egress) + t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Egress, size) } } // TrackInbound processes an inbound TCP packet and updates connection state -func (t *TCPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) { - t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Ingress) +func (t *TCPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, size int) { + t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Ingress, size) } // track is the common implementation for tracking both inbound and outbound connections -func (t *TCPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, direction nftypes.Direction) { - key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort, flags) +func (t *TCPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, direction nftypes.Direction, size int) { + key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort, flags, direction, size) if exists { return } conn := &TCPConnTrack{ BaseConnTrack: BaseConnTrack{ - FlowId: uuid.New(), - Direction: direction, - SourceIP: srcIP, - DestIP: dstIP, - SourcePort: srcPort, - DestPort: dstPort, + FlowId: uuid.New(), + Direction: direction, + SourceIP: srcIP, + DestIP: dstIP, }, + SourcePort: srcPort, + DestPort: dstPort, } conn.established.Store(false) @@ -212,7 +216,7 @@ func (t *TCPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, d } // IsValidInbound checks if an inbound TCP packet matches a tracked connection -func (t *TCPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8) bool { +func (t *TCPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, size int) bool { key := ConnKey{ SrcIP: dstIP, DstIP: srcIP, @@ -239,6 +243,7 @@ func (t *TCPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort conn.State = TCPStateClosed conn.SetEstablished(false) conn.Unlock() + conn.UpdateCounters(nftypes.Ingress, size) t.logger.Trace("TCP connection reset: %s", key) t.sendEvent(nftypes.TypeEnd, conn) @@ -427,7 +432,7 @@ func (t *TCPTracker) cleanup() { // Return IPs to pool delete(t.connections, key) - t.logger.Trace("Cleaned up timed-out TCP connection %s", &key) + t.logger.Trace("Cleaned up timed-out TCP connection %s", key) // event already handled by state change if conn.State != TCPStateTimeWait { @@ -472,5 +477,9 @@ func (t *TCPTracker) sendEvent(typ nftypes.Type, conn *TCPConnTrack) { DestIP: conn.DestIP, SourcePort: conn.SourcePort, DestPort: conn.DestPort, + RxPackets: conn.PacketsRx.Load(), + TxPackets: conn.PacketsTx.Load(), + RxBytes: conn.BytesRx.Load(), + TxBytes: conn.BytesTx.Load(), }) } diff --git a/client/firewall/uspfilter/conntrack/tcp_test.go b/client/firewall/uspfilter/conntrack/tcp_test.go index 122deae1e..96558583d 100644 --- a/client/firewall/uspfilter/conntrack/tcp_test.go +++ b/client/firewall/uspfilter/conntrack/tcp_test.go @@ -58,7 +58,7 @@ func TestTCPStateMachine(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - isValid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, tt.flags) + isValid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, tt.flags, 0) require.Equal(t, !tt.wantDrop, isValid, tt.desc) }) } @@ -76,17 +76,17 @@ func TestTCPStateMachine(t *testing.T) { t.Helper() // Send initial SYN - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPSyn) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPSyn, 0) // Receive SYN-ACK - valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPSyn|TCPAck) + valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPSyn|TCPAck, 0) require.True(t, valid, "SYN-ACK should be allowed") // Send ACK - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck, 0) // Test data transfer - valid = tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPPush|TCPAck) + valid = tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPPush|TCPAck, 0) require.True(t, valid, "Data should be allowed after handshake") }, }, @@ -99,18 +99,18 @@ func TestTCPStateMachine(t *testing.T) { establishConnection(t, tracker, srcIP, dstIP, srcPort, dstPort) // Send FIN - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPFin|TCPAck) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPFin|TCPAck, 0) // Receive ACK for FIN - valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPAck) + valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPAck, 0) require.True(t, valid, "ACK for FIN should be allowed") // Receive FIN from other side - valid = tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPFin|TCPAck) + valid = tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPFin|TCPAck, 0) require.True(t, valid, "FIN should be allowed") // Send final ACK - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck, 0) }, }, { @@ -122,7 +122,7 @@ func TestTCPStateMachine(t *testing.T) { establishConnection(t, tracker, srcIP, dstIP, srcPort, dstPort) // Receive RST - valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPRst) + valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPRst, 0) require.True(t, valid, "RST should be allowed for established connection") // Connection is logically dead but we don't enforce blocking subsequent packets @@ -138,13 +138,13 @@ func TestTCPStateMachine(t *testing.T) { establishConnection(t, tracker, srcIP, dstIP, srcPort, dstPort) // Both sides send FIN+ACK - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPFin|TCPAck) - valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPFin|TCPAck) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPFin|TCPAck, 0) + valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPFin|TCPAck, 0) require.True(t, valid, "Simultaneous FIN should be allowed") // Both sides send final ACK - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck) - valid = tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPAck) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck, 0) + valid = tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPAck, 0) require.True(t, valid, "Final ACKs should be allowed") }, }, @@ -181,12 +181,12 @@ func TestRSTHandling(t *testing.T) { name: "RST in established", setupState: func() { // Establish connection first - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPSyn) - tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPSyn|TCPAck) - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPSyn, 0) + tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPSyn|TCPAck, 0) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck, 0) }, sendRST: func() { - tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPRst) + tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPRst, 0) }, wantValid: true, desc: "Should accept RST for established connection", @@ -195,7 +195,7 @@ func TestRSTHandling(t *testing.T) { name: "RST without connection", setupState: func() {}, sendRST: func() { - tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPRst) + tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPRst, 0) }, wantValid: false, desc: "Should reject RST without connection", @@ -228,12 +228,12 @@ func TestRSTHandling(t *testing.T) { func establishConnection(t *testing.T, tracker *TCPTracker, srcIP, dstIP netip.Addr, srcPort, dstPort uint16) { t.Helper() - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPSyn) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPSyn, 0) - valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPSyn|TCPAck) + valid := tracker.IsValidInbound(dstIP, srcIP, dstPort, srcPort, TCPSyn|TCPAck, 0) require.True(t, valid, "SYN-ACK should be allowed") - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, TCPAck, 0) } func BenchmarkTCPTracker(b *testing.B) { @@ -246,7 +246,7 @@ func BenchmarkTCPTracker(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 80, TCPSyn) + tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 80, TCPSyn, 0) } }) @@ -259,12 +259,12 @@ func BenchmarkTCPTracker(b *testing.B) { // Pre-populate some connections for i := 0; i < 1000; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i), 80, TCPSyn) + tracker.TrackOutbound(srcIP, dstIP, uint16(i), 80, TCPSyn, 0) } b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.IsValidInbound(dstIP, srcIP, 80, uint16(i%1000), TCPAck) + tracker.IsValidInbound(dstIP, srcIP, 80, uint16(i%1000), TCPAck, 0) } }) @@ -279,9 +279,9 @@ func BenchmarkTCPTracker(b *testing.B) { i := 0 for pb.Next() { if i%2 == 0 { - tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 80, TCPSyn) + tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 80, TCPSyn, 0) } else { - tracker.IsValidInbound(dstIP, srcIP, 80, uint16(i%65535), TCPAck) + tracker.IsValidInbound(dstIP, srcIP, 80, uint16(i%65535), TCPAck, 0) } i++ } @@ -299,7 +299,7 @@ func BenchmarkCleanup(b *testing.B) { srcIP := netip.MustParseAddr("192.168.1.1") dstIP := netip.MustParseAddr("192.168.1.2") for i := 0; i < 10000; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i), 80, TCPSyn) + tracker.TrackOutbound(srcIP, dstIP, uint16(i), 80, TCPSyn, 0) } // Wait for connections to expire diff --git a/client/firewall/uspfilter/conntrack/udp.go b/client/firewall/uspfilter/conntrack/udp.go index 465e400be..351b3c93a 100644 --- a/client/firewall/uspfilter/conntrack/udp.go +++ b/client/firewall/uspfilter/conntrack/udp.go @@ -21,6 +21,8 @@ const ( // UDPConnTrack represents a UDP connection state type UDPConnTrack struct { BaseConnTrack + SourcePort uint16 + DestPort uint16 } // UDPTracker manages UDP connection states @@ -54,19 +56,19 @@ func NewUDPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nftyp } // TrackOutbound records an outbound UDP connection -func (t *UDPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) { - if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort); !exists { +func (t *UDPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, size int) { + if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort, nftypes.Egress, size); !exists { // if (inverted direction) conn is not tracked, track this direction - t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Egress) + t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Egress, size) } } // TrackInbound records an inbound UDP connection -func (t *UDPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) { - t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Ingress) +func (t *UDPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, size int) { + t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Ingress, size) } -func (t *UDPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) (ConnKey, bool) { +func (t *UDPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, direction nftypes.Direction, size int) (ConnKey, bool) { key := ConnKey{ SrcIP: srcIP, DstIP: dstIP, @@ -80,6 +82,7 @@ func (t *UDPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort if exists { conn.UpdateLastSeen() + conn.UpdateCounters(direction, size) return key, true } @@ -87,21 +90,21 @@ func (t *UDPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort } // track is the common implementation for tracking both inbound and outbound connections -func (t *UDPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, direction nftypes.Direction) { - key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort) +func (t *UDPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, direction nftypes.Direction, size int) { + key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort, direction, size) if exists { return } conn := &UDPConnTrack{ BaseConnTrack: BaseConnTrack{ - FlowId: uuid.New(), - Direction: direction, - SourceIP: srcIP, - DestIP: dstIP, - SourcePort: srcPort, - DestPort: dstPort, + FlowId: uuid.New(), + Direction: direction, + SourceIP: srcIP, + DestIP: dstIP, }, + SourcePort: srcPort, + DestPort: dstPort, } conn.UpdateLastSeen() @@ -114,7 +117,7 @@ func (t *UDPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, d } // IsValidInbound checks if an inbound packet matches a tracked connection -func (t *UDPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16) bool { +func (t *UDPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, size int) bool { key := ConnKey{ SrcIP: dstIP, DstIP: srcIP, @@ -131,6 +134,7 @@ func (t *UDPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort } conn.UpdateLastSeen() + conn.UpdateCounters(nftypes.Ingress, size) return true } @@ -155,7 +159,8 @@ func (t *UDPTracker) cleanup() { if conn.timeoutExceeded(t.timeout) { delete(t.connections, key) - t.logger.Trace("Removed UDP connection %s (timeout)", key) + t.logger.Trace("Removed UDP 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) } } @@ -201,5 +206,9 @@ func (t *UDPTracker) sendEvent(typ nftypes.Type, conn *UDPConnTrack) { DestIP: conn.DestIP, SourcePort: conn.SourcePort, DestPort: conn.DestPort, + RxPackets: conn.PacketsRx.Load(), + TxPackets: conn.PacketsTx.Load(), + RxBytes: conn.BytesRx.Load(), + TxBytes: conn.BytesTx.Load(), }) } diff --git a/client/firewall/uspfilter/conntrack/udp_test.go b/client/firewall/uspfilter/conntrack/udp_test.go index db7fa0f51..14b912908 100644 --- a/client/firewall/uspfilter/conntrack/udp_test.go +++ b/client/firewall/uspfilter/conntrack/udp_test.go @@ -48,7 +48,7 @@ func TestUDPTracker_TrackOutbound(t *testing.T) { srcPort := uint16(12345) dstPort := uint16(53) - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, 0) // Verify connection was tracked key := ConnKey{ @@ -76,7 +76,7 @@ func TestUDPTracker_IsValidInbound(t *testing.T) { dstPort := uint16(53) // Track outbound connection - tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort) + tracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, 0) tests := []struct { name string @@ -148,7 +148,7 @@ func TestUDPTracker_IsValidInbound(t *testing.T) { if tt.sleep > 0 { time.Sleep(tt.sleep) } - got := tracker.IsValidInbound(tt.srcIP, tt.dstIP, tt.srcPort, tt.dstPort) + got := tracker.IsValidInbound(tt.srcIP, tt.dstIP, tt.srcPort, tt.dstPort, 0) assert.Equal(t, tt.want, got) }) } @@ -194,7 +194,7 @@ func TestUDPTracker_Cleanup(t *testing.T) { } for _, conn := range connections { - tracker.TrackOutbound(conn.srcIP, conn.dstIP, conn.srcPort, conn.dstPort) + tracker.TrackOutbound(conn.srcIP, conn.dstIP, conn.srcPort, conn.dstPort, 0) } // Verify initial connections @@ -224,7 +224,7 @@ func BenchmarkUDPTracker(b *testing.B) { b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 80) + tracker.TrackOutbound(srcIP, dstIP, uint16(i%65535), 80, 0) } }) @@ -237,12 +237,12 @@ func BenchmarkUDPTracker(b *testing.B) { // Pre-populate some connections for i := 0; i < 1000; i++ { - tracker.TrackOutbound(srcIP, dstIP, uint16(i), 80) + tracker.TrackOutbound(srcIP, dstIP, uint16(i), 80, 0) } b.ResetTimer() for i := 0; i < b.N; i++ { - tracker.IsValidInbound(dstIP, srcIP, 80, uint16(i%1000)) + tracker.IsValidInbound(dstIP, srcIP, 80, uint16(i%1000), 0) } }) } diff --git a/client/firewall/uspfilter/forwarder/icmp.go b/client/firewall/uspfilter/forwarder/icmp.go index 3d7e8857b..833854a21 100644 --- a/client/firewall/uspfilter/forwarder/icmp.go +++ b/client/firewall/uspfilter/forwarder/icmp.go @@ -15,13 +15,16 @@ import ( // handleICMP handles ICMP packets from the network stack func (f *Forwarder) handleICMP(id stack.TransportEndpointID, pkt stack.PacketBufferPtr) bool { - flowID := uuid.New() - - // Extract ICMP header to get type and code icmpHdr := header.ICMPv4(pkt.TransportHeader().View().AsSlice()) icmpType := uint8(icmpHdr.Type()) icmpCode := uint8(icmpHdr.Code()) + if header.ICMPv4Type(icmpType) == header.ICMPv4EchoReply { + // dont process our own replies + return true + } + + flowID := uuid.New() f.sendICMPEvent(nftypes.TypeStart, flowID, id, icmpType, icmpCode) ctx, cancel := context.WithTimeout(f.ctx, 5*time.Second) @@ -33,8 +36,6 @@ func (f *Forwarder) handleICMP(id stack.TransportEndpointID, pkt stack.PacketBuf if err != nil { f.logger.Error("Failed to create ICMP socket for %v: %v", epID(id), err) - f.sendICMPEvent(nftypes.TypeEnd, flowID, id, icmpType, icmpCode) - // This will make netstack reply on behalf of the original destination, that's ok for now return false } @@ -42,30 +43,15 @@ func (f *Forwarder) handleICMP(id stack.TransportEndpointID, pkt stack.PacketBuf if err := conn.Close(); err != nil { f.logger.Debug("Failed to close ICMP socket: %v", err) } - - f.sendICMPEvent(nftypes.TypeEnd, flowID, id, icmpType, icmpCode) }() dstIP := f.determineDialAddr(id.LocalAddress) dst := &net.IPAddr{IP: dstIP} - // Get the complete ICMP message (header + data) fullPacket := stack.PayloadSince(pkt.TransportHeader()) payload := fullPacket.AsSlice() - // For Echo Requests, send and handle response - switch icmpHdr.Type() { - case header.ICMPv4Echo: - return f.handleEchoResponse(icmpHdr, payload, dst, conn, id, flowID) - case header.ICMPv4EchoReply: - // dont process our own replies - return true - default: - } - - // For other ICMP types (Time Exceeded, Destination Unreachable, etc) - _, err = conn.WriteTo(payload, dst) - if err != nil { + if _, err = conn.WriteTo(payload, dst); err != nil { f.logger.Error("Failed to write ICMP packet for %v: %v", epID(id), err) return true } @@ -73,21 +59,20 @@ func (f *Forwarder) handleICMP(id stack.TransportEndpointID, pkt stack.PacketBuf f.logger.Trace("Forwarded ICMP packet %v type %v code %v", epID(id), icmpHdr.Type(), icmpHdr.Code()) + // For Echo Requests, send and handle response + if header.ICMPv4Type(icmpType) == header.ICMPv4Echo { + f.handleEchoResponse(icmpHdr, conn, id) + f.sendICMPEvent(nftypes.TypeEnd, flowID, id, icmpType, icmpCode) + } + + // For other ICMP types (Time Exceeded, Destination Unreachable, etc) do nothing return true } -func (f *Forwarder) handleEchoResponse(icmpHdr header.ICMPv4, payload []byte, dst *net.IPAddr, conn net.PacketConn, id stack.TransportEndpointID, flowID uuid.UUID) bool { - if _, err := conn.WriteTo(payload, dst); err != nil { - f.logger.Error("Failed to write ICMP packet for %v: %v", epID(id), err) - return true - } - - f.logger.Trace("Forwarded ICMP packet %v type %v code %v", - epID(id), icmpHdr.Type(), icmpHdr.Code()) - +func (f *Forwarder) handleEchoResponse(icmpHdr header.ICMPv4, conn net.PacketConn, id stack.TransportEndpointID) { if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { f.logger.Error("Failed to set read deadline for ICMP response: %v", err) - return true + return } response := make([]byte, f.endpoint.mtu) @@ -96,7 +81,7 @@ func (f *Forwarder) handleEchoResponse(icmpHdr header.ICMPv4, payload []byte, ds if !isTimeout(err) { f.logger.Error("Failed to read ICMP response: %v", err) } - return true + return } ipHdr := make([]byte, header.IPv4MinimumSize) @@ -117,13 +102,11 @@ func (f *Forwarder) handleEchoResponse(icmpHdr header.ICMPv4, payload []byte, ds if err := f.InjectIncomingPacket(fullPacket); err != nil { f.logger.Error("Failed to inject ICMP response: %v", err) - return true + return } f.logger.Trace("Forwarded ICMP echo reply for %v type %v code %v", epID(id), icmpHdr.Type(), icmpHdr.Code()) - - return true } // sendICMPEvent stores flow events for ICMP packets @@ -138,5 +121,7 @@ func (f *Forwarder) sendICMPEvent(typ nftypes.Type, flowID uuid.UUID, id stack.T DestIP: netip.AddrFrom4(id.RemoteAddress.As4()), ICMPType: icmpType, ICMPCode: icmpCode, + + // TODO: get packets/bytes }) } diff --git a/client/firewall/uspfilter/forwarder/tcp.go b/client/firewall/uspfilter/forwarder/tcp.go index 44aafa989..c3e1eca80 100644 --- a/client/firewall/uspfilter/forwarder/tcp.go +++ b/client/firewall/uspfilter/forwarder/tcp.go @@ -22,7 +22,14 @@ func (f *Forwarder) handleTCP(r *tcp.ForwarderRequest) { id := r.ID() flowID := uuid.New() - f.sendTCPEvent(nftypes.TypeStart, flowID, id) + + f.sendTCPEvent(nftypes.TypeStart, flowID, id, nil) + var success bool + defer func() { + if !success { + f.sendTCPEvent(nftypes.TypeEnd, flowID, id, nil) + } + }() dialAddr := fmt.Sprintf("%s:%d", f.determineDialAddr(id.LocalAddress), id.LocalPort) @@ -51,6 +58,7 @@ func (f *Forwarder) handleTCP(r *tcp.ForwarderRequest) { inConn := gonet.NewTCPConn(&wq, ep) + success = true f.logger.Trace("forwarder: established TCP connection %v", epID(id)) go f.proxyTCP(id, inConn, outConn, ep, flowID) @@ -66,7 +74,7 @@ func (f *Forwarder) proxyTCP(id stack.TransportEndpointID, inConn *gonet.TCPConn } ep.Close() - f.sendTCPEvent(nftypes.TypeEnd, flowID, id) + f.sendTCPEvent(nftypes.TypeEnd, flowID, id, ep) }() // Create context for managing the proxy goroutines @@ -98,17 +106,27 @@ func (f *Forwarder) proxyTCP(id stack.TransportEndpointID, inConn *gonet.TCPConn } } -func (f *Forwarder) sendTCPEvent(typ nftypes.Type, flowID uuid.UUID, id stack.TransportEndpointID) { - - f.flowLogger.StoreEvent(nftypes.EventFields{ +func (f *Forwarder) sendTCPEvent(typ nftypes.Type, flowID uuid.UUID, id stack.TransportEndpointID, ep tcpip.Endpoint) { + fields := nftypes.EventFields{ FlowID: flowID, Type: typ, Direction: nftypes.Ingress, - Protocol: 6, + Protocol: nftypes.TCP, // TODO: handle ipv6 SourceIP: netip.AddrFrom4(id.LocalAddress.As4()), DestIP: netip.AddrFrom4(id.RemoteAddress.As4()), SourcePort: id.LocalPort, DestPort: id.RemotePort, - }) + } + + if ep != nil { + if tcpStats, ok := ep.Stats().(*tcp.Stats); ok { + // fields are flipped since this is the in conn + // TODO: get bytes + fields.RxPackets = tcpStats.SegmentsSent.Value() + fields.TxPackets = tcpStats.SegmentsReceived.Value() + } + } + + f.flowLogger.StoreEvent(fields) } diff --git a/client/firewall/uspfilter/forwarder/udp.go b/client/firewall/uspfilter/forwarder/udp.go index db8aa1a2f..20e1ee3a7 100644 --- a/client/firewall/uspfilter/forwarder/udp.go +++ b/client/firewall/uspfilter/forwarder/udp.go @@ -165,13 +165,19 @@ func (f *Forwarder) handleUDP(r *udp.ForwarderRequest) { } flowID := uuid.New() - f.sendUDPEvent(nftypes.TypeStart, flowID, id) + + f.sendUDPEvent(nftypes.TypeStart, flowID, id, nil) + var success bool + defer func() { + if !success { + f.sendUDPEvent(nftypes.TypeEnd, flowID, id, nil) + } + }() dstAddr := fmt.Sprintf("%s:%d", f.determineDialAddr(id.LocalAddress), id.LocalPort) outConn, err := (&net.Dialer{}).DialContext(f.ctx, "udp", dstAddr) if err != nil { f.logger.Debug("forwarder: UDP dial error for %v: %v", epID(id), err) - f.sendUDPEvent(nftypes.TypeEnd, flowID, id) // TODO: Send ICMP error message return } @@ -184,7 +190,6 @@ func (f *Forwarder) handleUDP(r *udp.ForwarderRequest) { if err := outConn.Close(); err != nil { f.logger.Debug("forwarder: UDP outConn close error for %v: %v", epID(id), err) } - f.sendUDPEvent(nftypes.TypeEnd, flowID, id) return } @@ -212,13 +217,14 @@ func (f *Forwarder) handleUDP(r *udp.ForwarderRequest) { f.logger.Debug("forwarder: UDP outConn close error for %v: %v", epID(id), err) } - f.sendUDPEvent(nftypes.TypeEnd, flowID, id) return } f.udpForwarder.conns[id] = pConn f.udpForwarder.Unlock() + success = true f.logger.Trace("forwarder: established UDP connection %v", epID(id)) + go f.proxyUDP(connCtx, pConn, id, ep) } @@ -238,7 +244,7 @@ func (f *Forwarder) proxyUDP(ctx context.Context, pConn *udpPacketConn, id stack delete(f.udpForwarder.conns, id) f.udpForwarder.Unlock() - f.sendUDPEvent(nftypes.TypeEnd, pConn.flowID, id) + f.sendUDPEvent(nftypes.TypeEnd, pConn.flowID, id, ep) }() errChan := make(chan error, 2) @@ -265,8 +271,8 @@ func (f *Forwarder) proxyUDP(ctx context.Context, pConn *udpPacketConn, id stack } // sendUDPEvent stores flow events for UDP connections, mirrors the TCP version -func (f *Forwarder) sendUDPEvent(typ nftypes.Type, flowID uuid.UUID, id stack.TransportEndpointID) { - f.flowLogger.StoreEvent(nftypes.EventFields{ +func (f *Forwarder) sendUDPEvent(typ nftypes.Type, flowID uuid.UUID, id stack.TransportEndpointID, ep tcpip.Endpoint) { + fields := nftypes.EventFields{ FlowID: flowID, Type: typ, Direction: nftypes.Ingress, @@ -276,7 +282,18 @@ func (f *Forwarder) sendUDPEvent(typ nftypes.Type, flowID uuid.UUID, id stack.Tr DestIP: netip.AddrFrom4(id.RemoteAddress.As4()), SourcePort: id.LocalPort, DestPort: id.RemotePort, - }) + } + + if ep != nil { + if tcpStats, ok := ep.Stats().(*tcpip.TransportEndpointStats); ok { + // fields are flipped since this is the in conn + // TODO: get bytes + fields.RxPackets = tcpStats.PacketsSent.Value() + fields.TxPackets = tcpStats.PacketsReceived.Value() + } + } + + f.flowLogger.StoreEvent(fields) } func (c *udpPacketConn) updateLastSeen() { diff --git a/client/firewall/uspfilter/tracer.go b/client/firewall/uspfilter/tracer.go index a980cda29..cc5edc554 100644 --- a/client/firewall/uspfilter/tracer.go +++ b/client/firewall/uspfilter/tracer.go @@ -281,7 +281,7 @@ func (m *Manager) traceInbound(packetData []byte, trace *PacketTrace, d *decoder } func (m *Manager) handleConntrackState(trace *PacketTrace, d *decoder, srcIP, dstIP netip.Addr) bool { - allowed := m.isValidTrackedConnection(d, srcIP, dstIP) + allowed := m.isValidTrackedConnection(d, srcIP, dstIP, 0) msg := "No existing connection found" if allowed { msg = m.buildConntrackStateMessage(d) @@ -391,7 +391,7 @@ func (m *Manager) addForwardingResult(trace *PacketTrace, action, remoteAddr str func (m *Manager) traceOutbound(packetData []byte, trace *PacketTrace) *PacketTrace { // will create or update the connection state - dropped := m.processOutgoingHooks(packetData) + dropped := m.processOutgoingHooks(packetData, 0) if dropped { trace.AddResult(StageCompleted, "Packet dropped by outgoing hook", false) } else { diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go index e663966ab..73b963d06 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/uspfilter.go @@ -510,13 +510,13 @@ func (m *Manager) DeleteDNATRule(rule firewall.Rule) error { } // DropOutgoing filter outgoing packets -func (m *Manager) DropOutgoing(packetData []byte) bool { - return m.processOutgoingHooks(packetData) +func (m *Manager) DropOutgoing(packetData []byte, size int) bool { + return m.processOutgoingHooks(packetData, size) } // DropIncoming filter incoming packets -func (m *Manager) DropIncoming(packetData []byte) bool { - return m.dropFilter(packetData) +func (m *Manager) DropIncoming(packetData []byte, size int) bool { + return m.dropFilter(packetData, size) } // UpdateLocalIPs updates the list of local IPs @@ -524,7 +524,7 @@ func (m *Manager) UpdateLocalIPs() error { return m.localipmanager.UpdateLocalIPs(m.wgIface) } -func (m *Manager) processOutgoingHooks(packetData []byte) bool { +func (m *Manager) processOutgoingHooks(packetData []byte, size int) bool { d := m.decoders.Get().(*decoder) defer m.decoders.Put(d) @@ -544,7 +544,7 @@ func (m *Manager) processOutgoingHooks(packetData []byte) bool { // Track all protocols if stateful mode is enabled if m.stateful { - m.trackOutbound(d, srcIP, dstIP) + m.trackOutbound(d, srcIP, dstIP, size) } // Process UDP hooks even if stateful mode is disabled @@ -593,29 +593,29 @@ func getTCPFlags(tcp *layers.TCP) uint8 { return flags } -func (m *Manager) trackOutbound(d *decoder, srcIP, dstIP netip.Addr) { +func (m *Manager) trackOutbound(d *decoder, srcIP, dstIP netip.Addr, size int) { transport := d.decoded[1] switch transport { case layers.LayerTypeUDP: - m.udpTracker.TrackOutbound(srcIP, dstIP, uint16(d.udp.SrcPort), uint16(d.udp.DstPort)) + m.udpTracker.TrackOutbound(srcIP, dstIP, uint16(d.udp.SrcPort), uint16(d.udp.DstPort), size) case layers.LayerTypeTCP: flags := getTCPFlags(&d.tcp) - m.tcpTracker.TrackOutbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags) + m.tcpTracker.TrackOutbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags, size) case layers.LayerTypeICMPv4: - m.icmpTracker.TrackOutbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode) + m.icmpTracker.TrackOutbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode, size) } } -func (m *Manager) trackInbound(d *decoder, srcIP, dstIP netip.Addr) { +func (m *Manager) trackInbound(d *decoder, srcIP, dstIP netip.Addr, size int) { transport := d.decoded[1] switch transport { case layers.LayerTypeUDP: - m.udpTracker.TrackInbound(srcIP, dstIP, uint16(d.udp.SrcPort), uint16(d.udp.DstPort)) + m.udpTracker.TrackInbound(srcIP, dstIP, uint16(d.udp.SrcPort), uint16(d.udp.DstPort), size) case layers.LayerTypeTCP: flags := getTCPFlags(&d.tcp) - m.tcpTracker.TrackInbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags) + m.tcpTracker.TrackInbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags, size) case layers.LayerTypeICMPv4: - m.icmpTracker.TrackInbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode) + m.icmpTracker.TrackInbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode, size) } } @@ -637,7 +637,7 @@ func (m *Manager) checkUDPHooks(d *decoder, dstIP netip.Addr, packetData []byte) // dropFilter implements filtering logic for incoming packets. // If it returns true, the packet should be dropped. -func (m *Manager) dropFilter(packetData []byte) bool { +func (m *Manager) dropFilter(packetData []byte, size int) bool { d := m.decoders.Get().(*decoder) defer m.decoders.Put(d) @@ -653,12 +653,12 @@ func (m *Manager) dropFilter(packetData []byte) bool { // For all inbound traffic, first check if it matches a tracked connection. // This must happen before any other filtering because the packets are statefully tracked. - if m.stateful && m.isValidTrackedConnection(d, srcIP, dstIP) { + if m.stateful && m.isValidTrackedConnection(d, srcIP, dstIP, size) { return false } if m.localipmanager.IsLocalIP(dstIP) { - return m.handleLocalTraffic(d, srcIP, dstIP, packetData) + return m.handleLocalTraffic(d, srcIP, dstIP, packetData, size) } return m.handleRoutedTraffic(d, srcIP, dstIP, packetData) @@ -666,7 +666,7 @@ func (m *Manager) dropFilter(packetData []byte) bool { // handleLocalTraffic handles local traffic. // If it returns true, the packet should be dropped. -func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP netip.Addr, packetData []byte) bool { +func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP netip.Addr, packetData []byte, size int) bool { if ruleId, blocked := m.peerACLsBlock(srcIP, packetData, m.incomingRules, d); blocked { _, pnum := getProtocolFromPacket(d) srcPort, dstPort := getPortsFromPacket(d) @@ -685,6 +685,8 @@ func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP netip.Addr, packet SourcePort: srcPort, DestPort: dstPort, // TODO: icmp type/code + RxPackets: 1, + RxBytes: uint64(size), }) return true } @@ -695,7 +697,7 @@ func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP netip.Addr, packet } // track inbound packets to get the correct direction and session id for flows - m.trackInbound(d, srcIP, dstIP) + m.trackInbound(d, srcIP, dstIP, size) return false } @@ -802,7 +804,7 @@ func (m *Manager) isValidPacket(d *decoder, packetData []byte) bool { return true } -func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP netip.Addr) bool { +func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP netip.Addr, size int) bool { switch d.decoded[1] { case layers.LayerTypeTCP: return m.tcpTracker.IsValidInbound( @@ -811,6 +813,7 @@ func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP netip.Addr) uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), getTCPFlags(&d.tcp), + size, ) case layers.LayerTypeUDP: @@ -819,6 +822,7 @@ func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP netip.Addr) dstIP, uint16(d.udp.SrcPort), uint16(d.udp.DstPort), + size, ) case layers.LayerTypeICMPv4: @@ -827,6 +831,7 @@ func (m *Manager) isValidTrackedConnection(d *decoder, srcIP, dstIP netip.Addr) dstIP, d.icmp4.Id, d.icmp4.TypeCode.Type(), + size, ) // TODO: ICMPv6 diff --git a/client/firewall/uspfilter/uspfilter_bench_test.go b/client/firewall/uspfilter/uspfilter_bench_test.go index 9b06b2803..b43ac2b16 100644 --- a/client/firewall/uspfilter/uspfilter_bench_test.go +++ b/client/firewall/uspfilter/uspfilter_bench_test.go @@ -193,13 +193,13 @@ func BenchmarkCoreFiltering(b *testing.B) { // For stateful scenarios, establish the connection if sc.stateful { - manager.processOutgoingHooks(outbound) + manager.processOutgoingHooks(outbound, 0) } // Measure inbound packet processing b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(inbound) + manager.dropFilter(inbound, 0) } }) } @@ -230,7 +230,7 @@ func BenchmarkStateScaling(b *testing.B) { for i := 0; i < count; i++ { outbound := generatePacket(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, layers.IPProtocolTCP) - manager.processOutgoingHooks(outbound) + manager.processOutgoingHooks(outbound, 0) } // Test packet @@ -238,11 +238,11 @@ func BenchmarkStateScaling(b *testing.B) { testIn := generatePacket(b, dstIPs[0], srcIPs[0], 80, 1024, layers.IPProtocolTCP) // First establish our test connection - manager.processOutgoingHooks(testOut) + manager.processOutgoingHooks(testOut, 0) b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(testIn) + manager.dropFilter(testIn, 0) } }) } @@ -278,12 +278,12 @@ func BenchmarkEstablishmentOverhead(b *testing.B) { inbound := generatePacket(b, dstIP, srcIP, 80, 1024, layers.IPProtocolTCP) if sc.established { - manager.processOutgoingHooks(outbound) + manager.processOutgoingHooks(outbound, 0) } b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(inbound) + manager.dropFilter(inbound, 0) } }) } @@ -477,25 +477,25 @@ func BenchmarkRoutedNetworkReturn(b *testing.B) { // For stateful cases and established connections if !strings.Contains(sc.name, "allow_non_wg") || (strings.Contains(sc.state, "established") || sc.state == "post_handshake") { - manager.processOutgoingHooks(outbound) + manager.processOutgoingHooks(outbound, 0) // For TCP post-handshake, simulate full handshake if sc.state == "post_handshake" { // SYN syn := generateTCPPacketWithFlags(b, srcIP, dstIP, 1024, 80, uint16(conntrack.TCPSyn)) - manager.processOutgoingHooks(syn) + manager.processOutgoingHooks(syn, 0) // SYN-ACK synack := generateTCPPacketWithFlags(b, dstIP, srcIP, 80, 1024, uint16(conntrack.TCPSyn|conntrack.TCPAck)) - manager.dropFilter(synack) + manager.dropFilter(synack, 0) // ACK ack := generateTCPPacketWithFlags(b, srcIP, dstIP, 1024, 80, uint16(conntrack.TCPAck)) - manager.processOutgoingHooks(ack) + manager.processOutgoingHooks(ack, 0) } } b.ResetTimer() for i := 0; i < b.N; i++ { - manager.dropFilter(inbound) + manager.dropFilter(inbound, 0) } }) } @@ -624,17 +624,17 @@ func BenchmarkLongLivedConnections(b *testing.B) { // Initial SYN syn := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPSyn)) - manager.processOutgoingHooks(syn) + manager.processOutgoingHooks(syn, 0) // SYN-ACK synack := generateTCPPacketWithFlags(b, dstIPs[i], srcIPs[i], 80, uint16(1024+i), uint16(conntrack.TCPSyn|conntrack.TCPAck)) - manager.dropFilter(synack) + manager.dropFilter(synack, 0) // ACK ack := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPAck)) - manager.processOutgoingHooks(ack) + manager.processOutgoingHooks(ack, 0) } // Prepare test packets simulating bidirectional traffic @@ -655,9 +655,9 @@ func BenchmarkLongLivedConnections(b *testing.B) { // Simulate bidirectional traffic // First outbound data - manager.processOutgoingHooks(outPackets[connIdx]) + manager.processOutgoingHooks(outPackets[connIdx], 0) // Then inbound response - this is what we're actually measuring - manager.dropFilter(inPackets[connIdx]) + manager.dropFilter(inPackets[connIdx], 0) } }) } @@ -761,19 +761,19 @@ func BenchmarkShortLivedConnections(b *testing.B) { p := patterns[connIdx] // Connection establishment - manager.processOutgoingHooks(p.syn) - manager.dropFilter(p.synAck) - manager.processOutgoingHooks(p.ack) + manager.processOutgoingHooks(p.syn, 0) + manager.dropFilter(p.synAck, 0) + manager.processOutgoingHooks(p.ack, 0) // Data transfer - manager.processOutgoingHooks(p.request) - manager.dropFilter(p.response) + manager.processOutgoingHooks(p.request, 0) + manager.dropFilter(p.response, 0) // Connection teardown - manager.processOutgoingHooks(p.finClient) - manager.dropFilter(p.ackServer) - manager.dropFilter(p.finServer) - manager.processOutgoingHooks(p.ackClient) + manager.processOutgoingHooks(p.finClient, 0) + manager.dropFilter(p.ackServer, 0) + manager.dropFilter(p.finServer, 0) + manager.processOutgoingHooks(p.ackClient, 0) } }) } @@ -826,15 +826,15 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) { for i := 0; i < sc.connCount; i++ { syn := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPSyn)) - manager.processOutgoingHooks(syn) + manager.processOutgoingHooks(syn, 0) synack := generateTCPPacketWithFlags(b, dstIPs[i], srcIPs[i], 80, uint16(1024+i), uint16(conntrack.TCPSyn|conntrack.TCPAck)) - manager.dropFilter(synack) + manager.dropFilter(synack, 0) ack := generateTCPPacketWithFlags(b, srcIPs[i], dstIPs[i], uint16(1024+i), 80, uint16(conntrack.TCPAck)) - manager.processOutgoingHooks(ack) + manager.processOutgoingHooks(ack, 0) } // Pre-generate test packets @@ -856,8 +856,8 @@ func BenchmarkParallelLongLivedConnections(b *testing.B) { counter++ // Simulate bidirectional traffic - manager.processOutgoingHooks(outPackets[connIdx]) - manager.dropFilter(inPackets[connIdx]) + manager.processOutgoingHooks(outPackets[connIdx], 0) + manager.dropFilter(inPackets[connIdx], 0) } }) }) @@ -950,17 +950,17 @@ func BenchmarkParallelShortLivedConnections(b *testing.B) { p := patterns[connIdx] // Full connection lifecycle - manager.processOutgoingHooks(p.syn) - manager.dropFilter(p.synAck) - manager.processOutgoingHooks(p.ack) + manager.processOutgoingHooks(p.syn, 0) + manager.dropFilter(p.synAck, 0) + manager.processOutgoingHooks(p.ack, 0) - manager.processOutgoingHooks(p.request) - manager.dropFilter(p.response) + manager.processOutgoingHooks(p.request, 0) + manager.dropFilter(p.response, 0) - manager.processOutgoingHooks(p.finClient) - manager.dropFilter(p.ackServer) - manager.dropFilter(p.finServer) - manager.processOutgoingHooks(p.ackClient) + manager.processOutgoingHooks(p.finClient, 0) + manager.dropFilter(p.ackServer, 0) + manager.dropFilter(p.finServer, 0) + manager.processOutgoingHooks(p.ackClient, 0) } }) }) diff --git a/client/firewall/uspfilter/uspfilter_filter_test.go b/client/firewall/uspfilter/uspfilter_filter_test.go index 7005d501c..3a97506f1 100644 --- a/client/firewall/uspfilter/uspfilter_filter_test.go +++ b/client/firewall/uspfilter/uspfilter_filter_test.go @@ -192,7 +192,7 @@ func TestPeerACLFiltering(t *testing.T) { t.Run("Implicit DROP (no rules)", func(t *testing.T) { packet := createTestPacket(t, "100.10.0.1", "100.10.0.100", fw.ProtocolTCP, 12345, 443) - isDropped := manager.DropIncoming(packet) + isDropped := manager.DropIncoming(packet, 0) require.True(t, isDropped, "Packet should be dropped when no rules exist") }) @@ -217,7 +217,7 @@ func TestPeerACLFiltering(t *testing.T) { }) packet := createTestPacket(t, tc.srcIP, tc.dstIP, tc.proto, tc.srcPort, tc.dstPort) - isDropped := manager.DropIncoming(packet) + isDropped := manager.DropIncoming(packet, 0) require.Equal(t, tc.shouldBeBlocked, isDropped) }) } diff --git a/client/firewall/uspfilter/uspfilter_test.go b/client/firewall/uspfilter/uspfilter_test.go index 429265794..1db572618 100644 --- a/client/firewall/uspfilter/uspfilter_test.go +++ b/client/firewall/uspfilter/uspfilter_test.go @@ -328,7 +328,7 @@ func TestNotMatchByIP(t *testing.T) { return } - if m.dropFilter(buf.Bytes()) { + if m.dropFilter(buf.Bytes(), 0) { t.Errorf("expected packet to be accepted") return } @@ -458,7 +458,7 @@ func TestProcessOutgoingHooks(t *testing.T) { require.NoError(t, err) // Test hook gets called - result := manager.processOutgoingHooks(buf.Bytes()) + result := manager.processOutgoingHooks(buf.Bytes(), 0) require.True(t, result) require.True(t, hookCalled) @@ -468,7 +468,7 @@ func TestProcessOutgoingHooks(t *testing.T) { err = gopacket.SerializeLayers(buf, opts, ipv4) require.NoError(t, err) - result = manager.processOutgoingHooks(buf.Bytes()) + result = manager.processOutgoingHooks(buf.Bytes(), 0) require.False(t, result) } @@ -569,7 +569,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { require.NoError(t, err) // Process outbound packet and verify connection tracking - drop := manager.DropOutgoing(outboundBuf.Bytes()) + drop := manager.DropOutgoing(outboundBuf.Bytes(), 0) require.False(t, drop, "Initial outbound packet should not be dropped") // Verify connection was tracked @@ -636,7 +636,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { for _, cp := range checkPoints { time.Sleep(cp.sleep) - drop = manager.dropFilter(inboundBuf.Bytes()) + drop = manager.dropFilter(inboundBuf.Bytes(), 0) require.Equal(t, cp.shouldAllow, !drop, cp.description) // If the connection should still be valid, verify it exists @@ -685,7 +685,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { } // Create a new outbound connection for invalid tests - drop = manager.processOutgoingHooks(outboundBuf.Bytes()) + drop = manager.processOutgoingHooks(outboundBuf.Bytes(), 0) require.False(t, drop, "Second outbound packet should not be dropped") for _, tc := range invalidCases { @@ -707,7 +707,7 @@ func TestStatefulFirewall_UDPTracking(t *testing.T) { require.NoError(t, err) // Verify the invalid packet is dropped - drop = manager.dropFilter(testBuf.Bytes()) + drop = manager.dropFilter(testBuf.Bytes(), 0) require.True(t, drop, tc.description) }) } diff --git a/client/iface/device/device_filter.go b/client/iface/device/device_filter.go index f21804683..c9b7e2448 100644 --- a/client/iface/device/device_filter.go +++ b/client/iface/device/device_filter.go @@ -11,10 +11,10 @@ import ( // PacketFilter interface for firewall abilities type PacketFilter interface { // DropOutgoing filter outgoing packets from host to external destinations - DropOutgoing(packetData []byte) bool + DropOutgoing(packetData []byte, size int) bool // DropIncoming filter incoming packets from external sources to host - DropIncoming(packetData []byte) bool + DropIncoming(packetData []byte, size int) bool // AddUDPPacketHook calls hook when UDP packet from given direction matched // @@ -58,7 +58,7 @@ func (d *FilteredDevice) Read(bufs [][]byte, sizes []int, offset int) (n int, er } for i := 0; i < n; i++ { - if filter.DropOutgoing(bufs[i][offset : offset+sizes[i]]) { + if filter.DropOutgoing(bufs[i][offset:offset+sizes[i]], sizes[i]) { bufs = append(bufs[:i], bufs[i+1:]...) sizes = append(sizes[:i], sizes[i+1:]...) n-- @@ -82,7 +82,7 @@ func (d *FilteredDevice) Write(bufs [][]byte, offset int) (int, error) { filteredBufs := make([][]byte, 0, len(bufs)) dropped := 0 for _, buf := range bufs { - if !filter.DropIncoming(buf[offset:]) { + if !filter.DropIncoming(buf[offset:], len(buf)) { filteredBufs = append(filteredBufs, buf) dropped++ } diff --git a/client/iface/device/device_filter_test.go b/client/iface/device/device_filter_test.go index d3278b918..c90269e82 100644 --- a/client/iface/device/device_filter_test.go +++ b/client/iface/device/device_filter_test.go @@ -146,7 +146,7 @@ func TestDeviceWrapperRead(t *testing.T) { tun.EXPECT().Write(mockBufs, 0).Return(0, nil) filter := mocks.NewMockPacketFilter(ctrl) - filter.EXPECT().DropIncoming(gomock.Any()).Return(true) + filter.EXPECT().DropIncoming(gomock.Any(), gomock.Any()).Return(true) wrapped := newDeviceFilter(tun) wrapped.filter = filter @@ -201,7 +201,7 @@ func TestDeviceWrapperRead(t *testing.T) { return 1, nil }) filter := mocks.NewMockPacketFilter(ctrl) - filter.EXPECT().DropOutgoing(gomock.Any()).Return(true) + filter.EXPECT().DropOutgoing(gomock.Any(), gomock.Any()).Return(true) wrapped := newDeviceFilter(tun) wrapped.filter = filter diff --git a/client/iface/mocks/filter.go b/client/iface/mocks/filter.go index f00024e38..faac55d68 100644 --- a/client/iface/mocks/filter.go +++ b/client/iface/mocks/filter.go @@ -50,31 +50,31 @@ func (mr *MockPacketFilterMockRecorder) AddUDPPacketHook(arg0, arg1, arg2, arg3 } // DropIncoming mocks base method. -func (m *MockPacketFilter) DropIncoming(arg0 []byte) bool { +func (m *MockPacketFilter) DropIncoming(arg0 []byte, arg1 int) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropIncoming", arg0) + ret := m.ctrl.Call(m, "DropIncoming", arg0, arg1) ret0, _ := ret[0].(bool) return ret0 } // DropIncoming indicates an expected call of DropIncoming. -func (mr *MockPacketFilterMockRecorder) DropIncoming(arg0 interface{}) *gomock.Call { +func (mr *MockPacketFilterMockRecorder) DropIncoming(arg0 interface{}, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropIncoming", reflect.TypeOf((*MockPacketFilter)(nil).DropIncoming), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropIncoming", reflect.TypeOf((*MockPacketFilter)(nil).DropIncoming), arg0, arg1) } // DropOutgoing mocks base method. -func (m *MockPacketFilter) DropOutgoing(arg0 []byte) bool { +func (m *MockPacketFilter) DropOutgoing(arg0 []byte, arg1 int) bool { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DropOutgoing", arg0) + ret := m.ctrl.Call(m, "DropOutgoing", arg0, arg1) ret0, _ := ret[0].(bool) return ret0 } // DropOutgoing indicates an expected call of DropOutgoing. -func (mr *MockPacketFilterMockRecorder) DropOutgoing(arg0 interface{}) *gomock.Call { +func (mr *MockPacketFilterMockRecorder) DropOutgoing(arg0 interface{}, arg1 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropOutgoing", reflect.TypeOf((*MockPacketFilter)(nil).DropOutgoing), arg0) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DropOutgoing", reflect.TypeOf((*MockPacketFilter)(nil).DropOutgoing), arg0, arg1) } // RemovePacketHook mocks base method. diff --git a/client/internal/dns/server_test.go b/client/internal/dns/server_test.go index 7c75f6bed..80db2561b 100644 --- a/client/internal/dns/server_test.go +++ b/client/internal/dns/server_test.go @@ -458,7 +458,7 @@ func TestDNSFakeResolverHandleUpdates(t *testing.T) { } packetfilter := pfmock.NewMockPacketFilter(ctrl) - packetfilter.EXPECT().DropOutgoing(gomock.Any()).AnyTimes() + packetfilter.EXPECT().DropOutgoing(gomock.Any(), gomock.Any()).AnyTimes() packetfilter.EXPECT().AddUDPPacketHook(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()) packetfilter.EXPECT().RemovePacketHook(gomock.Any()) packetfilter.EXPECT().SetNetwork(ipNet) diff --git a/client/internal/netflow/types/types.go b/client/internal/netflow/types/types.go index a0f2eb95e..f57fca1c9 100644 --- a/client/internal/netflow/types/types.go +++ b/client/internal/netflow/types/types.go @@ -2,6 +2,7 @@ package types import ( "net/netip" + "strconv" "time" "github.com/google/uuid" @@ -27,8 +28,10 @@ func (p Protocol) String() string { return "TCP" case 17: return "UDP" + case 132: + return "SCTP" default: - return "unknown" + return strconv.FormatUint(uint64(p), 10) } } From 3c3a454e61f22cccd3c2d01da66cb2bf3b05ad02 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Thu, 6 Mar 2025 16:54:15 +0100 Subject: [PATCH 06/11] Fix merge regression --- client/firewall/uspfilter/conntrack/icmp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/firewall/uspfilter/conntrack/icmp.go b/client/firewall/uspfilter/conntrack/icmp.go index 730edd6ce..5a736d132 100644 --- a/client/firewall/uspfilter/conntrack/icmp.go +++ b/client/firewall/uspfilter/conntrack/icmp.go @@ -90,7 +90,7 @@ func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint } // TrackOutbound records an outbound ICMP connection -func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, seq uint16, typecode layers.ICMPv4TypeCode, size int) { +func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, size int) { if _, exists := t.updateIfExists(dstIP, srcIP, id, nftypes.Egress, size); !exists { // if (inverted direction) conn is not tracked, track this direction t.track(srcIP, dstIP, id, typecode, nftypes.Egress, size) From 54be772ffd6ce80e674a59937558e879ad22def1 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 7 Mar 2025 13:56:00 +0100 Subject: [PATCH 07/11] Handle flow updates (#3455) --- client/internal/netflow/logger/logger.go | 4 +- client/internal/netflow/manager.go | 129 ++++++++++++++++------- client/internal/netflow/store/memory.go | 4 +- flow/client/client.go | 111 ++++++++++--------- 4 files changed, 154 insertions(+), 94 deletions(-) diff --git a/client/internal/netflow/logger/logger.go b/client/internal/netflow/logger/logger.go index 1e23c1dce..6569b75b7 100644 --- a/client/internal/netflow/logger/logger.go +++ b/client/internal/netflow/logger/logger.go @@ -58,13 +58,14 @@ func (l *Logger) startReceiver() { if l.enabled.Load() { return } + l.mux.Lock() ctx, cancel := context.WithCancel(l.ctx) l.cancelReceiver = cancel l.mux.Unlock() c := make(rcvChan, 100) - l.rcvChan.Swap(&c) + l.rcvChan.Store(&c) l.enabled.Store(true) for { @@ -100,6 +101,7 @@ func (l *Logger) stop() { l.cancelReceiver() l.cancelReceiver = nil } + l.rcvChan.Store(nil) l.mux.Unlock() } diff --git a/client/internal/netflow/manager.go b/client/internal/netflow/manager.go index 8ab81f4ff..ed5655f8a 100644 --- a/client/internal/netflow/manager.go +++ b/client/internal/netflow/manager.go @@ -2,6 +2,7 @@ package netflow import ( "context" + "errors" "fmt" "runtime" "sync" @@ -45,46 +46,80 @@ func NewManager(ctx context.Context, iface nftypes.IFaceMapper, publicKey []byte } } +// Update applies new flow configuration settings +// needsNewClient checks if a new client needs to be created +func (m *Manager) needsNewClient(previous *nftypes.FlowConfig) bool { + current := m.flowConfig + return previous == nil || + !previous.Enabled || + previous.TokenPayload != current.TokenPayload || + previous.TokenSignature != current.TokenSignature || + previous.URL != current.URL +} + +// enableFlow starts components for flow tracking +func (m *Manager) enableFlow(previous *nftypes.FlowConfig) error { + // first make sender ready so events don't pile up + if m.needsNewClient(previous) { + if m.receiverClient != nil { + if err := m.receiverClient.Close(); err != nil { + log.Warnf("error closing previous flow client: %s", err) + } + } + + flowClient, err := client.NewClient(m.flowConfig.URL, m.flowConfig.TokenPayload, m.flowConfig.TokenSignature, m.flowConfig.Interval) + if err != nil { + return fmt.Errorf("create client: %w", err) + } + log.Infof("flow client configured to connect to %s", m.flowConfig.URL) + + m.receiverClient = flowClient + go m.receiveACKs(flowClient) + go m.startSender() + } + + m.logger.Enable() + + if m.conntrack != nil { + if err := m.conntrack.Start(m.flowConfig.Counters); err != nil { + return fmt.Errorf("start conntrack: %w", err) + } + } + + return nil +} + +// disableFlow stops components for flow tracking +func (m *Manager) disableFlow() error { + if m.conntrack != nil { + m.conntrack.Stop() + } + + m.logger.Disable() + + if m.receiverClient != nil { + return m.receiverClient.Close() + } + return nil +} + // Update applies new flow configuration settings func (m *Manager) Update(update *nftypes.FlowConfig) error { if update == nil { return nil } + m.mux.Lock() defer m.mux.Unlock() + previous := m.flowConfig m.flowConfig = update if update.Enabled { - if m.conntrack != nil { - if err := m.conntrack.Start(update.Counters); err != nil { - return fmt.Errorf("start conntrack: %w", err) - } - } - - m.logger.Enable() - if previous == nil || !previous.Enabled { - flowClient, err := client.NewClient(m.ctx, m.flowConfig.URL, m.flowConfig.TokenPayload, m.flowConfig.TokenSignature) - if err != nil { - return err - } - log.Infof("flow client connected to %s", m.flowConfig.URL) - m.receiverClient = flowClient - go m.receiveACKs() - go m.startSender() - } - return nil + return m.enableFlow(previous) } - if m.conntrack != nil { - m.conntrack.Stop() - } - m.logger.Disable() - if previous != nil && previous.Enabled { - return m.receiverClient.Close() - } - - return nil + return m.disableFlow() } // Close cleans up all resources @@ -95,6 +130,13 @@ func (m *Manager) Close() { if m.conntrack != nil { m.conntrack.Close() } + + if m.receiverClient != nil { + if err := m.receiverClient.Close(); err != nil { + log.Warnf("failed to close receiver client: %s", err) + } + } + m.logger.Close() } @@ -106,6 +148,7 @@ func (m *Manager) GetLogger() nftypes.FlowLogger { func (m *Manager) startSender() { ticker := time.NewTicker(m.flowConfig.Interval) defer ticker.Stop() + for { select { case <-m.ctx.Done(): @@ -113,35 +156,38 @@ func (m *Manager) startSender() { case <-ticker.C: events := m.logger.GetEvents() for _, event := range events { - log.Infof("send flow event to server: %s", event.ID) - err := m.send(event) - if err != nil { - log.Errorf("send flow event to server: %s", err) + if err := m.send(event); err != nil { + log.Errorf("failed to send flow event to server: %s", err) + continue } + log.Tracef("sent flow event: %s", event.ID) } } } } -func (m *Manager) receiveACKs() { - if m.receiverClient == nil { - return - } - err := m.receiverClient.Receive(m.ctx, func(ack *proto.FlowEventAck) error { - log.Infof("receive flow event ack: %s", ack.EventId) +func (m *Manager) receiveACKs(client *client.GRPCClient) { + err := client.Receive(m.ctx, m.flowConfig.Interval, func(ack *proto.FlowEventAck) error { + log.Tracef("received flow event ack: %s", ack.EventId) m.logger.DeleteEvents([]string{ack.EventId}) return nil }) - if err != nil { - log.Errorf("receive flow event ack: %s", err) + + if err != nil && !errors.Is(err, context.Canceled) { + log.Errorf("failed to receive flow event ack: %s", err) } } func (m *Manager) send(event *nftypes.Event) error { - if m.receiverClient == nil { + m.mux.Lock() + client := m.receiverClient + m.mux.Unlock() + + if client == nil { return nil } - return m.receiverClient.Send(m.ctx, toProtoEvent(m.publicKey, event)) + + return client.Send(toProtoEvent(m.publicKey, event)) } func toProtoEvent(publicKey []byte, event *nftypes.Event) *proto.FlowEvent { @@ -163,6 +209,7 @@ func toProtoEvent(publicKey []byte, event *nftypes.Event) *proto.FlowEvent { TxBytes: event.TxBytes, }, } + if event.Protocol == nftypes.ICMP { protoEvent.FlowFields.ConnectionInfo = &proto.FlowFields_IcmpInfo{ IcmpInfo: &proto.ICMPInfo{ diff --git a/client/internal/netflow/store/memory.go b/client/internal/netflow/store/memory.go index b0dcbd6f8..7fa08b510 100644 --- a/client/internal/netflow/store/memory.go +++ b/client/internal/netflow/store/memory.go @@ -3,6 +3,8 @@ package store import ( "sync" + "golang.org/x/exp/maps" + "github.com/netbirdio/netbird/client/internal/netflow/types" ) @@ -26,7 +28,7 @@ func (m *Memory) StoreEvent(event *types.Event) { func (m *Memory) Close() { m.mux.Lock() defer m.mux.Unlock() - m.events = make(map[string]*types.Event) + maps.Clear(m.events) } func (m *Memory) GetEvents() []*types.Event { diff --git a/flow/client/client.go b/flow/client/client.go index 47c80ef0d..b16b28c64 100644 --- a/flow/client/client.go +++ b/flow/client/client.go @@ -4,8 +4,10 @@ import ( "context" "crypto/tls" "crypto/x509" + "errors" "fmt" "strings" + "sync" "time" "github.com/cenkalti/backoff/v4" @@ -25,95 +27,99 @@ type GRPCClient struct { realClient proto.FlowServiceClient clientConn *grpc.ClientConn stream proto.FlowService_EventsClient + streamMu sync.Mutex } -func NewClient(ctx context.Context, addr, payload, signature string) (*GRPCClient, error) { +func NewClient(addr, payload, signature string, interval time.Duration) (*GRPCClient, error) { + var opts []grpc.DialOption - transportOption := grpc.WithTransportCredentials(insecure.NewCredentials()) if strings.Contains(addr, "443") { - certPool, err := x509.SystemCertPool() if err != nil || certPool == nil { log.Debugf("System cert pool not available; falling back to embedded cert, error: %v", err) certPool = embeddedroots.Get() } - transportOption = grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{ + opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(&tls.Config{ RootCAs: certPool, - })) + }))) + } else { + opts = append(opts, grpc.WithTransportCredentials(insecure.NewCredentials())) } - connCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - conn, err := grpc.DialContext( - connCtx, - addr, - transportOption, + opts = append(opts, nbgrpc.WithCustomDialer(), - grpc.WithBlock(), + grpc.WithIdleTimeout(interval*2), grpc.WithKeepaliveParams(keepalive.ClientParameters{ Time: 30 * time.Second, Timeout: 10 * time.Second, }), withAuthToken(payload, signature), + grpc.WithDefaultServiceConfig(`{"healthCheckConfig": {"serviceName": ""}}`), ) + conn, err := grpc.NewClient(addr, opts...) if err != nil { - return nil, fmt.Errorf("dialing with context: %s", err) + return nil, fmt.Errorf("creating new grpc client: %w", err) } - client := &GRPCClient{ + return &GRPCClient{ realClient: proto.NewFlowServiceClient(conn), clientConn: conn, - } - return client, nil + }, nil } func (c *GRPCClient) Close() error { + c.streamMu.Lock() + defer c.streamMu.Unlock() + + c.stream = nil return c.clientConn.Close() } -func (c *GRPCClient) Receive(ctx context.Context, msgHandler func(msg *proto.FlowEventAck) error) error { - backOff := defaultBackoff(ctx) +func (c *GRPCClient) Receive(ctx context.Context, interval time.Duration, msgHandler func(msg *proto.FlowEventAck) error) error { + backOff := defaultBackoff(ctx, interval) operation := func() error { - connState := c.clientConn.GetState() - if connState == connectivity.Shutdown { - return backoff.Permanent(fmt.Errorf("connection to signal has been shut down")) - } - - stream, err := c.realClient.Events(ctx, grpc.WaitForReady(true)) - if err != nil { - return err - } - c.stream = stream - - err = checkHeader(stream) - if err != nil { - return err - } - - return c.receive(stream, msgHandler) + return c.establishStreamAndReceive(ctx, msgHandler) } - err := backoff.Retry(operation, backOff) - if err != nil { - log.Errorf("exiting the flow receiver service connection retry loop due to the unrecoverable error: %v", err) - return err + if err := backoff.Retry(operation, backOff); err != nil { + return fmt.Errorf("receive failed permanently: %w", err) } return nil } +func (c *GRPCClient) establishStreamAndReceive(ctx context.Context, msgHandler func(msg *proto.FlowEventAck) error) error { + if c.clientConn.GetState() == connectivity.Shutdown { + return backoff.Permanent(errors.New("connection to flow receiver has been shut down")) + } + + stream, err := c.realClient.Events(ctx, grpc.WaitForReady(true)) + if err != nil { + return fmt.Errorf("create event stream: %w", err) + } + + if err = checkHeader(stream); err != nil { + return fmt.Errorf("check header: %w", err) + } + + c.streamMu.Lock() + c.stream = stream + c.streamMu.Unlock() + + return c.receive(stream, msgHandler) +} + func (c *GRPCClient) receive(stream proto.FlowService_EventsClient, msgHandler func(msg *proto.FlowEventAck) error) error { for { msg, err := stream.Recv() if err != nil { - return err + return fmt.Errorf("receive from stream: %w", err) } if err := msgHandler(msg); err != nil { - return err + return fmt.Errorf("handle message: %w", err) } } } @@ -122,7 +128,7 @@ func checkHeader(stream proto.FlowService_EventsClient) error { header, err := stream.Header() if err != nil { log.Errorf("waiting for flow receiver header: %s", err) - return err + return fmt.Errorf("wait for header: %w", err) } if len(header) == 0 { @@ -132,26 +138,29 @@ func checkHeader(stream proto.FlowService_EventsClient) error { return nil } -func defaultBackoff(ctx context.Context) backoff.BackOff { +func defaultBackoff(ctx context.Context, interval time.Duration) backoff.BackOff { return backoff.WithContext(&backoff.ExponentialBackOff{ InitialInterval: 800 * time.Millisecond, RandomizationFactor: 1, Multiplier: 1.7, - MaxInterval: 10 * time.Second, + MaxInterval: interval / 2, MaxElapsedTime: 3 * 30 * 24 * time.Hour, // 3 months Stop: backoff.Stop, Clock: backoff.SystemClock, }, ctx) } -func (c *GRPCClient) Send(ctx context.Context, event *proto.FlowEvent) error { - if c.stream == nil { - return fmt.Errorf("stream not initialized") +func (c *GRPCClient) Send(event *proto.FlowEvent) error { + c.streamMu.Lock() + stream := c.stream + c.streamMu.Unlock() + + if stream == nil { + return errors.New("stream not initialized") } - err := c.stream.Send(event) - if err != nil { - return fmt.Errorf("sending flow event: %s", err) + if err := stream.Send(event); err != nil { + return fmt.Errorf("send flow event: %w", err) } return nil From 8b07f21c28d6882c3924e28f7447c53f3aa1272f Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 7 Mar 2025 13:56:16 +0100 Subject: [PATCH 08/11] Don't track intercepted packets (#3448) --- client/firewall/uspfilter/uspfilter.go | 43 ++++++++++++++++++-------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go index 73b963d06..23dc95517 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/uspfilter.go @@ -542,14 +542,12 @@ func (m *Manager) processOutgoingHooks(packetData []byte, size int) bool { return false } - // Track all protocols if stateful mode is enabled - if m.stateful { - m.trackOutbound(d, srcIP, dstIP, size) + if d.decoded[1] == layers.LayerTypeUDP && m.udpHooksDrop(uint16(d.udp.DstPort), dstIP, packetData) { + return true } - // Process UDP hooks even if stateful mode is disabled - if d.decoded[1] == layers.LayerTypeUDP { - return m.checkUDPHooks(d, dstIP, packetData) + if m.stateful { + m.trackOutbound(d, srcIP, dstIP, size) } return false @@ -619,19 +617,38 @@ func (m *Manager) trackInbound(d *decoder, srcIP, dstIP netip.Addr, size int) { } } -func (m *Manager) checkUDPHooks(d *decoder, dstIP netip.Addr, packetData []byte) bool { +// udpHooksDrop checks if any UDP hooks should drop the packet +func (m *Manager) udpHooksDrop(dport uint16, dstIP netip.Addr, packetData []byte) bool { m.mutex.RLock() defer m.mutex.RUnlock() - for _, ipKey := range []netip.Addr{dstIP, netip.IPv4Unspecified(), netip.IPv6Unspecified()} { - if rules, exists := m.outgoingRules[ipKey]; exists { - for _, rule := range rules { - if rule.udpHook != nil && portsMatch(rule.dPort, uint16(d.udp.DstPort)) { - return rule.udpHook(packetData) - } + // Check specific destination IP first + if rules, exists := m.outgoingRules[dstIP]; exists { + for _, rule := range rules { + if rule.udpHook != nil && portsMatch(rule.dPort, dport) { + return rule.udpHook(packetData) } } } + + // Check IPv4 unspecified address + if rules, exists := m.outgoingRules[netip.IPv4Unspecified()]; exists { + for _, rule := range rules { + if rule.udpHook != nil && portsMatch(rule.dPort, dport) { + return rule.udpHook(packetData) + } + } + } + + // Check IPv6 unspecified address + if rules, exists := m.outgoingRules[netip.IPv6Unspecified()]; exists { + for _, rule := range rules { + if rule.udpHook != nil && portsMatch(rule.dPort, dport) { + return rule.udpHook(packetData) + } + } + } + return false } From e8d8bd8f182a4b0b7d11561ce95c8274598c63ad Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 7 Mar 2025 13:56:26 +0100 Subject: [PATCH 09/11] Add peer traffic rule IDs to allowed connections in flows (#3442) --- client/firewall/uspfilter/conntrack/icmp.go | 21 ++++++++-------- client/firewall/uspfilter/conntrack/tcp.go | 27 +++++++++++---------- client/firewall/uspfilter/conntrack/udp.go | 15 ++++++------ client/firewall/uspfilter/uspfilter.go | 23 +++++++++--------- 4 files changed, 45 insertions(+), 41 deletions(-) diff --git a/client/firewall/uspfilter/conntrack/icmp.go b/client/firewall/uspfilter/conntrack/icmp.go index 5a736d132..49cc832e6 100644 --- a/client/firewall/uspfilter/conntrack/icmp.go +++ b/client/firewall/uspfilter/conntrack/icmp.go @@ -93,18 +93,17 @@ func (t *ICMPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, id uint func (t *ICMPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, size int) { if _, exists := t.updateIfExists(dstIP, srcIP, id, nftypes.Egress, size); !exists { // if (inverted direction) conn is not tracked, track this direction - t.track(srcIP, dstIP, id, typecode, nftypes.Egress, size) + t.track(srcIP, dstIP, id, typecode, nftypes.Egress, nil, size) } } // TrackInbound records an inbound ICMP Echo Request -func (t *ICMPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, size int) { - t.track(srcIP, dstIP, id, typecode, nftypes.Ingress, size) +func (t *ICMPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, ruleId []byte, size int) { + t.track(srcIP, dstIP, id, typecode, nftypes.Ingress, ruleId, size) } // track is the common implementation for tracking both inbound and outbound ICMP connections -func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction, size int) { - // TODO: icmp doesn't need to extend the timeout +func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typecode layers.ICMPv4TypeCode, direction nftypes.Direction, ruleId []byte, size int) { key, exists := t.updateIfExists(srcIP, dstIP, id, direction, size) if exists { return @@ -115,7 +114,7 @@ func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typec // non echo requests don't need tracking if typ != uint8(layers.ICMPv4TypeEchoRequest) { t.logger.Trace("New %s ICMP connection %s type %d code %d", direction, key, typ, code) - t.sendStartEvent(direction, srcIP, dstIP, typ, code, size) + t.sendStartEvent(direction, srcIP, dstIP, typ, code, ruleId, size) return } @@ -136,7 +135,7 @@ func (t *ICMPTracker) track(srcIP netip.Addr, dstIP netip.Addr, id uint16, typec t.mutex.Unlock() t.logger.Trace("New %s ICMP connection %s type %d code %d", direction, key, typ, code) - t.sendEvent(nftypes.TypeStart, conn) + t.sendEvent(nftypes.TypeStart, conn, ruleId) } // IsValidInbound checks if an inbound ICMP Echo Reply matches a tracked request @@ -186,7 +185,7 @@ func (t *ICMPTracker) cleanup() { t.logger.Debug("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) + t.sendEvent(nftypes.TypeEnd, conn, nil) } } } @@ -201,10 +200,11 @@ func (t *ICMPTracker) Close() { t.mutex.Unlock() } -func (t *ICMPTracker) sendEvent(typ nftypes.Type, conn *ICMPConnTrack) { +func (t *ICMPTracker) sendEvent(typ nftypes.Type, conn *ICMPConnTrack, ruleID []byte) { t.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: conn.FlowId, Type: typ, + RuleID: ruleID, Direction: conn.Direction, Protocol: nftypes.ICMP, // TODO: adjust for IPv6/icmpv6 SourceIP: conn.SourceIP, @@ -218,10 +218,11 @@ func (t *ICMPTracker) sendEvent(typ nftypes.Type, conn *ICMPConnTrack) { }) } -func (t *ICMPTracker) sendStartEvent(direction nftypes.Direction, srcIP netip.Addr, dstIP netip.Addr, typ uint8, code uint8, size int) { +func (t *ICMPTracker) sendStartEvent(direction nftypes.Direction, srcIP netip.Addr, dstIP netip.Addr, typ uint8, code uint8, ruleID []byte, size int) { fields := nftypes.EventFields{ FlowID: uuid.New(), Type: nftypes.TypeStart, + RuleID: ruleID, Direction: direction, Protocol: nftypes.ICMP, SourceIP: srcIP, diff --git a/client/firewall/uspfilter/conntrack/tcp.go b/client/firewall/uspfilter/conntrack/tcp.go index 96bbf0220..b5e470bf9 100644 --- a/client/firewall/uspfilter/conntrack/tcp.go +++ b/client/firewall/uspfilter/conntrack/tcp.go @@ -175,17 +175,17 @@ func (t *TCPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort func (t *TCPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, size int) { if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort, flags, 0, 0); !exists { // if (inverted direction) conn is not tracked, track this direction - t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Egress, size) + t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Egress, nil, size) } } // TrackInbound processes an inbound TCP packet and updates connection state -func (t *TCPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, size int) { - t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Ingress, size) +func (t *TCPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, ruleID []byte, size int) { + t.track(srcIP, dstIP, srcPort, dstPort, flags, nftypes.Ingress, ruleID, size) } // track is the common implementation for tracking both inbound and outbound connections -func (t *TCPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, direction nftypes.Direction, size int) { +func (t *TCPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, flags uint8, direction nftypes.Direction, ruleID []byte, size int) { key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort, flags, direction, size) if exists { return @@ -212,7 +212,7 @@ func (t *TCPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, d t.connections[key] = conn t.mutex.Unlock() - t.sendEvent(nftypes.TypeStart, conn) + t.sendEvent(nftypes.TypeStart, conn, ruleID) } // IsValidInbound checks if an inbound TCP packet matches a tracked connection @@ -246,7 +246,7 @@ func (t *TCPTracker) IsValidInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort conn.UpdateCounters(nftypes.Ingress, size) t.logger.Trace("TCP connection reset: %s", key) - t.sendEvent(nftypes.TypeEnd, conn) + t.sendEvent(nftypes.TypeEnd, conn, nil) return true } @@ -304,7 +304,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i } else if flags&TCPRst != 0 { conn.State = TCPStateClosed conn.SetTombstone() - t.sendEvent(nftypes.TypeEnd, conn) + t.sendEvent(nftypes.TypeEnd, conn, nil) } case TCPStateFinWait1: @@ -318,7 +318,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i case flags&TCPRst != 0: conn.State = TCPStateClosed conn.SetTombstone() - t.sendEvent(nftypes.TypeEnd, conn) + t.sendEvent(nftypes.TypeEnd, conn, nil) } case TCPStateFinWait2: @@ -326,7 +326,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i conn.State = TCPStateTimeWait t.logger.Trace("TCP connection %s completed", key) - t.sendEvent(nftypes.TypeEnd, conn) + t.sendEvent(nftypes.TypeEnd, conn, nil) } case TCPStateClosing: @@ -335,7 +335,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i // Keep established = false from previous state t.logger.Trace("TCP connection %s closed (simultaneous)", key) - t.sendEvent(nftypes.TypeEnd, conn) + t.sendEvent(nftypes.TypeEnd, conn, nil) } case TCPStateCloseWait: @@ -349,7 +349,7 @@ func (t *TCPTracker) updateState(key ConnKey, conn *TCPConnTrack, flags uint8, i conn.SetTombstone() // Send close event for gracefully closed connections - t.sendEvent(nftypes.TypeEnd, conn) + t.sendEvent(nftypes.TypeEnd, conn, nil) t.logger.Trace("TCP connection %s closed gracefully", key) } } @@ -436,7 +436,7 @@ func (t *TCPTracker) cleanup() { // event already handled by state change if conn.State != TCPStateTimeWait { - t.sendEvent(nftypes.TypeEnd, conn) + t.sendEvent(nftypes.TypeEnd, conn, nil) } } } @@ -467,10 +467,11 @@ func isValidFlagCombination(flags uint8) bool { return true } -func (t *TCPTracker) sendEvent(typ nftypes.Type, conn *TCPConnTrack) { +func (t *TCPTracker) sendEvent(typ nftypes.Type, conn *TCPConnTrack, ruleID []byte) { t.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: conn.FlowId, Type: typ, + RuleID: ruleID, Direction: conn.Direction, Protocol: nftypes.TCP, SourceIP: conn.SourceIP, diff --git a/client/firewall/uspfilter/conntrack/udp.go b/client/firewall/uspfilter/conntrack/udp.go index 351b3c93a..94db24f5f 100644 --- a/client/firewall/uspfilter/conntrack/udp.go +++ b/client/firewall/uspfilter/conntrack/udp.go @@ -59,13 +59,13 @@ func NewUDPTracker(timeout time.Duration, logger *nblog.Logger, flowLogger nftyp func (t *UDPTracker) TrackOutbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, size int) { if _, exists := t.updateIfExists(dstIP, srcIP, dstPort, srcPort, nftypes.Egress, size); !exists { // if (inverted direction) conn is not tracked, track this direction - t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Egress, size) + t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Egress, nil, size) } } // TrackInbound records an inbound UDP connection -func (t *UDPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, size int) { - t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Ingress, size) +func (t *UDPTracker) TrackInbound(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, ruleID []byte, size int) { + t.track(srcIP, dstIP, srcPort, dstPort, nftypes.Ingress, ruleID, size) } func (t *UDPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, direction nftypes.Direction, size int) (ConnKey, bool) { @@ -90,7 +90,7 @@ func (t *UDPTracker) updateIfExists(srcIP netip.Addr, dstIP netip.Addr, srcPort } // track is the common implementation for tracking both inbound and outbound connections -func (t *UDPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, direction nftypes.Direction, size int) { +func (t *UDPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, dstPort uint16, direction nftypes.Direction, ruleID []byte, size int) { key, exists := t.updateIfExists(srcIP, dstIP, srcPort, dstPort, direction, size) if exists { return @@ -113,7 +113,7 @@ func (t *UDPTracker) track(srcIP netip.Addr, dstIP netip.Addr, srcPort uint16, d t.mutex.Unlock() t.logger.Trace("New %s UDP connection: %s", direction, key) - t.sendEvent(nftypes.TypeStart, conn) + t.sendEvent(nftypes.TypeStart, conn, ruleID) } // IsValidInbound checks if an inbound packet matches a tracked connection @@ -161,7 +161,7 @@ func (t *UDPTracker) cleanup() { t.logger.Trace("Removed UDP 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) + t.sendEvent(nftypes.TypeEnd, conn, nil) } } } @@ -196,10 +196,11 @@ func (t *UDPTracker) Timeout() time.Duration { return t.timeout } -func (t *UDPTracker) sendEvent(typ nftypes.Type, conn *UDPConnTrack) { +func (t *UDPTracker) sendEvent(typ nftypes.Type, conn *UDPConnTrack, ruleID []byte) { t.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: conn.FlowId, Type: typ, + RuleID: ruleID, Direction: conn.Direction, Protocol: nftypes.UDP, SourceIP: conn.SourceIP, diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go index 23dc95517..723ef6299 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/uspfilter.go @@ -604,16 +604,16 @@ func (m *Manager) trackOutbound(d *decoder, srcIP, dstIP netip.Addr, size int) { } } -func (m *Manager) trackInbound(d *decoder, srcIP, dstIP netip.Addr, size int) { +func (m *Manager) trackInbound(d *decoder, srcIP, dstIP netip.Addr, ruleID []byte, size int) { transport := d.decoded[1] switch transport { case layers.LayerTypeUDP: - m.udpTracker.TrackInbound(srcIP, dstIP, uint16(d.udp.SrcPort), uint16(d.udp.DstPort), size) + m.udpTracker.TrackInbound(srcIP, dstIP, uint16(d.udp.SrcPort), uint16(d.udp.DstPort), ruleID, size) case layers.LayerTypeTCP: flags := getTCPFlags(&d.tcp) - m.tcpTracker.TrackInbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags, size) + m.tcpTracker.TrackInbound(srcIP, dstIP, uint16(d.tcp.SrcPort), uint16(d.tcp.DstPort), flags, ruleID, size) case layers.LayerTypeICMPv4: - m.icmpTracker.TrackInbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode, size) + m.icmpTracker.TrackInbound(srcIP, dstIP, d.icmp4.Id, d.icmp4.TypeCode, ruleID, size) } } @@ -684,17 +684,18 @@ func (m *Manager) dropFilter(packetData []byte, size int) bool { // handleLocalTraffic handles local traffic. // If it returns true, the packet should be dropped. func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP netip.Addr, packetData []byte, size int) bool { - if ruleId, blocked := m.peerACLsBlock(srcIP, packetData, m.incomingRules, d); blocked { + ruleID, blocked := m.peerACLsBlock(srcIP, packetData, m.incomingRules, d) + if blocked { _, pnum := getProtocolFromPacket(d) srcPort, dstPort := getPortsFromPacket(d) m.logger.Trace("Dropping local packet (ACL denied): rule_id=%s proto=%v src=%s:%d dst=%s:%d", - ruleId, pnum, srcIP, srcPort, dstIP, dstPort) + ruleID, pnum, srcIP, srcPort, dstIP, dstPort) m.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: uuid.New(), Type: nftypes.TypeDrop, - RuleID: ruleId, + RuleID: ruleID, Direction: nftypes.Ingress, Protocol: pnum, SourceIP: srcIP, @@ -714,7 +715,7 @@ func (m *Manager) handleLocalTraffic(d *decoder, srcIP, dstIP netip.Addr, packet } // track inbound packets to get the correct direction and session id for flows - m.trackInbound(d, srcIP, dstIP, size) + m.trackInbound(d, srcIP, dstIP, ruleID, size) return false } @@ -756,14 +757,14 @@ func (m *Manager) handleRoutedTraffic(d *decoder, srcIP, dstIP netip.Addr, packe proto, pnum := getProtocolFromPacket(d) srcPort, dstPort := getPortsFromPacket(d) - if id, pass := m.routeACLsPass(srcIP, dstIP, proto, srcPort, dstPort); !pass { + if ruleID, pass := m.routeACLsPass(srcIP, dstIP, proto, srcPort, dstPort); !pass { m.logger.Trace("Dropping routed packet (ACL denied): rule_id=%s proto=%v src=%s:%d dst=%s:%d", - id, pnum, srcIP, srcPort, dstIP, dstPort) + ruleID, pnum, srcIP, srcPort, dstIP, dstPort) m.flowLogger.StoreEvent(nftypes.EventFields{ FlowID: uuid.New(), Type: nftypes.TypeDrop, - RuleID: id, + RuleID: ruleID, Direction: nftypes.Ingress, Protocol: pnum, SourceIP: srcIP, From cb16d0f45faec03532f27408bafef88c0d1762f5 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 7 Mar 2025 14:03:45 +0100 Subject: [PATCH 10/11] Align packet tracer behavior with actual code paths (#3424) --- client/firewall/uspfilter/tracer.go | 36 +- client/firewall/uspfilter/tracer_test.go | 440 +++++++++++++++++++++++ 2 files changed, 463 insertions(+), 13 deletions(-) create mode 100644 client/firewall/uspfilter/tracer_test.go diff --git a/client/firewall/uspfilter/tracer.go b/client/firewall/uspfilter/tracer.go index cc5edc554..53350797c 100644 --- a/client/firewall/uspfilter/tracer.go +++ b/client/firewall/uspfilter/tracer.go @@ -265,8 +265,10 @@ func (m *Manager) traceInbound(packetData []byte, trace *PacketTrace, d *decoder return trace } - if m.handleLocalDelivery(trace, packetData, d, srcIP, dstIP) { - return trace + if m.localipmanager.IsLocalIP(dstIP) { + if m.handleLocalDelivery(trace, packetData, d, srcIP, dstIP) { + return trace + } } if !m.handleRouting(trace) { @@ -310,32 +312,40 @@ func (m *Manager) buildConntrackStateMessage(d *decoder) string { } func (m *Manager) handleLocalDelivery(trace *PacketTrace, packetData []byte, d *decoder, srcIP, dstIP netip.Addr) bool { - if !m.localForwarding { - trace.AddResult(StageRouting, "Local forwarding disabled", false) - trace.AddResult(StageCompleted, "Packet dropped - local forwarding disabled", false) - return true - } trace.AddResult(StageRouting, "Packet destined for local delivery", true) ruleId, blocked := m.peerACLsBlock(srcIP, packetData, m.incomingRules, d) - strRuleId := "implicit" + strRuleId := "" if ruleId != nil { strRuleId = string(ruleId) } - msg := fmt.Sprintf("Allowed by peer ACL rules (%s)", strRuleId) if blocked { msg = fmt.Sprintf("Blocked by peer ACL rules (%s)", strRuleId) + trace.AddResult(StagePeerACL, msg, false) + trace.AddResult(StageCompleted, "Packet dropped - ACL denied", false) + return true } - trace.AddResult(StagePeerACL, msg, !blocked) + trace.AddResult(StagePeerACL, msg, true) + // Handle netstack mode if m.netstack { - m.addForwardingResult(trace, "proxy-local", "127.0.0.1", !blocked) + switch { + case !m.localForwarding: + trace.AddResult(StageCompleted, "Packet sent to virtual stack", true) + case m.forwarder.Load() != nil: + m.addForwardingResult(trace, "proxy-local", "127.0.0.1", true) + trace.AddResult(StageCompleted, msgProcessingCompleted, true) + default: + trace.AddResult(StageCompleted, "Packet dropped - forwarder not initialized", false) + } + return true } - trace.AddResult(StageCompleted, msgProcessingCompleted, !blocked) + // In normal mode, packets are allowed through for local delivery + trace.AddResult(StageCompleted, msgProcessingCompleted, true) return true } @@ -363,7 +373,7 @@ func (m *Manager) handleRouteACLs(trace *PacketTrace, d *decoder, srcIP, dstIP n strId := string(id) if id == nil { - strId = "implicit" + strId = "" } msg := fmt.Sprintf("Allowed by route ACLs (%s)", strId) diff --git a/client/firewall/uspfilter/tracer_test.go b/client/firewall/uspfilter/tracer_test.go new file mode 100644 index 000000000..19ddceda7 --- /dev/null +++ b/client/firewall/uspfilter/tracer_test.go @@ -0,0 +1,440 @@ +package uspfilter + +import ( + "net" + "net/netip" + "testing" + + "github.com/stretchr/testify/require" + + fw "github.com/netbirdio/netbird/client/firewall/manager" + "github.com/netbirdio/netbird/client/firewall/uspfilter/conntrack" + "github.com/netbirdio/netbird/client/firewall/uspfilter/forwarder" + "github.com/netbirdio/netbird/client/iface" + "github.com/netbirdio/netbird/client/iface/device" +) + +func verifyTraceStages(t *testing.T, trace *PacketTrace, expectedStages []PacketStage) { + t.Logf("Trace results: %v", trace.Results) + actualStages := make([]PacketStage, 0, len(trace.Results)) + for _, result := range trace.Results { + actualStages = append(actualStages, result.Stage) + t.Logf("Stage: %s, Message: %s, Allowed: %v", result.Stage, result.Message, result.Allowed) + } + + require.ElementsMatch(t, expectedStages, actualStages, "Trace stages don't match expected stages") +} + +func verifyFinalDisposition(t *testing.T, trace *PacketTrace, expectedAllowed bool) { + require.NotEmpty(t, trace.Results, "Trace should have results") + lastResult := trace.Results[len(trace.Results)-1] + require.Equal(t, StageCompleted, lastResult.Stage, "Last stage should be 'Completed'") + require.Equal(t, expectedAllowed, lastResult.Allowed, "Final disposition incorrect") +} + +func TestTracePacket(t *testing.T) { + setupTracerTest := func(statefulMode bool) *Manager { + ifaceMock := &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + AddressFunc: func() iface.WGAddress { + return iface.WGAddress{ + IP: net.ParseIP("100.10.0.100"), + Network: &net.IPNet{ + IP: net.ParseIP("100.10.0.0"), + Mask: net.CIDRMask(16, 32), + }, + } + }, + } + + m, err := Create(ifaceMock, false, flowLogger) + require.NoError(t, err) + + if !statefulMode { + m.stateful = false + } + + return m + } + + createPacketBuilder := func(srcIP, dstIP string, protocol fw.Protocol, srcPort, dstPort uint16, direction fw.RuleDirection) *PacketBuilder { + builder := &PacketBuilder{ + SrcIP: netip.MustParseAddr(srcIP), + DstIP: netip.MustParseAddr(dstIP), + Protocol: protocol, + SrcPort: srcPort, + DstPort: dstPort, + Direction: direction, + } + + if protocol == "tcp" { + builder.TCPState = &TCPState{SYN: true} + } + + return builder + } + + createICMPPacketBuilder := func(srcIP, dstIP string, icmpType, icmpCode uint8, direction fw.RuleDirection) *PacketBuilder { + return &PacketBuilder{ + SrcIP: netip.MustParseAddr(srcIP), + DstIP: netip.MustParseAddr(dstIP), + Protocol: "icmp", + ICMPType: icmpType, + ICMPCode: icmpCode, + Direction: direction, + } + } + + testCases := []struct { + name string + setup func(*Manager) + packetBuilder func() *PacketBuilder + expectedStages []PacketStage + expectedAllow bool + }{ + { + name: "LocalTraffic_ACLAllowed", + setup: func(m *Manager) { + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolTCP + port := &fw.Port{Values: []uint16{80}} + action := fw.ActionAccept + _, err := m.AddPeerFiltering(nil, ip, proto, nil, port, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "100.10.0.100", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "LocalTraffic_ACLDenied", + setup: func(m *Manager) { + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolTCP + port := &fw.Port{Values: []uint16{80}} + action := fw.ActionDrop + _, err := m.AddPeerFiltering(nil, ip, proto, nil, port, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "100.10.0.100", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: false, + }, + { + name: "LocalTraffic_WithForwarder", + setup: func(m *Manager) { + m.netstack = true + m.localForwarding = true + + m.forwarder.Store(&forwarder.Forwarder{}) + + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolTCP + port := &fw.Port{Values: []uint16{80}} + action := fw.ActionAccept + _, err := m.AddPeerFiltering(nil, ip, proto, nil, port, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "100.10.0.100", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageForwarding, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "LocalTraffic_WithoutForwarder", + setup: func(m *Manager) { + m.netstack = true + m.localForwarding = false + + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolTCP + port := &fw.Port{Values: []uint16{80}} + action := fw.ActionAccept + _, err := m.AddPeerFiltering(nil, ip, proto, nil, port, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "100.10.0.100", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "RoutedTraffic_ACLAllowed", + setup: func(m *Manager) { + m.routingEnabled.Store(true) + m.nativeRouter.Store(false) + + m.forwarder.Store(&forwarder.Forwarder{}) + + src := netip.PrefixFrom(netip.AddrFrom4([4]byte{1, 1, 1, 1}), 32) + dst := netip.PrefixFrom(netip.AddrFrom4([4]byte{172, 17, 0, 2}), 32) + _, err := m.AddRouteFiltering(nil, []netip.Prefix{src}, dst, fw.ProtocolTCP, nil, &fw.Port{Values: []uint16{80}}, fw.ActionAccept) + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "172.17.0.2", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StageRouteACL, + StageForwarding, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "RoutedTraffic_ACLDenied", + setup: func(m *Manager) { + m.routingEnabled.Store(true) + m.nativeRouter.Store(false) + + src := netip.PrefixFrom(netip.AddrFrom4([4]byte{1, 1, 1, 1}), 32) + dst := netip.PrefixFrom(netip.AddrFrom4([4]byte{172, 17, 0, 2}), 32) + _, err := m.AddRouteFiltering(nil, []netip.Prefix{src}, dst, fw.ProtocolTCP, nil, &fw.Port{Values: []uint16{80}}, fw.ActionDrop) + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "172.17.0.2", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StageRouteACL, + StageCompleted, + }, + expectedAllow: false, + }, + { + name: "RoutedTraffic_NativeRouter", + setup: func(m *Manager) { + m.routingEnabled.Store(true) + m.nativeRouter.Store(true) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "172.17.0.2", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StageRouteACL, + StageForwarding, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "RoutedTraffic_RoutingDisabled", + setup: func(m *Manager) { + m.routingEnabled.Store(false) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "172.17.0.2", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StageCompleted, + }, + expectedAllow: false, + }, + { + name: "ConnectionTracking_Hit", + setup: func(m *Manager) { + srcIP := netip.MustParseAddr("100.10.0.100") + dstIP := netip.MustParseAddr("1.1.1.1") + srcPort := uint16(12345) + dstPort := uint16(80) + + m.tcpTracker.TrackOutbound(srcIP, dstIP, srcPort, dstPort, conntrack.TCPSyn) + }, + packetBuilder: func() *PacketBuilder { + pb := createPacketBuilder("1.1.1.1", "100.10.0.100", "tcp", 80, 12345, fw.RuleDirectionIN) + pb.TCPState = &TCPState{SYN: true, ACK: true} + return pb + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "OutboundTraffic", + setup: func(m *Manager) { + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("100.10.0.100", "1.1.1.1", "tcp", 12345, 80, fw.RuleDirectionOUT) + }, + expectedStages: []PacketStage{ + StageReceived, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "ICMPEchoRequest", + setup: func(m *Manager) { + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolICMP + action := fw.ActionAccept + _, err := m.AddPeerFiltering(nil, ip, proto, nil, nil, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createICMPPacketBuilder("1.1.1.1", "100.10.0.100", 8, 0, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "ICMPDestinationUnreachable", + setup: func(m *Manager) { + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolICMP + action := fw.ActionDrop + _, err := m.AddPeerFiltering(nil, ip, proto, nil, nil, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createICMPPacketBuilder("1.1.1.1", "100.10.0.100", 3, 0, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "UDPTraffic_WithoutHook", + setup: func(m *Manager) { + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolUDP + port := &fw.Port{Values: []uint16{53}} + action := fw.ActionAccept + _, err := m.AddPeerFiltering(nil, ip, proto, nil, port, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "100.10.0.100", "udp", 12345, 53, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: true, + }, + { + name: "UDPTraffic_WithHook", + setup: func(m *Manager) { + hookFunc := func([]byte) bool { + return true + } + m.AddUDPPacketHook(true, netip.MustParseAddr("1.1.1.1"), 53, hookFunc) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "100.10.0.100", "udp", 12345, 53, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageConntrack, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: false, + }, + { + name: "StatefulDisabled_NoTracking", + setup: func(m *Manager) { + m.stateful = false + + ip := net.ParseIP("1.1.1.1") + proto := fw.ProtocolTCP + port := &fw.Port{Values: []uint16{80}} + action := fw.ActionDrop + _, err := m.AddPeerFiltering(nil, ip, proto, nil, port, action, "") + require.NoError(t, err) + }, + packetBuilder: func() *PacketBuilder { + return createPacketBuilder("1.1.1.1", "100.10.0.100", "tcp", 12345, 80, fw.RuleDirectionIN) + }, + expectedStages: []PacketStage{ + StageReceived, + StageRouting, + StagePeerACL, + StageCompleted, + }, + expectedAllow: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + m := setupTracerTest(true) + + tc.setup(m) + + require.True(t, m.localipmanager.IsLocalIP(netip.MustParseAddr("100.10.0.100")), + "100.10.0.100 should be recognized as a local IP") + require.False(t, m.localipmanager.IsLocalIP(netip.MustParseAddr("172.17.0.2")), + "172.17.0.2 should not be recognized as a local IP") + + pb := tc.packetBuilder() + + trace, err := m.TracePacketFromBuilder(pb) + require.NoError(t, err) + + verifyTraceStages(t, trace, tc.expectedStages) + verifyFinalDisposition(t, trace, tc.expectedAllow) + }) + } +} From 86370a0e7bab3c28d2873d8426c9920efdefe95d Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Fri, 7 Mar 2025 16:12:47 +0100 Subject: [PATCH 11/11] Use bytes for flows event id (#3439) --- client/internal/netflow/logger/logger.go | 4 ++-- client/internal/netflow/manager.go | 5 +++-- client/internal/netflow/store/memory.go | 8 +++++--- client/internal/netflow/types/types.go | 6 +++--- flow/proto/flow.pb.go | 16 ++++++++-------- flow/proto/flow.proto | 4 ++-- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/client/internal/netflow/logger/logger.go b/client/internal/netflow/logger/logger.go index 6569b75b7..403bdb45c 100644 --- a/client/internal/netflow/logger/logger.go +++ b/client/internal/netflow/logger/logger.go @@ -74,7 +74,7 @@ func (l *Logger) startReceiver() { log.Info("flow Memory store receiver stopped") return case eventFields := <-c: - id := uuid.NewString() + id := uuid.New() event := types.Event{ ID: id, EventFields: *eventFields, @@ -109,7 +109,7 @@ func (l *Logger) GetEvents() []*types.Event { return l.Store.GetEvents() } -func (l *Logger) DeleteEvents(ids []string) { +func (l *Logger) DeleteEvents(ids []uuid.UUID) { l.Store.DeleteEvents(ids) } diff --git a/client/internal/netflow/manager.go b/client/internal/netflow/manager.go index ed5655f8a..6c5e26711 100644 --- a/client/internal/netflow/manager.go +++ b/client/internal/netflow/manager.go @@ -8,6 +8,7 @@ import ( "sync" "time" + "github.com/google/uuid" log "github.com/sirupsen/logrus" "google.golang.org/protobuf/types/known/timestamppb" @@ -169,7 +170,7 @@ func (m *Manager) startSender() { func (m *Manager) receiveACKs(client *client.GRPCClient) { err := client.Receive(m.ctx, m.flowConfig.Interval, func(ack *proto.FlowEventAck) error { log.Tracef("received flow event ack: %s", ack.EventId) - m.logger.DeleteEvents([]string{ack.EventId}) + m.logger.DeleteEvents([]uuid.UUID{uuid.UUID(ack.EventId)}) return nil }) @@ -192,7 +193,7 @@ func (m *Manager) send(event *nftypes.Event) error { func toProtoEvent(publicKey []byte, event *nftypes.Event) *proto.FlowEvent { protoEvent := &proto.FlowEvent{ - EventId: event.ID, + EventId: event.ID[:], Timestamp: timestamppb.New(event.Timestamp), PublicKey: publicKey, FlowFields: &proto.FlowFields{ diff --git a/client/internal/netflow/store/memory.go b/client/internal/netflow/store/memory.go index 7fa08b510..b695a0a12 100644 --- a/client/internal/netflow/store/memory.go +++ b/client/internal/netflow/store/memory.go @@ -5,18 +5,20 @@ import ( "golang.org/x/exp/maps" + "github.com/google/uuid" + "github.com/netbirdio/netbird/client/internal/netflow/types" ) func NewMemoryStore() *Memory { return &Memory{ - events: make(map[string]*types.Event), + events: make(map[uuid.UUID]*types.Event), } } type Memory struct { mux sync.Mutex - events map[string]*types.Event + events map[uuid.UUID]*types.Event } func (m *Memory) StoreEvent(event *types.Event) { @@ -41,7 +43,7 @@ func (m *Memory) GetEvents() []*types.Event { return events } -func (m *Memory) DeleteEvents(ids []string) { +func (m *Memory) DeleteEvents(ids []uuid.UUID) { m.mux.Lock() defer m.mux.Unlock() for _, id := range ids { diff --git a/client/internal/netflow/types/types.go b/client/internal/netflow/types/types.go index f57fca1c9..fc90a53d6 100644 --- a/client/internal/netflow/types/types.go +++ b/client/internal/netflow/types/types.go @@ -64,7 +64,7 @@ const ( ) type Event struct { - ID string + ID uuid.UUID Timestamp time.Time EventFields } @@ -111,7 +111,7 @@ type FlowLogger interface { // GetEvents returns all stored events GetEvents() []*Event // DeleteEvents deletes events from the store - DeleteEvents([]string) + DeleteEvents([]uuid.UUID) // Close closes the logger Close() // Enable enables the flow logger receiver @@ -126,7 +126,7 @@ type Store interface { // GetEvents returns all stored events GetEvents() []*Event // DeleteEvents deletes events from the store - DeleteEvents([]string) + DeleteEvents([]uuid.UUID) // Close closes the store Close() } diff --git a/flow/proto/flow.pb.go b/flow/proto/flow.pb.go index b2857c2b2..1a94fd103 100644 --- a/flow/proto/flow.pb.go +++ b/flow/proto/flow.pb.go @@ -130,7 +130,7 @@ type FlowEvent struct { unknownFields protoimpl.UnknownFields // Unique client event identifier - EventId string `protobuf:"bytes,1,opt,name=event_id,json=eventId,proto3" json:"event_id,omitempty"` + EventId []byte `protobuf:"bytes,1,opt,name=event_id,json=eventId,proto3" json:"event_id,omitempty"` // When the event occurred Timestamp *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=timestamp,proto3" json:"timestamp,omitempty"` // Public key of the sending peer @@ -170,11 +170,11 @@ func (*FlowEvent) Descriptor() ([]byte, []int) { return file_flow_proto_rawDescGZIP(), []int{0} } -func (x *FlowEvent) GetEventId() string { +func (x *FlowEvent) GetEventId() []byte { if x != nil { return x.EventId } - return "" + return nil } func (x *FlowEvent) GetTimestamp() *timestamppb.Timestamp { @@ -204,7 +204,7 @@ type FlowEventAck struct { unknownFields protoimpl.UnknownFields // Unique client event identifier that has been ack'ed - EventId string `protobuf:"bytes,1,opt,name=event_id,json=eventId,proto3" json:"event_id,omitempty"` + EventId []byte `protobuf:"bytes,1,opt,name=event_id,json=eventId,proto3" json:"event_id,omitempty"` } func (x *FlowEventAck) Reset() { @@ -239,11 +239,11 @@ func (*FlowEventAck) Descriptor() ([]byte, []int) { return file_flow_proto_rawDescGZIP(), []int{1} } -func (x *FlowEventAck) GetEventId() string { +func (x *FlowEventAck) GetEventId() []byte { if x != nil { return x.EventId } - return "" + return nil } type FlowFields struct { @@ -548,7 +548,7 @@ var file_flow_proto_rawDesc = []byte{ 0x62, 0x75, 0x66, 0x2f, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x22, 0xb2, 0x01, 0x0a, 0x09, 0x46, 0x6c, 0x6f, 0x77, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x12, 0x19, 0x0a, 0x08, 0x65, 0x76, 0x65, 0x6e, 0x74, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, - 0x01, 0x28, 0x09, 0x52, 0x07, 0x65, 0x76, 0x65, 0x6e, 0x74, 0x49, 0x64, 0x12, 0x38, 0x0a, 0x09, + 0x01, 0x28, 0x0c, 0x52, 0x07, 0x65, 0x76, 0x65, 0x6e, 0x74, 0x49, 0x64, 0x12, 0x38, 0x0a, 0x09, 0x74, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x1a, 0x2e, 0x67, 0x6f, 0x6f, 0x67, 0x6c, 0x65, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x62, 0x75, 0x66, 0x2e, 0x54, 0x69, 0x6d, 0x65, 0x73, 0x74, 0x61, 0x6d, 0x70, 0x52, 0x09, 0x74, 0x69, 0x6d, @@ -559,7 +559,7 @@ var file_flow_proto_rawDesc = []byte{ 0x77, 0x2e, 0x46, 0x6c, 0x6f, 0x77, 0x46, 0x69, 0x65, 0x6c, 0x64, 0x73, 0x52, 0x0a, 0x66, 0x6c, 0x6f, 0x77, 0x46, 0x69, 0x65, 0x6c, 0x64, 0x73, 0x22, 0x29, 0x0a, 0x0c, 0x46, 0x6c, 0x6f, 0x77, 0x45, 0x76, 0x65, 0x6e, 0x74, 0x41, 0x63, 0x6b, 0x12, 0x19, 0x0a, 0x08, 0x65, 0x76, 0x65, 0x6e, - 0x74, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x65, 0x76, 0x65, 0x6e, + 0x74, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x07, 0x65, 0x76, 0x65, 0x6e, 0x74, 0x49, 0x64, 0x22, 0xc4, 0x03, 0x0a, 0x0a, 0x46, 0x6c, 0x6f, 0x77, 0x46, 0x69, 0x65, 0x6c, 0x64, 0x73, 0x12, 0x17, 0x0a, 0x07, 0x66, 0x6c, 0x6f, 0x77, 0x5f, 0x69, 0x64, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x06, 0x66, 0x6c, 0x6f, 0x77, 0x49, 0x64, 0x12, 0x1e, 0x0a, 0x04, 0x74, diff --git a/flow/proto/flow.proto b/flow/proto/flow.proto index 742de82f9..a4bd56137 100644 --- a/flow/proto/flow.proto +++ b/flow/proto/flow.proto @@ -13,7 +13,7 @@ service FlowService { message FlowEvent { // Unique client event identifier - string event_id = 1; + bytes event_id = 1; // When the event occurred google.protobuf.Timestamp timestamp = 2; @@ -26,7 +26,7 @@ message FlowEvent { message FlowEventAck { // Unique client event identifier that has been ack'ed - string event_id = 1; + bytes event_id = 1; } message FlowFields {