From 236531c5fc6d7295f8832f47a569e49fceca76eb Mon Sep 17 00:00:00 2001
From: Will Norris <will@tailscale.com>
Date: Wed, 10 Jan 2024 13:58:51 -0800
Subject: [PATCH] ipn/ipnserver: always allow Windows SYSTEM user to connect

When establishing connections to the ipnserver, we validate that the
local user is allowed to connect.  If Tailscale is currently being
managed by a different user (primarily for multi-user Windows installs),
we don't allow the connection.

With the new device web UI, the inbound connection is coming from
tailscaled itself, which is often running as "NT AUTHORITY\SYSTEM".
In this case, we still want to allow the connection, even though it
doesn't match the user running the Tailscale GUI. The SYSTEM user has
full access to everything on the system anyway, so this doesn't escalate
privileges.

Eventually, we want the device web UI to run outside of the tailscaled
process, at which point this exception would probably not be needed.

Updates tailscale/corp#16393

Signed-off-by: Will Norris <will@tailscale.com>
---
 ipn/ipnauth/ipnauth.go         |  2 ++
 ipn/ipnauth/ipnauth_windows.go |  6 ++++++
 ipn/ipnlocal/local.go          | 10 ++++++++++
 ipn/ipnserver/server.go        | 12 ++++++++++--
 4 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/ipn/ipnauth/ipnauth.go b/ipn/ipnauth/ipnauth.go
index 7ae9ff3e4..5ae6ef23b 100644
--- a/ipn/ipnauth/ipnauth.go
+++ b/ipn/ipnauth/ipnauth.go
@@ -46,6 +46,8 @@ type WindowsToken interface {
 	// IsElevated reports whether the receiver is currently executing as an
 	// elevated administrative user.
 	IsElevated() bool
+	// IsLocalSystem reports whether the receiver is the built-in SYSTEM user.
+	IsLocalSystem() bool
 	// UserDir returns the special directory identified by folderID as associated
 	// with the receiver. folderID must be one of the KNOWNFOLDERID values from
 	// the x/sys/windows package, serialized as a stringified GUID.
diff --git a/ipn/ipnauth/ipnauth_windows.go b/ipn/ipnauth/ipnauth_windows.go
index d3421a5dc..9abd04cd1 100644
--- a/ipn/ipnauth/ipnauth_windows.go
+++ b/ipn/ipnauth/ipnauth_windows.go
@@ -93,6 +93,12 @@ func (t *token) IsElevated() bool {
 	return t.t.IsElevated()
 }
 
+func (t *token) IsLocalSystem() bool {
+	// https://web.archive.org/web/2024/https://learn.microsoft.com/en-us/windows-server/identity/ad-ds/manage/understand-security-identifiers
+	const systemUID = ipn.WindowsUserID("S-1-5-18")
+	return t.IsUID(systemUID)
+}
+
 func (t *token) UserDir(folderID string) (string, error) {
 	guid, err := windows.GUIDFromString(folderID)
 	if err != nil {
diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go
index a5f3ae693..a26b53228 100644
--- a/ipn/ipnlocal/local.go
+++ b/ipn/ipnlocal/local.go
@@ -2735,6 +2735,16 @@ func (b *LocalBackend) CheckIPNConnectionAllowed(ci *ipnauth.ConnIdentity) error
 	if !b.pm.CurrentPrefs().ForceDaemon() {
 		return nil
 	}
+
+	// Always allow Windows SYSTEM user to connect,
+	// even if Tailscale is currently being used by another user.
+	if tok, err := ci.WindowsToken(); err == nil {
+		defer tok.Close()
+		if tok.IsLocalSystem() {
+			return nil
+		}
+	}
+
 	uid := ci.WindowsUserID()
 	if uid == "" {
 		return errors.New("empty user uid in connection identity")
diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go
index 90b5cfd65..b414e05a4 100644
--- a/ipn/ipnserver/server.go
+++ b/ipn/ipnserver/server.go
@@ -251,6 +251,12 @@ func (s *Server) checkConnIdentityLocked(ci *ipnauth.ConnIdentity) error {
 				return err
 			}
 
+			// Always allow Windows SYSTEM user to connect,
+			// even if Tailscale is currently being used by another user.
+			if chkTok != nil && chkTok.IsLocalSystem() {
+				return nil
+			}
+
 			activeTok, err := active.WindowsToken()
 			if err == nil {
 				defer activeTok.Close()
@@ -401,8 +407,10 @@ func (s *Server) addActiveHTTPRequest(req *http.Request, ci *ipnauth.ConnIdentit
 			if !errors.Is(err, ipnauth.ErrNotImplemented) {
 				s.logf("error obtaining access token: %v", err)
 			}
-		} else {
-			// Tell the LocalBackend about the identity we're now running as.
+		} else if !token.IsLocalSystem() {
+			// Tell the LocalBackend about the identity we're now running as,
+			// unless its the SYSTEM user. That user is not a real account and
+			// doesn't have a home directory.
 			uid, err := lb.SetCurrentUser(token)
 			if err != nil {
 				token.Close()