policy/v2: validate SSH test sources and destinations like SaaS

Reject address-range sources and autogroup:member destinations.

hscontrol/policy/v2/sshtest_test.go

@@ -748,32 +748,9 @@ func TestRunSSHTests(t *testing.T) {
 			wantPass:   false,
 			wantErrSub: []string{"autogroup:self", "resolved to no nodes"},
 		},
-		{
-			// A src naming address space is evaluated through the node
-			// addresses inside it, never address by address: ::/0
-			// holds alice's IPv6, which the rule allows.
-			name: "src-cidr-deny-fails-on-covered-node",
-			policy: `{
-				"tagOwners": { "tag:server": ["alice@headscale.net"] },
-				"ssh": [{
-					"action": "accept",
-					"src":    ["alice@headscale.net"],
-					"dst":    ["tag:server"],
-					"users":  ["root"]
-				}],
-				"sshTests": [{
-					"src":  "::/0",
-					"dst":  ["tag:server"],
-					"deny": ["root"]
-				}]
-			}`,
-			wantPass:   false,
-			wantErrSub: []string{"::/0", "expected DENIED, got ALLOWED"},
-		},
 		{
 			name: "src-address-space-deny-pass",
 			policy: `{
-				"hosts":     { "lan": "10.0.0.0/8" },
 				"tagOwners": { "tag:server": ["alice@headscale.net"] },
 				"ssh": [{
 					"action": "accept",
@@ -784,8 +761,7 @@ func TestRunSSHTests(t *testing.T) {
 				"sshTests": [
 					{ "src": "*",                    "dst": ["tag:server"], "deny": ["ubuntu"] },
 					{ "src": "autogroup:danger-all", "dst": ["tag:server"], "deny": ["ubuntu"] },
-					{ "src": "autogroup:internet",   "dst": ["tag:server"], "deny": ["root"] },
-					{ "src": "lan",                  "dst": ["tag:server"], "deny": ["root"] }
+					{ "src": "autogroup:internet",   "dst": ["tag:server"], "deny": ["root"] }
 				]
 			}`,
 			wantPass: true,
@@ -811,25 +787,6 @@ func TestRunSSHTests(t *testing.T) {
 			wantPass:   false,
 			wantErrSub: []string{"expected ALLOWED, got DENIED"},
 		},
-		{
-			// 100.64.0.4/31 is exactly alice-tablet and thor-laptop.
-			name: "src-cidr-fully-assigned-accept-pass",
-			policy: `{
-				"tagOwners": { "tag:server": ["alice@headscale.net"] },
-				"ssh": [{
-					"action": "accept",
-					"src":    ["autogroup:member"],
-					"dst":    ["tag:server"],
-					"users":  ["root"]
-				}],
-				"sshTests": [{
-					"src":    "100.64.0.4/31",
-					"dst":    ["tag:server"],
-					"accept": ["root"]
-				}]
-			}`,
-			wantPass: true,
-		},
 	}
 
 	for _, tt := range tests {

hscontrol/policy/v2/sshtester_compat_test.go

@@ -31,16 +31,6 @@ var knownSSHTesterDivergences = map[string]string{
 	"sshtest-report-multi/label":                       sshTestNodeLabel,
 	"sshtest-src-unassigned-ip-accept/label":           sshTestNodeLabel,
 
-	"sshtest-src-cidr24-deny/message":           sshTestSrcRange,
-	"sshtest-src-cidr31-empty-accept/message":   sshTestSrcRange,
-	"sshtest-src-cidr31-empty-deny/verdict":     sshTestSrcRange,
-	"sshtest-src-cidr31-hole-accept/message":    sshTestSrcRange,
-	"sshtest-src-cidr31-members-accept/verdict": sshTestSrcRange,
-	"sshtest-src-hosts-cidr-deny/verdict":       sshTestSrcRange,
-
-	"sshtest-fail-label-autogroup-member/message": sshTestDstMember,
-	"sshtest-check-reason-mixed-alias/message":    sshTestDstMember,
-
 	"sshtest-fail-label-same-node-twice/breakdown": "SSHTEST_PER_ALIAS: SaaS reports each dst alias, also two naming one node; headscale checks each node once",
 	"sshtest-fail-label-ip-no-device/breakdown":    sshTestNoDevice,
 	"sshtest-report-multi/breakdown":               "SSHTEST_PER_ALIAS+SSHTEST_NO_DEVICE: SaaS reports one line per dst alias and a deviceless alias as not reached; headscale one per node and an error",
@@ -49,8 +39,6 @@ var knownSSHTesterDivergences = map[string]string{
 
 const (
 	sshTestNodeLabel = "SSHTEST_NODE_LABEL: headscale names each failing dst node by hostname; SaaS names the dst as written"
-	sshTestSrcRange  = "SSHTEST_SRC_RANGE: SaaS refuses a CIDR, or a host alias holding one, as an sshTests source when parsing; headscale evaluates it"
-	sshTestDstMember = "SSHTEST_DST_MEMBER: SaaS refuses autogroup:member as an sshTests destination when parsing; headscale evaluates it"
 	sshTestNoDevice  = "SSHTEST_NO_DEVICE: SaaS reports a dst that selects no device as not reached; headscale as an error"
 )
 

hscontrol/policy/v2/types.go

@@ -156,6 +156,7 @@ var (
 	ErrSSHTestEmptyDst             = errors.New("SSH tests entry must have at least one dst")
 	ErrSSHTestDstUnknownTag        = errors.New("SSH tests dst contains unknown tag")
 	ErrSSHTestDstDisallowedElement = errors.New("SSH tests dst contains disallowed element")
+	ErrSSHTestSrcDisallowedElement = errors.New("SSH tests src contains disallowed element")
 )
 
 type resolved struct {
@@ -3397,6 +3398,10 @@ func validateSSHTests(pol *Policy, tests []SSHPolicyTest) error {
 	for i, t := range tests {
 		if t.Src == nil {
 			errs = append(errs, fmt.Errorf("sshTest %d: %w", i, ErrSSHTestEmptySrc))
+		} else if isAddressRange(pol, t.Src) {
+			// Tailscale SaaS refuses a range: a test speaks for one source.
+			errs = append(errs, fmt.Errorf("sshTest %d: %w %q",
+				i, ErrSSHTestSrcDisallowedElement, t.Src.String()))
 		}
 
 		if len(t.Dst) == 0 {
@@ -3418,31 +3423,54 @@ func validateSSHTests(pol *Policy, tests []SSHPolicyTest) error {
 	return nil
 }
 
+// isAddressRange reports whether alias is a CIDR, or a hosts: entry
+// holding one, that names more than one address. A bare IP parses as a
+// `/BitLen` prefix and names one.
+func isAddressRange(pol *Policy, alias Alias) bool {
+	var p netip.Prefix
+
+	switch a := alias.(type) {
+	case *Prefix:
+		p = netip.Prefix(*a)
+	case *Host:
+		if pol == nil {
+			return false
+		}
+
+		pref, ok := pol.Hosts[*a]
+		if !ok {
+			return false
+		}
+
+		p = netip.Prefix(pref)
+	default:
+		return false
+	}
+
+	return p.Bits() < p.Addr().BitLen()
+}
+
 // validateSSHTestDestination rejects sshTests dst shapes that cannot
 // name a single SSH-reachable host:
 //
 //   - `host:port` suffixes (parsed as an unknown tag),
 //   - multi-host CIDRs (raw `/N` or a hosts: entry resolving wider),
-//   - autogroup:internet (valid as ACL dst only).
+//   - autogroup:internet (valid as ACL dst only),
+//   - autogroup:member, which Tailscale SaaS refuses here.
 //
 // A bare IP literal (single-host /BitLen prefix) is accepted. Tag
 // entries must exist in tagOwners.
 func validateSSHTestDestination(pol *Policy, alias Alias) error {
 	dst := alias.String()
 
+	if isAddressRange(pol, alias) {
+		return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst)
+	}
+
 	switch a := alias.(type) {
 	case *AutoGroup:
-		// autogroup:internet is the only autogroup not valid here;
-		// member/tagged/self/nonroot pass to engine evaluation.
-		if *a == AutoGroupInternet {
-			return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst)
-		}
-
-	case *Prefix:
-		// A bare IP parses as `/BitLen` and is a valid single-host dst;
-		// any narrower CIDR is a multi-host range and is rejected.
-		p := netip.Prefix(*a)
-		if p.Bits() < p.Addr().BitLen() {
+		// tagged/self/nonroot pass to engine evaluation.
+		if *a == AutoGroupInternet || *a == AutoGroupMember {
 			return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst)
 		}
 
@@ -3458,23 +3486,6 @@ func validateSSHTestDestination(pol *Policy, alias Alias) error {
 		if err != nil {
 			return fmt.Errorf("%w %q", ErrSSHTestDstUnknownTag, string(*a))
 		}
-
-	case *Host:
-		// A hosts: alias that resolves to multiple addresses is a CIDR
-		// in disguise.
-		if pol == nil {
-			return nil
-		}
-
-		pref, ok := pol.Hosts[*a]
-		if !ok {
-			return nil
-		}
-
-		p := netip.Prefix(pref)
-		if p.Bits() < p.Addr().BitLen() {
-			return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst)
-		}
 	}
 
 	return nil

hscontrol/policy/v2/types_test.go

@@ -6032,6 +6032,42 @@ func TestUnmarshalPolicySSHTests(t *testing.T) {
 				require.Equal(t, "fd7a:115c:a1e0::10/128", pref.String())
 			},
 		},
+		{
+			name: "src-cidr",
+			input: `
+{
+  "tagOwners": {"tag:server": ["admin@example.org"]},
+  "sshTests": [
+    {"src": "100.64.0.0/24", "dst": ["tag:server"], "deny": ["root"]}
+  ]
+}
+`,
+			wantErr: ErrSSHTestSrcDisallowedElement,
+		},
+		{
+			name: "src-hosts-cidr",
+			input: `
+{
+  "hosts": {"lan": "192.168.1.0/24"},
+  "tagOwners": {"tag:server": ["admin@example.org"]},
+  "sshTests": [
+    {"src": "lan", "dst": ["tag:server"], "deny": ["root"]}
+  ]
+}
+`,
+			wantErr: ErrSSHTestSrcDisallowedElement,
+		},
+		{
+			name: "src-bare-ip-accepted",
+			input: `
+{
+  "tagOwners": {"tag:server": ["admin@example.org"]},
+  "sshTests": [
+    {"src": "100.64.0.30", "dst": ["tag:server"], "accept": ["root"]}
+  ]
+}
+`,
+		},
 		{
 			name: "dst-autogroup-internet",
 			input: `
@@ -6041,6 +6077,18 @@ func TestUnmarshalPolicySSHTests(t *testing.T) {
     {"src": "thor@example.org", "dst": ["autogroup:internet"], "accept": ["root"]}
   ]
 }
+`,
+			wantErr: ErrSSHTestDstDisallowedElement,
+		},
+		{
+			name: "dst-autogroup-member",
+			input: `
+{
+  "tagOwners": {"tag:server": ["admin@example.org"]},
+  "sshTests": [
+    {"src": "thor@example.org", "dst": ["autogroup:member"], "accept": ["root"]}
+  ]
+}
 `,
 			wantErr: ErrSSHTestDstDisallowedElement,
 		},

hscontrol/servertest/policy_test.go

@@ -327,10 +327,6 @@ var rejectedPolicySkipReasons = map[string]string{
 	"policytest-src-cidr-accept/refused":              "TEST_SRC_SHAPE: headscale evaluates a CIDR tests source",
 	"policytest-src-wildcard-deny/refused":            "TEST_SRC_SHAPE: headscale evaluates \"*\" as a tests source",
 	"policytest-dst-multinode-partial-accept/refused": "TEST_DST_EVERY_NODE: headscale accepts when one node of the dst alias is reached",
-
-	"sshtest-src-cidr31-empty-deny/refused":     "SSHTEST_SRC_RANGE: headscale evaluates a CIDR sshTests source",
-	"sshtest-src-cidr31-members-accept/refused": "SSHTEST_SRC_RANGE: headscale evaluates a CIDR sshTests source",
-	"sshtest-src-hosts-cidr-deny/refused":       "SSHTEST_SRC_RANGE: headscale evaluates a host alias holding a CIDR as an sshTests source",
 }
 
 const noTailnetDomains = "NO_TAILNET_DOMAINS: headscale does not check user:*@domain or localpart:*@domain against the tailnet's domains"

integration/cli_policy_test.go

@@ -201,8 +201,8 @@ func TestPolicyCheckCommand(t *testing.T) {
 
 // TestSSHTestsRejectFailingPolicy asserts `headscale policy set` rejects
 // a policy whose sshTests fail, surfaces the engine's "test(s) failed"
-// sentinel, and leaves the stored policy unchanged. autogroup:member as
-// dst lets every scenario node count, so no tagged node is needed.
+// sentinel, and leaves the stored policy unchanged. The tests name user1's
+// node by its user, so no tagged node is needed.
 func TestSSHTestsRejectFailingPolicy(t *testing.T) {
 	IntegrationSkip(t)
 
@@ -226,7 +226,7 @@ func TestSSHTestsRejectFailingPolicy(t *testing.T) {
 		SSHTests: []policyv2.SSHPolicyTest{
 			{
 				Src:    usernamep(user1),
-				Dst:    policyv2.SSHTestDestinations{new(policyv2.AutoGroupMember)},
+				Dst:    policyv2.SSHTestDestinations{usernamep(user1)},
 				Accept: []policyv2.SSHUser{policyv2.SSHUser("root")},
 			},
 		},
@@ -238,7 +238,7 @@ func TestSSHTestsRejectFailingPolicy(t *testing.T) {
 	badPolicy.SSHTests = []policyv2.SSHPolicyTest{
 		{
 			Src:    usernamep(user2),
-			Dst:    policyv2.SSHTestDestinations{new(policyv2.AutoGroupMember)},
+			Dst:    policyv2.SSHTestDestinations{usernamep(user1)},
 			Accept: []policyv2.SSHUser{policyv2.SSHUser("root")},
 		},
 	}