diff --git a/hscontrol/policy/policy_test.go b/hscontrol/policy/policy_test.go index ebce8de5..f28fd090 100644 --- a/hscontrol/policy/policy_test.go +++ b/hscontrol/policy/policy_test.go @@ -768,6 +768,65 @@ func TestReduceNodes(t *testing.T) { }, }, }, + // Subnet-to-subnet: routers must see each other when ACL + // uses only subnet CIDRs. Issue #3157. + { + name: "subnet-to-subnet-routers-see-each-other-3157", + args: args{ + nodes: []*types.Node{ + { + ID: 1, + IPv4: ap("100.64.0.1"), + Hostname: "router-a", + User: &types.User{Name: "router-a"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")}, + }, + ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")}, + }, + { + ID: 2, + IPv4: ap("100.64.0.2"), + Hostname: "router-b", + User: &types.User{Name: "router-b"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")}, + }, + ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")}, + }, + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + node: &types.Node{ + ID: 1, + IPv4: ap("100.64.0.1"), + Hostname: "router-a", + User: &types.User{Name: "router-a"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")}, + }, + ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")}, + }, + }, + want: []*types.Node{ + { + ID: 2, + IPv4: ap("100.64.0.2"), + Hostname: "router-b", + User: &types.User{Name: "router-b"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")}, + }, + ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")}, + }, + }, + }, } for _, tt := range tests { @@ -2230,6 +2289,127 @@ func TestReduceRoutes(t *testing.T) { netip.MustParsePrefix("192.168.1.0/14"), }, }, + // Subnet-to-subnet tests for issue #3157. + // When an ACL references subnet CIDRs as both source and destination, + // the subnet routers for those subnets must receive routes to each + // other's subnets. + { + name: "subnet-to-subnet-src-router-gets-dst-route-3157", + args: args{ + node: &types.Node{ + ID: 1, + IPv4: ap("100.64.0.1"), + User: &types.User{Name: "router-a"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + routes: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + }, + want: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + { + name: "subnet-to-subnet-dst-router-gets-src-route-3157", + args: args{ + node: &types.Node{ + ID: 2, + IPv4: ap("100.64.0.2"), + User: &types.User{Name: "router-b"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + routes: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + }, + want: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + { + name: "subnet-to-subnet-regular-node-no-route-leak-3157", + args: args{ + node: &types.Node{ + ID: 3, + IPv4: ap("100.64.0.3"), + User: &types.User{Name: "regular-node"}, + }, + routes: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + netip.MustParsePrefix("10.99.9.0/24"), + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + }, + want: nil, + }, + { + name: "subnet-to-subnet-unrelated-router-no-route-leak-3157", + args: args{ + node: &types.Node{ + ID: 4, + IPv4: ap("100.64.0.4"), + User: &types.User{Name: "router-c"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("172.16.0.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("172.16.0.0/24"), + }, + }, + routes: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + }, + want: nil, + }, } for _, tt := range tests { diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go index 567f21fb..cbb8192f 100644 --- a/hscontrol/types/node_test.go +++ b/hscontrol/types/node_test.go @@ -113,6 +113,183 @@ func Test_NodeCanAccess(t *testing.T) { }, want: true, }, + // Subnet-to-subnet tests for issue #3157. + // When ACL src and dst are both subnet CIDRs, subnet + // routers advertising those subnets must see each other. + { + name: "subnet-to-subnet-src-router-sees-dst-router-3157", + node1: Node{ + IPv4: iap("100.64.0.1"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + node2: Node{ + IPv4: iap("100.64.0.2"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + want: true, + }, + { + // With a unidirectional ACL (src=A→dst=B), the dst + // router cannot access the src router. Bidirectional + // peer visibility comes from ReduceNodes checking + // both A.CanAccess(B) || B.CanAccess(A). + name: "subnet-to-subnet-unidirectional-dst-cannot-access-src-3157", + node1: Node{ + IPv4: iap("100.64.0.2"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + node2: Node{ + IPv4: iap("100.64.0.1"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + want: false, + }, + { + // With a bidirectional ACL, both routers can access + // each other. + name: "subnet-to-subnet-bidirectional-3157", + node1: Node{ + IPv4: iap("100.64.0.2"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + node2: Node{ + IPv4: iap("100.64.0.1"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.88.8.0/24"), + }, + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + { + SrcIPs: []string{"10.99.9.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.88.8.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + want: true, + }, + { + name: "subnet-to-subnet-regular-node-excluded-3157", + node1: Node{ + IPv4: iap("100.64.0.3"), + }, + node2: Node{ + IPv4: iap("100.64.0.2"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + want: false, + }, + { + name: "subnet-to-subnet-unrelated-router-excluded-3157", + node1: Node{ + IPv4: iap("100.64.0.3"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("172.16.0.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("172.16.0.0/24"), + }, + }, + node2: Node{ + IPv4: iap("100.64.0.2"), + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + ApprovedRoutes: []netip.Prefix{ + netip.MustParsePrefix("10.99.9.0/24"), + }, + }, + rules: []tailcfg.FilterRule{ + { + SrcIPs: []string{"10.88.8.0/24"}, + DstPorts: []tailcfg.NetPortRange{ + {IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny}, + }, + }, + }, + want: false, + }, } for _, tt := range tests {