From 955b2b98e1ae73fc99f21e5cd576b2754ccdb1f7 Mon Sep 17 00:00:00 2001 From: Viktor Liu Date: Fri, 3 Jan 2025 14:19:59 +0100 Subject: [PATCH] Complete route ACLs and add tests --- .../firewall/uspfilter/forwarder/endpoint.go | 2 - client/firewall/uspfilter/forwarder/tcp.go | 1 - client/firewall/uspfilter/uspfilter.go | 69 +- .../uspfilter/uspfilter_bench_test.go | 70 ++ .../uspfilter/uspfilter_filter_test.go | 744 ++++++++++++++++++ client/firewall/uspfilter/uspfilter_test.go | 16 +- 6 files changed, 881 insertions(+), 21 deletions(-) create mode 100644 client/firewall/uspfilter/uspfilter_filter_test.go diff --git a/client/firewall/uspfilter/forwarder/endpoint.go b/client/firewall/uspfilter/forwarder/endpoint.go index 7ff6101d0..e8a265c94 100644 --- a/client/firewall/uspfilter/forwarder/endpoint.go +++ b/client/firewall/uspfilter/forwarder/endpoint.go @@ -53,8 +53,6 @@ func (e *endpoint) WritePackets(pkts stack.PacketBufferList) (int, tcpip.Error) // Send the packet through WireGuard address := netHeader.DestinationAddress() - - // TODO: handle dest ip addresses outside our network err := e.device.CreateOutboundPacket(data.AsSlice(), address.AsSlice()) if err != nil { e.logger.Error("CreateOutboundPacket: %v", err) diff --git a/client/firewall/uspfilter/forwarder/tcp.go b/client/firewall/uspfilter/forwarder/tcp.go index efe94bae9..3cdc45218 100644 --- a/client/firewall/uspfilter/forwarder/tcp.go +++ b/client/firewall/uspfilter/forwarder/tcp.go @@ -19,7 +19,6 @@ func (f *Forwarder) handleTCP(r *tcp.ForwarderRequest) { dialAddr := fmt.Sprintf("%s:%d", f.determineDialAddr(id.LocalAddress), id.LocalPort) - f.logger.Trace("forwarder: handling TCP connection %v", id) outConn, err := (&net.Dialer{}).DialContext(f.ctx, "tcp", dialAddr) if err != nil { r.Complete(true) diff --git a/client/firewall/uspfilter/uspfilter.go b/client/firewall/uspfilter/uspfilter.go index e869b0f4b..b6be57ff6 100644 --- a/client/firewall/uspfilter/uspfilter.go +++ b/client/firewall/uspfilter/uspfilter.go @@ -517,7 +517,6 @@ func (m *Manager) dropFilter(packetData []byte, rules map[string]RuleSet) bool { // if running in netstack mode we need to pass this to the forwarder if m.netstack { - m.logger.Trace("Passing local packet to netstack: src=%s dst=%s", srcIP, dstIP) m.handleNetstackLocalTraffic(packetData) // don't process this packet further return true @@ -729,20 +728,39 @@ func (m *Manager) routeACLsPass(srcIP, dstIP net.IP, proto firewall.Protocol, sr m.mutex.RLock() defer m.mutex.RUnlock() - srcAddr, _ := netip.AddrFromSlice(srcIP) - dstAddr, _ := netip.AddrFromSlice(dstIP) + srcAddr := netip.AddrFrom4([4]byte(srcIP.To4())) + dstAddr := netip.AddrFrom4([4]byte(dstIP.To4())) - matched := false for _, rule := range m.routeRules { - if m.ruleMatches(rule, srcAddr, dstAddr, proto, srcPort, dstPort) { - matched = true - if rule.action == firewall.ActionDrop { - return false + if !rule.destination.Contains(dstAddr) { + continue + } + + sourceMatched := false + for _, src := range rule.sources { + if src.Contains(srcAddr) { + sourceMatched = true + break } } + if !sourceMatched { + continue + } + + if rule.proto != firewall.ProtocolALL && rule.proto != proto { + continue + } + + if proto == firewall.ProtocolTCP || proto == firewall.ProtocolUDP { + if !m.portsMatch(rule.srcPort, srcPort) || !m.portsMatch(rule.dstPort, dstPort) { + continue + } + } + + return rule.action == firewall.ActionAccept } - return matched + return false } func (m *Manager) ruleMatches(rule RouteRule, srcAddr, dstAddr netip.Addr, proto firewall.Protocol, srcPort, dstPort uint16) bool { @@ -765,16 +783,39 @@ func (m *Manager) ruleMatches(rule RouteRule, srcAddr, dstAddr netip.Addr, proto return false } - if rule.srcPort != nil && rule.srcPort.Values[0] != int(srcPort) { - return false - } - if rule.dstPort != nil && rule.dstPort.Values[0] != int(dstPort) { - return false + // Port matches for TCP/UDP only + if proto == firewall.ProtocolTCP || proto == firewall.ProtocolUDP { + return m.portsMatch(rule.srcPort, srcPort) && m.portsMatch(rule.dstPort, dstPort) } return true } +// Add to uspfilter.go, replace existing portsMatch method +func (m *Manager) portsMatch(rulePort *firewall.Port, packetPort uint16) bool { + if rulePort == nil || len(rulePort.Values) == 0 { + return true + } + + if rulePort.IsRange { + if len(rulePort.Values) != 2 { + m.logger.Error("Invalid port range configuration: expected 2 values for range") + return false + } + startPort := rulePort.Values[0] + endPort := rulePort.Values[1] + return int(packetPort) >= startPort && int(packetPort) <= endPort + } + + // Handle list of individual ports + for _, p := range rulePort.Values { + if uint16(p) == packetPort { + return true + } + } + return false +} + // SetNetwork of the wireguard interface to which filtering applied func (m *Manager) SetNetwork(network *net.IPNet) { m.wgNetwork = network diff --git a/client/firewall/uspfilter/uspfilter_bench_test.go b/client/firewall/uspfilter/uspfilter_bench_test.go index 4ae88559d..c3974b492 100644 --- a/client/firewall/uspfilter/uspfilter_bench_test.go +++ b/client/firewall/uspfilter/uspfilter_bench_test.go @@ -6,6 +6,7 @@ import ( "fmt" "math/rand" "net" + "net/netip" "os" "strings" "testing" @@ -998,3 +999,72 @@ func generateTCPPacketWithFlags(b *testing.B, srcIP, dstIP net.IP, srcPort, dstP require.NoError(b, gopacket.SerializeLayers(buf, opts, ipv4, tcp, gopacket.Payload("test"))) return buf.Bytes() } + +func BenchmarkRouteACLs(b *testing.B) { + manager := setupRoutedManager(b, "10.10.0.100/16") + + // Add several route rules to simulate real-world scenario + rules := []struct { + sources []netip.Prefix + dest netip.Prefix + proto fw.Protocol + port *fw.Port + }{ + { + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + port: &fw.Port{Values: []int{80, 443}}, + }, + { + sources: []netip.Prefix{ + netip.MustParsePrefix("172.16.0.0/12"), + netip.MustParsePrefix("10.0.0.0/8"), + }, + dest: netip.MustParsePrefix("0.0.0.0/0"), + proto: fw.ProtocolICMP, + }, + { + sources: []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0")}, + dest: netip.MustParsePrefix("192.168.0.0/16"), + proto: fw.ProtocolUDP, + port: &fw.Port{Values: []int{53}}, + }, + } + + for _, r := range rules { + _, err := manager.AddRouteFiltering( + r.sources, + r.dest, + r.proto, + nil, + r.port, + fw.ActionAccept, + ) + if err != nil { + b.Fatal(err) + } + } + + // Test cases that exercise different matching scenarios + cases := []struct { + srcIP string + dstIP string + proto fw.Protocol + dstPort uint16 + }{ + {"100.10.0.1", "192.168.1.100", fw.ProtocolTCP, 443}, // Match first rule + {"172.16.0.1", "8.8.8.8", fw.ProtocolICMP, 0}, // Match second rule + {"1.1.1.1", "192.168.1.53", fw.ProtocolUDP, 53}, // Match third rule + {"192.168.1.1", "10.0.0.1", fw.ProtocolTCP, 8080}, // No match + } + + b.ResetTimer() + for i := 0; i < b.N; i++ { + for _, tc := range cases { + srcIP := net.ParseIP(tc.srcIP) + dstIP := net.ParseIP(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 new file mode 100644 index 000000000..abfa4e54d --- /dev/null +++ b/client/firewall/uspfilter/uspfilter_filter_test.go @@ -0,0 +1,744 @@ +package uspfilter + +import ( + "net" + "net/netip" + "testing" + + "github.com/golang/mock/gomock" + "github.com/google/gopacket" + "github.com/google/gopacket/layers" + "github.com/stretchr/testify/require" + wgdevice "golang.zx2c4.com/wireguard/device" + + fw "github.com/netbirdio/netbird/client/firewall/manager" + "github.com/netbirdio/netbird/client/iface" + "github.com/netbirdio/netbird/client/iface/device" + "github.com/netbirdio/netbird/client/iface/mocks" +) + +func TestPeerACLFiltering(t *testing.T) { + localIP := net.ParseIP("100.10.0.100") + wgNet := &net.IPNet{ + IP: net.ParseIP("100.10.0.0"), + Mask: net.CIDRMask(16, 32), + } + + ifaceMock := &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + AddressFunc: func() iface.WGAddress { + return iface.WGAddress{ + IP: localIP, + Network: wgNet, + } + }, + } + + manager, err := Create(ifaceMock) + require.NoError(t, err) + require.NotNil(t, manager) + + t.Cleanup(func() { + require.NoError(t, manager.Reset(nil)) + }) + + manager.wgNetwork = wgNet + + err = manager.UpdateLocalIPs() + require.NoError(t, err) + + testCases := []struct { + name string + srcIP string + dstIP string + proto fw.Protocol + srcPort uint16 + dstPort uint16 + ruleIP string + ruleProto fw.Protocol + ruleSrcPort *fw.Port + ruleDstPort *fw.Port + ruleAction fw.Action + shouldBeBlocked bool + }{ + { + name: "Allow TCP traffic from WG peer", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{Values: []int{443}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow UDP traffic from WG peer", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolUDP, + srcPort: 12345, + dstPort: 53, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolUDP, + ruleDstPort: &fw.Port{Values: []int{53}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow ICMP traffic from WG peer", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolICMP, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolICMP, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow all traffic from WG peer", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolALL, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow traffic from non-WG source", + srcIP: "192.168.1.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + ruleIP: "192.168.1.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{Values: []int{443}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow all traffic with 0.0.0.0 rule", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + ruleIP: "0.0.0.0", + ruleProto: fw.ProtocolALL, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + } + + 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) + require.True(t, isDropped, "Packet should be dropped when no rules exist") + }) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rules, err := manager.AddPeerFiltering( + net.ParseIP(tc.ruleIP), + tc.ruleProto, + tc.ruleSrcPort, + tc.ruleDstPort, + fw.RuleDirectionIN, + tc.ruleAction, + "", + tc.name, + ) + require.NoError(t, err) + require.NotEmpty(t, rules) + + t.Cleanup(func() { + for _, rule := range rules { + require.NoError(t, manager.DeletePeerRule(rule)) + } + }) + + packet := createTestPacket(t, tc.srcIP, tc.dstIP, tc.proto, tc.srcPort, tc.dstPort) + isDropped := manager.DropIncoming(packet) + require.Equal(t, tc.shouldBeBlocked, isDropped) + }) + } +} + +func createTestPacket(t *testing.T, srcIP, dstIP string, proto fw.Protocol, srcPort, dstPort uint16) []byte { + t.Helper() + + buf := gopacket.NewSerializeBuffer() + opts := gopacket.SerializeOptions{ + ComputeChecksums: true, + FixLengths: true, + } + + ipLayer := &layers.IPv4{ + Version: 4, + TTL: 64, + SrcIP: net.ParseIP(srcIP), + DstIP: net.ParseIP(dstIP), + } + + var err error + switch proto { + case fw.ProtocolTCP: + ipLayer.Protocol = layers.IPProtocolTCP + tcp := &layers.TCP{ + SrcPort: layers.TCPPort(srcPort), + DstPort: layers.TCPPort(dstPort), + } + err = tcp.SetNetworkLayerForChecksum(ipLayer) + require.NoError(t, err) + err = gopacket.SerializeLayers(buf, opts, ipLayer, tcp) + + case fw.ProtocolUDP: + ipLayer.Protocol = layers.IPProtocolUDP + udp := &layers.UDP{ + SrcPort: layers.UDPPort(srcPort), + DstPort: layers.UDPPort(dstPort), + } + err = udp.SetNetworkLayerForChecksum(ipLayer) + require.NoError(t, err) + err = gopacket.SerializeLayers(buf, opts, ipLayer, udp) + + case fw.ProtocolICMP: + ipLayer.Protocol = layers.IPProtocolICMPv4 + icmp := &layers.ICMPv4{ + TypeCode: layers.CreateICMPv4TypeCode(layers.ICMPv4TypeEchoRequest, 0), + } + err = gopacket.SerializeLayers(buf, opts, ipLayer, icmp) + + default: + err = gopacket.SerializeLayers(buf, opts, ipLayer) + } + + require.NoError(t, err) + return buf.Bytes() +} + +func setupRoutedManager(t testing.TB, network string) *Manager { + t.Helper() + + ctrl := gomock.NewController(t) + dev := mocks.NewMockDevice(ctrl) + dev.EXPECT().MTU().Return(1500, nil).AnyTimes() + + localIP, wgNet, err := net.ParseCIDR(network) + require.NoError(t, err) + + ifaceMock := &IFaceMock{ + SetFilterFunc: func(device.PacketFilter) error { return nil }, + AddressFunc: func() iface.WGAddress { + return iface.WGAddress{ + IP: localIP, + Network: wgNet, + } + }, + GetDeviceFunc: func() *device.FilteredDevice { + return &device.FilteredDevice{Device: dev} + }, + GetWGDeviceFunc: func() *wgdevice.Device { + return &wgdevice.Device{} + }, + } + + manager, err := Create(ifaceMock) + require.NoError(t, err) + require.NotNil(t, manager) + require.True(t, manager.routingEnabled) + require.False(t, manager.nativeRouter) + + t.Cleanup(func() { + require.NoError(t, manager.Reset(nil)) + }) + + return manager +} + +func TestRouteACLFiltering(t *testing.T) { + manager := setupRoutedManager(t, "10.10.0.100/16") + + type rule struct { + sources []netip.Prefix + dest netip.Prefix + proto fw.Protocol + srcPort *fw.Port + dstPort *fw.Port + action fw.Action + } + + testCases := []struct { + name string + srcIP string + dstIP string + proto fw.Protocol + srcPort uint16 + dstPort uint16 + rule rule + shouldPass bool + }{ + { + name: "Allow TCP with specific source and destination", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{443}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow any source to specific destination", + srcIP: "172.16.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{443}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow any source to any destination", + srcIP: "172.16.0.1", + dstIP: "203.0.113.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0")}, + dest: netip.MustParsePrefix("0.0.0.0/0"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{443}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow UDP DNS traffic", + srcIP: "100.10.0.1", + dstIP: "192.168.1.53", + proto: fw.ProtocolUDP, + srcPort: 54321, + dstPort: 53, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolUDP, + dstPort: &fw.Port{Values: []int{53}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow ICMP to any destination", + srcIP: "100.10.0.1", + dstIP: "8.8.8.8", + proto: fw.ProtocolICMP, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("0.0.0.0/0"), + proto: fw.ProtocolICMP, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow all protocols but specific port", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolALL, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Implicit deny - wrong destination port", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "Implicit deny - wrong protocol", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolUDP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "Implicit deny - wrong source network", + srcIP: "172.16.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "Source port match", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + srcPort: &fw.Port{Values: []int{12345}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Multiple source networks", + srcIP: "172.16.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{ + netip.MustParsePrefix("100.10.0.0/16"), + netip.MustParsePrefix("172.16.0.0/16"), + }, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow ALL protocol without ports", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolICMP, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolALL, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow ALL protocol with specific ports", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolALL, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Multiple source networks with mismatched protocol", + srcIP: "172.16.0.1", + dstIP: "192.168.1.100", + // Should not match TCP rule + proto: fw.ProtocolUDP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{ + netip.MustParsePrefix("100.10.0.0/16"), + netip.MustParsePrefix("172.16.0.0/16"), + }, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "Allow multiple destination ports", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{Values: []int{80, 8080, 443}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow multiple source ports", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + srcPort: &fw.Port{Values: []int{12345, 12346, 12347}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow ALL protocol with both src and dst ports", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolALL, + srcPort: &fw.Port{Values: []int{12345}}, + dstPort: &fw.Port{Values: []int{80}}, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Port Range - Within Range", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{ + IsRange: true, + Values: []int{8000, 8100}, + }, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Port Range - Outside Range", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 7999, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{ + IsRange: true, + Values: []int{8000, 8100}, + }, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "Source Port Range - Within Range", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 32100, + dstPort: 80, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + srcPort: &fw.Port{ + IsRange: true, + Values: []int{32000, 33000}, + }, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Mixed Port Specification - Range and Single", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 32100, + dstPort: 443, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + srcPort: &fw.Port{ + IsRange: true, + Values: []int{32000, 33000}, + }, + dstPort: &fw.Port{ + Values: []int{443}, + }, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Invalid Port Range Configuration", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{ + IsRange: true, + Values: []int{8000}, // Invalid: only one value for range + }, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "Edge Case - Port at Range Boundary", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8100, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolTCP, + dstPort: &fw.Port{ + IsRange: true, + Values: []int{8000, 8100}, + }, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "UDP Port Range", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolUDP, + srcPort: 12345, + dstPort: 5060, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolUDP, + dstPort: &fw.Port{ + IsRange: true, + Values: []int{5060, 5070}, + }, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "ALL Protocol with Port Range", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + rule: rule{ + sources: []netip.Prefix{netip.MustParsePrefix("100.10.0.0/16")}, + dest: netip.MustParsePrefix("192.168.1.0/24"), + proto: fw.ProtocolALL, + dstPort: &fw.Port{ + IsRange: true, + Values: []int{8000, 8100}, + }, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + rule, err := manager.AddRouteFiltering( + tc.rule.sources, + tc.rule.dest, + tc.rule.proto, + tc.rule.srcPort, + tc.rule.dstPort, + tc.rule.action, + ) + require.NoError(t, err) + require.NotNil(t, rule) + + t.Cleanup(func() { + require.NoError(t, manager.DeleteRouteRule(rule)) + }) + + srcIP := net.ParseIP(tc.srcIP) + dstIP := net.ParseIP(tc.dstIP) + + // testing routeACLsPass only and not DropIncoming, as routed packets are dropped after being passed + // to the forwarder + isAllowed := manager.routeACLsPass(srcIP, dstIP, tc.proto, tc.srcPort, tc.dstPort) + require.Equal(t, tc.shouldPass, isAllowed) + }) + } +} diff --git a/client/firewall/uspfilter/uspfilter_test.go b/client/firewall/uspfilter/uspfilter_test.go index a0c0d23c3..95f79115a 100644 --- a/client/firewall/uspfilter/uspfilter_test.go +++ b/client/firewall/uspfilter/uspfilter_test.go @@ -23,16 +23,24 @@ import ( var logger = log.NewFromLogrus(logrus.StandardLogger()) type IFaceMock struct { - SetFilterFunc func(device.PacketFilter) error - AddressFunc func() iface.WGAddress + SetFilterFunc func(device.PacketFilter) error + AddressFunc func() iface.WGAddress + GetWGDeviceFunc func() *wgdevice.Device + GetDeviceFunc func() *device.FilteredDevice } func (i *IFaceMock) GetWGDevice() *wgdevice.Device { - return nil + if i.GetWGDeviceFunc == nil { + return nil + } + return i.GetWGDeviceFunc() } func (i *IFaceMock) GetDevice() *device.FilteredDevice { - return nil + if i.GetDeviceFunc == nil { + return nil + } + return i.GetDeviceFunc() } func (i *IFaceMock) SetFilter(iface device.PacketFilter) error {