policy/v2: refuse address ranges as sshTests sources

A CIDR, or a hosts entry holding one, as an sshTests src is refused,
as on Tailscale SaaS. Destinations share the range check.

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,13 +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,
 
@@ -49,7 +42,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,6 +3423,33 @@ 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:
 //
@@ -3430,6 +3462,10 @@ func validateSSHTests(pol *Policy, tests []SSHPolicyTest) error {
 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;
@@ -3438,14 +3474,6 @@ func validateSSHTestDestination(pol *Policy, alias Alias) error {
 			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() {
-			return fmt.Errorf("%w %q", ErrSSHTestDstDisallowedElement, dst)
-		}
-
 	case *Tag:
 		// A tag must be declared in tagOwners. `tag:server:22` lands
 		// here too because isTag only checks the prefix, so the lookup
@@ -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: `

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"