From a79fb203724cee26b979e7267f3443dad014a517 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 13 May 2026 10:04:39 +0000 Subject: [PATCH] policy/v2: reject hosts-table aliases as SSH dst SaaS rejects any hosts-table alias on an SSH rule dst with `invalid dst "alias"`, irrespective of whether the alias resolves to a single IP or a CIDR. headscale was resolving the alias through the same path as ACL dsts and accepting the policy. Validate at the per-SSH-rule pass so the error body matches SaaS. The existing host-alias-as-dst sshtest_test row tested the now-rejected shape and is dropped along with its unused commonHosts fixture. --- hscontrol/policy/v2/sshtest_test.go | 22 ------------------- .../v2/tailscale_ssh_data_compat_test.go | 12 ---------- hscontrol/policy/v2/types.go | 8 +++++++ hscontrol/policy/v2/types_test.go | 16 +++++++++++++- 4 files changed, 23 insertions(+), 35 deletions(-) diff --git a/hscontrol/policy/v2/sshtest_test.go b/hscontrol/policy/v2/sshtest_test.go index 98253d4a..1fa2dd05 100644 --- a/hscontrol/policy/v2/sshtest_test.go +++ b/hscontrol/policy/v2/sshtest_test.go @@ -83,10 +83,6 @@ func sshTestNodes(users types.Users) types.Nodes { } } -// commonHosts contains every host alias the table below references -// as an sshTests dst. Embedded in the JSON policy under "hosts". -const commonHosts = `"hosts": { "alice-laptop": "100.64.0.1", "bob-laptop": "100.64.0.2" }` - // TestRunSSHTests covers the engine's per-test outcome reporting. Each // row constructs a PolicyManager (whose SetPolicy sandbox also exercises // evaluateSSHTests) and asserts on the resulting RunSSHTests behaviour. @@ -563,24 +559,6 @@ func TestRunSSHTests(t *testing.T) { }`, wantPass: true, }, - { - name: "host-alias-as-dst", - policy: `{ - ` + commonHosts + `, - "ssh": [{ - "action": "accept", - "src": ["bob@headscale.net"], - "dst": ["alice-laptop"], - "users": ["root"] - }], - "sshTests": [{ - "src": "bob@headscale.net", - "dst": ["alice-laptop"], - "accept": ["root"] - }] - }`, - wantPass: true, - }, { name: "acl-allows-tcp22-no-ssh-rule", policy: `{ diff --git a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go index a0bdc171..54d908bd 100644 --- a/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_ssh_data_compat_test.go @@ -107,18 +107,6 @@ var sshRejectSkipReasons = map[string]string{ "ssh-e2": "domain validation: headscale has no 'associated tailnet domains' concept", "ssh-malformed-user-localpart-multi-glob": "domain validation: headscale has no 'associated tailnet domains' concept (same gap as ssh-b4/d1/e1/e2)", - // HOSTS_ALIAS_AS_SSH_DST (2 tests) - // - // SaaS rejects every hosts-alias dst on an SSH rule with - // `invalid dst "alias"`, regardless of whether the alias resolves - // to a single IP or a CIDR. headscale's compileSSHPolicy resolves - // host aliases through the same path as ACL dsts and accepts - // them. Either reject hosts-aliases at SSH dst parse time to - // match, or document that headscale is intentionally more - // permissive than SaaS for SSH dsts. - "ssh-hosts-as-dst-single-ip": "SaaS rejects host alias as SSH dst, headscale accepts", - "ssh-hosts-as-dst-multi-host-prefix": "SaaS rejects host alias as SSH dst, headscale accepts", - // NON_ASCII_TAG_NAME (1 test) // // SaaS rejects non-ASCII characters in tag names with diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 2c882224..b43415c9 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -52,6 +52,7 @@ var ( ErrSSHAcceptEnvEmpty = errors.New("acceptEnv values cannot be empty") ErrSSHActionMustBeSpecified = errors.New("action must be specified") ErrSSHActionInvalid = errors.New("is not a valid action") + ErrSSHDestinationHostAlias = errors.New("invalid dst") ) // SSH check period constants per Tailscale docs: @@ -2579,6 +2580,13 @@ func (p *Policy) validate() error { if err != nil { errs = append(errs, err) } + case *Host: + // SaaS rejects every hosts-table alias on an SSH + // dst with `invalid dst "alias"`, whether the + // alias resolves to a single IP or a CIDR. The + // equivalent ACL rule accepts the same aliases, + // so reject here rather than at parse time. + errs = append(errs, fmt.Errorf("%w %q", ErrSSHDestinationHostAlias, string(*dst))) } } diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 24a14ec3..46a75131 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -4660,11 +4660,25 @@ func TestSSHRuleSaaSValidation(t *testing.T) { s.AcceptEnv = []string{"**"} }), }, + { + // SaaS rejects hosts-table aliases on SSH dst with + // `invalid dst "srv"`. headscale validates the same + // regardless of whether the alias resolves to a + // single IP or a CIDR. + name: "host alias as SSH dst rejected", + ssh: baseSSH(func(s *SSH) { + s.Destinations = SSHDstAliases{hp("srv")} + }), + wantErr: ErrSSHDestinationHostAlias, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - pol := &Policy{SSHs: []SSH{tt.ssh}} + pol := &Policy{ + Hosts: Hosts{Host("srv"): Prefix(mp("100.64.0.16/32"))}, + SSHs: []SSH{tt.ssh}, + } err := pol.validate() if tt.wantErr != nil {