From 6750414db116aa4b0241c07b2d8f52f13ba045fe Mon Sep 17 00:00:00 2001 From: Vitalij Dovhanyc <45185420+vdovhanych@users.noreply.github.com> Date: Sat, 17 May 2025 11:07:34 +0200 Subject: [PATCH] feat: add autogroup:member, autogroup:tagged (#2572) --- .github/workflows/test-integration.yaml | 2 + CHANGELOG.md | 2 + docs/about/features.md | 2 +- hscontrol/policy/v2/types.go | 93 ++++++++++++++--- hscontrol/policy/v2/types_test.go | 133 +++++++++++++++++++++++- integration/acl_test.go | 112 ++++++++++++++++++++ 6 files changed, 329 insertions(+), 15 deletions(-) diff --git a/.github/workflows/test-integration.yaml b/.github/workflows/test-integration.yaml index 3c8141c7..61213ea6 100644 --- a/.github/workflows/test-integration.yaml +++ b/.github/workflows/test-integration.yaml @@ -22,6 +22,8 @@ jobs: - TestACLNamedHostsCanReach - TestACLDevice1CanAccessDevice2 - TestPolicyUpdateWhileRunningWithCLIInDatabase + - TestACLAutogroupMember + - TestACLAutogroupTagged - TestAuthKeyLogoutAndReloginSameUser - TestAuthKeyLogoutAndReloginNewUser - TestAuthKeyLogoutAndReloginSameUserExpiredKey diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe63f3e..91a23a05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -155,6 +155,8 @@ working in v1 and not tested might be broken in v2 (and vice versa). [#2438](https://github.com/juanfont/headscale/pull/2438) - Add documentation for routes [#2496](https://github.com/juanfont/headscale/pull/2496) +- Add support for `autogroup:member`, `autogroup:tagged` + [#2572](https://github.com/juanfont/headscale/pull/2572) ## 0.25.1 (2025-02-25) diff --git a/docs/about/features.md b/docs/about/features.md index 22a4be62..3ee913db 100644 --- a/docs/about/features.md +++ b/docs/about/features.md @@ -23,7 +23,7 @@ provides on overview of Headscale's feature and compatibility with the Tailscale - [x] Access control lists ([GitHub label "policy"](https://github.com/juanfont/headscale/labels/policy%20%F0%9F%93%9D)) - [x] ACL management via API - [x] Some [Autogroups](https://tailscale.com/kb/1396/targets#autogroups), currently: `autogroup:internet`, - `autogroup:nonroot` + `autogroup:nonroot`, `autogroup:member`, `autogroup:tagged` - [x] [Auto approvers](https://tailscale.com/kb/1337/acl-syntax#auto-approvers) for [subnet routers](../ref/routes.md#automatically-approve-routes-of-a-subnet-router) and [exit nodes](../ref/routes.md#automatically-approve-an-exit-node-with-auto-approvers) diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index d10136a0..580a1980 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -384,15 +384,20 @@ type AutoGroup string const ( AutoGroupInternet AutoGroup = "autogroup:internet" + AutoGroupMember AutoGroup = "autogroup:member" AutoGroupNonRoot AutoGroup = "autogroup:nonroot" + AutoGroupTagged AutoGroup = "autogroup:tagged" // These are not yet implemented. - AutoGroupSelf AutoGroup = "autogroup:self" - AutoGroupMember AutoGroup = "autogroup:member" - AutoGroupTagged AutoGroup = "autogroup:tagged" + AutoGroupSelf AutoGroup = "autogroup:self" ) -var autogroups = []AutoGroup{AutoGroupInternet} +var autogroups = []AutoGroup{ + AutoGroupInternet, + AutoGroupMember, + AutoGroupNonRoot, + AutoGroupTagged, +} func (ag AutoGroup) Validate() error { if slices.Contains(autogroups, ag) { @@ -410,13 +415,76 @@ func (ag *AutoGroup) UnmarshalJSON(b []byte) error { return nil } -func (ag AutoGroup) Resolve(_ *Policy, _ types.Users, _ types.Nodes) (*netipx.IPSet, error) { +func (ag AutoGroup) Resolve(p *Policy, users types.Users, nodes types.Nodes) (*netipx.IPSet, error) { + var build netipx.IPSetBuilder + switch ag { case AutoGroupInternet: return util.TheInternet(), nil - } - return nil, nil + case AutoGroupMember: + // autogroup:member represents all untagged devices in the tailnet. + tagMap, err := resolveTagOwners(p, users, nodes) + if err != nil { + return nil, err + } + + for _, node := range nodes { + // Skip if node has forced tags + if len(node.ForcedTags) != 0 { + continue + } + + // Skip if node has any allowed requested tags + hasAllowedTag := false + if node.Hostinfo != nil && len(node.Hostinfo.RequestTags) != 0 { + for _, tag := range node.Hostinfo.RequestTags { + if tagips, ok := tagMap[Tag(tag)]; ok && node.InIPSet(tagips) { + hasAllowedTag = true + break + } + } + } + if hasAllowedTag { + continue + } + + // Node is a member if it has no forced tags and no allowed requested tags + node.AppendToIPSet(&build) + } + + return build.IPSet() + + case AutoGroupTagged: + // autogroup:tagged represents all devices with a tag in the tailnet. + tagMap, err := resolveTagOwners(p, users, nodes) + if err != nil { + return nil, err + } + + for _, node := range nodes { + // Include if node has forced tags + if len(node.ForcedTags) != 0 { + node.AppendToIPSet(&build) + continue + } + + // Include if node has any allowed requested tags + if node.Hostinfo != nil && len(node.Hostinfo.RequestTags) != 0 { + for _, tag := range node.Hostinfo.RequestTags { + if _, ok := tagMap[Tag(tag)]; ok { + node.AppendToIPSet(&build) + break + } + } + } + } + + return build.IPSet() + + default: + return nil, fmt.Errorf("unknown autogroup %q", ag) + } } func (ag *AutoGroup) Is(c AutoGroup) bool { @@ -952,12 +1020,13 @@ type Policy struct { } var ( - autogroupForSrc = []AutoGroup{} - autogroupForDst = []AutoGroup{AutoGroupInternet} - autogroupForSSHSrc = []AutoGroup{} - autogroupForSSHDst = []AutoGroup{} + // TODO(kradalby): Add these checks for tagOwners and autoApprovers + autogroupForSrc = []AutoGroup{AutoGroupMember, AutoGroupTagged} + autogroupForDst = []AutoGroup{AutoGroupInternet, AutoGroupMember, AutoGroupTagged} + autogroupForSSHSrc = []AutoGroup{AutoGroupMember, AutoGroupTagged} + autogroupForSSHDst = []AutoGroup{AutoGroupMember, AutoGroupTagged} autogroupForSSHUser = []AutoGroup{AutoGroupNonRoot} - autogroupNotSupported = []AutoGroup{AutoGroupSelf, AutoGroupMember, AutoGroupTagged} + autogroupNotSupported = []AutoGroup{AutoGroupSelf} ) func validateAutogroupSupported(ag *AutoGroup) error { diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index e9fa6263..3e9de7d7 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -359,7 +359,7 @@ func TestUnmarshalPolicy(t *testing.T) { ], } `, - wantErr: `AutoGroup is invalid, got: "autogroup:invalid", must be one of [autogroup:internet]`, + wantErr: `AutoGroup is invalid, got: "autogroup:invalid", must be one of [autogroup:internet autogroup:member autogroup:nonroot autogroup:tagged]`, }, { name: "undefined-hostname-errors-2490", @@ -998,6 +998,135 @@ func TestResolvePolicy(t *testing.T) { toResolve: Wildcard, want: []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()}, }, + { + name: "autogroup-member-comprehensive", + toResolve: ptr.To(AutoGroup(AutoGroupMember)), + nodes: types.Nodes{ + // Node with no tags (should be included) + { + User: users["testuser"], + IPv4: ap("100.100.101.1"), + }, + // Node with forced tags (should be excluded) + { + User: users["testuser"], + ForcedTags: []string{"tag:test"}, + IPv4: ap("100.100.101.2"), + }, + // Node with allowed requested tag (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test"}, + }, + IPv4: ap("100.100.101.3"), + }, + // Node with non-allowed requested tag (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed"}, + }, + IPv4: ap("100.100.101.4"), + }, + // Node with multiple requested tags, one allowed (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test", "tag:notallowed"}, + }, + IPv4: ap("100.100.101.5"), + }, + // Node with multiple requested tags, none allowed (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed1", "tag:notallowed2"}, + }, + IPv4: ap("100.100.101.6"), + }, + }, + pol: &Policy{ + TagOwners: TagOwners{ + Tag("tag:test"): Owners{ptr.To(Username("testuser@"))}, + }, + }, + want: []netip.Prefix{ + mp("100.100.101.1/32"), // No tags + mp("100.100.101.4/32"), // Non-allowed requested tag + mp("100.100.101.6/32"), // Multiple non-allowed requested tags + }, + }, + { + name: "autogroup-tagged", + toResolve: ptr.To(AutoGroup(AutoGroupTagged)), + nodes: types.Nodes{ + // Node with no tags (should be excluded) + { + User: users["testuser"], + IPv4: ap("100.100.101.1"), + }, + // Node with forced tag (should be included) + { + User: users["testuser"], + ForcedTags: []string{"tag:test"}, + IPv4: ap("100.100.101.2"), + }, + // Node with allowed requested tag (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test"}, + }, + IPv4: ap("100.100.101.3"), + }, + // Node with non-allowed requested tag (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed"}, + }, + IPv4: ap("100.100.101.4"), + }, + // Node with multiple requested tags, one allowed (should be included) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:test", "tag:notallowed"}, + }, + IPv4: ap("100.100.101.5"), + }, + // Node with multiple requested tags, none allowed (should be excluded) + { + User: users["testuser"], + Hostinfo: &tailcfg.Hostinfo{ + RequestTags: []string{"tag:notallowed1", "tag:notallowed2"}, + }, + IPv4: ap("100.100.101.6"), + }, + // Node with multiple forced tags (should be included) + { + User: users["testuser"], + ForcedTags: []string{"tag:test", "tag:other"}, + IPv4: ap("100.100.101.7"), + }, + }, + pol: &Policy{ + TagOwners: TagOwners{ + Tag("tag:test"): Owners{ptr.To(Username("testuser@"))}, + }, + }, + want: []netip.Prefix{ + mp("100.100.101.2/31"), // Forced tag and allowed requested tag consecutive IPs are put in 31 prefix + mp("100.100.101.5/32"), // Multiple requested tags, one allowed + mp("100.100.101.7/32"), // Multiple forced tags + }, + }, + { + name: "autogroup-invalid", + toResolve: ptr.To(AutoGroup("autogroup:invalid")), + wantErr: "unknown autogroup", + }, } for _, tt := range tests { @@ -1161,7 +1290,7 @@ func TestResolveAutoApprovers(t *testing.T) { name: "mixed-routes-and-exit-nodes", policy: &Policy{ Groups: Groups{ - "group:testgroup": Usernames{"user1", "user2"}, + "group:testgroup": Usernames{"user1@", "user2@"}, }, AutoApprovers: AutoApproverPolicy{ Routes: map[netip.Prefix]AutoApprovers{ diff --git a/integration/acl_test.go b/integration/acl_test.go index bb18b3b3..116f298d 100644 --- a/integration/acl_test.go +++ b/integration/acl_test.go @@ -1139,3 +1139,115 @@ func TestPolicyUpdateWhileRunningWithCLIInDatabase(t *testing.T) { } } } + +func TestACLAutogroupMember(t *testing.T) { + IntegrationSkip(t) + t.Parallel() + + scenario := aclScenario(t, + &policyv1.ACLPolicy{ + ACLs: []policyv1.ACL{ + { + Action: "accept", + Sources: []string{"autogroup:member"}, + Destinations: []string{"autogroup:member:*"}, + }, + }, + }, + 2, + ) + defer scenario.ShutdownAssertNoPanics(t) + + allClients, err := scenario.ListTailscaleClients() + require.NoError(t, err) + + err = scenario.WaitForTailscaleSync() + require.NoError(t, err) + + // Test that untagged nodes can access each other + for _, client := range allClients { + status, err := client.Status() + require.NoError(t, err) + if status.Self.Tags != nil && status.Self.Tags.Len() > 0 { + continue + } + + for _, peer := range allClients { + if client.Hostname() == peer.Hostname() { + continue + } + + status, err := peer.Status() + require.NoError(t, err) + if status.Self.Tags != nil && status.Self.Tags.Len() > 0 { + continue + } + + fqdn, err := peer.FQDN() + require.NoError(t, err) + + url := fmt.Sprintf("http://%s/etc/hostname", fqdn) + t.Logf("url from %s to %s", client.Hostname(), url) + + result, err := client.Curl(url) + assert.Len(t, result, 13) + require.NoError(t, err) + } + } +} + +func TestACLAutogroupTagged(t *testing.T) { + IntegrationSkip(t) + t.Parallel() + + scenario := aclScenario(t, + &policyv1.ACLPolicy{ + ACLs: []policyv1.ACL{ + { + Action: "accept", + Sources: []string{"autogroup:tagged"}, + Destinations: []string{"autogroup:tagged:*"}, + }, + }, + }, + 2, + ) + defer scenario.ShutdownAssertNoPanics(t) + + allClients, err := scenario.ListTailscaleClients() + require.NoError(t, err) + + err = scenario.WaitForTailscaleSync() + require.NoError(t, err) + + // Test that tagged nodes can access each other + for _, client := range allClients { + status, err := client.Status() + require.NoError(t, err) + if status.Self.Tags == nil || status.Self.Tags.Len() == 0 { + continue + } + + for _, peer := range allClients { + if client.Hostname() == peer.Hostname() { + continue + } + + status, err := peer.Status() + require.NoError(t, err) + if status.Self.Tags == nil || status.Self.Tags.Len() == 0 { + continue + } + + fqdn, err := peer.FQDN() + require.NoError(t, err) + + url := fmt.Sprintf("http://%s/etc/hostname", fqdn) + t.Logf("url from %s to %s", client.Hostname(), url) + + result, err := client.Curl(url) + assert.Len(t, result, 13) + require.NoError(t, err) + } + } +}