From 0bfae13d6db7d58aa6faed483841941da68ea8fb Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 21 Oct 2026 16:20:36 +0000 Subject: [PATCH] policy/v2: normalize emitted ACL and via destinations Write single-address via destinations bare. Deduplicate repeated ACL destination/port pairs while preserving grant duplicates. --- hscontrol/policy/v2/compiled.go | 2 +- hscontrol/policy/v2/filter.go | 22 ++++++++----- .../v2/tailscale_acl_data_compat_test.go | 13 +------- .../policy/v2/tailscale_grants_compat_test.go | 5 --- .../v2/tailscale_sequence_compat_test.go | 7 ---- hscontrol/policy/v2/types.go | 33 +++++++++++++++++++ 6 files changed, 48 insertions(+), 34 deletions(-) diff --git a/hscontrol/policy/v2/compiled.go b/hscontrol/policy/v2/compiled.go index 514edd1b0..90450cee7 100644 --- a/hscontrol/policy/v2/compiled.go +++ b/hscontrol/policy/v2/compiled.go @@ -1042,7 +1042,7 @@ func compileViaForNode( destPorts = append( destPorts, tailcfg.NetPortRange{ - IP: prefix.String(), + IP: filterIP(prefix), Ports: port, }, ) diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go index 54d93b4fb..53dc7e915 100644 --- a/hscontrol/policy/v2/filter.go +++ b/hscontrol/policy/v2/filter.go @@ -173,16 +173,10 @@ func (pol *Policy) destinationsToNetPortRange( for _, pref := range prefixes { for _, port := range ports { - pr := tailcfg.NetPortRange{ - IP: pref.String(), + ret = append(ret, tailcfg.NetPortRange{ + IP: filterIP(pref), Ports: port, - } - // Drop the prefix bits if its a single IP. - if pref.IsSingleIP() { - pr.IP = pref.Addr().String() - } - - ret = append(ret, pr) + }) } } } @@ -190,6 +184,16 @@ func (pol *Policy) destinationsToNetPortRange( return ret } +// filterIP writes pref as a filter destination: a single address bare, +// any other prefix with its bits, as Tailscale SaaS writes them. +func filterIP(pref netip.Prefix) string { + if pref.IsSingleIP() { + return pref.Addr().String() + } + + return pref.String() +} + // compileFilterRulesForNode compiles filter rules for a specific node. func (pol *Policy) compileFilterRulesForNode( users types.Users, diff --git a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go index c4a4b32c8..3af51c138 100644 --- a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go @@ -216,25 +216,14 @@ var aclSkipReasons = map[string]string{ "acl-n01/venusaur/filter": allowAllForm, "acl-n01/weedle/filter": allowAllForm, - "acl-dup-src-dst/beedrill/filter": aclDstDedupe, - "acl-dup-src-dst/bulbasaur/filter": aclDstDedupe, - "acl-dup-src-dst/charmander/filter": aclDstDedupe, - "acl-dup-src-dst/ivysaur/filter": aclDstDedupe, - "acl-dup-src-dst/kakuna/filter": aclDstDedupe, - "acl-dup-src-dst/squirtle/filter": aclDstDedupe, - "acl-dup-src-dst/venusaur/filter": aclDstDedupe, - "acl-dup-src-dst/weedle/filter": aclDstDedupe, - "acl-dxp-dup-ports/beedrill/filter": aclDstDedupe, - "acl-self-plus-dsts/kakuna/filter": srcAddressForm, - "acl-self-plus-dsts/beedrill/filter": "FILTER_ADDRESS_FORM+ACL_DST_DEDUPE: sources listed one by one, a repeated destination written once", + "acl-self-plus-dsts/beedrill/filter": srcAddressForm, "acl-self-plus-dsts/bulbasaur/filter": "FILTER_ADDRESS_FORM+SELF_DST_REPEAT: sources listed one by one, the autogroup:self destination written once per source alias", } const ( routeHostBits = "ROUTE_HOST_BITS: SaaS refuses an autoApprovers route with host bits; headscale accepts it" allowAllForm = "ALLOW_ALL_FORM: SaaS writes the allow-all default's sources as the tailnet ranges, headscale as \"*\"" - aclDstDedupe = "ACL_DST_DEDUPE: SaaS writes a destination repeated within one ACL once, headscale once per mention" srcAddressForm = "FILTER_ADDRESS_FORM: SaaS lists contiguous source addresses one by one, headscale as a range" ) diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index c084b4344..a8799ef81 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -172,9 +172,6 @@ var grantSkipReasons = map[string]string{ // Checks keyed //filter: a listed check that matches // fails, so a fix must delete its entry. - "via-grant-v63/blastoise/filter": viaDstAddressForm, - "via-grant-v63/squirtle/filter": viaDstAddressForm, - "grant-dxp-comma-ports/refused": grantPortList, // Contiguous pinned addresses; TestViaGrantHACompat compares these @@ -195,8 +192,6 @@ var grantSkipReasons = map[string]string{ const grantPortList = "GRANT_PORT_LIST: SaaS refuses a comma port list in a grant ip entry (only one port range allowed); headscale accepts it" -const viaDstAddressForm = "FILTER_ADDRESS_FORM: SaaS writes a single-address via destination as a bare address, headscale as a /32 prefix" - // TestGrantsCompat is a data-driven test that loads all GRANT-*.json // test files captured from Tailscale SaaS and compares headscale's grants // engine output against the real Tailscale behavior. diff --git a/hscontrol/policy/v2/tailscale_sequence_compat_test.go b/hscontrol/policy/v2/tailscale_sequence_compat_test.go index b02d898bf..81d19e149 100644 --- a/hscontrol/policy/v2/tailscale_sequence_compat_test.go +++ b/hscontrol/policy/v2/tailscale_sequence_compat_test.go @@ -34,13 +34,6 @@ import ( // keyed ///. A listed check that matches fails, // so a fix must delete its entry. var policySequenceSkipReasons = map[string]string{ - "policy-generation/G0/srv/filter": aclDstDedupe, - "policy-generation/G1/srv/filter": aclDstDedupe, - "policy-generation/G2/srv/filter": aclDstDedupe, - "policy-generation/G3/srv/filter": aclDstDedupe, - "policy-generation/G4/srv/filter": aclDstDedupe, - "policy-generation/G5/srv/filter": aclDstDedupe, - "policy-generation/G0/rt/filter": viaRuleMerged, "policy-generation/G1/rt/filter": viaRuleMerged, "policy-generation/G2/rt/filter": viaRuleMerged, diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 7ff9f564a..fadf5dcf2 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -1886,6 +1886,7 @@ type NodeAttrGrant struct { // aclToGrants converts an [ACL] rule to one or more equivalent [Grant] rules. func aclToGrants(acl ACL) []Grant { + acl.Destinations = uniqueDestinations(acl.Destinations) ret := make([]Grant, 0, len(acl.Destinations)) // Check if the ACL has any non-autogroup destinations. If so, @@ -1952,6 +1953,38 @@ func aclToGrants(acl ACL) []Grant { return ret } +// uniqueDestinations keeps each destination and port range of one ACL the +// first time it is named, as Tailscale SaaS writes a repeated one once. +// Grants keep their repeats, as there. +func uniqueDestinations(dsts []AliasWithPorts) []AliasWithPorts { + type named struct { + alias string + ports tailcfg.PortRange + } + + seen := make(map[named]bool) + out := make([]AliasWithPorts, 0, len(dsts)) + + for _, dst := range dsts { + var ports []tailcfg.PortRange + + for _, p := range dst.Ports { + k := named{dst.String(), p} + if !seen[k] { + seen[k] = true + + ports = append(ports, p) + } + } + + if len(ports) > 0 || len(dst.Ports) == 0 { + out = append(out, AliasWithPorts{Alias: dst.Alias, Ports: ports}) + } + } + + return out +} + // Policy represents a Tailscale Network Policy. // TODO(kradalby): // Add validation method checking: