From 5d6648babf7cec7f1ef90465db96857560e3a95f Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 21 Oct 2026 16:41:36 +0000 Subject: [PATCH] policy/v2: write an ACL's repeated destination once An ACL that names a destination and port twice wrote the pair twice. Grants keep their repeats, as on Tailscale SaaS. --- .../v2/tailscale_acl_data_compat_test.go | 13 +------- .../v2/tailscale_sequence_compat_test.go | 7 ---- hscontrol/policy/v2/types.go | 33 +++++++++++++++++++ 3 files changed, 34 insertions(+), 19 deletions(-) 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_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: