From e4e209f9191a1f071b557fe6d0b9bdedbb380bbf Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 29 Apr 2026 12:25:01 +0000 Subject: [PATCH] policy/v2: canonicalize Protocol form during unmarshal Tailscale accepts both named ("tcp") and numeric IANA ("6") protocol forms wherever a Protocol value is allowed. Headscale stored whichever form the user wrote, leaving downstream code with two equivalents to handle separately. validateProtocolPortCompatibility only recognised the named constants and rejected the numeric form, so a policy with `proto: "6", dst: ["host:443"]` was rejected at parse time even though SaaS accepts it. Resolve the disagreement by normalising to the named form during Protocol.UnmarshalJSON. Every downstream consumer now sees one form regardless of what the user wrote, so layered guards like `|| protocol == "6"` in the validator are unnecessary. Updates #1803 --- .../policy/v2/policytester_compat_test.go | 4 +-- hscontrol/policy/v2/types.go | 16 ++++++++- hscontrol/policy/v2/types_test.go | 36 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/hscontrol/policy/v2/policytester_compat_test.go b/hscontrol/policy/v2/policytester_compat_test.go index ba0a2b1f..9feb1537 100644 --- a/hscontrol/policy/v2/policytester_compat_test.go +++ b/hscontrol/policy/v2/policytester_compat_test.go @@ -37,9 +37,7 @@ import ( // Each entry is a real bug to fix in a follow-up; documenting them here // keeps the compat suite green and the divergence list visible. var knownPolicyTesterDivergences = map[string]string{ //nolint:gosec // strings here are human-readable notes, not credentials - "policytest-allpass-acls-and-grants-mixed": "evaluator denies tag:client → webserver:80 in mixed acls+grants policy; SaaS accepts (Updates #1803)", - "policytest-proto-numeric": "validateProtocolPortCompatibility rejects numeric proto \"6\" with specific ports; SaaS accepts (Updates #1803)", - "policytest-accept-fail-proto-numeric-mismatch": "validateProtocolPortCompatibility rejects numeric proto \"6\" with specific ports; SaaS accepts (Updates #1803)", + "policytest-allpass-acls-and-grants-mixed": "evaluator denies tag:client → webserver:80 in mixed acls+grants policy; SaaS accepts (Updates #1803)", } // policyTesterCompatUsers / policyTesterCompatNodes mirror the small diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 435616ba..5d305075 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -1756,13 +1756,27 @@ func (p *Protocol) toIANAProtocolNumbers() []int { } // UnmarshalJSON implements JSON unmarshaling for Protocol. +// +// Tailscale accepts both named ("tcp") and numeric IANA ("6") forms. +// Storing whichever form the user wrote leaves downstream code with +// two equivalents to handle separately, and any consumer that +// branches on the named form would silently mishandle the numeric +// equivalent. Canonicalising to the named form here makes Protocol +// hold one value post-parse — every downstream consumer sees the +// same form regardless of what the user wrote. func (p *Protocol) UnmarshalJSON(b []byte) error { str := strings.Trim(string(b), `"`) // Normalize to lowercase for case-insensitive matching *p = Protocol(strings.ToLower(str)) - // Validate the protocol + num, atoiErr := strconv.Atoi(string(*p)) + if atoiErr == nil && num >= 0 && num <= 255 { + if name, ok := ProtocolNumberToName[num]; ok { + *p = name + } + } + err := p.validate() if err != nil { return err diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index e9e024b3..55350c03 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -2187,6 +2187,42 @@ func TestUnmarshalPolicy(t *testing.T) { `, wantErr: `test destination must be a single host, not a CIDR range`, }, + // Tailscale accepts numeric IANA protocol form ("6", "17", + // "132") wherever the named form is allowed, including with + // specific ports. validateProtocolPortCompatibility today only + // recognises the named constants and rejects the numeric form. + { + name: "protocol-numeric-tcp-with-specific-port-allowed", + input: ` +{ + "acls": [ + { + "action": "accept", + "proto": "6", + "src": ["*"], + "dst": ["*:443"] + } + ] +} +`, + want: &Policy{ + ACLs: []ACL{ + { + Action: "accept", + Protocol: "tcp", + Sources: Aliases{ + Wildcard, + }, + Destinations: []AliasWithPorts{ + { + Alias: Wildcard, + Ports: []tailcfg.PortRange{{First: 443, Last: 443}}, + }, + }, + }, + }, + }, + }, } cmps := append(util.Comparers,