diff --git a/client/firewall/uspfilter/uspfilter_filter_test.go b/client/firewall/uspfilter/uspfilter_filter_test.go index ba97c2643..9c0a54e3f 100644 --- a/client/firewall/uspfilter/uspfilter_filter_test.go +++ b/client/firewall/uspfilter/uspfilter_filter_test.go @@ -188,6 +188,281 @@ func TestPeerACLFiltering(t *testing.T) { ruleAction: fw.ActionAccept, shouldBeBlocked: true, }, + { + name: "Allow TCP traffic without port specification", + 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, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow UDP traffic without port specification", + 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, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "TCP packet doesn't match UDP filter with same port", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 443, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolUDP, + ruleDstPort: &fw.Port{Values: []uint16{443}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: true, + }, + { + name: "UDP packet doesn't match TCP filter with same port", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolUDP, + srcPort: 12345, + dstPort: 443, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{Values: []uint16{443}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: true, + }, + { + name: "ICMP packet doesn't match TCP filter", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolICMP, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleAction: fw.ActionAccept, + shouldBeBlocked: true, + }, + { + name: "ICMP packet doesn't match UDP filter", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolICMP, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolUDP, + ruleAction: fw.ActionAccept, + shouldBeBlocked: true, + }, + { + name: "Allow TCP traffic within port range", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{IsRange: true, Values: []uint16{8000, 8100}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Block TCP traffic outside port range", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 7999, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{IsRange: true, Values: []uint16{8000, 8100}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: true, + }, + { + name: "Edge Case - Port at Range Boundary", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8100, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{IsRange: true, Values: []uint16{8000, 8100}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "UDP Port Range", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolUDP, + srcPort: 12345, + dstPort: 5060, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolUDP, + ruleDstPort: &fw.Port{IsRange: true, Values: []uint16{5060, 5070}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow multiple destination ports", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{Values: []uint16{80, 8080, 443}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + { + name: "Allow multiple source ports", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleSrcPort: &fw.Port{Values: []uint16{12345, 12346, 12347}}, + ruleAction: fw.ActionAccept, + shouldBeBlocked: false, + }, + // New drop test cases + { + name: "Drop 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: []uint16{443}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Drop 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: []uint16{53}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Drop 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.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Drop 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.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Drop traffic from multiple source ports", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 80, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleSrcPort: &fw.Port{Values: []uint16{12345, 12346, 12347}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Drop multiple destination ports", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{Values: []uint16{80, 8080, 443}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Drop TCP traffic within port range", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 8080, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{IsRange: true, Values: []uint16{8000, 8100}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Accept TCP traffic outside drop port range", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 12345, + dstPort: 7999, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleDstPort: &fw.Port{IsRange: true, Values: []uint16{8000, 8100}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: false, + }, + { + name: "Drop TCP traffic with source port range", + srcIP: "100.10.0.1", + dstIP: "100.10.0.100", + proto: fw.ProtocolTCP, + srcPort: 32100, + dstPort: 80, + ruleIP: "100.10.0.1", + ruleProto: fw.ProtocolTCP, + ruleSrcPort: &fw.Port{IsRange: true, Values: []uint16{32000, 33000}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: true, + }, + { + name: "Mixed rule - drop specific port but allow other ports", + 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: []uint16{443}}, + ruleAction: fw.ActionDrop, + shouldBeBlocked: true, + }, } t.Run("Implicit DROP (no rules)", func(t *testing.T) { @@ -198,6 +473,28 @@ func TestPeerACLFiltering(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + + if tc.ruleAction == fw.ActionDrop { + // add general accept rule to test drop rule + // TODO: this only works because 0.0.0.0 is tested last, we need to implement order + rules, err := manager.AddPeerFiltering( + nil, + net.ParseIP("0.0.0.0"), + fw.ProtocolALL, + nil, + nil, + fw.ActionAccept, + "", + ) + require.NoError(t, err) + require.NotEmpty(t, rules) + t.Cleanup(func() { + for _, rule := range rules { + require.NoError(t, manager.DeletePeerRule(rule)) + } + }) + } + rules, err := manager.AddPeerFiltering( nil, net.ParseIP(tc.ruleIP), @@ -543,26 +840,6 @@ func TestRouteACLFiltering(t *testing.T) { }, 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: []uint16{80}}, - action: fw.ActionAccept, - }, - shouldPass: false, - }, { name: "Allow multiple destination ports", srcIP: "100.10.0.1", @@ -798,10 +1075,132 @@ func TestRouteACLFiltering(t *testing.T) { }, shouldPass: false, }, + { + name: "Accept TCP traffic outside drop port 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: []uint16{8000, 8100}}, + action: fw.ActionDrop, + }, + shouldPass: true, + }, + { + name: "Allow TCP traffic without port specification", + 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, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "Allow UDP traffic without port specification", + srcIP: "100.10.0.1", + dstIP: "192.168.1.100", + proto: fw.ProtocolUDP, + srcPort: 12345, + 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, + action: fw.ActionAccept, + }, + shouldPass: true, + }, + { + name: "TCP packet doesn't match UDP filter with same 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.ProtocolUDP, + dstPort: &fw.Port{Values: []uint16{80}}, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "UDP packet doesn't match TCP filter with same port", + 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: []uint16{80}}, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "ICMP packet doesn't match TCP filter", + 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.ProtocolTCP, + action: fw.ActionAccept, + }, + shouldPass: false, + }, + { + name: "ICMP packet doesn't match UDP filter", + 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.ProtocolUDP, + action: fw.ActionAccept, + }, + shouldPass: false, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + if tc.rule.action == fw.ActionDrop { + // add general accept rule to test drop rule + rule, err := manager.AddRouteFiltering( + nil, + []netip.Prefix{netip.MustParsePrefix("0.0.0.0/0")}, + netip.MustParsePrefix("0.0.0.0/0"), + fw.ProtocolALL, + nil, + nil, + fw.ActionAccept, + ) + require.NoError(t, err) + require.NotNil(t, rule) + t.Cleanup(func() { + require.NoError(t, manager.DeleteRouteRule(rule)) + }) + } + rule, err := manager.AddRouteFiltering( nil, tc.rule.sources,