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>
This commit is contained in:
Will Norris 2024-01-10 13:58:51 -08:00 committed by Will Norris
parent 7100b6e721
commit 236531c5fc
4 changed files with 28 additions and 2 deletions

View File

@ -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.

View File

@ -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 {

View File

@ -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")

View File

@ -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()