ssh/tailssh: check if user matching autogroup:nonroot is root

Add a check to ensure that the user being matched to an
autogroup:nonroot rule is in fact a non-root user on the system.

Updates https://github.com/tailscale/corp/issues/43245

Signed-off-by: Mario Minardi <mario@tailscale.com>

ssh/tailssh/tailssh.go

@@ -22,7 +22,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"regexp"
 	"runtime"
 	"strconv"
 	"strings"
@@ -48,6 +47,7 @@ import (
 	"tailscale.com/util/clientmetric"
 	"tailscale.com/util/httpm"
 	"tailscale.com/util/mak"
+	"tailscale.com/version/distro"
 )
 
 var (
@@ -64,8 +64,6 @@ var (
 	// hookSSHLoginSuccess is called after successful SSH authentication.
 	// It is set by platform-specific code (e.g., auditd_linux.go).
 	hookSSHLoginSuccess feature.Hook[func(logf logger.Logf, c *conn)]
-
-	uidRegex = regexp.MustCompile("^[0-9]+$")
 )
 
 const (
@@ -344,10 +342,6 @@ func (c *conn) clientAuth(cm ssh.ConnMetadata) (perms *ssh.Permissions, retErr e
 		return &ssh.Permissions{}, nil
 	}
 
-	if uidRegex.MatchString(cm.User()) {
-		return nil, c.errBanner(fmt.Sprintf("rejecting username %q. Usernames that consist of only digits are not allowed as they are ambiguous with numerical UIDs", cm.User()), nil)
-	}
-
 	if err := c.setInfo(cm); err != nil {
 		return nil, c.errBanner("failed to get connection info", err)
 	}
@@ -1262,6 +1256,22 @@ func mapLocalUser(ruleSSHUsers map[string]string, reqSSHUser string) (localUser
 		v = ruleSSHUsers["*"]
 	}
 	if v == "=" {
+		// Skip lookup for gokrazy as we intentionally fall back to a synthesized
+		// root user there when user lookup fails.
+		if distro.Get() == distro.Gokrazy {
+			return reqSSHUser
+		}
+
+		// Immediately look up user information for purposes of generating
+		// hold and delegate URL (if necessary).
+		lu, err := userLookup(reqSSHUser)
+		if err != nil {
+			return ""
+		}
+		// Don't match as root for autogroup:nonroot
+		if lu.Uid == "0" || lu.Username == "root" {
+			return ""
+		}
 		return reqSSHUser
 	}
 	return v

ssh/tailssh/tailssh_test.go

@@ -6,6 +6,7 @@
 package tailssh
 
 import (
+	"bufio"
 	"bytes"
 	"context"
 	"crypto/ecdsa"
@@ -58,6 +59,10 @@ import (
 
 func TestMatchRule(t *testing.T) {
 	someAction := new(tailcfg.SSHAction)
+	// nonRootUser is a real non-root user in the local passwd database,
+	// used by the "ssh-user-equal" case because mapLocalUser now does a
+	// userLookup (via getent) when the mapped value is "=" and rejects root.
+	nonRootUser := aNonRootUser(t)
 	tests := []struct {
 		name          string
 		rule          *tailcfg.SSHRule
@@ -219,8 +224,8 @@ func TestMatchRule(t *testing.T) {
 					"*": "=",
 				},
 			},
-			ci:       &sshConnInfo{sshUser: "alice"},
+			ci:       &sshConnInfo{sshUser: nonRootUser},
-			wantUser: "alice",
+			wantUser: nonRootUser,
 		},
 	}
 	for _, tt := range tests {
@@ -396,6 +401,39 @@ var currentUser = func() string {
 	return os.Getenv("USER")
 }()
 
+// aNonRootUser returns the username of a non-root user that exists in the
+// local passwd database (verified via userLookup, which uses getent on
+// Linux). It prefers the current user, falling back to reading /etc/passwd
+// for a real non-root account when the tests are run as root (e.g. in CI).
+// It skips the test if no such user can be found.
+func aNonRootUser(t *testing.T) string {
+	t.Helper()
+	if u, err := userLookup(currentUser); err == nil && u.Uid != "0" && u.Username != "root" {
+		return currentUser
+	}
+	f, err := os.Open("/etc/passwd")
+	if err != nil {
+		t.Skipf("can't find a non-root user: %v", err)
+	}
+	defer f.Close()
+	scan := bufio.NewScanner(f)
+	for scan.Scan() {
+		fields := strings.Split(scan.Text(), ":")
+		if len(fields) < 3 {
+			continue
+		}
+		name, uid := fields[0], fields[2]
+		if uid == "0" || name == "root" {
+			continue
+		}
+		if _, err := userLookup(name); err == nil {
+			return name
+		}
+	}
+	t.Skip("can't find a non-root user in /etc/passwd")
+	return ""
+}
+
 func (ts *localState) Dialer() *tsdial.Dialer {
 	return &tsdial.Dialer{}
 }
@@ -468,6 +506,7 @@ func newSSHRule(action *tailcfg.SSHAction) *tailcfg.SSHRule {
 	return &tailcfg.SSHRule{
 		SSHUsers: map[string]string{
 			"alice": currentUser,
+			"*":     "=",
 		},
 		Action: action,
 		Principals: []*tailcfg.SSHPrincipal{
@@ -803,6 +842,11 @@ func TestSSHAuthFlow(t *testing.T) {
 		Reject:  true,
 		Message: "Go Away!",
 	})
+	autogroupNonrootRule := newSSHRule(&tailcfg.SSHAction{
+		Accept:  true,
+		Message: "autogroup:nonroot",
+	})
+	autogroupNonrootRule.SSHUsers = map[string]string{"*": "="}
 
 	tests := []struct {
 		name         string
@@ -832,7 +876,7 @@ func TestSSHAuthFlow(t *testing.T) {
 			wantBanners: []string{`tailscale: tailnet policy does not permit you to SSH as user "alice"` + "\n"},
 		},
 		{
-			name:    "digit-only-username",
+			name:    "user-mismatch-numeric-username",
 			sshUser: "321",
 			state: &localState{
 				sshEnabled:   true,
@@ -840,7 +884,62 @@ func TestSSHAuthFlow(t *testing.T) {
 				matchingRule: bobRule,
 			},
 			authErr:     true,
-			wantBanners: []string{`tailscale: rejecting username "321". Usernames that consist of only digits are not allowed as they are ambiguous with numerical UIDs` + "\n"},
+			wantBanners: []string{`tailscale: tailnet policy does not permit you to SSH as user "321"` + "\n"},
+		},
+		{
+			name:    "user-mismatch-root-uid",
+			sshUser: "0",
+			state: &localState{
+				sshEnabled:   true,
+				varRoot:      varRoot,
+				matchingRule: autogroupNonrootRule,
+			},
+			authErr:     true,
+			wantBanners: []string{`tailscale: tailnet policy does not permit you to SSH as user "0"` + "\n"},
+		},
+		{
+			name:    "user-mismatch-root-uid-leading-space",
+			sshUser: " 0",
+			state: &localState{
+				sshEnabled:   true,
+				varRoot:      varRoot,
+				matchingRule: autogroupNonrootRule,
+			},
+			authErr:     true,
+			wantBanners: []string{`tailscale: tailnet policy does not permit you to SSH as user " 0"` + "\n"},
+		},
+		{
+			name:    "user-mismatch-root-uid-force-password-auth",
+			sshUser: "0+password",
+			state: &localState{
+				sshEnabled:   true,
+				varRoot:      varRoot,
+				matchingRule: autogroupNonrootRule,
+			},
+			authErr:     true,
+			wantBanners: []string{`tailscale: tailnet policy does not permit you to SSH as user "0"` + "\n"},
+		},
+		{
+			name:    "user-mismatch-double-zero-force-password-auth",
+			sshUser: "00+password",
+			state: &localState{
+				sshEnabled:   true,
+				varRoot:      varRoot,
+				matchingRule: autogroupNonrootRule,
+			},
+			authErr:     true,
+			wantBanners: []string{`tailscale: tailnet policy does not permit you to SSH as user "00"` + "\n"},
+		},
+		{
+			name:    "user-mismatch-leading-plus",
+			sshUser: "+0",
+			state: &localState{
+				sshEnabled:   true,
+				varRoot:      varRoot,
+				matchingRule: autogroupNonrootRule,
+			},
+			authErr:     true,
+			wantBanners: []string{`tailscale: tailnet policy does not permit you to SSH as user "+0"` + "\n"},
 		},
 		{
 			name: "accept",

tstest/integration/testcontrol/testcontrol.go

@@ -698,6 +698,15 @@ func (s *Server) SetMasqueradeAddresses(pairs []MasqueradePair) {
 	s.updateLocked("SetMasqueradeAddresses", s.nodeIDsLocked(0))
 }
 
+// SetSSHPolicy sets the SSH policy sent in MapResponses and notifies all
+// connected nodes so they pick up the change.
+func (s *Server) SetSSHPolicy(policy *tailcfg.SSHPolicy) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.SSHPolicy = policy
+	s.updateLocked("SetSSHPolicy", s.nodeIDsLocked(0))
+}
+
 // SetNodeCapMap overrides the capability map the specified client receives.
 func (s *Server) SetNodeCapMap(nodeKey key.NodePublic, capMap tailcfg.NodeCapMap) {
 	s.mu.Lock()

tstest/natlab/vmtest/ssh_test.go

@@ -12,6 +12,8 @@ import (
 	"time"
 
 	"github.com/creachadair/mds/shell"
+	"tailscale.com/tailcfg"
+	"tailscale.com/tstest"
 	"tailscale.com/tstest/natlab/vmtest"
 	"tailscale.com/tstest/natlab/vnet"
 )
@@ -37,6 +39,8 @@ func TestTailscaleSSH_Ubuntu(t *testing.T) {
 	if out, code := testSuite.ssh(t, "nosuchuser", "true"); code == 0 {
 		t.Errorf("ubuntu nonexistent user: ssh succeeded, want failure:\n%s", out)
 	}
+
+	testSuite.checkAutogroupNonroot(t, "ubuntu")
 }
 
 // TestTailscaleSSH_FreeBSD exercises the Tailscale SSH server ("tailscale up
@@ -60,6 +64,8 @@ func TestTailscaleSSH_FreeBSD(t *testing.T) {
 	if out, code := testSuite.ssh(t, "nosuchuser", "true"); code == 0 {
 		t.Errorf("freebsd nonexistent user: ssh succeeded, want failure:\n%s", out)
 	}
+
+	testSuite.checkAutogroupNonroot(t, "freebsd")
 }
 
 // TestTailscaleSSH_Gokrazy exercises the gokrazy-specific cases in the
@@ -157,6 +163,34 @@ func (st *sshTest) check(t *testing.T, desc, user string, cmd, want string) {
 	}
 }
 
+// checkAutogroupNonroot adjusts the server's SSH policy to be equivalent to "autogroup:nonroot"
+// and verifies that attempting to SSH as root fails.
+func (st *sshTest) checkAutogroupNonroot(t *testing.T, name string) {
+	t.Helper()
+	orignalSSHPolicy := st.env.ControlServer().SSHPolicy
+	defer func() {
+		st.env.ControlServer().SetSSHPolicy(orignalSSHPolicy)
+	}()
+
+	st.env.ControlServer().SetSSHPolicy(&tailcfg.SSHPolicy{
+		Rules: []*tailcfg.SSHRule{{
+			Principals: []*tailcfg.SSHPrincipal{{Any: true}},
+			SSHUsers:   map[string]string{"*": "="},
+			Action:     &tailcfg.SSHAction{Accept: true},
+		}},
+	})
+
+	if err := tstest.WaitFor(30*time.Second, func() error {
+		_, code := st.ssh(t, "root", "true")
+		if code == 0 {
+			return fmt.Errorf("root SSH still succeeds")
+		}
+		return nil
+	}); err != nil {
+		t.Fatalf("%s: root SSH still succeeds after policy update to autogroup:nonroot-only; policy may not have propagated", name)
+	}
+}
+
 func newTestSuite(t *testing.T, serverName string, serverOS vmtest.OSImage) *sshTest {
 	t.Helper()
 	env := vmtest.New(t)

tstest/natlab/vmtest/vmtest.go

@@ -1779,8 +1779,9 @@ func (e *Env) initVnet() {
 				e.server.ControlServer().SSHPolicy = &tailcfg.SSHPolicy{
 					Rules: []*tailcfg.SSHRule{{
 						Principals: []*tailcfg.SSHPrincipal{{Any: true}},
-						SSHUsers:   map[string]string{"*": "="},
+						// Allow permissive login + root login by default
-						Action:     &tailcfg.SSHAction{Accept: true},
+						SSHUsers: map[string]string{"*": "=", "root": "root"},
+						Action:   &tailcfg.SSHAction{Accept: true},
 					}},
 				}
 				break