From 170ae26ad7250b8cede78f05733e7f6edc096dc0 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 21 Oct 2026 18:37:12 +0000 Subject: [PATCH] policy/v2: refuse check SSH rules with tag sources A check asks the source's user to log in again, and a tagged node has none. Tailscale SaaS refuses the rule; headscale accepted it. --- .../v2/tailscale_ssh_data_compat_test.go | 1 - hscontrol/policy/v2/types.go | 12 ++++++++ hscontrol/policy/v2/types_test.go | 30 +++++++++++++++++++ hscontrol/servertest/policy_test.go | 2 -- 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go index e9f4c11a3..b5cf20511 100644 --- a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go @@ -139,7 +139,6 @@ var sshRejectSkipReasons = map[string]string{ "ssh-e2": "headscale has no associated-tailnet-domains config; user:*@domain / localpart:*@domain are not domain-validated", "ssh-malformed-user-localpart-multi-glob": "headscale has no associated-tailnet-domains config; user:*@domain / localpart:*@domain are not domain-validated", - "ssh-check-tag-src/refused": "SSH_CHECK_TAG_SRC: SaaS refuses a check rule with a tag source; headscale accepts it", "ssh-malformed-checkperiod-too-long/message": "SSH_CHECKPERIOD_TEXT: SaaS quotes a refused checkPeriod as written; headscale prints the parsed duration", } diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 495bc1e8c..2b6d29793 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -57,6 +57,7 @@ var ( ErrSSHActionMustBeSpecified = errors.New("action must be specified") ErrSSHActionInvalid = errors.New("is not a valid action") ErrSSHDestinationHostAlias = errors.New("invalid dst") + ErrSSHCheckTagSource = errors.New(`[ssh] "check" action does not support tags in src`) ErrTagNameMustStartWithLetter = errors.New("tag names must start with a letter, after 'tag:'") ErrGroupMembersCannotBeRecursive = errors.New("group members cannot be recursive") ) @@ -2592,6 +2593,17 @@ func (p *Policy) validate() error { } } + // A check asks the source's user to log in again, and a tagged + // node has none; Tailscale SaaS refuses this too. + if ssh.Action == SSHActionCheck && slices.ContainsFunc(ssh.Sources, func(a Alias) bool { + _, isTag := a.(*Tag) + ag, _ := a.(*AutoGroup) + + return isTag || ag.Is(AutoGroupTagged) + }) { + errs = append(errs, ErrSSHCheckTagSource) + } + for _, dst := range ssh.Destinations { switch dst := dst.(type) { case *AutoGroup: diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 44d02fca2..a2996e96b 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -1547,6 +1547,36 @@ func TestUnmarshalPolicy(t *testing.T) { wantErr: `tagOwners["tag:child"]: "tag:nonexistent": does not exist`, }, // SSH source/destination validation tests (#3009, #3010) + { + name: "ssh-check-tag-src-rejected", + input: ` +{ + "tagOwners": {"tag:client": ["admin@"], "tag:server": ["admin@"]}, + "ssh": [{ + "action": "check", + "src": ["admin@", "tag:client"], + "dst": ["tag:server"], + "users": ["root"] + }] +} +`, + wantErr: `[ssh] "check" action does not support tags in src`, + }, + { + name: "ssh-check-autogroup-tagged-src-rejected", + input: ` +{ + "tagOwners": {"tag:server": ["admin@"]}, + "ssh": [{ + "action": "check", + "src": ["admin@", "autogroup:tagged"], + "dst": ["tag:server"], + "users": ["root"] + }] +} +`, + wantErr: `[ssh] "check" action does not support tags in src`, + }, { name: "ssh-tag-to-user-rejected", input: ` diff --git a/hscontrol/servertest/policy_test.go b/hscontrol/servertest/policy_test.go index 7279aee72..70bcb9444 100644 --- a/hscontrol/servertest/policy_test.go +++ b/hscontrol/servertest/policy_test.go @@ -319,8 +319,6 @@ var rejectedPolicySkipReasons = map[string]string{ "acl-err10/refused": "ROUTE_HOST_BITS: headscale accepts an autoApprovers route with host bits", "acl-err11/refused": "ROUTE_HOST_BITS: headscale accepts an autoApprovers route with host bits", - - "ssh-check-tag-src/refused": "SSH_CHECK_TAG_SRC: headscale accepts a check rule with a tag source", } const noTailnetDomains = "NO_TAILNET_DOMAINS: headscale does not check user:*@domain or localpart:*@domain against the tailnet's domains"