From ebdbe0363924fb222bcbd61a2c46c0b42e1b7db6 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 23 Jan 2026 20:37:27 +0000 Subject: [PATCH] =?UTF-8?q?policy:=20validate=20autogroup:self=20sources?= =?UTF-8?q?=20in=20ACL=20rules=20Tailscale=20validates=20that=20autogroup:?= =?UTF-8?q?self=20destinations=20in=20ACL=20rules=20can=20only=20be=20used?= =?UTF-8?q?=20when=20ALL=20sources=20are=20users,=20groups,=20autogroup:me?= =?UTF-8?q?mber,=20or=20wildcard=20(*).=20Previously,=20Headscale=20only?= =?UTF-8?q?=20performed=20this=20validation=20for=20SSH=20rules.=20Add=20v?= =?UTF-8?q?alidateACLSrcDstCombination()=20to=20enforce=20that=20tags,=20a?= =?UTF-8?q?utogroup:tagged,=20hosts,=20and=20raw=20IPs=20cannot=20be=20use?= =?UTF-8?q?d=20as=20sources=20with=20autogroup:self=20destinations.=20Inva?= =?UTF-8?q?lid=20policies=20like=20`tag:client=20=E2=86=92=20autogroup:sel?= =?UTF-8?q?f:*`=20are=20now=20rejected=20at=20validation=20time,=20matchin?= =?UTF-8?q?g=20Tailscale=20behavior.=20Wildcard=20(*)=20is=20allowed=20bec?= =?UTF-8?q?ause=20autogroup:self=20evaluation=20narrows=20it=20per-node=20?= =?UTF-8?q?to=20only=20the=20node's=20own=20IPs.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates #3036 --- CHANGELOG.md | 2 +- hscontrol/policy/v2/tailscale_compat_test.go | 54 +++++++++---------- hscontrol/policy/v2/types.go | 56 ++++++++++++++++++++ 3 files changed, 83 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12696add..6e7b4354 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Changes -- **ACL Policy**: Add ICMP and IPv6-ICMP protocols to default filter rules and export protocol constants [#3036](https://github.com/juanfont/headscale/pull/3036) +- **ACL Policy**: Add ICMP and IPv6-ICMP protocols to default filter rules when no protocol is specified [#3036](https://github.com/juanfont/headscale/pull/3036) - **ACL Policy**: Fix autogroup:self handling for tagged nodes - tagged nodes no longer incorrectly receive autogroup:self filter rules [#3036](https://github.com/juanfont/headscale/pull/3036) ## 0.28.0 (2026-02-04) diff --git a/hscontrol/policy/v2/tailscale_compat_test.go b/hscontrol/policy/v2/tailscale_compat_test.go index 31ae5be9..4a386362 100644 --- a/hscontrol/policy/v2/tailscale_compat_test.go +++ b/hscontrol/policy/v2/tailscale_compat_test.go @@ -10142,29 +10142,28 @@ func TestTailscaleCompatErrorCases(t *testing.T) { } } -// TestTailscaleCompatErrorCasesHeadscaleDiffers documents cases where Tailscale produces -// validation errors but Headscale does NOT. These represent compatibility gaps. +// TestTailscaleCompatErrorCasesHeadscaleDiffers validates that Headscale correctly rejects +// policies that Tailscale also rejects. These tests verify that autogroup:self destination +// validation for ACL rules matches Tailscale's behavior. // -// TODO: Tailscale validates that autogroup:self can only be used when ALL sources are -// users, groups, or autogroup:member. Headscale does NOT currently perform this validation -// for ACL rules (only for SSH rules). This means invalid policies like tag:client → autogroup:self:* -// will be accepted by Headscale but rejected by Tailscale. +// Tailscale validates that autogroup:self can only be used when ALL sources are +// users, groups, or autogroup:member. Headscale now performs this same validation. // // Reference: /home/kradalby/acl-explore/findings/09-mixed-scenarios.md. func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { t.Parallel() - // These tests document where Headscale behavior DIFFERS from Tailscale. + // These tests verify that Headscale rejects policies the same way Tailscale does. // Tailscale rejects these policies at validation time (400 Bad Request), - // but Headscale currently accepts them. + // and Headscale now does the same. tests := []struct { name string policy string - tailscaleError string // What Tailscale would return + tailscaleError string // What Tailscale returns (and Headscale should match) reference string }{ // Test 2.5: tag:client → autogroup:self:* + tag:server:22 - // TODO: Tailscale REJECTS this - autogroup:self requires user/group sources + // Tailscale REJECTS this - autogroup:self requires user/group sources { name: "tag_source_with_self_dest_2_5", policy: `{ @@ -10184,7 +10183,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 4.5: tag:client → autogroup:self:* - // TODO: Tailscale REJECTS this - autogroup:self requires user/group sources + // Tailscale REJECTS this - autogroup:self requires user/group sources { name: "tag_source_to_self_dest_only_4_5", policy: `{ @@ -10203,7 +10202,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 6.1: autogroup:tagged → autogroup:self:* - // TODO: Tailscale REJECTS this - autogroup:tagged is NOT a valid source for autogroup:self + // Tailscale REJECTS this - autogroup:tagged is NOT a valid source for autogroup:self { name: "autogroup_tagged_to_self_6_1", policy: `{ @@ -10222,7 +10221,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 9.5: [autogroup:member, autogroup:tagged] → [autogroup:self:*, tag:server:22] - // TODO: Tailscale REJECTS this - ANY invalid source (autogroup:tagged) invalidates the rule + // Tailscale REJECTS this - ANY invalid source (autogroup:tagged) invalidates the rule { name: "both_autogroups_to_self_plus_tag_9_5", policy: `{ @@ -10241,7 +10240,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 13.6: autogroup:tagged → self:* - // TODO: Tailscale REJECTS this - same as 6.1 + // Tailscale REJECTS this - same as 6.1 { name: "autogroup_tagged_to_self_13_6", policy: `{ @@ -10260,7 +10259,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 13.10: tag:client → self:* - // TODO: Tailscale REJECTS this - tags are not valid sources for autogroup:self + // Tailscale REJECTS this - tags are not valid sources for autogroup:self { name: "tag_to_self_13_10", policy: `{ @@ -10279,7 +10278,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 13.13: Host → self:* - // TODO: Tailscale REJECTS this - hosts are not valid sources for autogroup:self + // Tailscale REJECTS this - hosts are not valid sources for autogroup:self { name: "host_to_self_13_13", policy: `{ @@ -10301,7 +10300,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 13.14: Raw IP → self:* - // TODO: Tailscale REJECTS this - raw IPs are not valid sources for autogroup:self + // Tailscale REJECTS this - raw IPs are not valid sources for autogroup:self { name: "raw_ip_to_self_13_14", policy: `{ @@ -10320,7 +10319,7 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { }, // Test 13.25: [autogroup:member, tag:client] → self:* - // TODO: Tailscale REJECTS this - ANY invalid source (tag:client) invalidates the rule + // Tailscale REJECTS this - ANY invalid source (tag:client) invalidates the rule { name: "mixed_valid_invalid_sources_to_self_13_25", policy: `{ @@ -10343,16 +10342,15 @@ func TestTailscaleCompatErrorCasesHeadscaleDiffers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - pol, err := unmarshalPolicy([]byte(tt.policy)) - require.NoError(t, err, "test %s (%s): policy should parse", tt.name, tt.reference) - - // TODO: Headscale does NOT validate autogroup:self source restrictions for ACL rules. - // Tailscale would reject these policies with: %s - // For now, document that Headscale accepts these policies. - err = pol.validate() - require.NoError(t, err, - "test %s (%s): Headscale currently accepts this policy (Tailscale rejects with: %s)", - tt.name, tt.reference, tt.tailscaleError) + // unmarshalPolicy calls validate() internally, so we expect it to fail + // with our validation error + _, err := unmarshalPolicy([]byte(tt.policy)) + require.Error(t, err, + "test %s (%s): should reject policy like Tailscale", + tt.name, tt.reference) + require.ErrorIs(t, err, ErrACLAutogroupSelfInvalidSource, + "test %s (%s): expected autogroup:self validation error", + tt.name, tt.reference) }) } } diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index f92e99f0..b43aea7c 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -46,6 +46,11 @@ var ( ErrSSHWildcardDestination = errors.New("wildcard (*) is not supported as SSH destination") ) +// ACL validation errors. +var ( + ErrACLAutogroupSelfInvalidSource = errors.New("autogroup:self destination requires sources to be users, groups, or autogroup:member only") +) + type Asterix int func (a Asterix) Validate() error { @@ -1680,6 +1685,51 @@ func validateSSHSrcDstCombination(sources SSHSrcAliases, destinations SSHDstAlia return nil } +// validateACLSrcDstCombination validates that ACL source/destination combinations +// follow Tailscale's security model: +// - autogroup:self destinations require ALL sources to be users, groups, autogroup:member, or wildcard (*) +// - Tags, autogroup:tagged, hosts, and raw IPs are NOT valid sources for autogroup:self +// - Wildcard (*) is allowed because autogroup:self evaluation narrows it per-node to the node's own IPs. +func validateACLSrcDstCombination(sources Aliases, destinations []AliasWithPorts) error { + // Check if any destination is autogroup:self + hasAutogroupSelf := false + + for _, dst := range destinations { + if ag, ok := dst.Alias.(*AutoGroup); ok && ag.Is(AutoGroupSelf) { + hasAutogroupSelf = true + break + } + } + + if !hasAutogroupSelf { + return nil // No autogroup:self, no validation needed + } + + // Validate all sources are valid for autogroup:self + for _, src := range sources { + switch v := src.(type) { + case *Username, *Group, Asterix: + // Valid sources - users, groups, and wildcard (*) are allowed + // Wildcard is allowed because autogroup:self evaluation narrows it per-node + continue + case *AutoGroup: + if v.Is(AutoGroupMember) { + continue // autogroup:member is valid + } + // autogroup:tagged and others are NOT valid + return ErrACLAutogroupSelfInvalidSource + case *Tag, *Host, *Prefix: + // Tags, hosts, and IPs are NOT valid sources for autogroup:self + return ErrACLAutogroupSelfInvalidSource + default: + // Unknown type - be conservative and reject + return ErrACLAutogroupSelfInvalidSource + } + } + + return nil +} + // validate reports if there are any errors in a policy after // the unmarshaling process. // It runs through all rules and checks if there are any inconsistencies @@ -1762,6 +1812,12 @@ func (p *Policy) validate() error { if err := validateProtocolPortCompatibility(acl.Protocol, acl.Destinations); err != nil { errs = append(errs, err) } + + // Validate ACL source/destination combinations follow Tailscale's security model + err := validateACLSrcDstCombination(acl.Sources, acl.Destinations) + if err != nil { + errs = append(errs, err) + } } for _, ssh := range p.SSHs {