Repository: juanfont/headscale

Issue:
[Bug] v0.26 policy: reduce routes sent to peers based on packetfilter #2561
### Is this a support request?

- [x] This is not a support request

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current Behavior

Hi,

v0.26 introduced this feature:
policy: reduce routes sent to peers based on packetfilter https://github.com/juanfont/headscale/pull/2561
described in this bug report: https://github.com/juanfont/headscale/issues/2365

Assume the following situation:

Node A (100.123.45.67) is a Router (part of group routers). Network 192.168.1.0/14 is connected to this Router.
Node B (100.123.45.89) is a regular Node.

Policy in place (simplified):
```json
{
  "acls": [
    {
      "action": "accept",
      "src": [
        "192.168.1.0/24",
        "group:routers"
      ],
      "dst": [
        "*:*"
      ]
    }
  ]
}
```
This policy allows traffic from all routers and the network 192.168.1.0/24 to everywhere.

Before v0.26 it was possible to connect (for example ssh) directly from Node A (100.123.45.67) to Node B (100.123.45.89), and also from any machine from the 192.168.1.0/24 network to Node B (100.123.45.89). Connections from Node B to Node A or any machine on 192.168.1.0/24 was restricted though.

With the reduction of routes sent to peers based on packetfilter, the connection from the network 192.168.1.0/24 to Node B (100.123.45.89) does not work any more, since Node B has no route to send the response back to.

So while the policy in place allows said connectivity, it pratically cannot work any more.

### Expected Behavior

Established and related connections should be allowed, like it was before 0.26. For this, the routes need to be known to the node.

### Steps To Reproduce

Set up per above scenario.

### Environment

```markdown
- OS: Ubuntu 24.04.2 LTS
- Headscale version: 0.26
- Tailscale version:1.82.5
```

### Runtime environment

- [x] Headscale is behind a (reverse) proxy
- [x] Headscale runs in a container

### Debug information

Happy to provide this if really required, however I believe the description above explains it in sufficient detail.

Discussion:
@ArcticLampyrid: Try add:
```json
    {
      "action": "accept",
      "src": [
        "*"
      ],
      "dst": [
        "192.168.1.0/24:0",
        "group:routers:0"
      ]
    }
```@hanjo: I cannot test if this would work, I went back to 0.25 for the time being, but even if this works, this is not what I want, since it would grant everybody access to the 192.168.1.0/24 network, which is not desired.

The goal is that devices from 192.168.1.0/24 can connect to node B, but not vice versa. This works fine in 0.25, since the node B has the route for the 192.168.1.0/24 network and the policy allows related, established cconnections, but drops new connections. In 0.26, since there is no route specific route, the device would send the traffic to default route, which obviously doesn't work.@ArcticLampyrid: > this is not what I want, since it would grant everybody access to the 192.168.1.0/24 network, which is not desired.

No. It blocks all ports (`192.168.1.0/24:0` ends with `:0`), so nobody has permission to proactively establish a connection to 192.168.1.0/24. (ICMP ping may be an exception.)@hanjo: ah - that's smart. Okay, let me give that a try. Nevertheless, I think it should be fixed by allowing some possibility to control which routes are announced and which are not. I understand the desire to limit the routes for clarity and I also acknowledge that it leaks details that some people probably would like to avoid, but in the case described above, the current implementation breaks functionality.@S-NoF3aR: This prevents us from updating to 0.26.1. 

I looked into the code: the function for the reduced routes only checks whether there is an ACL rule from this node to Prefix in a route. This would have to be extended by a check “From the prefix of  a route to this node”, i. in other words the other way around
https://github.com/juanfont/headscale/blob/e73b2a9fb9db82ea5dd1a1a5d554a585188b6b21/hscontrol/types/node.go#L288C1-L302C2

a quickfix would look like this:
```go
func (node *Node) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) bool {
	src := node.IPs()

	for _, matcher := range matchers {
		if !matcher.SrcsContainsIPs(src...) && !matcher.DestsContainsIP(src...) {
			continue
		}

		if matcher.SrcsOverlapsPrefixes(route) || matcher.DestsOverlapsPrefixes(route) {
			return true
		}
	}

	return false
}
```@nblock: > No. It blocks all ports (`192.168.1.0/24:0` ends with `:0`), so nobody has permission to proactively establish a connection to 192.168.1.0/24. (ICMP ping may be an exception.)

This will stop working with the next headscale version, see: https://github.com/juanfont/headscale/pull/2606@ArcticLampyrid: Then we can change the workaround to
```
    {
      "action": "accept",
      "proto": "icmp",
      "src": [
        "*"
      ],
      "dst": [
        "192.168.1.0/24:*",
        "group:routers:*"
      ]
    }
```

or just use a fake port:
```
    // assume nobody will really use port 11
    {
      "action": "accept",
      "src": [
        "*"
      ],
      "dst": [
        "192.168.1.0/24:11",
        "group:routers:11"
      ]
    }
```@kradalby: This should be fixed in https://github.com/juanfont/headscale/pull/2767, give it a go please.

Solution patch:
diff --git a/.goreleaser.yml b/.goreleaser.yml
index dc2378a9..7bc2171c 100644
--- a/.goreleaser.yml
+++ b/.goreleaser.yml
@@ -23,10 +23,6 @@ builds:
       - linux_arm64
     flags:
       - -mod=readonly
-    ldflags:
-      - -s -w
-      - -X github.com/juanfont/headscale/hscontrol/types.Version={{ .Version }}
-      - -X github.com/juanfont/headscale/hscontrol/types.GitCommitHash={{ .Commit }}
     tags:
       - ts2019
 
diff --git a/CHANGELOG.md b/CHANGELOG.md
index e77eb3e8..9ab70873 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -70,6 +70,8 @@ upstream is changed.
   [#2741](https://github.com/juanfont/headscale/pull/2741)
 - Add support for `autogroup:member`, `autogroup:tagged`
   [#2572](https://github.com/juanfont/headscale/pull/2572)
+- Fix bug where return routes were being removed by policy
+  [#2767](https://github.com/juanfont/headscale/pull/2767)
 - Remove policy v1 code [#2600](https://github.com/juanfont/headscale/pull/2600)
 - Refactor Debian/Ubuntu packaging and drop support for Ubuntu 20.04.
   [#2614](https://github.com/juanfont/headscale/pull/2614)
diff --git a/cmd/headscale/cli/root.go b/cmd/headscale/cli/root.go
index f3a16018..420cf363 100644
--- a/cmd/headscale/cli/root.go
+++ b/cmd/headscale/cli/root.go
@@ -71,19 +71,20 @@ func initConfig() {
 
 	disableUpdateCheck := viper.GetBool("disable_check_updates")
 	if !disableUpdateCheck && !machineOutput {
+		versionInfo := types.GetVersionInfo()
 		if (runtime.GOOS == "linux" || runtime.GOOS == "darwin") &&
-			types.Version != "dev" {
+			!versionInfo.Dirty {
 			githubTag := &latest.GithubTag{
 				Owner:      "juanfont",
 				Repository: "headscale",
 			}
-			res, err := latest.Check(githubTag, types.Version)
+			res, err := latest.Check(githubTag, versionInfo.Version)
 			if err == nil && res.Outdated {
 				//nolint
 				log.Warn().Msgf(
 					"An updated version of Headscale has been found (%s vs. your current %s). Check it out https://github.com/juanfont/headscale/releases\n",
 					res.Current,
-					types.Version,
+					versionInfo.Version,
 				)
 			}
 		}
diff --git a/cmd/headscale/cli/version.go b/cmd/headscale/cli/version.go
index b007d05c..df8a0be4 100644
--- a/cmd/headscale/cli/version.go
+++ b/cmd/headscale/cli/version.go
@@ -7,6 +7,7 @@ import (
 
 func init() {
 	rootCmd.AddCommand(versionCmd)
+	versionCmd.Flags().StringP("output", "o", "", "Output format. Empty for human-readable, 'json', 'json-line' or 'yaml'")
 }
 
 var versionCmd = &cobra.Command{
@@ -15,9 +16,9 @@ var versionCmd = &cobra.Command{
 	Long:  "The version of headscale.",
 	Run: func(cmd *cobra.Command, args []string) {
 		output, _ := cmd.Flags().GetString("output")
-		SuccessOutput(map[string]string{
-			"version": types.Version,
-			"commit":  types.GitCommitHash,
-		}, types.Version, output)
+
+		info := types.GetVersionInfo()
+
+		SuccessOutput(info, info.String(), output)
 	},
 }
diff --git a/hscontrol/app.go b/hscontrol/app.go
index 885066a0..6880c6be 100644
--- a/hscontrol/app.go
+++ b/hscontrol/app.go
@@ -511,7 +511,8 @@ func (h *Headscale) Serve() error {
 		spew.Dump(h.cfg)
 	}
 
-	log.Info().Str("version", types.Version).Str("commit", types.GitCommitHash).Msg("Starting Headscale")
+	versionInfo := types.GetVersionInfo()
+	log.Info().Str("version", versionInfo.Version).Str("commit", versionInfo.Commit).Msg("Starting Headscale")
 	log.Info().
 		Str("minimum_version", capver.TailscaleVersion(capver.MinSupportedCapabilityVersion)).
 		Msg("Clients with a lower minimum version will be rejected")
diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go
index 338e513b..5793d96c 100644
--- a/hscontrol/policy/v2/filter.go
+++ b/hscontrol/policy/v2/filter.go
@@ -89,11 +89,12 @@ func (pol *Policy) compileFilterRules(
 
 func sshAction(accept bool, duration time.Duration) tailcfg.SSHAction {
 	return tailcfg.SSHAction{
-		Reject:                   !accept,
-		Accept:                   accept,
-		SessionDuration:          duration,
-		AllowAgentForwarding:     true,
-		AllowLocalPortForwarding: true,
+		Reject:                    !accept,
+		Accept:                    accept,
+		SessionDuration:           duration,
+		AllowAgentForwarding:      true,
+		AllowLocalPortForwarding:  true,
+		AllowRemotePortForwarding: true,
 	}
 }
 
@@ -153,8 +154,17 @@ func (pol *Policy) compileSSHPolicy(
 		}
 
 		userMap := make(map[string]string, len(rule.Users))
-		for _, user := range rule.Users {
-			userMap[user.String()] = "="
+		if rule.Users.ContainsNonRoot() {
+			userMap["*"] = "="
+
+			// by default, we do not allow root unless explicitly stated
+			userMap["root"] = ""
+		}
+		if rule.Users.ContainsRoot() {
+			userMap["root"] = "root"
+		}
+		for _, u := range rule.Users.NormalUsers() {
+			userMap[u.String()] = u.String()
 		}
 		rules = append(rules, &tailcfg.SSHRule{
 			Principals: principals,
diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go
index a2541da6..80797e17 100644
--- a/hscontrol/policy/v2/types.go
+++ b/hscontrol/policy/v2/types.go
@@ -20,6 +20,7 @@ import (
 	"tailscale.com/types/ptr"
 	"tailscale.com/types/views"
 	"tailscale.com/util/multierr"
+	"tailscale.com/util/slicesx"
 )
 
 const Wildcard = Asterix(0)
@@ -506,6 +507,10 @@ func (ag *AutoGroup) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
+func (ag AutoGroup) String() string {
+	return string(ag)
+}
+
 // MarshalJSON marshals the AutoGroup to JSON.
 func (ag AutoGroup) MarshalJSON() ([]byte, error) {
 	return json.Marshal(string(ag))
@@ -1562,7 +1567,7 @@ type SSH struct {
 	Action       string         `json:"action"`
 	Sources      SSHSrcAliases  `json:"src"`
 	Destinations SSHDstAliases  `json:"dst"`
-	Users        []SSHUser      `json:"users"`
+	Users        SSHUsers       `json:"users"`
 	CheckPeriod  model.Duration `json:"checkPeriod,omitempty"`
 }
 
@@ -1715,6 +1720,22 @@ func (a SSHSrcAliases) Resolve(p *Policy, users types.Users, nodes views.Slice[t
 // It can be a list of usernames, tags or autogroups.
 type SSHDstAliases []Alias
 
+type SSHUsers []SSHUser
+
+func (u SSHUsers) ContainsRoot() bool {
+	return slices.Contains(u, "root")
+}
+
+func (u SSHUsers) ContainsNonRoot() bool {
+	return slices.Contains(u, SSHUser(AutoGroupNonRoot))
+}
+
+func (u SSHUsers) NormalUsers() []SSHUser {
+	return slicesx.Filter(nil, u, func(user SSHUser) bool {
+		return user != "root" && user != SSHUser(AutoGroupNonRoot)
+	})
+}
+
 type SSHUser string
 
 func (u SSHUser) String() string {
diff --git a/hscontrol/types/node.go b/hscontrol/types/node.go
index 1d0b6cc3..a7d25e11 100644
--- a/hscontrol/types/node.go
+++ b/hscontrol/types/node.go
@@ -317,11 +317,11 @@ func (node *Node) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) b
 	src := node.IPs()
 
 	for _, matcher := range matchers {
-		if !matcher.SrcsContainsIPs(src...) {
-			continue
+		if matcher.SrcsContainsIPs(src...) && matcher.DestsOverlapsPrefixes(route) {
+			return true
 		}
 
-		if matcher.DestsOverlapsPrefixes(route) {
+		if matcher.SrcsOverlapsPrefixes(route) && matcher.DestsContainsIP(src...) {
 			return true
 		}
 	}
@@ -680,19 +680,8 @@ func (v NodeView) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) b
 	if !v.Valid() {
 		return false
 	}
-	src := v.IPs()
-
-	for _, matcher := range matchers {
-		if !matcher.SrcsContainsIPs(src...) {
-			continue
-		}
-
-		if matcher.DestsOverlapsPrefixes(route) {
-			return true
-		}
-	}
 
-	return false
+	return v.ж.CanAccessRoute(matchers, route)
 }
 
 func (v NodeView) AnnouncedRoutes() []netip.Prefix {
diff --git a/hscontrol/types/version.go b/hscontrol/types/version.go
index 7fe23250..6676c92f 100644
--- a/hscontrol/types/version.go
+++ b/hscontrol/types/version.go
@@ -1,6 +1,81 @@
 package types
 
-var (
-	Version       = "dev"
-	GitCommitHash = "dev"
+import (
+	"fmt"
+	"runtime"
+	"runtime/debug"
+	"strings"
+	"sync"
 )
+
+type GoInfo struct {
+	Version string `json:"version"`
+	OS      string `json:"os"`
+	Arch    string `json:"arch"`
+}
+
+type VersionInfo struct {
+	Version   string `json:"version"`
+	Commit    string `json:"commit"`
+	BuildTime string `json:"buildTime"`
+	Go        GoInfo `json:"go"`
+	Dirty     bool   `json:"dirty"`
+}
+
+func (v *VersionInfo) String() string {
+	var sb strings.Builder
+
+	version := v.Version
+	if v.Dirty && !strings.Contains(version, "dirty") {
+		version += "-dirty"
+	}
+
+	sb.WriteString(fmt.Sprintf("headscale version %s\n", version))
+	sb.WriteString(fmt.Sprintf("commit: %s\n", v.Commit))
+	sb.WriteString(fmt.Sprintf("build time: %s\n", v.BuildTime))
+	sb.WriteString(fmt.Sprintf("built with: %s %s/%s\n", v.Go.Version, v.Go.OS, v.Go.Arch))
+
+	return sb.String()
+}
+
+var buildInfo = sync.OnceValues(func() (*debug.BuildInfo, bool) {
+	return debug.ReadBuildInfo()
+})
+
+var GetVersionInfo = sync.OnceValue(func() *VersionInfo {
+	info := &VersionInfo{
+		Version:   "dev",
+		Commit:    "unknown",
+		BuildTime: "unknown",
+		Go: GoInfo{
+			Version: runtime.Version(),
+			OS:      runtime.GOOS,
+			Arch:    runtime.GOARCH,
+		},
+		Dirty: false,
+	}
+
+	buildInfo, ok := buildInfo()
+	if !ok {
+		return info
+	}
+
+	// Extract version from module path or main version
+	if buildInfo.Main.Version != "" && buildInfo.Main.Version != "(devel)" {
+		info.Version = buildInfo.Main.Version
+	}
+
+	// Extract build settings
+	for _, setting := range buildInfo.Settings {
+		switch setting.Key {
+		case "vcs.revision":
+			info.Commit = setting.Value
+		case "vcs.modified":
+			info.Dirty = setting.Value == "true"
+		case "vcs.time":
+			info.BuildTime = setting.Value
+		}
+	}
+
+	return info
+})



Test patch:
diff --git a/hscontrol/policy/policy_test.go b/hscontrol/policy/policy_test.go
index f19ac3d3..d2ff019d 100644
--- a/hscontrol/policy/policy_test.go
+++ b/hscontrol/policy/policy_test.go
@@ -1612,13 +1612,7 @@ func TestSSHPolicyRules(t *testing.T) {
 		UserID:   2,
 		User:     users[1],
 	}
-	taggedServer := types.Node{
-		Hostname:   "tagged-server",
-		IPv4:       ap("100.64.0.3"),
-		UserID:     3,
-		User:       users[2],
-		ForcedTags: []string{"tag:server"},
-	}
+
 	taggedClient := types.Node{
 		Hostname:   "tagged-client",
 		IPv4:       ap("100.64.0.4"),
@@ -1659,32 +1653,32 @@ func TestSSHPolicyRules(t *testing.T) {
 						{NodeIP: "100.64.0.2"},
 					},
 					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
+						"*":    "=",
+						"root": "",
 					},
 					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
+						Accept:                    true,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
 					},
 				},
 			}},
 		},
 		{
-			name:       "group-to-tag",
-			targetNode: taggedServer,
-			peers:      types.Nodes{&nodeUser1, &nodeUser2},
+			name:       "check-period-specified",
+			targetNode: nodeUser1,
+			peers:      types.Nodes{&taggedClient},
 			policy: `{
 				"tagOwners": {
-				  "tag:server": ["user3@"],
-				},
-				"groups": {
-					"group:users": ["user1@", "user2@"]
+					"tag:client": ["user1@"],
 				},
 				"ssh": [
 					{
-						"action": "accept",
-						"src": ["group:users"],
-						"dst": ["tag:server"],
+						"action": "check",
+						"checkPeriod": "24h",
+						"src": ["tag:client"],
+						"dst": ["user1@"],
 						"users": ["autogroup:nonroot"]
 					}
 				]
@@ -1692,28 +1686,30 @@ func TestSSHPolicyRules(t *testing.T) {
 			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
 				{
 					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.1"},
-						{NodeIP: "100.64.0.2"},
+						{NodeIP: "100.64.0.4"},
 					},
 					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
+						"*":    "=",
+						"root": "",
 					},
 					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
+						Accept:                    true,
+						SessionDuration:           24 * time.Hour,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
 					},
 				},
 			}},
 		},
 		{
-			name:       "tag-to-user",
-			targetNode: nodeUser1,
-			peers:      types.Nodes{&taggedClient},
+			name:       "no-matching-rules",
+			targetNode: nodeUser2,
+			peers:      types.Nodes{&nodeUser1},
 			policy: `{
 			    "tagOwners": {
-					"tag:client": ["user1@"],
-				},
+			    	"tag:client": ["user1@"],
+			    },
 				"ssh": [
 					{
 						"action": "accept",
@@ -1723,60 +1719,64 @@ func TestSSHPolicyRules(t *testing.T) {
 					}
 				]
 			}`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
-				{
-					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.4"},
-					},
-					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
-					},
-					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
-					},
-				},
-			}},
+			wantSSH: &tailcfg.SSHPolicy{Rules: nil},
 		},
 		{
-			name:       "tag-to-tag",
-			targetNode: taggedServer,
-			peers:      types.Nodes{&taggedClient},
+			name:       "invalid-action",
+			targetNode: nodeUser1,
+			peers:      types.Nodes{&nodeUser2},
 			policy: `{
-				"tagOwners": {
-					"tag:client": ["user2@"],
-					"tag:server": ["user3@"],
-				},
 				"ssh": [
 					{
-						"action": "accept",
-						"src": ["tag:client"],
-						"dst": ["tag:server"],
+						"action": "invalid",
+						"src": ["group:admins"],
+						"dst": ["user1@"],
 						"users": ["autogroup:nonroot"]
 					}
 				]
 			}`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
-				{
-					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.4"},
-					},
-					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
-					},
-					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
-					},
-				},
-			}},
+			expectErr:    true,
+			errorMessage: `SSH action "invalid" is not valid, must be accept or check`,
+		},
+		{
+			name:       "invalid-check-period",
+			targetNode: nodeUser1,
+			peers:      types.Nodes{&nodeUser2},
+			policy: `{
+				"ssh": [
+					{
+						"action": "check",
+						"checkPeriod": "invalid",
+						"src": ["group:admins"],
+						"dst": ["user1@"],
+						"users": ["autogroup:nonroot"]
+					}
+				]
+			}`,
+			expectErr:    true,
+			errorMessage: "not a valid duration string",
 		},
 		{
-			name:       "group-to-wildcard",
+			name:       "unsupported-autogroup",
 			targetNode: nodeUser1,
-			peers:      types.Nodes{&nodeUser2, &taggedClient},
+			peers:      types.Nodes{&taggedClient},
+			policy: `{
+        "ssh": [
+            {
+                "action": "accept",
+                "src": ["tag:client"],
+                "dst": ["user1@"],
+                "users": ["autogroup:invalid"]
+            }
+        ]
+    }`,
+			expectErr:    true,
+			errorMessage: "autogroup \"autogroup:invalid\" is not supported",
+		},
+		{
+			name:       "autogroup-nonroot-should-use-wildcard-with-root-excluded",
+			targetNode: nodeUser1,
+			peers:      types.Nodes{&nodeUser2},
 			policy: `{
 				"groups": {
 					"group:admins": ["user2@"]
@@ -1785,167 +1785,102 @@ func TestSSHPolicyRules(t *testing.T) {
 					{
 						"action": "accept",
 						"src": ["group:admins"],
-						"dst": ["*"],
+						"dst": ["user1@"],
 						"users": ["autogroup:nonroot"]
 					}
 				]
 			}`,
+			// autogroup:nonroot should map to wildcard "*" with root excluded
 			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
 				{
 					Principals: []*tailcfg.SSHPrincipal{
 						{NodeIP: "100.64.0.2"},
 					},
 					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
+						"*":    "=",
+						"root": "",
 					},
 					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
+						Accept:                    true,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
 					},
 				},
 			}},
 		},
 		{
-			name:       "check-period-specified",
+			name:       "autogroup-nonroot-plus-root-should-use-wildcard-with-root-mapped",
 			targetNode: nodeUser1,
-			peers:      types.Nodes{&taggedClient},
+			peers:      types.Nodes{&nodeUser2},
 			policy: `{
-				"tagOwners": {
-					"tag:client": ["user1@"],
+				"groups": {
+					"group:admins": ["user2@"]
 				},
 				"ssh": [
 					{
-						"action": "check",
-						"checkPeriod": "24h",
-						"src": ["tag:client"],
+						"action": "accept",
+						"src": ["group:admins"],
 						"dst": ["user1@"],
-						"users": ["autogroup:nonroot"]
+						"users": ["autogroup:nonroot", "root"]
 					}
 				]
 			}`,
+			// autogroup:nonroot + root should map to wildcard "*" with root mapped to itself
 			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
 				{
 					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.4"},
+						{NodeIP: "100.64.0.2"},
 					},
 					SSHUsers: map[string]string{
-						"autogroup:nonroot": "=",
+						"*":    "=",
+						"root": "root",
 					},
 					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						SessionDuration:          24 * time.Hour,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
+						Accept:                    true,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
 					},
 				},
 			}},
 		},
 		{
-			name:       "no-matching-rules",
-			targetNode: nodeUser2,
-			peers:      types.Nodes{&nodeUser1},
-			policy: `{
-			    "tagOwners": {
-			    	"tag:client": ["user1@"],
-			    },
-				"ssh": [
-					{
-						"action": "accept",
-						"src": ["tag:client"],
-						"dst": ["user1@"],
-						"users": ["autogroup:nonroot"]
-					}
-				]
-			}`,
-			wantSSH: &tailcfg.SSHPolicy{Rules: nil},
-		},
-		{
-			name:       "invalid-action",
-			targetNode: nodeUser1,
-			peers:      types.Nodes{&nodeUser2},
-			policy: `{
-				"ssh": [
-					{
-						"action": "invalid",
-						"src": ["group:admins"],
-						"dst": ["user1@"],
-						"users": ["autogroup:nonroot"]
-					}
-				]
-			}`,
-			expectErr:    true,
-			errorMessage: `SSH action "invalid" is not valid, must be accept or check`,
-		},
-		{
-			name:       "invalid-check-period",
+			name:       "specific-users-should-map-to-themselves-not-equals",
 			targetNode: nodeUser1,
 			peers:      types.Nodes{&nodeUser2},
 			policy: `{
+				"groups": {
+					"group:admins": ["user2@"]
+				},
 				"ssh": [
 					{
-						"action": "check",
-						"checkPeriod": "invalid",
+						"action": "accept",
 						"src": ["group:admins"],
 						"dst": ["user1@"],
-						"users": ["autogroup:nonroot"]
+						"users": ["ubuntu", "root"]
 					}
 				]
 			}`,
-			expectErr:    true,
-			errorMessage: "not a valid duration string",
-		},
-		{
-			name:       "multiple-ssh-users-with-autogroup",
-			targetNode: nodeUser1,
-			peers:      types.Nodes{&taggedClient},
-			policy: `{
-			"tagOwners": {
-				"tag:client": ["user1@"],
-			},
-        	"ssh": [
-        	    {
-        	        "action": "accept",
-        	        "src": ["tag:client"],
-        	        "dst": ["user1@"],
-        	        "users": ["alice", "bob"]
-        	    }
-        	]
-    }`,
+			// specific usernames should map to themselves, not "="
 			wantSSH: &tailcfg.SSHPolicy{Rules: []*tailcfg.SSHRule{
 				{
 					Principals: []*tailcfg.SSHPrincipal{
-						{NodeIP: "100.64.0.4"},
+						{NodeIP: "100.64.0.2"},
 					},
 					SSHUsers: map[string]string{
-						"alice": "=",
-						"bob":   "=",
+						"root":   "root",
+						"ubuntu": "ubuntu",
 					},
 					Action: &tailcfg.SSHAction{
-						Accept:                   true,
-						AllowAgentForwarding:     true,
-						AllowLocalPortForwarding: true,
+						Accept:                    true,
+						AllowAgentForwarding:      true,
+						AllowLocalPortForwarding:  true,
+						AllowRemotePortForwarding: true,
 					},
 				},
 			}},
 		},
-		{
-			name:       "unsupported-autogroup",
-			targetNode: nodeUser1,
-			peers:      types.Nodes{&taggedClient},
-			policy: `{
-        "ssh": [
-            {
-                "action": "accept",
-                "src": ["tag:client"],
-                "dst": ["user1@"],
-                "users": ["autogroup:invalid"]
-            }
-        ]
-    }`,
-			expectErr:    true,
-			errorMessage: "autogroup \"autogroup:invalid\" is not supported",
-		},
 	}
 
 	for _, tt := range tests {
@@ -2425,6 +2360,177 @@ func TestReduceRoutes(t *testing.T) {
 				netip.MustParsePrefix("10.10.12.0/24"),
 			},
 		},
+		{
+			name: "return-path-subnet-router-to-regular-node-issue-2608",
+			args: args{
+				node: &types.Node{
+					ID:   2,
+					IPv4: ap("100.123.45.89"), // Node B - regular node
+					User: types.User{Name: "node-b"},
+				},
+				routes: []netip.Prefix{
+					netip.MustParsePrefix("192.168.1.0/24"), // Subnet connected to Node A
+				},
+				rules: []tailcfg.FilterRule{
+					{
+						// Policy allows 192.168.1.0/24 and group:routers to access *:*
+						SrcIPs: []string{
+							"192.168.1.0/24", // Subnet behind router
+							"100.123.45.67",  // Node A (router, part of group:routers)
+						},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "*", Ports: tailcfg.PortRangeAny}, // Access to everything
+						},
+					},
+				},
+			},
+			// Node B should receive the 192.168.1.0/24 route for return traffic
+			// even though Node B cannot initiate connections to that network
+			want: []netip.Prefix{
+				netip.MustParsePrefix("192.168.1.0/24"),
+			},
+		},
+		{
+			name: "return-path-router-perspective-2608",
+			args: args{
+				node: &types.Node{
+					ID:   1,
+					IPv4: ap("100.123.45.67"), // Node A - router node
+					User: types.User{Name: "router"},
+				},
+				routes: []netip.Prefix{
+					netip.MustParsePrefix("192.168.1.0/24"), // Subnet connected to this router
+				},
+				rules: []tailcfg.FilterRule{
+					{
+						// Policy allows 192.168.1.0/24 and group:routers to access *:*
+						SrcIPs: []string{
+							"192.168.1.0/24", // Subnet behind router
+							"100.123.45.67",  // Node A (router, part of group:routers)
+						},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "*", Ports: tailcfg.PortRangeAny}, // Access to everything
+						},
+					},
+				},
+			},
+			// Router should have access to its own routes
+			want: []netip.Prefix{
+				netip.MustParsePrefix("192.168.1.0/24"),
+			},
+		},
+		{
+			name: "subnet-behind-router-bidirectional-connectivity-issue-2608",
+			args: args{
+				node: &types.Node{
+					ID:   2,
+					IPv4: ap("100.123.45.89"), // Node B - regular node that should be reachable
+					User: types.User{Name: "node-b"},
+				},
+				routes: []netip.Prefix{
+					netip.MustParsePrefix("192.168.1.0/24"), // Subnet behind router
+					netip.MustParsePrefix("10.0.0.0/24"),    // Another subnet
+				},
+				rules: []tailcfg.FilterRule{
+					{
+						// Only 192.168.1.0/24 and routers can access everything
+						SrcIPs: []string{
+							"192.168.1.0/24", // Subnet that can connect to Node B
+							"100.123.45.67",  // Router node
+						},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "*", Ports: tailcfg.PortRangeAny},
+						},
+					},
+					{
+						// Node B cannot access anything (no rules with Node B as source)
+						SrcIPs:   []string{"100.123.45.89"},
+						DstPorts: []tailcfg.NetPortRange{
+							// No destinations - Node B cannot initiate connections
+						},
+					},
+				},
+			},
+			// Node B should still get the 192.168.1.0/24 route for return traffic
+			// but should NOT get 10.0.0.0/24 since nothing allows that subnet to connect to Node B
+			want: []netip.Prefix{
+				netip.MustParsePrefix("192.168.1.0/24"),
+			},
+		},
+		{
+			name: "no-route-leakage-when-no-connection-allowed-2608",
+			args: args{
+				node: &types.Node{
+					ID:   3,
+					IPv4: ap("100.123.45.99"), // Node C - isolated node
+					User: types.User{Name: "isolated-node"},
+				},
+				routes: []netip.Prefix{
+					netip.MustParsePrefix("192.168.1.0/24"), // Subnet behind router
+					netip.MustParsePrefix("10.0.0.0/24"),    // Another private subnet
+					netip.MustParsePrefix("172.16.0.0/24"),  // Yet another subnet
+				},
+				rules: []tailcfg.FilterRule{
+					{
+						// Only specific subnets and routers can access specific destinations
+						SrcIPs: []string{
+							"192.168.1.0/24", // This subnet can access everything
+							"100.123.45.67",  // Router node can access everything
+						},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "100.123.45.89", Ports: tailcfg.PortRangeAny}, // Only to Node B
+						},
+					},
+					{
+						// 10.0.0.0/24 can only access router
+						SrcIPs: []string{"10.0.0.0/24"},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "100.123.45.67", Ports: tailcfg.PortRangeAny}, // Only to router
+						},
+					},
+					{
+						// 172.16.0.0/24 has no access rules at all
+					},
+				},
+			},
+			// Node C should get NO routes because:
+			// - 192.168.1.0/24 can only connect to Node B (not Node C)
+			// - 10.0.0.0/24 can only connect to router (not Node C)
+			// - 172.16.0.0/24 has no rules allowing it to connect anywhere
+			// - Node C is not in any rules as a destination
+			want: nil,
+		},
+		{
+			name: "original-issue-2608-with-slash14-network",
+			args: args{
+				node: &types.Node{
+					ID:   2,
+					IPv4: ap("100.123.45.89"), // Node B - regular node
+					User: types.User{Name: "node-b"},
+				},
+				routes: []netip.Prefix{
+					netip.MustParsePrefix("192.168.1.0/14"), // Network 192.168.1.0/14 as mentioned in original issue
+				},
+				rules: []tailcfg.FilterRule{
+					{
+						// Policy allows 192.168.1.0/24 (part of /14) and group:routers to access *:*
+						SrcIPs: []string{
+							"192.168.1.0/24", // Subnet behind router (part of the larger /14 network)
+							"100.123.45.67",  // Node A (router, part of group:routers)
+						},
+						DstPorts: []tailcfg.NetPortRange{
+							{IP: "*", Ports: tailcfg.PortRangeAny}, // Access to everything
+						},
+					},
+				},
+			},
+			// Node B should receive the 192.168.1.0/14 route for return traffic
+			// even though only 192.168.1.0/24 (part of /14) can connect to Node B
+			// This is the exact scenario from the original issue
+			want: []netip.Prefix{
+				netip.MustParsePrefix("192.168.1.0/14"),
+			},
+		},
 	}
 
 	for _, tt := range tests {
diff --git a/hscontrol/policy/v2/filter_test.go b/hscontrol/policy/v2/filter_test.go
index 12c60fbb..8b73a6f5 100644
--- a/hscontrol/policy/v2/filter_test.go
+++ b/hscontrol/policy/v2/filter_test.go
@@ -1,10 +1,16 @@
 package v2
 
 import (
+	"encoding/json"
+	"net/netip"
 	"testing"
+	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/juanfont/headscale/hscontrol/types"
+	"github.com/prometheus/common/model"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"gorm.io/gorm"
 	"tailscale.com/tailcfg"
 )
@@ -376,3 +382,406 @@ func TestParsing(t *testing.T) {
 		})
 	}
 }
+
+func TestCompileSSHPolicy_UserMapping(t *testing.T) {
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+		{Name: "user2", Model: gorm.Model{ID: 2}},
+	}
+
+	// Create test nodes
+	nodeUser1 := types.Node{
+		Hostname: "user1-device",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+	nodeUser2 := types.Node{
+		Hostname: "user2-device",
+		IPv4:     createAddr("100.64.0.2"),
+		UserID:   2,
+		User:     users[1],
+	}
+
+	nodes := types.Nodes{&nodeUser1, &nodeUser2}
+
+	tests := []struct {
+		name         string
+		targetNode   types.Node
+		policy       *Policy
+		wantSSHUsers map[string]string
+		wantEmpty    bool
+	}{
+		{
+			name:       "specific user mapping",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{"ssh-it-user"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"ssh-it-user": "ssh-it-user",
+			},
+		},
+		{
+			name:       "multiple specific users",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{"ubuntu", "admin", "deploy"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"ubuntu": "ubuntu",
+				"admin":  "admin",
+				"deploy": "deploy",
+			},
+		},
+		{
+			name:       "autogroup:nonroot only",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{SSHUser(AutoGroupNonRoot)},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"*":    "=",
+				"root": "",
+			},
+		},
+		{
+			name:       "root only",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{"root"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"root": "root",
+			},
+		},
+		{
+			name:       "autogroup:nonroot plus root",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{SSHUser(AutoGroupNonRoot), "root"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"*":    "=",
+				"root": "root",
+			},
+		},
+		{
+			name:       "mixed specific users and autogroups",
+			targetNode: nodeUser1,
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")},
+						Users:        []SSHUser{SSHUser(AutoGroupNonRoot), "root", "ubuntu", "admin"},
+					},
+				},
+			},
+			wantSSHUsers: map[string]string{
+				"*":      "=",
+				"root":   "root",
+				"ubuntu": "ubuntu",
+				"admin":  "admin",
+			},
+		},
+		{
+			name:       "no matching destination",
+			targetNode: nodeUser2, // Target node2, but policy only allows user1
+			policy: &Policy{
+				Groups: Groups{
+					Group("group:admins"): []Username{Username("user2@")},
+				},
+				SSHs: []SSH{
+					{
+						Action:       "accept",
+						Sources:      SSHSrcAliases{gp("group:admins")},
+						Destinations: SSHDstAliases{up("user1@")}, // Only user1, not user2
+						Users:        []SSHUser{"ssh-it-user"},
+					},
+				},
+			},
+			wantEmpty: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// Validate the policy
+			err := tt.policy.validate()
+			require.NoError(t, err)
+
+			// Compile SSH policy
+			sshPolicy, err := tt.policy.compileSSHPolicy(users, tt.targetNode.View(), nodes.ViewSlice())
+			require.NoError(t, err)
+
+			if tt.wantEmpty {
+				if sshPolicy == nil {
+					return // Expected empty result
+				}
+				assert.Empty(t, sshPolicy.Rules, "SSH policy should be empty when no rules match")
+				return
+			}
+
+			require.NotNil(t, sshPolicy)
+			require.Len(t, sshPolicy.Rules, 1, "Should have exactly one SSH rule")
+
+			rule := sshPolicy.Rules[0]
+			assert.Equal(t, tt.wantSSHUsers, rule.SSHUsers, "SSH users mapping should match expected")
+
+			// Verify principals are set correctly (should contain user2's IP since that's the source)
+			require.Len(t, rule.Principals, 1)
+			assert.Equal(t, "100.64.0.2", rule.Principals[0].NodeIP)
+
+			// Verify action is set correctly
+			assert.True(t, rule.Action.Accept)
+			assert.True(t, rule.Action.AllowAgentForwarding)
+			assert.True(t, rule.Action.AllowLocalPortForwarding)
+			assert.True(t, rule.Action.AllowRemotePortForwarding)
+		})
+	}
+}
+
+func TestCompileSSHPolicy_CheckAction(t *testing.T) {
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+		{Name: "user2", Model: gorm.Model{ID: 2}},
+	}
+
+	nodeUser1 := types.Node{
+		Hostname: "user1-device",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+	nodeUser2 := types.Node{
+		Hostname: "user2-device",
+		IPv4:     createAddr("100.64.0.2"),
+		UserID:   2,
+		User:     users[1],
+	}
+
+	nodes := types.Nodes{&nodeUser1, &nodeUser2}
+
+	policy := &Policy{
+		Groups: Groups{
+			Group("group:admins"): []Username{Username("user2@")},
+		},
+		SSHs: []SSH{
+			{
+				Action:       "check",
+				CheckPeriod:  model.Duration(24 * time.Hour),
+				Sources:      SSHSrcAliases{gp("group:admins")},
+				Destinations: SSHDstAliases{up("user1@")},
+				Users:        []SSHUser{"ssh-it-user"},
+			},
+		},
+	}
+
+	err := policy.validate()
+	require.NoError(t, err)
+
+	sshPolicy, err := policy.compileSSHPolicy(users, nodeUser1.View(), nodes.ViewSlice())
+	require.NoError(t, err)
+	require.NotNil(t, sshPolicy)
+	require.Len(t, sshPolicy.Rules, 1)
+
+	rule := sshPolicy.Rules[0]
+
+	// Verify SSH users are correctly mapped
+	expectedUsers := map[string]string{
+		"ssh-it-user": "ssh-it-user",
+	}
+	assert.Equal(t, expectedUsers, rule.SSHUsers)
+
+	// Verify check action with session duration
+	assert.True(t, rule.Action.Accept)
+	assert.Equal(t, 24*time.Hour, rule.Action.SessionDuration)
+}
+
+// TestSSHIntegrationReproduction reproduces the exact scenario from the integration test
+// TestSSHOneUserToAll that was failing with empty sshUsers
+func TestSSHIntegrationReproduction(t *testing.T) {
+	// Create users matching the integration test
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+		{Name: "user2", Model: gorm.Model{ID: 2}},
+	}
+
+	// Create simple nodes for testing
+	node1 := &types.Node{
+		Hostname: "user1-node",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+
+	node2 := &types.Node{
+		Hostname: "user2-node",
+		IPv4:     createAddr("100.64.0.2"),
+		UserID:   2,
+		User:     users[1],
+	}
+
+	nodes := types.Nodes{node1, node2}
+
+	// Create a simple policy that reproduces the issue
+	policy := &Policy{
+		Groups: Groups{
+			Group("group:integration-test"): []Username{Username("user1@")},
+		},
+		SSHs: []SSH{
+			{
+				Action:       "accept",
+				Sources:      SSHSrcAliases{gp("group:integration-test")},
+				Destinations: SSHDstAliases{up("user2@")},       // Target user2
+				Users:        []SSHUser{SSHUser("ssh-it-user")}, // This is the key - specific user
+			},
+		},
+	}
+
+	// Validate policy
+	err := policy.validate()
+	require.NoError(t, err)
+
+	// Test SSH policy compilation for node2 (target)
+	sshPolicy, err := policy.compileSSHPolicy(users, node2.View(), nodes.ViewSlice())
+	require.NoError(t, err)
+	require.NotNil(t, sshPolicy)
+	require.Len(t, sshPolicy.Rules, 1)
+
+	rule := sshPolicy.Rules[0]
+
+	// This was the failing assertion in integration test - sshUsers was empty
+	assert.NotEmpty(t, rule.SSHUsers, "SSH users should not be empty")
+	assert.Contains(t, rule.SSHUsers, "ssh-it-user", "ssh-it-user should be present in SSH users")
+	assert.Equal(t, "ssh-it-user", rule.SSHUsers["ssh-it-user"], "ssh-it-user should map to itself")
+
+	// Verify that ssh-it-user is correctly mapped
+	expectedUsers := map[string]string{
+		"ssh-it-user": "ssh-it-user",
+	}
+	assert.Equal(t, expectedUsers, rule.SSHUsers, "ssh-it-user should be mapped to itself")
+}
+
+// TestSSHJSONSerialization verifies that the SSH policy can be properly serialized
+// to JSON and that the sshUsers field is not empty
+func TestSSHJSONSerialization(t *testing.T) {
+	users := types.Users{
+		{Name: "user1", Model: gorm.Model{ID: 1}},
+	}
+
+	node := &types.Node{
+		Hostname: "test-node",
+		IPv4:     createAddr("100.64.0.1"),
+		UserID:   1,
+		User:     users[0],
+	}
+
+	nodes := types.Nodes{node}
+
+	policy := &Policy{
+		SSHs: []SSH{
+			{
+				Action:       "accept",
+				Sources:      SSHSrcAliases{up("user1@")},
+				Destinations: SSHDstAliases{up("user1@")},
+				Users:        []SSHUser{"ssh-it-user", "ubuntu", "admin"},
+			},
+		},
+	}
+
+	err := policy.validate()
+	require.NoError(t, err)
+
+	sshPolicy, err := policy.compileSSHPolicy(users, node.View(), nodes.ViewSlice())
+	require.NoError(t, err)
+	require.NotNil(t, sshPolicy)
+
+	// Serialize to JSON to verify structure
+	jsonData, err := json.MarshalIndent(sshPolicy, "", "  ")
+	require.NoError(t, err)
+
+	// Parse back to verify structure
+	var parsed tailcfg.SSHPolicy
+	err = json.Unmarshal(jsonData, &parsed)
+	require.NoError(t, err)
+
+	// Verify the parsed structure has the expected SSH users
+	require.Len(t, parsed.Rules, 1)
+	rule := parsed.Rules[0]
+
+	expectedUsers := map[string]string{
+		"ssh-it-user": "ssh-it-user",
+		"ubuntu":      "ubuntu",
+		"admin":       "admin",
+	}
+	assert.Equal(t, expectedUsers, rule.SSHUsers, "SSH users should survive JSON round-trip")
+
+	// Verify JSON contains the SSH users (not empty)
+	assert.Contains(t, string(jsonData), `"ssh-it-user"`)
+	assert.Contains(t, string(jsonData), `"ubuntu"`)
+	assert.Contains(t, string(jsonData), `"admin"`)
+	assert.NotContains(t, string(jsonData), `"sshUsers": {}`, "SSH users should not be empty")
+	assert.NotContains(t, string(jsonData), `"sshUsers": null`, "SSH users should not be null")
+}
+
+// Helper function to create IP addresses for testing
+func createAddr(ip string) *netip.Addr {
+	addr, _ := netip.ParseAddr(ip)
+	return &addr
+}
diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go
index 5bdb7885..2a3ab578 100644
--- a/hscontrol/policy/v2/types_test.go
+++ b/hscontrol/policy/v2/types_test.go
@@ -1615,6 +1615,134 @@ func TestResolveAutoApprovers(t *testing.T) {
 	}
 }
 
+func TestSSHUsers_NormalUsers(t *testing.T) {
+	tests := []struct {
+		name     string
+		users    SSHUsers
+		expected []SSHUser
+	}{
+		{
+			name:     "empty users",
+			users:    SSHUsers{},
+			expected: []SSHUser{},
+		},
+		{
+			name:     "only root",
+			users:    SSHUsers{"root"},
+			expected: []SSHUser{},
+		},
+		{
+			name:     "only autogroup:nonroot",
+			users:    SSHUsers{SSHUser(AutoGroupNonRoot)},
+			expected: []SSHUser{},
+		},
+		{
+			name:     "only normal user",
+			users:    SSHUsers{"ssh-it-user"},
+			expected: []SSHUser{"ssh-it-user"},
+		},
+		{
+			name:     "multiple normal users",
+			users:    SSHUsers{"ubuntu", "admin", "user1"},
+			expected: []SSHUser{"ubuntu", "admin", "user1"},
+		},
+		{
+			name:     "mixed users with root",
+			users:    SSHUsers{"ubuntu", "root", "admin"},
+			expected: []SSHUser{"ubuntu", "admin"},
+		},
+		{
+			name:     "mixed users with autogroup:nonroot",
+			users:    SSHUsers{"ubuntu", SSHUser(AutoGroupNonRoot), "admin"},
+			expected: []SSHUser{"ubuntu", "admin"},
+		},
+		{
+			name:     "mixed users with both root and autogroup:nonroot",
+			users:    SSHUsers{"ubuntu", "root", SSHUser(AutoGroupNonRoot), "admin"},
+			expected: []SSHUser{"ubuntu", "admin"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.users.NormalUsers()
+			assert.ElementsMatch(t, tt.expected, result, "NormalUsers() should return expected normal users")
+		})
+	}
+}
+
+func TestSSHUsers_ContainsRoot(t *testing.T) {
+	tests := []struct {
+		name     string
+		users    SSHUsers
+		expected bool
+	}{
+		{
+			name:     "empty users",
+			users:    SSHUsers{},
+			expected: false,
+		},
+		{
+			name:     "contains root",
+			users:    SSHUsers{"root"},
+			expected: true,
+		},
+		{
+			name:     "does not contain root",
+			users:    SSHUsers{"ubuntu", "admin"},
+			expected: false,
+		},
+		{
+			name:     "contains root among others",
+			users:    SSHUsers{"ubuntu", "root", "admin"},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.users.ContainsRoot()
+			assert.Equal(t, tt.expected, result, "ContainsRoot() should return expected result")
+		})
+	}
+}
+
+func TestSSHUsers_ContainsNonRoot(t *testing.T) {
+	tests := []struct {
+		name     string
+		users    SSHUsers
+		expected bool
+	}{
+		{
+			name:     "empty users",
+			users:    SSHUsers{},
+			expected: false,
+		},
+		{
+			name:     "contains autogroup:nonroot",
+			users:    SSHUsers{SSHUser(AutoGroupNonRoot)},
+			expected: true,
+		},
+		{
+			name:     "does not contain autogroup:nonroot",
+			users:    SSHUsers{"ubuntu", "admin", "root"},
+			expected: false,
+		},
+		{
+			name:     "contains autogroup:nonroot among others",
+			users:    SSHUsers{"ubuntu", SSHUser(AutoGroupNonRoot), "admin"},
+			expected: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := tt.users.ContainsNonRoot()
+			assert.Equal(t, tt.expected, result, "ContainsNonRoot() should return expected result")
+		})
+	}
+}
+
 func mustIPSet(prefixes ...string) *netipx.IPSet {
 	var builder netipx.IPSetBuilder
 	for _, p := range prefixes {