diff --git a/hscontrol/policy/policy_autoapprove_test.go b/hscontrol/policy/policy_autoapprove_test.go index 68266645..ed3fa980 100644 --- a/hscontrol/policy/policy_autoapprove_test.go +++ b/hscontrol/policy/policy_autoapprove_test.go @@ -19,11 +19,11 @@ import ( func TestApproveRoutesWithPolicy_NeverRemovesApprovedRoutes(t *testing.T) { user1 := types.User{ Model: gorm.Model{ID: 1}, - Name: "testuser@", + Name: "testuser", } user2 := types.User{ Model: gorm.Model{ID: 2}, - Name: "otheruser@", + Name: "otheruser", } users := []types.User{user1, user2} diff --git a/hscontrol/policy/policyutil/reduce_test.go b/hscontrol/policy/policyutil/reduce_test.go index aa02ac07..d21e3e9d 100644 --- a/hscontrol/policy/policyutil/reduce_test.go +++ b/hscontrol/policy/policyutil/reduce_test.go @@ -9,7 +9,6 @@ import ( "github.com/google/go-cmp/cmp" "github.com/juanfont/headscale/hscontrol/policy" "github.com/juanfont/headscale/hscontrol/policy/policyutil" - v2 "github.com/juanfont/headscale/hscontrol/policy/v2" "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" @@ -206,21 +205,14 @@ func TestReduceFilterRules(t *testing.T) { }, }, want: []tailcfg.FilterRule{ - // Merged: Both ACL rules combined (same SrcIPs and IPProto) + // Merged: Both ACL rules combined (same SrcIPs) { SrcIPs: []string{ - "100.64.0.1/32", - "100.64.0.2/32", - "fd7a:115c:a1e0::1/128", - "fd7a:115c:a1e0::2/128", + "100.64.0.1-100.64.0.2", }, DstPorts: []tailcfg.NetPortRange{ { - IP: "100.64.0.1/32", - Ports: tailcfg.PortRangeAny, - }, - { - IP: "fd7a:115c:a1e0::1/128", + IP: "100.64.0.1", Ports: tailcfg.PortRangeAny, }, { @@ -228,7 +220,6 @@ func TestReduceFilterRules(t *testing.T) { Ports: tailcfg.PortRangeAny, }, }, - IPProto: []int{v2.ProtocolTCP, v2.ProtocolUDP, v2.ProtocolICMP, v2.ProtocolIPv6ICMP}, }, }, }, @@ -356,18 +347,13 @@ func TestReduceFilterRules(t *testing.T) { // autogroup:internet does NOT generate packet filters - it's handled // by exit node routing via AllowedIPs, not by packet filtering. { - SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"}, + SrcIPs: []string{"100.64.0.1-100.64.0.2"}, DstPorts: []tailcfg.NetPortRange{ { - IP: "100.64.0.100/32", - Ports: tailcfg.PortRangeAny, - }, - { - IP: "fd7a:115c:a1e0::100/128", + IP: "100.64.0.100", Ports: tailcfg.PortRangeAny, }, }, - IPProto: []int{v2.ProtocolTCP, v2.ProtocolUDP, v2.ProtocolICMP, v2.ProtocolIPv6ICMP}, }, }, }, @@ -459,16 +445,12 @@ func TestReduceFilterRules(t *testing.T) { }, }, want: []tailcfg.FilterRule{ - // Merged: Both ACL rules combined (same SrcIPs and IPProto) + // Merged: Both ACL rules combined (same SrcIPs) { - SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"}, + SrcIPs: []string{"100.64.0.1-100.64.0.2"}, DstPorts: []tailcfg.NetPortRange{ { - IP: "100.64.0.100/32", - Ports: tailcfg.PortRangeAny, - }, - { - IP: "fd7a:115c:a1e0::100/128", + IP: "100.64.0.100", Ports: tailcfg.PortRangeAny, }, {IP: "0.0.0.0/5", Ports: tailcfg.PortRangeAny}, @@ -502,7 +484,6 @@ func TestReduceFilterRules(t *testing.T) { {IP: "200.0.0.0/5", Ports: tailcfg.PortRangeAny}, {IP: "208.0.0.0/4", Ports: tailcfg.PortRangeAny}, }, - IPProto: []int{v2.ProtocolTCP, v2.ProtocolUDP, v2.ProtocolICMP, v2.ProtocolIPv6ICMP}, }, }, }, @@ -566,16 +547,12 @@ func TestReduceFilterRules(t *testing.T) { }, }, want: []tailcfg.FilterRule{ - // Merged: Both ACL rules combined (same SrcIPs and IPProto) + // Merged: Both ACL rules combined (same SrcIPs) { - SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"}, + SrcIPs: []string{"100.64.0.1-100.64.0.2"}, DstPorts: []tailcfg.NetPortRange{ { - IP: "100.64.0.100/32", - Ports: tailcfg.PortRangeAny, - }, - { - IP: "fd7a:115c:a1e0::100/128", + IP: "100.64.0.100", Ports: tailcfg.PortRangeAny, }, { @@ -587,7 +564,6 @@ func TestReduceFilterRules(t *testing.T) { Ports: tailcfg.PortRangeAny, }, }, - IPProto: []int{v2.ProtocolTCP, v2.ProtocolUDP, v2.ProtocolICMP, v2.ProtocolIPv6ICMP}, }, }, }, @@ -651,16 +627,12 @@ func TestReduceFilterRules(t *testing.T) { }, }, want: []tailcfg.FilterRule{ - // Merged: Both ACL rules combined (same SrcIPs and IPProto) + // Merged: Both ACL rules combined (same SrcIPs) { - SrcIPs: []string{"100.64.0.1/32", "100.64.0.2/32", "fd7a:115c:a1e0::1/128", "fd7a:115c:a1e0::2/128"}, + SrcIPs: []string{"100.64.0.1-100.64.0.2"}, DstPorts: []tailcfg.NetPortRange{ { - IP: "100.64.0.100/32", - Ports: tailcfg.PortRangeAny, - }, - { - IP: "fd7a:115c:a1e0::100/128", + IP: "100.64.0.100", Ports: tailcfg.PortRangeAny, }, { @@ -672,7 +644,6 @@ func TestReduceFilterRules(t *testing.T) { Ports: tailcfg.PortRangeAny, }, }, - IPProto: []int{v2.ProtocolTCP, v2.ProtocolUDP, v2.ProtocolICMP, v2.ProtocolIPv6ICMP}, }, }, }, @@ -725,22 +696,17 @@ func TestReduceFilterRules(t *testing.T) { }, want: []tailcfg.FilterRule{ { - SrcIPs: []string{"100.64.0.1/32", "fd7a:115c:a1e0::1/128"}, + SrcIPs: []string{"100.64.0.1"}, DstPorts: []tailcfg.NetPortRange{ { - IP: "100.64.0.100/32", + IP: "100.64.0.100", Ports: tailcfg.PortRangeAny, }, { - IP: "fd7a:115c:a1e0::100/128", - Ports: tailcfg.PortRangeAny, - }, - { - IP: "172.16.0.21/32", + IP: "172.16.0.21", Ports: tailcfg.PortRangeAny, }, }, - IPProto: []int{v2.ProtocolTCP, v2.ProtocolUDP, v2.ProtocolICMP, v2.ProtocolIPv6ICMP}, }, }, }, diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go index aa2d5355..3d366334 100644 --- a/hscontrol/policy/v2/filter.go +++ b/hscontrol/policy/v2/filter.go @@ -45,7 +45,7 @@ func (pol *Policy) compileFilterRules( log.Trace().Caller().Err(err).Msgf("resolving source ips") } - if srcIPs == nil || len(srcIPs.Prefixes()) == 0 { + if srcIPs.Empty() { continue } @@ -54,7 +54,7 @@ func (pol *Policy) compileFilterRules( if len(destPorts) > 0 { rules = append(rules, tailcfg.FilterRule{ - SrcIPs: ipSetToPrefixStringList(srcIPs), + SrcIPs: srcIPs.Strings(), DstPorts: destPorts, IPProto: ipp.Protocol.toIANAProtocolNumbers(), }) @@ -77,7 +77,7 @@ func (pol *Policy) compileFilterRules( } rules = append(rules, tailcfg.FilterRule{ - SrcIPs: ipSetToPrefixStringList(srcIPs), + SrcIPs: srcIPs.Strings(), CapGrant: capGrants, }) } @@ -131,6 +131,10 @@ func (pol *Policy) destinationsToNetPortRange( IP: pref.String(), Ports: port, } + // Drop the prefix bits if its a single IP. + if pref.IsSingleIP() { + pr.IP = pref.Addr().String() + } ret = append(ret, pr) } } @@ -197,7 +201,7 @@ func (pol *Policy) compileGrantWithAutogroupSelf( var rules []tailcfg.FilterRule - var resolvedSrcIPs []*netipx.IPSet + var resolvedSrcs []ResolvedAddresses for _, src := range grant.Sources { if ag, ok := src.(*AutoGroup); ok && ag.Is(AutoGroupSelf) { @@ -210,11 +214,11 @@ func (pol *Policy) compileGrantWithAutogroupSelf( } if ips != nil { - resolvedSrcIPs = append(resolvedSrcIPs, ips) + resolvedSrcs = append(resolvedSrcs, ips) } } - if len(resolvedSrcIPs) == 0 { + if len(resolvedSrcs) == 0 { return rules, nil } @@ -235,7 +239,7 @@ func (pol *Policy) compileGrantWithAutogroupSelf( // Filter sources to only same-user untagged devices var srcIPs netipx.IPSetBuilder - for _, ips := range resolvedSrcIPs { + for _, ips := range resolvedSrcs { for _, n := range sameUserNodes { // Check if any of this node's IPs are in the source set if slices.ContainsFunc(n.IPs(), ips.Contains) { @@ -244,12 +248,12 @@ func (pol *Policy) compileGrantWithAutogroupSelf( } } - srcSet, err := srcIPs.IPSet() + srcResolved, err := newResolved(&srcIPs) if err != nil { return nil, err } - if srcSet != nil && len(srcSet.Prefixes()) > 0 { + if !srcResolved.Empty() { var destPorts []tailcfg.NetPortRange for _, n := range sameUserNodes { @@ -265,7 +269,7 @@ func (pol *Policy) compileGrantWithAutogroupSelf( if len(destPorts) > 0 { rules = append(rules, tailcfg.FilterRule{ - SrcIPs: ipSetToPrefixStringList(srcSet), + SrcIPs: srcResolved.Strings(), DstPorts: destPorts, IPProto: ipp.Protocol.toIANAProtocolNumbers(), }) @@ -277,21 +281,23 @@ func (pol *Policy) compileGrantWithAutogroupSelf( if len(otherDests) > 0 { var srcIPs netipx.IPSetBuilder - for _, ips := range resolvedSrcIPs { - srcIPs.AddSet(ips) + for _, ips := range resolvedSrcs { + for _, pref := range ips.Prefixes() { + srcIPs.AddPrefix(pref) + } } - srcSet, err := srcIPs.IPSet() + srcResolved, err := newResolved(&srcIPs) if err != nil { return nil, err } - if srcSet != nil && len(srcSet.Prefixes()) > 0 { + if !srcResolved.Empty() { destPorts := pol.destinationsToNetPortRange(users, nodes, otherDests, ipp.Ports) if len(destPorts) > 0 { rules = append(rules, tailcfg.FilterRule{ - SrcIPs: ipSetToPrefixStringList(srcSet), + SrcIPs: srcResolved.Strings(), DstPorts: destPorts, IPProto: ipp.Protocol.toIANAProtocolNumbers(), }) @@ -474,7 +480,9 @@ func (pol *Policy) compileSSHPolicy( } if ips != nil { - dest.AddSet(ips) + for _, pref := range ips.Prefixes() { + dest.AddPrefix(pref) + } } } @@ -497,7 +505,7 @@ func (pol *Policy) compileSSHPolicy( appendRules(taggedPrincipals, 0, false) } } else { - if principals := ipSetToPrincipals(srcIPs); len(principals) > 0 { + if principals := resolvedAddrsToPrincipals(srcIPs); len(principals) > 0 { rules = append(rules, &tailcfg.SSHRule{ Principals: principals, SSHUsers: baseUserMap, @@ -506,7 +514,7 @@ func (pol *Policy) compileSSHPolicy( }) } } - } else if hasLocalpart && node.InIPSet(srcIPs) { + } else if hasLocalpart && slices.ContainsFunc(node.IPs(), srcIPs.Contains) { // Self-access: source node not in destination set // receives rules scoped to its own user. if node.IsTagged() { @@ -552,6 +560,23 @@ func (pol *Policy) compileSSHPolicy( }, nil } +// resolvedAddrsToPrincipals converts ResolvedAddresses into SSH principals, one per address. +func resolvedAddrsToPrincipals(addrs ResolvedAddresses) []*tailcfg.SSHPrincipal { + if addrs == nil { + return nil + } + + var principals []*tailcfg.SSHPrincipal + + for addr := range addrs.Iter() { + principals = append(principals, &tailcfg.SSHPrincipal{ + NodeIP: addr.String(), + }) + } + + return principals +} + // ipSetToPrincipals converts an IPSet into SSH principals, one per address. func ipSetToPrincipals(ipSet *netipx.IPSet) []*tailcfg.SSHPrincipal { if ipSet == nil { @@ -619,7 +644,7 @@ func resolveLocalparts( // Only includes nodes whose IPs are in the srcIPs set. func groupSourcesByUser( nodes views.Slice[types.NodeView], - srcIPs *netipx.IPSet, + srcIPs ResolvedAddresses, ) ([]uint, map[uint][]*tailcfg.SSHPrincipal, []*tailcfg.SSHPrincipal) { userIPSets := make(map[uint]*netipx.IPSetBuilder) diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go index b6080eae..6805d9da 100644 --- a/hscontrol/policy/v2/filter_test.go +++ b/hscontrol/policy/v2/filter_test.go @@ -9,7 +9,9 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/juanfont/headscale/hscontrol/policy/policyutil" "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/util" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go4.org/netipx" @@ -98,9 +100,8 @@ func TestParsing(t *testing.T) { DstPorts: []tailcfg.NetPortRange{ {IP: "*", Ports: tailcfg.PortRange{First: 22, Last: 22}}, {IP: "*", Ports: tailcfg.PortRange{First: 3389, Last: 3389}}, - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, + {IP: "100.100.100.100", Ports: tailcfg.PortRangeAny}, }, - IPProto: []int{ProtocolTCP, ProtocolUDP, ProtocolICMP, ProtocolIPv6ICMP}, }, }, wantErr: false, @@ -150,23 +151,23 @@ func TestParsing(t *testing.T) { }`, want: []tailcfg.FilterRule{ { - SrcIPs: []string{"100.64.0.0/10", "fd7a:115c:a1e0::/48"}, + SrcIPs: []string{"100.64.0.0-100.115.91.255", "100.115.94.0-100.127.255.255", "fd7a:115c:a1e0::/48"}, DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, + {IP: "100.100.100.100", Ports: tailcfg.PortRangeAny}, }, IPProto: []int{ProtocolTCP}, }, { - SrcIPs: []string{"100.64.0.0/10", "fd7a:115c:a1e0::/48"}, + SrcIPs: []string{"100.64.0.0-100.115.91.255", "100.115.94.0-100.127.255.255", "fd7a:115c:a1e0::/48"}, DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRange{First: 53, Last: 53}}, + {IP: "100.100.100.100", Ports: tailcfg.PortRange{First: 53, Last: 53}}, }, IPProto: []int{ProtocolUDP}, }, { - SrcIPs: []string{"100.64.0.0/10", "fd7a:115c:a1e0::/48"}, + SrcIPs: []string{"100.64.0.0-100.115.91.255", "100.115.94.0-100.127.255.255", "fd7a:115c:a1e0::/48"}, DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, + {IP: "100.100.100.100", Ports: tailcfg.PortRangeAny}, }, // proto:icmp only includes ICMP (1), not ICMPv6 (58) IPProto: []int{ProtocolICMP}, @@ -199,11 +200,10 @@ func TestParsing(t *testing.T) { `, want: []tailcfg.FilterRule{ { - SrcIPs: []string{"100.64.0.0/10", "fd7a:115c:a1e0::/48"}, + SrcIPs: []string{"100.64.0.0-100.115.91.255", "100.115.94.0-100.127.255.255", "fd7a:115c:a1e0::/48"}, DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, + {IP: "100.100.100.100", Ports: tailcfg.PortRangeAny}, }, - IPProto: []int{ProtocolTCP, ProtocolUDP, ProtocolICMP, ProtocolIPv6ICMP}, }, }, wantErr: false, @@ -236,11 +236,10 @@ func TestParsing(t *testing.T) { SrcIPs: []string{"100.100.101.0/24"}, DstPorts: []tailcfg.NetPortRange{ { - IP: "100.100.100.100/32", + IP: "100.100.100.100", Ports: tailcfg.PortRange{First: 5400, Last: 5500}, }, }, - IPProto: []int{ProtocolTCP, ProtocolUDP, ProtocolICMP, ProtocolIPv6ICMP}, }, }, wantErr: false, @@ -276,11 +275,10 @@ func TestParsing(t *testing.T) { `, want: []tailcfg.FilterRule{ { - SrcIPs: []string{"200.200.200.200/32"}, + SrcIPs: []string{"200.200.200.200"}, DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, + {IP: "100.100.100.100", Ports: tailcfg.PortRangeAny}, }, - IPProto: []int{ProtocolTCP, ProtocolUDP, ProtocolICMP, ProtocolIPv6ICMP}, }, }, wantErr: false, @@ -310,11 +308,10 @@ func TestParsing(t *testing.T) { `, want: []tailcfg.FilterRule{ { - SrcIPs: []string{"200.200.200.200/32"}, + SrcIPs: []string{"200.200.200.200"}, DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, + {IP: "100.100.100.100", Ports: tailcfg.PortRangeAny}, }, - IPProto: []int{ProtocolTCP, ProtocolUDP, ProtocolICMP, ProtocolIPv6ICMP}, }, }, wantErr: false, @@ -344,11 +341,10 @@ func TestParsing(t *testing.T) { `, want: []tailcfg.FilterRule{ { - SrcIPs: []string{"100.64.0.0/10", "fd7a:115c:a1e0::/48"}, + SrcIPs: []string{"100.64.0.0-100.115.91.255", "100.115.94.0-100.127.255.255", "fd7a:115c:a1e0::/48"}, DstPorts: []tailcfg.NetPortRange{ - {IP: "100.100.100.100/32", Ports: tailcfg.PortRangeAny}, + {IP: "100.100.100.100", Ports: tailcfg.PortRangeAny}, }, - IPProto: []int{ProtocolTCP, ProtocolUDP, ProtocolICMP, ProtocolIPv6ICMP}, }, }, wantErr: false, @@ -1366,16 +1362,20 @@ func TestCompileFilterRulesForNodeWithAutogroupSelf(t *testing.T) { addr := netip.MustParseAddr(expectedIP) - for _, prefix := range rule.SrcIPs { - pref := netip.MustParsePrefix(prefix) - if pref.Contains(addr) { + for _, srcEntry := range rule.SrcIPs { + ipSet, err := util.ParseIPSet(srcEntry, nil) + if err != nil { + t.Fatalf("failed to parse SrcIP %q: %v", srcEntry, err) + } + + if ipSet.Contains(addr) { found = true break } } if !found { - t.Errorf("expected source IP %s to be covered by generated prefixes %v", expectedIP, rule.SrcIPs) + t.Errorf("expected source IP %s to be covered by generated SrcIPs %v", expectedIP, rule.SrcIPs) } } @@ -1384,15 +1384,19 @@ func TestCompileFilterRulesForNodeWithAutogroupSelf(t *testing.T) { for _, excludedIP := range excludedSourceIPs { addr := netip.MustParseAddr(excludedIP) - for _, prefix := range rule.SrcIPs { - pref := netip.MustParsePrefix(prefix) - if pref.Contains(addr) { - t.Errorf("SECURITY VIOLATION: source IP %s should not be included but found in prefix %s", excludedIP, prefix) + for _, srcEntry := range rule.SrcIPs { + ipSet, err := util.ParseIPSet(srcEntry, nil) + if err != nil { + t.Fatalf("failed to parse SrcIP %q: %v", srcEntry, err) + } + + if ipSet.Contains(addr) { + t.Errorf("SECURITY VIOLATION: source IP %s should not be included but found in SrcIP %s", excludedIP, srcEntry) } } } - expectedDestIPs := []string{"100.64.0.1/32", "100.64.0.2/32"} + expectedDestIPs := []string{"100.64.0.1", "100.64.0.2"} actualDestIPs := make([]string, 0, len(rule.DstPorts)) for _, dst := range rule.DstPorts { @@ -1400,7 +1404,21 @@ func TestCompileFilterRulesForNodeWithAutogroupSelf(t *testing.T) { } for _, expectedIP := range expectedDestIPs { - found := slices.Contains(actualDestIPs, expectedIP) + addr := netip.MustParseAddr(expectedIP) + + found := false + + for _, destIP := range actualDestIPs { + ipSet, err := util.ParseIPSet(destIP, nil) + if err != nil { + t.Fatalf("failed to parse DstPort IP %q: %v", destIP, err) + } + + if ipSet.Contains(addr) { + found = true + break + } + } if !found { t.Errorf("expected destination IP %s to be included, got: %v", expectedIP, actualDestIPs) @@ -1408,18 +1426,26 @@ func TestCompileFilterRulesForNodeWithAutogroupSelf(t *testing.T) { } // Verify that other users' devices and tagged devices are not in destinations - excludedDestIPs := []string{"100.64.0.3/32", "100.64.0.4/32", "100.64.0.5/32", "100.64.0.6/32"} + excludedDestIPs := []string{"100.64.0.3", "100.64.0.4", "100.64.0.5", "100.64.0.6"} for _, excludedIP := range excludedDestIPs { - for _, actualIP := range actualDestIPs { - if actualIP == excludedIP { - t.Errorf("SECURITY: destination IP %s should not be included but found in destinations", excludedIP) + addr := netip.MustParseAddr(excludedIP) + + for _, destIP := range actualDestIPs { + ipSet, err := util.ParseIPSet(destIP, nil) + if err != nil { + t.Fatalf("failed to parse DstPort IP %q: %v", destIP, err) + } + + if ipSet.Contains(addr) { + t.Errorf("SECURITY: destination IP %s should not be included but found in dest %s", excludedIP, destIP) } } } } -// TestTagUserMutualExclusivity tests that user-owned nodes and tagged nodes -// are treated as separate identity classes and cannot inadvertently access each other. +// TestTagUserMutualExclusivity tests that without explicit cross-identity ACL +// rules, user-owned nodes and tagged nodes are isolated from each other. +// It also verifies that tag-to-tag rules work correctly. func TestTagUserMutualExclusivity(t *testing.T) { users := types.Users{ {Model: gorm.Model{ID: 1}, Name: "user1"}, @@ -1449,21 +1475,13 @@ func TestTagUserMutualExclusivity(t *testing.T) { }, } - policy := &Policy{ + pol := &Policy{ TagOwners: TagOwners{ Tag("tag:server"): Owners{new(Username("user1@"))}, Tag("tag:database"): Owners{new(Username("user2@"))}, }, ACLs: []ACL{ - // Rule 1: user1 (user-owned) should NOT be able to reach tagged nodes - { - Action: "accept", - Sources: []Alias{up("user1@")}, - Destinations: []AliasWithPorts{ - aliasWithPorts(tp("tag:server"), tailcfg.PortRangeAny), - }, - }, - // Rule 2: tag:server should be able to reach tag:database + // Only tag-to-tag rule, no user-to-tag rules. { Action: "accept", Sources: []Alias{tp("tag:server")}, @@ -1474,63 +1492,143 @@ func TestTagUserMutualExclusivity(t *testing.T) { }, } - err := policy.validate() - if err != nil { - t.Fatalf("policy validation failed: %v", err) - } + err := pol.validate() + require.NoError(t, err) - // Test user1's user-owned node (100.64.0.1) + // User1's user-owned node should have no rules reaching tagged nodes + // since there is no explicit user→tag ACL rule. ReduceFilterRules + // filters compiled rules to only those where the node is a destination, + // matching the production pipeline in filterForNodeLocked. userNode := nodes[0].View() - userRules, err := policy.compileFilterRulesForNode(users, userNode, nodes.ViewSlice()) - if err != nil { - t.Fatalf("unexpected error for user node: %v", err) - } + compiled, err := pol.compileFilterRulesForNode(users, userNode, nodes.ViewSlice()) + require.NoError(t, err) + + userRules := policyutil.ReduceFilterRules(userNode, compiled) - // User1's user-owned node should NOT reach tag:server (100.64.0.10) - // because user1@ as a source only matches user1's user-owned devices, NOT tagged devices for _, rule := range userRules { for _, dst := range rule.DstPorts { - if dst.IP == "100.64.0.10" { - t.Errorf("SECURITY: user-owned node should NOT reach tagged node (got dest %s in rule)", dst.IP) + ipSet, parseErr := util.ParseIPSet(dst.IP, nil) + require.NoError(t, parseErr) + + if ipSet.Contains(netip.MustParseAddr("100.64.0.10")) { + t.Errorf("user-owned node should not reach tag:server without explicit grant (got dest %s)", dst.IP) + } + + if ipSet.Contains(netip.MustParseAddr("100.64.0.11")) { + t.Errorf("user-owned node should not reach tag:database without explicit grant (got dest %s)", dst.IP) } } } - // Test tag:server node (100.64.0.10) - // compileFilterRulesForNode returns rules for what the node can ACCESS (as source) - taggedNode := nodes[2].View() + // Tag:database should receive the tag:server → tag:database rule after reduction. + dbNode := nodes[3].View() - taggedRules, err := policy.compileFilterRulesForNode(users, taggedNode, nodes.ViewSlice()) - if err != nil { - t.Fatalf("unexpected error for tagged node: %v", err) - } + compiled, err = pol.compileFilterRulesForNode(users, dbNode, nodes.ViewSlice()) + require.NoError(t, err) - // Tag:server (as source) should be able to reach tag:database (100.64.0.11) - // Check destinations in the rules for this node - foundDatabaseDest := false + dbRules := policyutil.ReduceFilterRules(dbNode, compiled) - for _, rule := range taggedRules { - // Check if this rule applies to tag:server as source - if !slices.Contains(rule.SrcIPs, "100.64.0.10/32") { - continue - } + foundServerSrc := false - // Check if tag:database is in destinations - for _, dst := range rule.DstPorts { - if dst.IP == "100.64.0.11/32" { - foundDatabaseDest = true + for _, rule := range dbRules { + for _, srcEntry := range rule.SrcIPs { + ipSet, parseErr := util.ParseIPSet(srcEntry, nil) + require.NoError(t, parseErr) + + if ipSet.Contains(netip.MustParseAddr("100.64.0.10")) { + foundServerSrc = true break } } + } - if foundDatabaseDest { - break + assert.True(t, foundServerSrc, "tag:database should accept traffic from tag:server") +} + +// TestUserToTagCrossIdentityGrant tests that an explicit ACL rule granting +// user-owned nodes access to tagged nodes works correctly. The tags-as-identity +// model separates identity classes, but explicit ACL grants across classes +// are valid and should produce filter rules. +func TestUserToTagCrossIdentityGrant(t *testing.T) { + users := types.Users{ + {Model: gorm.Model{ID: 1}, Name: "user1"}, + {Model: gorm.Model{ID: 2}, Name: "user2"}, + } + + nodes := types.Nodes{ + { + User: new(users[0]), + IPv4: ap("100.64.0.1"), + }, + { + User: new(users[1]), + IPv4: ap("100.64.0.2"), + }, + { + User: &users[0], // "created by" tracking + IPv4: ap("100.64.0.10"), + Tags: []string{"tag:server"}, + }, + } + + pol := &Policy{ + TagOwners: TagOwners{ + Tag("tag:server"): Owners{new(Username("user1@"))}, + }, + ACLs: []ACL{ + // Explicit cross-identity grant: user1's devices can reach tag:server. + { + Action: "accept", + Sources: []Alias{up("user1@")}, + Destinations: []AliasWithPorts{ + aliasWithPorts(tp("tag:server"), tailcfg.PortRangeAny), + }, + }, + }, + } + + err := pol.validate() + require.NoError(t, err) + + // Compile and reduce rules for the tag:server node — it is the + // destination, so after ReduceFilterRules, the filter should include + // user1's IP as source. + taggedNode := nodes[2].View() + + compiled, err := pol.compileFilterRulesForNode(users, taggedNode, nodes.ViewSlice()) + require.NoError(t, err) + + rules := policyutil.ReduceFilterRules(taggedNode, compiled) + + // user1's IP should appear as a source that can reach tag:server. + foundUser1Src := false + + for _, rule := range rules { + for _, srcEntry := range rule.SrcIPs { + ipSet, parseErr := util.ParseIPSet(srcEntry, nil) + require.NoError(t, parseErr) + + if ipSet.Contains(netip.MustParseAddr("100.64.0.1")) { + foundUser1Src = true + break + } } } - if !foundDatabaseDest { - t.Errorf("tag:server should reach tag:database but didn't find 100.64.0.11 in destinations") + assert.True(t, foundUser1Src, + "explicit user1@ -> tag:server ACL should allow user1 devices to reach tagged node") + + // user2 should NOT appear as a source. + for _, rule := range rules { + for _, srcEntry := range rule.SrcIPs { + ipSet, parseErr := util.ParseIPSet(srcEntry, nil) + require.NoError(t, parseErr) + + if ipSet.Contains(netip.MustParseAddr("100.64.0.2")) { + t.Errorf("user2 should not reach tag:server (found in SrcIP %s)", srcEntry) + } + } } } @@ -1730,9 +1828,11 @@ func TestAutogroupSelfWithSpecificUserSource(t *testing.T) { found := false addr := netip.MustParseAddr(expectedIP) - for _, prefix := range rules[0].SrcIPs { - pref := netip.MustParsePrefix(prefix) - if pref.Contains(addr) { + for _, srcEntry := range rules[0].SrcIPs { + ipSet, err := util.ParseIPSet(srcEntry, nil) + require.NoError(t, err, "failed to parse SrcIP %q", srcEntry) + + if ipSet.Contains(addr) { found = true break } @@ -1802,9 +1902,11 @@ func TestAutogroupSelfWithGroupSource(t *testing.T) { found := false addr := netip.MustParseAddr(expectedIP) - for _, prefix := range rules[0].SrcIPs { - pref := netip.MustParsePrefix(prefix) - if pref.Contains(addr) { + for _, srcEntry := range rules[0].SrcIPs { + ipSet, err := util.ParseIPSet(srcEntry, nil) + require.NoError(t, err, "failed to parse SrcIP %q", srcEntry) + + if ipSet.Contains(addr) { found = true break } @@ -2228,23 +2330,12 @@ func TestAutogroupSelfWithNonExistentUserInGroup(t *testing.T) { for _, rule := range rules { for _, dp := range rule.DstPorts { - // DstPort IPs may be bare addresses or CIDR prefixes - pref, err := netip.ParsePrefix(dp.IP) + ipSet, err := util.ParseIPSet(dp.IP, nil) if err != nil { - // Try as bare address - a, err2 := netip.ParseAddr(dp.IP) - if err2 != nil { - continue - } - - if a == addr { - return true - } - continue } - if pref.Contains(addr) { + if ipSet.Contains(addr) { return true } } @@ -2258,21 +2349,12 @@ func TestAutogroupSelfWithNonExistentUserInGroup(t *testing.T) { for _, rule := range rules { for _, srcIP := range rule.SrcIPs { - pref, err := netip.ParsePrefix(srcIP) + ipSet, err := util.ParseIPSet(srcIP, nil) if err != nil { - a, err2 := netip.ParseAddr(srcIP) - if err2 != nil { - continue - } - - if a == addr { - return true - } - continue } - if pref.Contains(addr) { + if ipSet.Contains(addr) { return true } } @@ -2982,19 +3064,20 @@ func TestGroupSourcesByUser(t *testing.T) { } // Build an IPSet that includes all node IPs - allIPs := func() *netipx.IPSet { + allIPs := func() ResolvedAddresses { var b netipx.IPSetBuilder b.AddPrefix(netip.MustParsePrefix("100.64.0.0/24")) s, _ := b.IPSet() + r, _ := newResolvedAddresses(s, nil) - return s + return r }() tests := []struct { name string nodes types.Nodes - srcIPs *netipx.IPSet + srcIPs ResolvedAddresses wantUIDs []uint wantUserCount int wantHasTagged bool @@ -3035,13 +3118,14 @@ func TestGroupSourcesByUser(t *testing.T) { { name: "node not in srcIPs excluded", nodes: types.Nodes{&nodeAlice, &nodeBob}, - srcIPs: func() *netipx.IPSet { + srcIPs: func() ResolvedAddresses { var b netipx.IPSetBuilder b.Add(netip.MustParseAddr("100.64.0.1")) // only alice s, _ := b.IPSet() + r, _ := newResolvedAddresses(s, nil) - return s + return r }(), wantUIDs: []uint{1}, wantUserCount: 1, diff --git a/hscontrol/policy/v2/policy.go b/hscontrol/policy/v2/policy.go index 77de20eb..3ef72a16 100644 --- a/hscontrol/policy/v2/policy.go +++ b/hscontrol/policy/v2/policy.go @@ -1198,7 +1198,11 @@ func resolveTagOwners(p *Policy, users types.Users, nodes views.Slice[types.Node case Alias: // If it does not resolve, that means the tag is not associated with any IP addresses. resolved, _ := o.Resolve(p, users, nodes) - ips.AddSet(resolved) + if resolved != nil { + for _, pref := range resolved.Prefixes() { + ips.AddPrefix(pref) + } + } default: // Should never happen - after flattening, all owners should be Alias types diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index a25f4df6..f4aae1e2 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -210,181 +210,268 @@ func loadGrantTestFile(t *testing.T, path string) grantTestFile { // // Impact summary (highest first): // -// SRCIPS_FORMAT - 125 tests: Fix SrcIPs to use CGNAT split ranges -// CAPGRANT_COMPILATION - 41 tests: Implement app->CapGrant FilterRule compilation -// ERROR_VALIDATION_GAP - 14 tests: Implement missing grant validation rules -// CAPGRANT_AND_SRCIPS_FORMAT - 9 tests: Both CapGrant compilation + SrcIPs format -// VIA_AND_SRCIPS_FORMAT - 4 tests: Via route compilation + SrcIPs format -// AUTOGROUP_DANGER_ALL - 3 tests: Implement autogroup:danger-all support -// VALIDATION_STRICTNESS - 2 tests: headscale too strict (rejects what Tailscale accepts) +// CAPGRANT_COMPILATION - 49 tests: Implement app->CapGrant FilterRule compilation +// ERROR_VALIDATION_GAP - 23 tests: Implement missing grant validation rules +// MISSING_IPV6_ADDRS - 90 tests: Include IPv6 for identity-based alias resolution +// CAPGRANT_COMPILATION_AND_SRCIPS - 11 tests: Both CapGrant compilation + SrcIPs format +// SUBNET_ROUTE_FILTER_RULES - 10 tests: Generate filter rules for subnet-routed CIDRs +// VIA_COMPILATION_AND_SRCIPS_FORMAT - 7 tests: Via route compilation + SrcIPs format +// AUTOGROUP_SELF_CIDR_FORMAT - 4 tests: DstPorts IPs get /32 or /128 suffix for autogroup:self +// VIA_COMPILATION - 3 tests: Via route compilation +// AUTOGROUP_DANGER_ALL - 3 tests: Implement autogroup:danger-all support +// USER_PASSKEY_WILDCARD - 2 tests: user:*@passkey wildcard pattern unresolvable +// VALIDATION_STRICTNESS - 2 tests: headscale too strict (rejects what Tailscale accepts) +// RAW_IPV6_ADDR_EXPANSION - 2 tests: Raw fd7a: IPv6 src/dst expanded to include IPv4 +// SRCIPS_WILDCARD_NODE_DEDUP - 1 test: Wildcard+specific source node IP deduplication // -// Total: 193 tests skipped, 19 tests expected to pass. +// Total: 207 tests skipped, 30 tests expected to pass. var grantSkipReasons = map[string]string{ // ======================================================================== - // SRCIPS_FORMAT (125 tests) + // MISSING_IPV6_ADDRS (90 tests) // - // TODO: Implement CGNAT split range generation for SrcIPs. + // TODO: Include IPv6 addresses when resolving identity-based aliases in + // filter rules. // - // headscale currently generates ["100.64.0.0/10", "fd7a:115c:a1e0::/48"] - // for wildcard source matches. Tailscale generates split CGNAT ranges that - // exclude the ChromeOS VM range 100.115.92.0/23, and includes advertised - // subnet routes (e.g., "10.33.0.0/16") in the SrcIPs list. + // When compiling filter rules, headscale resolves identity-based aliases + // (tags, groups, users, autogroups) to only IPv4 addresses. Tailscale + // includes both IPv4 AND the corresponding fd7a:115c:a1e0:: IPv6 address + // in SrcIPs and DstPorts. // - // Additionally, headscale uses CIDR notation for host IPs in DstPorts - // (e.g., "100.108.74.26/32") while Tailscale uses bare IPs - // (e.g., "100.108.74.26"). + // IMPORTANT: This only applies to IDENTITY-based aliases. Address-based + // aliases (raw IPs like "100.108.74.26", host aliases like "webserver") + // correctly resolve to IPv4-only in both Tailscale and headscale. // - // Fixing SrcIPs generation and DstPorts IP format would resolve all 125 - // tests in this category. + // The rule (verified 100% across 790 node-IP references in test data): + // Identity aliases (tag:X, group:X, user@Y, autogroup:X, *) + // → include BOTH node.IPv4 and node.IPv6 + // Address aliases (raw IPv4/IPv6, host alias names) + // → include ONLY the literal/resolved IP + // + // Example diff (tag:client src → tagged-server node): + // SrcIPs: headscale=["100.83.200.69"] + // SrcIPs: tailscale=["100.83.200.69", "fd7a:115c:a1e0::c537:c845"] + // + // Fix: When resolving an identity alias (tag, group, user, autogroup, *) + // to IPs, include both node.IPv4 and node.IPv6 addresses. When resolving + // an address alias (raw IP, host alias), keep only the literal IP. // ======================================================================== - // J-series: Protocol-specific IP grants - "GRANT-J1": "SRCIPS_FORMAT", - "GRANT-J2": "SRCIPS_FORMAT", - "GRANT-J3": "SRCIPS_FORMAT", - "GRANT-J4": "SRCIPS_FORMAT", - "GRANT-J5": "SRCIPS_FORMAT", - "GRANT-J6": "SRCIPS_FORMAT", + // J-series: Protocol-specific IP grants with identity src/dst + "GRANT-J1": "MISSING_IPV6_ADDRS", + "GRANT-J2": "MISSING_IPV6_ADDRS", + "GRANT-J3": "MISSING_IPV6_ADDRS", + "GRANT-J4": "MISSING_IPV6_ADDRS", + "GRANT-J5": "MISSING_IPV6_ADDRS", + "GRANT-J6": "MISSING_IPV6_ADDRS", - // K-series: Various IP grant patterns - "GRANT-K1": "SRCIPS_FORMAT", - "GRANT-K2": "SRCIPS_FORMAT", - "GRANT-K4": "SRCIPS_FORMAT", - "GRANT-K14": "SRCIPS_FORMAT", - "GRANT-K15": "SRCIPS_FORMAT", - "GRANT-K16": "SRCIPS_FORMAT", - "GRANT-K17": "SRCIPS_FORMAT", - "GRANT-K20": "SRCIPS_FORMAT", - "GRANT-K21": "SRCIPS_FORMAT", - "GRANT-K22": "SRCIPS_FORMAT", - "GRANT-K26": "SRCIPS_FORMAT", - - // P01-series: Wildcard and basic IP grants - "GRANT-P01_1": "SRCIPS_FORMAT", - "GRANT-P01_2": "SRCIPS_FORMAT", - "GRANT-P01_3": "SRCIPS_FORMAT", - "GRANT-P01_4": "SRCIPS_FORMAT", + // K-series: Various IP grant patterns with identity aliases + "GRANT-K4": "MISSING_IPV6_ADDRS", + "GRANT-K16": "MISSING_IPV6_ADDRS", + "GRANT-K17": "MISSING_IPV6_ADDRS", + "GRANT-K22": "MISSING_IPV6_ADDRS", + "GRANT-K26": "MISSING_IPV6_ADDRS", // P02-series: Source targeting (user, group, tag) - "GRANT-P02_1": "SRCIPS_FORMAT", - "GRANT-P02_2": "SRCIPS_FORMAT", - "GRANT-P02_3": "SRCIPS_FORMAT", - "GRANT-P02_4": "SRCIPS_FORMAT", - "GRANT-P02_5_CORRECT": "SRCIPS_FORMAT", - "GRANT-P02_5_NAIVE": "SRCIPS_FORMAT", + "GRANT-P02_1": "MISSING_IPV6_ADDRS", + "GRANT-P02_2": "MISSING_IPV6_ADDRS", + "GRANT-P02_3": "MISSING_IPV6_ADDRS", + "GRANT-P02_4": "MISSING_IPV6_ADDRS", + "GRANT-P02_5_CORRECT": "MISSING_IPV6_ADDRS", + "GRANT-P02_5_NAIVE": "MISSING_IPV6_ADDRS", // P03-series: Destination targeting - "GRANT-P03_1": "SRCIPS_FORMAT", - "GRANT-P03_2": "SRCIPS_FORMAT", - "GRANT-P03_3": "SRCIPS_FORMAT", - "GRANT-P03_4": "SRCIPS_FORMAT", + "GRANT-P03_1": "MISSING_IPV6_ADDRS", + "GRANT-P03_2": "MISSING_IPV6_ADDRS", + "GRANT-P03_3": "MISSING_IPV6_ADDRS", + "GRANT-P03_4": "MISSING_IPV6_ADDRS", // P04-series: autogroup:member grants - "GRANT-P04_1": "SRCIPS_FORMAT", - "GRANT-P04_2": "SRCIPS_FORMAT", - - // P05-series: Tag-to-tag grants - "GRANT-P05_1": "SRCIPS_FORMAT", - "GRANT-P05_2": "SRCIPS_FORMAT", - "GRANT-P05_3": "SRCIPS_FORMAT", + "GRANT-P04_1": "MISSING_IPV6_ADDRS", + "GRANT-P04_2": "MISSING_IPV6_ADDRS", // P06-series: IP protocol grants - "GRANT-P06_1": "SRCIPS_FORMAT", - "GRANT-P06_2": "SRCIPS_FORMAT", - "GRANT-P06_3": "SRCIPS_FORMAT", - "GRANT-P06_4": "SRCIPS_FORMAT", - "GRANT-P06_5": "SRCIPS_FORMAT", - "GRANT-P06_6": "SRCIPS_FORMAT", - "GRANT-P06_7": "SRCIPS_FORMAT", + "GRANT-P06_1": "MISSING_IPV6_ADDRS", + "GRANT-P06_2": "MISSING_IPV6_ADDRS", + "GRANT-P06_3": "MISSING_IPV6_ADDRS", + "GRANT-P06_4": "MISSING_IPV6_ADDRS", + "GRANT-P06_5": "MISSING_IPV6_ADDRS", + "GRANT-P06_6": "MISSING_IPV6_ADDRS", + "GRANT-P06_7": "MISSING_IPV6_ADDRS", // P08-series: Multiple grants / rule merging - "GRANT-P08_1": "SRCIPS_FORMAT", - "GRANT-P08_2": "SRCIPS_FORMAT", - "GRANT-P08_4": "SRCIPS_FORMAT", - "GRANT-P08_5": "SRCIPS_FORMAT", - "GRANT-P08_6": "SRCIPS_FORMAT", - "GRANT-P08_7": "SRCIPS_FORMAT", - "GRANT-P08_8": "SRCIPS_FORMAT", + "GRANT-P08_1": "MISSING_IPV6_ADDRS", + "GRANT-P08_2": "MISSING_IPV6_ADDRS", + "GRANT-P08_4": "MISSING_IPV6_ADDRS", + "GRANT-P08_5": "MISSING_IPV6_ADDRS", + "GRANT-P08_6": "MISSING_IPV6_ADDRS", + "GRANT-P08_7": "MISSING_IPV6_ADDRS", // P09-series: ACL-to-grant conversion equivalence tests - "GRANT-P09_1A": "SRCIPS_FORMAT", - "GRANT-P09_1B": "SRCIPS_FORMAT", - "GRANT-P09_1C": "SRCIPS_FORMAT", - "GRANT-P09_1D": "SRCIPS_FORMAT", - "GRANT-P09_1E": "SRCIPS_FORMAT", - "GRANT-P09_2A_CORRECT": "SRCIPS_FORMAT", - "GRANT-P09_2A_NAIVE": "SRCIPS_FORMAT", - "GRANT-P09_2B_CORRECT": "SRCIPS_FORMAT", - "GRANT-P09_2B_NAIVE": "SRCIPS_FORMAT", - "GRANT-P09_2C": "SRCIPS_FORMAT", - "GRANT-P09_3A": "SRCIPS_FORMAT", - "GRANT-P09_3B": "SRCIPS_FORMAT", - "GRANT-P09_3C": "SRCIPS_FORMAT", - "GRANT-P09_4A": "SRCIPS_FORMAT", - "GRANT-P09_4B": "SRCIPS_FORMAT", - "GRANT-P09_4C": "SRCIPS_FORMAT", - "GRANT-P09_4D": "SRCIPS_FORMAT", - "GRANT-P09_4E": "SRCIPS_FORMAT", - "GRANT-P09_4F": "SRCIPS_FORMAT", - "GRANT-P09_4G": "SRCIPS_FORMAT", - "GRANT-P09_5A": "SRCIPS_FORMAT", - "GRANT-P09_5B": "SRCIPS_FORMAT", - "GRANT-P09_5C_NAIVE": "SRCIPS_FORMAT", - "GRANT-P09_6A": "SRCIPS_FORMAT", - "GRANT-P09_6C": "SRCIPS_FORMAT", - "GRANT-P09_6D": "SRCIPS_FORMAT", - "GRANT-P09_7A": "SRCIPS_FORMAT", - "GRANT-P09_7B_NAIVE": "SRCIPS_FORMAT", - "GRANT-P09_7C": "SRCIPS_FORMAT", - "GRANT-P09_7D_NAIVE": "SRCIPS_FORMAT", - "GRANT-P09_8A": "SRCIPS_FORMAT", - "GRANT-P09_8B": "SRCIPS_FORMAT", - "GRANT-P09_8C": "SRCIPS_FORMAT", - "GRANT-P09_9A": "SRCIPS_FORMAT", - "GRANT-P09_9B": "SRCIPS_FORMAT", - "GRANT-P09_9C": "SRCIPS_FORMAT", - "GRANT-P09_10A": "SRCIPS_FORMAT", - "GRANT-P09_10B": "SRCIPS_FORMAT", - "GRANT-P09_10C": "SRCIPS_FORMAT", - "GRANT-P09_10D": "SRCIPS_FORMAT", - "GRANT-P09_11A": "SRCIPS_FORMAT", - "GRANT-P09_11B": "SRCIPS_FORMAT", - "GRANT-P09_11C_NAIVE": "SRCIPS_FORMAT", - "GRANT-P09_11D": "SRCIPS_FORMAT", - "GRANT-P09_12A": "SRCIPS_FORMAT", - "GRANT-P09_12B": "SRCIPS_FORMAT", - "GRANT-P09_13E": "SRCIPS_FORMAT", - "GRANT-P09_13F": "SRCIPS_FORMAT", - "GRANT-P09_13G": "SRCIPS_FORMAT", - "GRANT-P09_14A": "SRCIPS_FORMAT", - "GRANT-P09_14B": "SRCIPS_FORMAT", - "GRANT-P09_14C": "SRCIPS_FORMAT", - "GRANT-P09_14D": "SRCIPS_FORMAT", - "GRANT-P09_14E": "SRCIPS_FORMAT", - "GRANT-P09_14F": "SRCIPS_FORMAT", - "GRANT-P09_14G": "SRCIPS_FORMAT", - "GRANT-P09_14H": "SRCIPS_FORMAT", - "GRANT-P09_14I": "SRCIPS_FORMAT", + "GRANT-P09_1A": "MISSING_IPV6_ADDRS", + "GRANT-P09_1B": "MISSING_IPV6_ADDRS", + "GRANT-P09_1C": "MISSING_IPV6_ADDRS", + "GRANT-P09_1D": "MISSING_IPV6_ADDRS", + "GRANT-P09_1E": "MISSING_IPV6_ADDRS", + "GRANT-P09_2A_CORRECT": "MISSING_IPV6_ADDRS", + "GRANT-P09_2A_NAIVE": "MISSING_IPV6_ADDRS", + "GRANT-P09_2B_CORRECT": "MISSING_IPV6_ADDRS", + "GRANT-P09_2B_NAIVE": "MISSING_IPV6_ADDRS", + "GRANT-P09_2C": "MISSING_IPV6_ADDRS", + "GRANT-P09_3A": "MISSING_IPV6_ADDRS", + "GRANT-P09_3B": "MISSING_IPV6_ADDRS", + "GRANT-P09_3C": "MISSING_IPV6_ADDRS", + "GRANT-P09_4A": "MISSING_IPV6_ADDRS", + "GRANT-P09_4B": "MISSING_IPV6_ADDRS", + "GRANT-P09_4C": "MISSING_IPV6_ADDRS", + "GRANT-P09_4D": "MISSING_IPV6_ADDRS", + "GRANT-P09_4F": "MISSING_IPV6_ADDRS", + "GRANT-P09_4G": "MISSING_IPV6_ADDRS", + "GRANT-P09_5A": "MISSING_IPV6_ADDRS", + "GRANT-P09_5B": "MISSING_IPV6_ADDRS", + "GRANT-P09_5C_NAIVE": "MISSING_IPV6_ADDRS", + "GRANT-P09_6C": "MISSING_IPV6_ADDRS", + "GRANT-P09_7B_NAIVE": "MISSING_IPV6_ADDRS", + "GRANT-P09_7C": "MISSING_IPV6_ADDRS", + "GRANT-P09_7D_NAIVE": "MISSING_IPV6_ADDRS", + "GRANT-P09_8A": "MISSING_IPV6_ADDRS", + "GRANT-P09_8B": "MISSING_IPV6_ADDRS", + "GRANT-P09_8C": "MISSING_IPV6_ADDRS", + "GRANT-P09_9A": "MISSING_IPV6_ADDRS", + "GRANT-P09_9B": "MISSING_IPV6_ADDRS", + "GRANT-P09_9C": "MISSING_IPV6_ADDRS", + "GRANT-P09_10A": "MISSING_IPV6_ADDRS", + "GRANT-P09_10B": "MISSING_IPV6_ADDRS", + "GRANT-P09_10C": "MISSING_IPV6_ADDRS", + "GRANT-P09_10D": "MISSING_IPV6_ADDRS", + "GRANT-P09_11A": "MISSING_IPV6_ADDRS", + "GRANT-P09_11B": "MISSING_IPV6_ADDRS", + "GRANT-P09_11C_NAIVE": "MISSING_IPV6_ADDRS", + "GRANT-P09_11D": "MISSING_IPV6_ADDRS", + "GRANT-P09_12A": "MISSING_IPV6_ADDRS", + "GRANT-P09_12B": "MISSING_IPV6_ADDRS + SUBNET_ROUTE_FILTER_RULES: tagged-server subtest missing IPv6; subnet-router subtest missing entire rule for 10.0.0.0/8", + "GRANT-P09_14A": "MISSING_IPV6_ADDRS", + "GRANT-P09_14B": "MISSING_IPV6_ADDRS", + "GRANT-P09_14C": "MISSING_IPV6_ADDRS", + "GRANT-P09_14D": "MISSING_IPV6_ADDRS", + "GRANT-P09_14E": "MISSING_IPV6_ADDRS", + "GRANT-P09_14F": "MISSING_IPV6_ADDRS", + "GRANT-P09_14G": "MISSING_IPV6_ADDRS", + "GRANT-P09_14H": "MISSING_IPV6_ADDRS", + "GRANT-P09_14I": "MISSING_IPV6_ADDRS", - // P10-series: Host alias grants - "GRANT-P10_1": "SRCIPS_FORMAT", - "GRANT-P10_2": "SRCIPS_FORMAT", - "GRANT-P10_3": "SRCIPS_FORMAT", - "GRANT-P10_4": "SRCIPS_FORMAT", + // P10-series: Host alias grants (only identity-src subtests fail) + "GRANT-P10_2": "MISSING_IPV6_ADDRS", // P11-series: autogroup:tagged grants - "GRANT-P11_1": "SRCIPS_FORMAT", - "GRANT-P11_2": "SRCIPS_FORMAT", + "GRANT-P11_2": "MISSING_IPV6_ADDRS", - // P13-series: CIDR destination grants - "GRANT-P13_1": "SRCIPS_FORMAT", - "GRANT-P13_2": "SRCIPS_FORMAT", - "GRANT-P13_3": "SRCIPS_FORMAT", - "GRANT-P13_4": "SRCIPS_FORMAT", + // P13-series: CIDR destination grants (identity-src subtests) + "GRANT-P13_4": "MISSING_IPV6_ADDRS", - // P15-series: Empty/no-match grants - "GRANT-P15_1": "SRCIPS_FORMAT", - "GRANT-P15_3": "SRCIPS_FORMAT", + // ======================================================================== + // SUBNET_ROUTE_FILTER_RULES (10 tests) + // + // TODO: Generate filter rules for non-Tailscale CIDR destinations on + // subnet-router nodes. + // + // When a grant targets a non-Tailscale CIDR (e.g., 10.0.0.0/8, + // 10.33.0.0/16, 10.33.1.0/24), Tailscale generates FilterRules on the + // subnet-router node that advertises overlapping routes. headscale + // produces no rules for these destinations, resulting in empty output + // on the subnet-router node. + // + // Example (GRANT-P13_1, dst=10.33.0.0/16): + // tailscale produces on subnet-router: + // SrcIPs=["100.103.90.82","100.110.121.96","100.90.199.68", + IPv6s] + // DstPorts=[{IP:"10.33.0.0/16", Ports:"22"}] + // headscale produces: [] (empty) + // + // Fix: During filter rule compilation, check if a destination CIDR + // overlaps with any subnet route advertised by the current node, and + // if so, generate the appropriate FilterRule. + // ======================================================================== + "GRANT-P08_8": "SUBNET_ROUTE_FILTER_RULES: dst=10.0.0.0/8 — subnet-router gets no rules", + "GRANT-P09_6D": "SUBNET_ROUTE_FILTER_RULES: dst=internal (host alias for 10.0.0.0/8) — subnet-router gets no rules", + "GRANT-P10_3": "SUBNET_ROUTE_FILTER_RULES: dst=host alias for 10.33.0.0/16 — subnet-router gets no rules", + "GRANT-P10_4": "SUBNET_ROUTE_FILTER_RULES: dst=host alias for 10.33.0.0/16 — subnet-router gets no rules", + "GRANT-P13_1": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.0.0/16 port 22 — subnet-router gets no rules", + "GRANT-P13_2": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.0.0/16 port 80-443 — subnet-router gets no rules", + "GRANT-P13_3": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.0.0/16 ports 22,80,443 — subnet-router gets no rules", + "GRANT-P15_1": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.1.0/24 port 22 — subnet-router gets no rules", + "GRANT-P15_3": "SUBNET_ROUTE_FILTER_RULES: dst=10.32.0.0/14 port 22 — subnet-router gets no rules", + // Note: GRANT-P09_12B also has a subnet-router subtest failure — listed under MISSING_IPV6_ADDRS above + + // ======================================================================== + // AUTOGROUP_SELF_CIDR_FORMAT (4 tests) + // + // TODO: Use bare IPs (not CIDR notation) in DstPorts for autogroup:self grants. + // + // When compiling autogroup:self grants, headscale appends /32 to IPv4 + // and /128 to IPv6 DstPort IPs. Tailscale uses bare IPs without a CIDR + // suffix. These tests also have missing IPv6 in SrcIPs. + // + // Example diff (user1 node, autogroup:member -> autogroup:self): + // DstPorts: tailscale=[{IP:"100.90.199.68"}, {IP:"fd7a:...::2d01:c747"}] + // DstPorts: headscale=[{IP:"100.90.199.68/32"}, {IP:"fd7a:...::2d01:c747/128"}] + // SrcIPs: tailscale=["100.90.199.68", "fd7a:...::2d01:c747"] + // SrcIPs: headscale=["100.90.199.68"] + // ======================================================================== + "GRANT-P09_4E": "AUTOGROUP_SELF_CIDR_FORMAT: autogroup:member -> autogroup:self — DstPorts IPs have /32 and /128 suffix + missing IPv6 in SrcIPs", + "GRANT-P09_13E": "AUTOGROUP_SELF_CIDR_FORMAT: autogroup:member -> autogroup:self with ip:[*] — DstPorts IPs have CIDR suffix + missing IPv6 in SrcIPs", + "GRANT-P09_13F": "AUTOGROUP_SELF_CIDR_FORMAT: single user -> autogroup:self with ip:[22] — DstPorts IPs have CIDR suffix + missing IPv6 in SrcIPs", + "GRANT-P09_13G": "AUTOGROUP_SELF_CIDR_FORMAT: single user -> autogroup:self with ip:[22,80,443] — DstPorts IPs have CIDR suffix + missing IPv6 in SrcIPs", + + // ======================================================================== + // USER_PASSKEY_WILDCARD (2 tests) + // + // TODO: Handle user:*@passkey wildcard pattern in grant src/dst. + // + // Tailscale SaaS policies can use user:*@passkey as a wildcard matching + // all passkey-authenticated users. headscale's convertPolicyUserEmails + // only converts specific user@passkey addresses (not the wildcard form), + // so the filter compiler logs "user not found: token user:*@passkey" + // and produces no rules. + // + // Fix: Either convert user:*@passkey to a headscale-compatible wildcard, + // or resolve it to all known users during filter compilation. + // ======================================================================== + "GRANT-K20": "USER_PASSKEY_WILDCARD: src=user:*@passkey, dst=tag:server — source can't be resolved, no rules produced", + "GRANT-K21": "USER_PASSKEY_WILDCARD: src=*, dst=user:*@passkey — destination can't be resolved, no rules produced", + + // ======================================================================== + // RAW_IPV6_ADDR_EXPANSION (2 tests) + // + // TODO: Don't expand raw IPv6 addresses to include the matching node's IPv4. + // + // When a grant uses a raw fd7a: IPv6 address as src or dst, headscale + // resolves it to BOTH the IPv4 and IPv6 of the matching node. Tailscale + // keeps only the specific address that was referenced in the grant. + // + // Example (GRANT-K14, src=fd7a:115c:a1e0::c537:c845): + // SrcIPs: tailscale=["fd7a:115c:a1e0::c537:c845"] + // SrcIPs: headscale=["100.83.200.69", "fd7a:115c:a1e0::c537:c845"] + // Example (GRANT-K15, dst=fd7a:115c:a1e0::b901:4a87): + // DstPorts: tailscale=[{IP:"fd7a:...::b901:4a87"}] + // DstPorts: headscale=[{IP:"100.108.74.26"}, {IP:"fd7a:...::b901:4a87"}] + // ======================================================================== + "GRANT-K14": "RAW_IPV6_ADDR_EXPANSION: src=fd7a:...::c537:c845 — headscale adds extra IPv4 SrcIP + missing IPv6 in DstPorts", + "GRANT-K15": "RAW_IPV6_ADDR_EXPANSION: dst=fd7a:...::b901:4a87 — headscale adds extra IPv4 DstPort entry", + + // ======================================================================== + // SRCIPS_WILDCARD_NODE_DEDUP (1 test) + // + // TODO: When src includes both * (wildcard) and specific identities, + // Tailscale unions individual node IPs with the wildcard CGNAT ranges. + // headscale only produces the wildcard ranges, omitting the individual + // node IPs that are technically covered by those ranges. + // + // Also has missing IPv6 in DstPorts. + // + // Example (GRANT-P09_7A, src=[*, autogroup:member, tag:client, ...]): + // SrcIPs: tailscale=[individual IPs + CGNAT ranges + IPv6s] (20 entries) + // SrcIPs: headscale=[10.33.0.0/16, CGNAT ranges, fd7a::/48] (4 entries) + // ======================================================================== + "GRANT-P09_7A": "SRCIPS_WILDCARD_NODE_DEDUP: src=[*,...] — individual node IPs missing from SrcIPs + missing IPv6 in DstPorts", // ======================================================================== // CAPGRANT_COMPILATION (49 tests) @@ -640,15 +727,21 @@ var grantSkipReasons = map[string]string{ // // Skip category impact summary (highest first): // -// SRCIPS_FORMAT - 125 tests: Fix SrcIPs to use CGNAT split ranges -// CAPGRANT_COMPILATION - 41 tests: Implement app->CapGrant FilterRule compilation -// ERROR_VALIDATION_GAP - 14 tests: Implement missing grant validation rules -// CAPGRANT_AND_SRCIPS_FORMAT - 9 tests: Both CapGrant compilation + SrcIPs format -// VIA_AND_SRCIPS_FORMAT - 4 tests: Via route compilation + SrcIPs format -// AUTOGROUP_DANGER_ALL - 3 tests: Implement autogroup:danger-all support -// VALIDATION_STRICTNESS - 2 tests: headscale too strict (rejects what Tailscale accepts) +// CAPGRANT_COMPILATION - 49 tests: Implement app->CapGrant FilterRule compilation +// ERROR_VALIDATION_GAP - 23 tests: Implement missing grant validation rules +// MISSING_IPV6_ADDRS - 90 tests: Include IPv6 for identity-based alias resolution +// CAPGRANT_COMPILATION_AND_SRCIPS - 11 tests: Both CapGrant compilation + SrcIPs format +// SUBNET_ROUTE_FILTER_RULES - 10 tests: Generate filter rules for subnet-routed CIDRs +// VIA_COMPILATION_AND_SRCIPS_FORMAT - 7 tests: Via route compilation + SrcIPs format +// AUTOGROUP_SELF_CIDR_FORMAT - 4 tests: DstPorts IPs get /32 or /128 suffix for autogroup:self +// VIA_COMPILATION - 3 tests: Via route compilation +// AUTOGROUP_DANGER_ALL - 3 tests: Implement autogroup:danger-all support +// USER_PASSKEY_WILDCARD - 2 tests: user:*@passkey wildcard pattern unresolvable +// VALIDATION_STRICTNESS - 2 tests: headscale too strict (rejects what Tailscale accepts) +// RAW_IPV6_ADDR_EXPANSION - 2 tests: Raw fd7a: IPv6 src/dst expanded to include IPv4 +// SRCIPS_WILDCARD_NODE_DEDUP - 1 test: Wildcard+specific source node IP deduplication // -// Total: 193 tests skipped, 19 tests expected to pass. +// Total: 207 tests skipped, 30 tests expected to pass. func TestGrantsCompat(t *testing.T) { t.Parallel() diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 090546f0..cc194e29 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -3,10 +3,12 @@ package v2 import ( "errors" "fmt" + "iter" "net/netip" "slices" "strconv" "strings" + "sync" "time" "github.com/go-json-experiment/json" @@ -124,6 +126,88 @@ var ( ErrProtocolNoSpecificPorts = errors.New("protocol does not support specific ports") ) +type resolved struct { + ips netipx.IPSet +} + +func newResolved(ipb *netipx.IPSetBuilder) (resolved, error) { + ips, err := ipb.IPSet() + if err != nil { + return resolved{}, err + } + + return resolved{ips: *ips}, nil +} + +func newResolvedAddresses(ips *netipx.IPSet, err error) (ResolvedAddresses, error) { + if ips == nil { + return nil, err + } + + return resolved{ips: *ips}, err +} + +func ipSetToStrings(ips *netipx.IPSet) []string { + var result []string + + for _, r := range ips.Ranges() { + if r.From() == r.To() { + result = append(result, r.From().String()) + continue + } + + if p, ok := r.Prefix(); ok { + result = append(result, p.String()) + continue + } + + result = append(result, r.String()) + } + + return result +} + +func (res resolved) Strings() []string { + return ipSetToStrings(&res.ips) +} + +func (res resolved) Prefixes() []netip.Prefix { + ret := res.ips.Prefixes() + + return ret +} + +func (res resolved) Empty() bool { + return len(res.ips.Prefixes()) == 0 +} + +func (res resolved) Iter() iter.Seq[netip.Addr] { + return util.IPSetAddrIter(&res.ips) +} + +func (res resolved) Contains(ip netip.Addr) bool { + return res.ips.Contains(ip) +} + +type ResolvedAddresses interface { + // Strings returns a slice of string representations of IP addresses, + // it will return the appropriate representation for the underlying Alias. + // Some should be returned as Prefixes and some as IP ranges. + Strings() []string + + // Prefixes returns a slice of netip.Prefix representations of IP addresses. + Prefixes() []netip.Prefix + + // Empty reports if there are no addresses in the ResolvedAddresses. + Empty() bool + + // Iter returns an iterator over netip.Addr representations of IP addresses. + Iter() iter.Seq[netip.Addr] + + // Contains reports if the given IP address is contained in the ResolvedAddresses. + Contains(ip netip.Addr) bool +} + type Asterix int func (a Asterix) Validate() error { @@ -194,16 +278,37 @@ func (a Asterix) UnmarshalJSON(b []byte) error { return nil } -func (a Asterix) Resolve(_ *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { - var ips netipx.IPSetBuilder +var asterixResolved = sync.OnceValue(func() *netipx.IPSet { + var ipb netipx.IPSetBuilder + ipb.AddPrefix(tsaddr.TailscaleULARange()) + ipb.AddPrefix(tsaddr.CGNATRange()) + ipb.RemovePrefix(tsaddr.ChromeOSVMRange()) - // Use Tailscale's CGNAT range for IPv4 and ULA range for IPv6. - // This matches Tailscale's behavior where wildcard (*) refers to - // "any node in the tailnet" which uses these address ranges. - ips.AddPrefix(tsaddr.CGNATRange()) - ips.AddPrefix(tsaddr.TailscaleULARange()) + ips, err := ipb.IPSet() + if err != nil { + panic(fmt.Sprintf("failed to build IPSet for wildcard: %v", err)) + } - return ips.IPSet() + return ips +}) + +func (a Asterix) Resolve(p *Policy, u types.Users, n views.Slice[types.NodeView]) (ResolvedAddresses, error) { + return newResolvedAddresses(a.resolve(p, u, n)) +} + +func (a Asterix) resolve(p *Policy, _ types.Users, _ views.Slice[types.NodeView]) (*netipx.IPSet, error) { + if p != nil && len(p.AutoApprovers.prefixes()) > 0 { + var ipb netipx.IPSetBuilder + ipb.AddSet(asterixResolved()) + + for _, pfx := range p.AutoApprovers.prefixes() { + ipb.AddPrefix(pfx) + } + + return ipb.IPSet() + } + + return asterixResolved(), nil } // Username is a string that represents a username, it must contain an @. @@ -286,7 +391,11 @@ func (u *Username) resolveUser(users types.Users) (types.User, error) { return potentialUsers[0], nil } -func (u *Username) Resolve(_ *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (u *Username) Resolve(_ *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { + return newResolvedAddresses(u.resolve(nil, users, nodes)) +} + +func (u *Username) resolve(_ *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { var ( ips netipx.IPSetBuilder errs []error @@ -365,14 +474,18 @@ func (g *Group) MarshalJSON() ([]byte, error) { return json.Marshal(string(*g)) } -func (g *Group) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (g *Group) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { + return newResolvedAddresses(g.resolve(p, users, nodes)) +} + +func (g *Group) resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { var ( ips netipx.IPSetBuilder errs []error ) for _, user := range p.Groups[*g] { - uips, err := user.Resolve(nil, users, nodes) + uips, err := user.resolve(nil, users, nodes) if err != nil { errs = append(errs, err) } @@ -405,7 +518,11 @@ func (t *Tag) UnmarshalJSON(b []byte) error { return nil } -func (t *Tag) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (t *Tag) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { + return newResolvedAddresses(t.resolve(p, users, nodes)) +} + +func (t *Tag) resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { var ips netipx.IPSetBuilder for _, node := range nodes.All() { @@ -457,7 +574,11 @@ func (h *Host) UnmarshalJSON(b []byte) error { return nil } -func (h *Host) Resolve(p *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (h *Host) Resolve(p *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { + return newResolvedAddresses(h.resolve(p, nil, nodes)) +} + +func (h *Host) resolve(p *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { var ( ips netipx.IPSetBuilder errs []error @@ -554,7 +675,11 @@ func (p *Prefix) UnmarshalJSON(b []byte) error { // of the Prefix and the Policy, Users, and Nodes. // // See [Policy], [types.Users], and [types.Nodes] for more details. -func (p *Prefix) Resolve(_ *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (p *Prefix) Resolve(_ *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { + return newResolvedAddresses(p.resolve(nil, nil, nodes)) +} + +func (p *Prefix) resolve(_ *Policy, _ types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { var ( ips netipx.IPSetBuilder errs []error @@ -629,7 +754,11 @@ func (ag *AutoGroup) MarshalJSON() ([]byte, error) { return json.Marshal(string(*ag)) } -func (ag *AutoGroup) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (ag *AutoGroup) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { + return newResolvedAddresses(ag.resolve(p, users, nodes)) +} + +func (ag *AutoGroup) resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { var build netipx.IPSetBuilder switch *ag { @@ -694,7 +823,9 @@ type Alias interface { // of the Alias and the Policy, Users and Nodes. // This is an interface definition and the implementation is independent of // the Alias type. - Resolve(pol *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) + Resolve(pol *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) + + resolve(pol *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) } type AliasWithPorts struct { @@ -960,14 +1091,14 @@ func (a *Aliases) MarshalJSON() ([]byte, error) { return json.Marshal(aliases) } -func (a *Aliases) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (a *Aliases) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { var ( ips netipx.IPSetBuilder errs []error ) for _, alias := range *a { - aips, err := alias.Resolve(p, users, nodes) + aips, err := alias.resolve(p, users, nodes) if err != nil { errs = append(errs, err) } @@ -975,7 +1106,7 @@ func (a *Aliases) Resolve(p *Policy, users types.Users, nodes views.Slice[types. ips.AddSet(aips) } - return buildIPSetMultiErr(&ips, errs) + return newResolvedAddresses(buildIPSetMultiErr(&ips, errs)) } func buildIPSetMultiErr(ipBuilder *netipx.IPSetBuilder, errs []error) (*netipx.IPSet, error) { @@ -1378,6 +1509,22 @@ func (ap AutoApproverPolicy) MarshalJSON() ([]byte, error) { return json.Marshal(&obj) } +// prefixes returns the prefixes that have auto-approvers defined in the policy. +// It filters out exit routes since they are not associated with a specific prefix and are handled separately. +func (ap AutoApproverPolicy) prefixes() []netip.Prefix { + prefixes := make([]netip.Prefix, 0, len(ap.Routes)) + + for prefix := range ap.Routes { + if tsaddr.IsExitRoute(prefix) { + continue + } + + prefixes = append(prefixes, prefix) + } + + return prefixes +} + // resolveAutoApprovers resolves the AutoApprovers to a map of netip.Prefix to netipx.IPSet. // The resulting map can be used to quickly look up if a node can self-approve a route. // It is intended for internal use in a PolicyManager. @@ -1402,7 +1549,7 @@ func resolveAutoApprovers(p *Policy, users types.Users, nodes views.Slice[types. return nil, nil, fmt.Errorf("%w: %v", ErrAutoApproverNotAlias, autoApprover) } // If it does not resolve, that means the autoApprover is not associated with any IP addresses. - ips, _ := aa.Resolve(p, users, nodes) + ips, _ := aa.resolve(p, users, nodes) routes[prefix].AddSet(ips) } } @@ -1417,7 +1564,7 @@ func resolveAutoApprovers(p *Policy, users types.Users, nodes views.Slice[types. return nil, nil, fmt.Errorf("%w: %v", ErrAutoApproverNotAlias, autoApprover) } // If it does not resolve, that means the autoApprover is not associated with any IP addresses. - ips, _ := aa.Resolve(p, users, nodes) + ips, _ := aa.resolve(p, users, nodes) exitNodeSetBuilder.AddSet(ips) } } @@ -1576,9 +1723,8 @@ func (p *Protocol) Description() string { func (p *Protocol) toIANAProtocolNumbers() []int { switch *p { case "": - // Empty protocol applies to TCP, UDP, ICMP, and ICMPv6 traffic - // This matches Tailscale's behavior for protocol defaults - return []int{ProtocolTCP, ProtocolUDP, ProtocolICMP, ProtocolIPv6ICMP} + // Empty means the same as wildcard-ish, the client will add the default protocols (TCP, UDP, ICMP) if empty. + return nil case ProtocolNameWildcard: // Wildcard protocol - defensive handling (should not reach here due to validation) return nil @@ -2601,14 +2747,14 @@ func (a *SSHSrcAliases) MarshalJSON() ([]byte, error) { return json.Marshal(aliases) } -func (a *SSHSrcAliases) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (*netipx.IPSet, error) { +func (a *SSHSrcAliases) Resolve(p *Policy, users types.Users, nodes views.Slice[types.NodeView]) (ResolvedAddresses, error) { var ( ips netipx.IPSetBuilder errs []error ) for _, alias := range *a { - aips, err := alias.Resolve(p, users, nodes) + aips, err := alias.resolve(p, users, nodes) if err != nil { errs = append(errs, err) } @@ -2616,7 +2762,7 @@ func (a *SSHSrcAliases) Resolve(p *Policy, users types.Users, nodes views.Slice[ ips.AddSet(aips) } - return buildIPSetMultiErr(&ips, errs) + return newResolvedAddresses(buildIPSetMultiErr(&ips, errs)) } // SSHDstAliases is a list of aliases that can be used as destinations in an SSH rule. diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 43e4d64a..70f95fbd 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -2378,7 +2378,22 @@ func TestResolvePolicy(t *testing.T) { { name: "wildcard-alias", toResolve: Wildcard, - want: []netip.Prefix{tsaddr.CGNATRange(), tsaddr.TailscaleULARange()}, + want: []netip.Prefix{ + mp("100.64.0.0/11"), + mp("100.96.0.0/12"), + mp("100.112.0.0/15"), + mp("100.114.0.0/16"), + mp("100.115.0.0/18"), + mp("100.115.64.0/20"), + mp("100.115.80.0/21"), + mp("100.115.88.0/22"), + mp("100.115.94.0/23"), + mp("100.115.96.0/19"), + mp("100.115.128.0/17"), + mp("100.116.0.0/14"), + mp("100.120.0.0/13"), + tsaddr.TailscaleULARange(), + }, }, { name: "autogroup-member-comprehensive", diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go index f26c191f..d260656b 100644 --- a/hscontrol/types/node.go +++ b/hscontrol/types/node.go @@ -300,9 +300,22 @@ func (node *Node) InIPSet(set *netipx.IPSet) bool { // AppendToIPSet adds the individual ips in NodeAddresses to a // given netipx.IPSetBuilder. func (node *Node) AppendToIPSet(build *netipx.IPSetBuilder) { - for _, ip := range node.IPs() { - build.Add(ip) + if node.IPv4 != nil { + build.Add(*node.IPv4) + return } + + if node.IPv6 != nil { + build.Add(*node.IPv6) + } + + // TODO(kradalby): Evaluate what we want to do here: + // Tailscale only adds the IPv4 addresses to any packet filter rule that is resolved to a given node. + // Presumably, it will add the IPv4 if a node does not have an IPv4. + // Until this change, we always added both, and that might be something people are dependent on, and we might want to keep it. + // for _, ip := range node.IPs() { + // build.Add(ip) + // } } func (node *Node) CanAccess(matchers []matcher.Match, node2 *Node) bool {