diff --git a/cmd/k8s-operator/depaware.txt b/cmd/k8s-operator/depaware.txt index 738d0cff2..34f933e37 100644 --- a/cmd/k8s-operator/depaware.txt +++ b/cmd/k8s-operator/depaware.txt @@ -796,7 +796,7 @@ tailscale.com/cmd/k8s-operator dependencies: (generated by github.com/tailscale/ 💣 tailscale.com/util/osdiag from tailscale.com/ipn/localapi W 💣 tailscale.com/util/osdiag/internal/wsc from tailscale.com/util/osdiag tailscale.com/util/osshare from tailscale.com/ipn/ipnlocal - tailscale.com/util/osuser from tailscale.com/ipn/ipnlocal+ + tailscale.com/util/osuser from tailscale.com/ipn/ipnlocal tailscale.com/util/progresstracking from tailscale.com/ipn/localapi tailscale.com/util/race from tailscale.com/net/dns/resolver tailscale.com/util/racebuild from tailscale.com/logpolicy diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 16c09cf95..4e5da410a 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -386,7 +386,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de 💣 tailscale.com/util/osdiag from tailscale.com/cmd/tailscaled+ W 💣 tailscale.com/util/osdiag/internal/wsc from tailscale.com/util/osdiag tailscale.com/util/osshare from tailscale.com/cmd/tailscaled+ - tailscale.com/util/osuser from tailscale.com/ipn/localapi+ + tailscale.com/util/osuser from tailscale.com/ipn/ipnlocal+ tailscale.com/util/progresstracking from tailscale.com/ipn/localapi tailscale.com/util/race from tailscale.com/net/dns/resolver tailscale.com/util/racebuild from tailscale.com/logpolicy diff --git a/ipn/ipnauth/actor.go b/ipn/ipnauth/actor.go new file mode 100644 index 000000000..db3192c91 --- /dev/null +++ b/ipn/ipnauth/actor.go @@ -0,0 +1,47 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package ipnauth + +import ( + "tailscale.com/ipn" +) + +// Actor is any actor using the [ipnlocal.LocalBackend]. +// +// It typically represents a specific OS user, indicating that an operation +// is performed on behalf of this user, should be evaluated against their +// access rights, and performed in their security context when applicable. +type Actor interface { + // UserID returns an OS-specific UID of the user represented by the receiver, + // or "" if the actor does not represent a specific user on a multi-user system. + // As of 2024-08-27, it is only used on Windows. + UserID() ipn.WindowsUserID + // Username returns the user name associated with the receiver, + // or "" if the actor does not represent a specific user. + Username() (string, error) + + // IsLocalSystem reports whether the actor is the Windows' Local System account. + // + // Deprecated: this method exists for compatibility with the current (as of 2024-08-27) + // permission model and will be removed as we progress on tailscale/corp#18342. + IsLocalSystem() bool + + // IsLocalAdmin reports whether the actor has administrative access to the + // local machine, for whatever that means with respect to the current OS. + // + // The operatorUID is only used on Unix-like platforms and specifies the ID + // of a local user (in the os/user.User.Uid string form) who is allowed to + // operate tailscaled without being root or using sudo. + // + // Deprecated: this method exists for compatibility with the current (as of 2024-08-27) + // permission model and will be removed as we progress on tailscale/corp#18342. + IsLocalAdmin(operatorUID string) bool +} + +// ActorCloser is an optional interface that might be implemented by an [Actor] +// that must be closed when done to release the resources. +type ActorCloser interface { + // Close releases resources associated with the receiver. + Close() error +} diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index da8b68744..b57be7a05 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -291,7 +291,7 @@ type LocalBackend struct { componentLogUntil map[string]componentLogState // c2nUpdateStatus is the status of c2n-triggered client update. c2nUpdateStatus updateStatus - currentUser ipnauth.WindowsToken + currentUser ipnauth.Actor selfUpdateProgress []ipnstate.UpdateProgress lastSelfUpdateState ipnstate.SelfUpdateStatus // capForcedNetfilter is the netfilter that control instructs Linux clients @@ -3101,13 +3101,13 @@ func (b *LocalBackend) InServerMode() bool { return b.pm.CurrentPrefs().ForceDaemon() } -// CheckIPNConnectionAllowed returns an error if the identity in ci should not +// CheckIPNConnectionAllowed returns an error if the specified actor should not // be allowed to connect or make requests to the LocalAPI currently. // -// Currently (as of 2022-11-23), this is only used on Windows to check if -// we started in server mode and ci is from an identity other than the one -// that started the server. -func (b *LocalBackend) CheckIPNConnectionAllowed(ci *ipnauth.ConnIdentity) error { +// Currently (as of 2024-08-26), this is only used on Windows. +// We plan to remove it as part of the multi-user and unattended mode improvements +// as we progress on tailscale/corp#18342. +func (b *LocalBackend) CheckIPNConnectionAllowed(actor ipnauth.Actor) error { b.mu.Lock() defer b.mu.Unlock() serverModeUid := b.pm.CurrentUserID() @@ -3122,14 +3122,11 @@ func (b *LocalBackend) CheckIPNConnectionAllowed(ci *ipnauth.ConnIdentity) error // 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 - } + if actor.IsLocalSystem() { + return nil } - uid := ci.WindowsUserID() + uid := actor.UserID() if uid == "" { return errors.New("empty user uid in connection identity") } @@ -3308,18 +3305,14 @@ func (b *LocalBackend) shouldUploadServices() bool { // unattended mode. The user must disable unattended mode before the user can be // changed. // -// On non-multi-user systems, the token should be set to nil. +// On non-multi-user systems, the user should be set to nil. // -// SetCurrentUser returns the ipn.WindowsUserID associated with token +// SetCurrentUser returns the ipn.WindowsUserID associated with the user // when successful. -func (b *LocalBackend) SetCurrentUser(token ipnauth.WindowsToken) (ipn.WindowsUserID, error) { +func (b *LocalBackend) SetCurrentUser(actor ipnauth.Actor) (ipn.WindowsUserID, error) { var uid ipn.WindowsUserID - if token != nil { - var err error - uid, err = token.UID() - if err != nil { - return "", err - } + if actor != nil { + uid = actor.UserID() } unlock := b.lockAndGetUnlock() @@ -3331,10 +3324,10 @@ func (b *LocalBackend) SetCurrentUser(token ipnauth.WindowsToken) (ipn.WindowsUs if err := b.pm.SetCurrentUserID(uid); err != nil { return uid, nil } - if b.currentUser != nil { - b.currentUser.Close() + if c, ok := b.currentUser.(ipnauth.ActorCloser); ok { + c.Close() } - b.currentUser = token + b.currentUser = actor b.resetForProfileChangeLockedOnEntry(unlock) return uid, nil } @@ -5037,7 +5030,9 @@ func (b *LocalBackend) ResetForClientDisconnect() { b.setNetMapLocked(nil) b.pm.Reset() if b.currentUser != nil { - b.currentUser.Close() + if c, ok := b.currentUser.(ipnauth.ActorCloser); ok { + c.Close() + } b.currentUser = nil } b.keyExpired = false diff --git a/ipn/ipnserver/actor.go b/ipn/ipnserver/actor.go new file mode 100644 index 000000000..761c9816c --- /dev/null +++ b/ipn/ipnserver/actor.go @@ -0,0 +1,188 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package ipnserver + +import ( + "context" + "errors" + "fmt" + "net" + "os/exec" + "runtime" + "time" + + "tailscale.com/ipn" + "tailscale.com/ipn/ipnauth" + "tailscale.com/types/logger" + "tailscale.com/util/ctxkey" + "tailscale.com/util/osuser" + "tailscale.com/version" +) + +var _ ipnauth.Actor = (*actor)(nil) + +// actor implements [ipnauth.Actor] and provides additional functionality that is +// specific to the current (as of 2024-08-27) permission model. +// +// Deprecated: this type exists for compatibility reasons and will be removed as +// we progress on tailscale/corp#18342. +type actor struct { + logf logger.Logf + ci *ipnauth.ConnIdentity + + isLocalSystem bool // whether the actor is the Windows' Local System identity. +} + +func newActor(logf logger.Logf, c net.Conn) (*actor, error) { + ci, err := ipnauth.GetConnIdentity(logf, c) + if err != nil { + return nil, err + } + return &actor{logf: logf, ci: ci, isLocalSystem: connIsLocalSystem(ci)}, nil +} + +// IsLocalSystem implements [ipnauth.Actor]. +func (a *actor) IsLocalSystem() bool { + return a.isLocalSystem +} + +// IsLocalAdmin implements [ipnauth.Actor]. +func (a *actor) IsLocalAdmin(operatorUID string) bool { + return a.isLocalSystem || connIsLocalAdmin(a.logf, a.ci, operatorUID) +} + +// UserID implements [ipnauth.Actor]. +func (a *actor) UserID() ipn.WindowsUserID { + return a.ci.WindowsUserID() +} + +func (a *actor) pid() int { + return a.ci.Pid() +} + +// Username implements [ipnauth.Actor]. +func (a *actor) Username() (string, error) { + if a.ci == nil { + a.logf("[unexpected] missing ConnIdentity in ipnserver.actor") + return "", errors.New("missing ConnIdentity") + } + switch runtime.GOOS { + case "windows": + tok, err := a.ci.WindowsToken() + if err != nil { + return "", fmt.Errorf("get windows token: %w", err) + } + defer tok.Close() + return tok.Username() + case "darwin", "linux": + uid, ok := a.ci.Creds().UserID() + if !ok { + return "", errors.New("missing user ID") + } + u, err := osuser.LookupByUID(uid) + if err != nil { + return "", fmt.Errorf("lookup user: %w", err) + } + return u.Username, nil + default: + return "", errors.New("unsupported OS") + } +} + +type actorOrError struct { + actor *actor + err error +} + +func (a actorOrError) unwrap() (*actor, error) { + return a.actor, a.err +} + +var errNoActor = errors.New("connection actor not available") + +var actorKey = ctxkey.New("ipnserver.actor", actorOrError{err: errNoActor}) + +// contextWithActor returns a new context that carries the identity of the actor +// owning the other end of the [net.Conn]. It can be retrieved with [actorFromContext]. +func contextWithActor(ctx context.Context, logf logger.Logf, c net.Conn) context.Context { + actor, err := newActor(logf, c) + return actorKey.WithValue(ctx, actorOrError{actor: actor, err: err}) +} + +// actorFromContext returns an [actor] associated with ctx, +// or an error if the context does not carry an actor's identity. +func actorFromContext(ctx context.Context) (*actor, error) { + return actorKey.Value(ctx).unwrap() +} + +func connIsLocalSystem(ci *ipnauth.ConnIdentity) bool { + token, err := ci.WindowsToken() + return err == nil && token.IsLocalSystem() +} + +// connIsLocalAdmin reports whether the connected client has administrative +// access to the local machine, for whatever that means with respect to the +// current OS. +// +// This is useful because tailscaled itself always runs with elevated rights: +// we want to avoid privilege escalation for certain mutative operations. +func connIsLocalAdmin(logf logger.Logf, ci *ipnauth.ConnIdentity, operatorUID string) bool { + if ci == nil { + logf("[unexpected] missing ConnIdentity in LocalAPI Handler") + return false + } + switch runtime.GOOS { + case "windows": + tok, err := ci.WindowsToken() + if err != nil { + if !errors.Is(err, ipnauth.ErrNotImplemented) { + logf("ipnauth.ConnIdentity.WindowsToken() error: %v", err) + } + return false + } + defer tok.Close() + + return tok.IsElevated() + + case "darwin": + // Unknown, or at least unchecked on sandboxed macOS variants. Err on + // the side of less permissions. + // + // authorizeServeConfigForGOOSAndUserContext should not call + // connIsLocalAdmin on sandboxed variants anyway. + if version.IsSandboxedMacOS() { + return false + } + // This is a standalone tailscaled setup, use the same logic as on + // Linux. + fallthrough + case "linux": + uid, ok := ci.Creds().UserID() + if !ok { + return false + } + // root is always admin. + if uid == "0" { + return true + } + // if non-root, must be operator AND able to execute "sudo tailscale". + if operatorUID != "" && uid != operatorUID { + return false + } + u, err := osuser.LookupByUID(uid) + if err != nil { + return false + } + // Short timeout just in case sudo hangs for some reason. + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + defer cancel() + if err := exec.CommandContext(ctx, "sudo", "--other-user="+u.Name, "--list", "tailscale").Run(); err != nil { + return false + } + return true + + default: + return false + } +} diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go index fd2075ba8..25c672e2e 100644 --- a/ipn/ipnserver/server.go +++ b/ipn/ipnserver/server.go @@ -23,7 +23,6 @@ "tailscale.com/envknob" "tailscale.com/ipn" - "tailscale.com/ipn/ipnauth" "tailscale.com/ipn/ipnlocal" "tailscale.com/ipn/localapi" "tailscale.com/net/netmon" @@ -52,7 +51,7 @@ type Server struct { // lock order: mu, then LocalBackend.mu mu sync.Mutex lastUserID ipn.WindowsUserID // tracks last userid; on change, Reset state for paranoia - activeReqs map[*http.Request]*ipnauth.ConnIdentity + activeReqs map[*http.Request]*actor backendWaiter waiterSet // of LocalBackend waiters zeroReqWaiter waiterSet // of blockUntilZeroConnections waiters } @@ -75,7 +74,7 @@ func (s *Server) mustBackend() *ipnlocal.LocalBackend { type waiterSet set.HandleSet[context.CancelFunc] // add registers a new waiter in the set. -// It aquires mu to add the waiter, and does so again when cleanup is called to remove it. +// It acquires mu to add the waiter, and does so again when cleanup is called to remove it. // ready is closed when the waiter is ready (or ctx is done). func (s *waiterSet) add(mu *sync.Mutex, ctx context.Context) (ready <-chan struct{}, cleanup func()) { ctx, cancel := context.WithCancel(ctx) @@ -173,15 +172,13 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) { return } - var ci *ipnauth.ConnIdentity - switch v := r.Context().Value(connIdentityContextKey{}).(type) { - case *ipnauth.ConnIdentity: - ci = v - case error: - http.Error(w, v.Error(), http.StatusUnauthorized) - return - case nil: - http.Error(w, "internal error: no connIdentityContextKey", http.StatusInternalServerError) + ci, err := actorFromContext(r.Context()) + if err != nil { + if errors.Is(err, errNoActor) { + http.Error(w, "internal error: "+err.Error(), http.StatusInternalServerError) + } else { + http.Error(w, err.Error(), http.StatusUnauthorized) + } return } @@ -199,9 +196,9 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) { if strings.HasPrefix(r.URL.Path, "/localapi/") { lah := localapi.NewHandler(lb, s.logf, s.backendLogID) - lah.PermitRead, lah.PermitWrite = s.localAPIPermissions(ci) - lah.PermitCert = s.connCanFetchCerts(ci) - lah.ConnIdentity = ci + lah.PermitRead, lah.PermitWrite = ci.Permissions(lb.OperatorUserID()) + lah.PermitCert = ci.CanFetchCerts() + lah.Actor = ci lah.ServeHTTP(w, r) return } @@ -234,42 +231,28 @@ func (e inUseOtherUserError) Unwrap() error { return e.error } // The returned error, when non-nil, will be of type inUseOtherUserError. // // s.mu must be held. -func (s *Server) checkConnIdentityLocked(ci *ipnauth.ConnIdentity) error { +func (s *Server) checkConnIdentityLocked(ci *actor) error { // If clients are already connected, verify they're the same user. // This mostly matters on Windows at the moment. if len(s.activeReqs) > 0 { - var active *ipnauth.ConnIdentity + var active *actor for _, active = range s.activeReqs { break } if active != nil { - chkTok, err := ci.WindowsToken() - if err == nil { - defer chkTok.Close() - } else if !errors.Is(err, ipnauth.ErrNotImplemented) { - return err - } - // Always allow Windows SYSTEM user to connect, // even if Tailscale is currently being used by another user. - if chkTok != nil && chkTok.IsLocalSystem() { + if ci.IsLocalSystem() { return nil } - activeTok, err := active.WindowsToken() - if err == nil { - defer activeTok.Close() - } else if !errors.Is(err, ipnauth.ErrNotImplemented) { - return err - } - - if chkTok != nil && !chkTok.EqualUIDs(activeTok) { + if ci.UserID() != active.UserID() { var b strings.Builder b.WriteString("Tailscale already in use") - if username, err := activeTok.Username(); err == nil { + if username, err := active.Username(); err == nil { fmt.Fprintf(&b, " by %s", username) } - fmt.Fprintf(&b, ", pid %d", active.Pid()) + fmt.Fprintf(&b, ", pid %d", active.pid()) return inUseOtherUserError{errors.New(b.String())} } } @@ -285,11 +268,11 @@ func (s *Server) checkConnIdentityLocked(ci *ipnauth.ConnIdentity) error { // // This is primarily used for the Windows GUI, to block until one user's done // controlling the tailscaled process. -func (s *Server) blockWhileIdentityInUse(ctx context.Context, ci *ipnauth.ConnIdentity) error { +func (s *Server) blockWhileIdentityInUse(ctx context.Context, actor *actor) error { inUse := func() bool { s.mu.Lock() defer s.mu.Unlock() - _, ok := s.checkConnIdentityLocked(ci).(inUseOtherUserError) + _, ok := s.checkConnIdentityLocked(actor).(inUseOtherUserError) return ok } for inUse() { @@ -304,24 +287,28 @@ func (s *Server) blockWhileIdentityInUse(ctx context.Context, ci *ipnauth.ConnId return nil } -// localAPIPermissions returns the permissions for the given identity accessing -// the Tailscale local daemon API. -// -// s.mu must not be held. -func (s *Server) localAPIPermissions(ci *ipnauth.ConnIdentity) (read, write bool) { +// Permissions returns the actor's permissions for accessing +// the Tailscale local daemon API. The operatorUID is only used on +// Unix-like platforms and specifies the ID of a local user +// (in the os/user.User.Uid string form) who is allowed +// to operate tailscaled without being root or using sudo. +func (a *actor) Permissions(operatorUID string) (read, write bool) { switch envknob.GOOS() { case "windows": - s.mu.Lock() - defer s.mu.Unlock() - if s.checkConnIdentityLocked(ci) == nil { - return true, true - } - return false, false + // As of 2024-08-27, according to the current permission model, + // Windows users always have read/write access to the local API if + // they're allowed to connect. Whether a user is allowed to connect + // is determined by [Server.checkConnIdentityLocked] when adding a + // new connection in [Server.addActiveHTTPRequest]. Therefore, it's + // acceptable to permit read and write access without any additional + // checks here. Note that this permission model is being changed in + // tailscale/corp#18342. + return true, true case "js": return true, true } - if ci.IsUnixSock() { - return true, !ci.IsReadonlyConn(s.mustBackend().OperatorUserID(), logger.Discard) + if a.ci.IsUnixSock() { + return true, !a.ci.IsReadonlyConn(operatorUID, logger.Discard) } return false, false } @@ -349,19 +336,19 @@ func isAllDigit(s string) bool { return true } -// connCanFetchCerts reports whether ci is allowed to fetch HTTPS +// CanFetchCerts reports whether the actor is allowed to fetch HTTPS // certs from this server when it wouldn't otherwise be able to. // -// That is, this reports whether ci should grant additional -// capabilities over what the conn would otherwise be able to do. +// That is, this reports whether the actor should grant additional +// capabilities over what the actor would otherwise be able to do. // // For now this only returns true on Unix machines when // TS_PERMIT_CERT_UID is set the to the userid of the peer // connection. It's intended to give your non-root webserver access // (www-data, caddy, nginx, etc) to certs. -func (s *Server) connCanFetchCerts(ci *ipnauth.ConnIdentity) bool { - if ci.IsUnixSock() && ci.Creds() != nil { - connUID, ok := ci.Creds().UserID() +func (a *actor) CanFetchCerts() bool { + if a.ci.IsUnixSock() && a.ci.Creds() != nil { + connUID, ok := a.ci.Creds().UserID() if ok && connUID == userIDFromString(envknob.String("TS_PERMIT_CERT_UID")) { return true } @@ -371,12 +358,13 @@ func (s *Server) connCanFetchCerts(ci *ipnauth.ConnIdentity) bool { // addActiveHTTPRequest adds c to the server's list of active HTTP requests. // -// If the returned error may be of type inUseOtherUserError. +// It returns an error if the specified actor is not allowed to connect. +// The returned error may be of type [inUseOtherUserError]. // // onDone must be called when the HTTP request is done. -func (s *Server) addActiveHTTPRequest(req *http.Request, ci *ipnauth.ConnIdentity) (onDone func(), err error) { - if ci == nil { - return nil, errors.New("internal error: nil connIdentity") +func (s *Server) addActiveHTTPRequest(req *http.Request, actor *actor) (onDone func(), err error) { + if actor == nil { + return nil, errors.New("internal error: nil actor") } lb := s.mustBackend() @@ -394,25 +382,19 @@ func (s *Server) addActiveHTTPRequest(req *http.Request, ci *ipnauth.ConnIdentit s.mu.Lock() defer s.mu.Unlock() - if err := s.checkConnIdentityLocked(ci); err != nil { + if err := s.checkConnIdentityLocked(actor); err != nil { return nil, err } - mak.Set(&s.activeReqs, req, ci) + mak.Set(&s.activeReqs, req, actor) if len(s.activeReqs) == 1 { - token, err := ci.WindowsToken() - if err != nil { - if !errors.Is(err, ipnauth.ErrNotImplemented) { - s.logf("error obtaining access token: %v", err) - } - } else if !token.IsLocalSystem() { + if envknob.GOOS() == "windows" && !actor.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) + uid, err := lb.SetCurrentUser(actor) if err != nil { - token.Close() return nil, err } if s.lastUserID != uid { @@ -488,10 +470,6 @@ func (s *Server) SetLocalBackend(lb *ipnlocal.LocalBackend) { // https://github.com/tailscale/tailscale/issues/6522 } -// connIdentityContextKey is the http.Request.Context's context.Value key for either an -// *ipnauth.ConnIdentity or an error. -type connIdentityContextKey struct{} - // Run runs the server, accepting connections from ln forever. // // If the context is done, the listener is closed. It is also the base context @@ -525,11 +503,7 @@ func (s *Server) Run(ctx context.Context, ln net.Listener) error { Handler: http.HandlerFunc(s.serveHTTP), BaseContext: func(_ net.Listener) context.Context { return ctx }, ConnContext: func(ctx context.Context, c net.Conn) context.Context { - ci, err := ipnauth.GetConnIdentity(s.logf, c) - if err != nil { - return context.WithValue(ctx, connIdentityContextKey{}, err) - } - return context.WithValue(ctx, connIdentityContextKey{}, ci) + return contextWithActor(ctx, s.logf, c) }, // Localhost connections are cheap; so only do // keep-alives for a short period of time, as these diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index af4c262e0..1902a967c 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -23,7 +23,6 @@ "net/netip" "net/url" "os" - "os/exec" "path" "runtime" "slices" @@ -60,7 +59,6 @@ "tailscale.com/util/httpm" "tailscale.com/util/mak" "tailscale.com/util/osdiag" - "tailscale.com/util/osuser" "tailscale.com/util/progresstracking" "tailscale.com/util/rands" "tailscale.com/util/testenv" @@ -183,12 +181,8 @@ type Handler struct { // cert fetching access. PermitCert bool - // ConnIdentity is the identity of the client connected to the Handler. - ConnIdentity *ipnauth.ConnIdentity - - // Test-only override for connIsLocalAdmin method. If non-nil, - // connIsLocalAdmin returns this value. - testConnIsLocalAdmin *bool + // Actor is the identity of the client connected to the Handler. + Actor ipnauth.Actor b *ipnlocal.LocalBackend logf logger.Logf @@ -1065,7 +1059,7 @@ func authorizeServeConfigForGOOSAndUserContext(goos string, configIn *ipn.ServeC if !configIn.HasPathHandler() { return nil } - if h.connIsLocalAdmin() { + if h.Actor.IsLocalAdmin(h.b.OperatorUserID()) { return nil } switch goos { @@ -1081,104 +1075,6 @@ func authorizeServeConfigForGOOSAndUserContext(goos string, configIn *ipn.ServeC } -// connIsLocalAdmin reports whether the connected client has administrative -// access to the local machine, for whatever that means with respect to the -// current OS. -// -// This is useful because tailscaled itself always runs with elevated rights: -// we want to avoid privilege escalation for certain mutative operations. -func (h *Handler) connIsLocalAdmin() bool { - if h.testConnIsLocalAdmin != nil { - return *h.testConnIsLocalAdmin - } - if h.ConnIdentity == nil { - h.logf("[unexpected] missing ConnIdentity in LocalAPI Handler") - return false - } - switch runtime.GOOS { - case "windows": - tok, err := h.ConnIdentity.WindowsToken() - if err != nil { - if !errors.Is(err, ipnauth.ErrNotImplemented) { - h.logf("ipnauth.ConnIdentity.WindowsToken() error: %v", err) - } - return false - } - defer tok.Close() - - return tok.IsElevated() - - case "darwin": - // Unknown, or at least unchecked on sandboxed macOS variants. Err on - // the side of less permissions. - // - // authorizeServeConfigForGOOSAndUserContext should not call - // connIsLocalAdmin on sandboxed variants anyway. - if version.IsSandboxedMacOS() { - return false - } - // This is a standalone tailscaled setup, use the same logic as on - // Linux. - fallthrough - case "linux": - uid, ok := h.ConnIdentity.Creds().UserID() - if !ok { - return false - } - // root is always admin. - if uid == "0" { - return true - } - // if non-root, must be operator AND able to execute "sudo tailscale". - operatorUID := h.b.OperatorUserID() - if operatorUID != "" && uid != operatorUID { - return false - } - u, err := osuser.LookupByUID(uid) - if err != nil { - return false - } - // Short timeout just in case sudo hangs for some reason. - ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) - defer cancel() - if err := exec.CommandContext(ctx, "sudo", "--other-user="+u.Name, "--list", "tailscale").Run(); err != nil { - return false - } - return true - - default: - return false - } -} - -func (h *Handler) getUsername() (string, error) { - if h.ConnIdentity == nil { - h.logf("[unexpected] missing ConnIdentity in LocalAPI Handler") - return "", errors.New("missing ConnIdentity") - } - switch runtime.GOOS { - case "windows": - tok, err := h.ConnIdentity.WindowsToken() - if err != nil { - return "", fmt.Errorf("get windows token: %w", err) - } - defer tok.Close() - return tok.Username() - case "darwin", "linux": - uid, ok := h.ConnIdentity.Creds().UserID() - if !ok { - return "", errors.New("missing user ID") - } - u, err := osuser.LookupByUID(uid) - if err != nil { - return "", fmt.Errorf("lookup user: %w", err) - } - return u.Username, nil - default: - return "", errors.New("unsupported OS") - } -} - func (h *Handler) serveCheckIPForwarding(w http.ResponseWriter, r *http.Request) { if !h.PermitRead { http.Error(w, "IP forwarding check access denied", http.StatusForbidden) @@ -2859,7 +2755,7 @@ func (h *Handler) serveShares(w http.ResponseWriter, r *http.Request) { } if drive.AllowShareAs() { // share as the connected user - username, err := h.getUsername() + username, err := h.Actor.Username() if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return diff --git a/ipn/localapi/localapi_test.go b/ipn/localapi/localapi_test.go index 095ec5e56..5ec873b3b 100644 --- a/ipn/localapi/localapi_test.go +++ b/ipn/localapi/localapi_test.go @@ -26,6 +26,7 @@ "tailscale.com/client/tailscale/apitype" "tailscale.com/ipn" + "tailscale.com/ipn/ipnauth" "tailscale.com/ipn/ipnlocal" "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" @@ -38,6 +39,23 @@ "tailscale.com/wgengine" ) +var _ ipnauth.Actor = (*testActor)(nil) + +type testActor struct { + uid ipn.WindowsUserID + name string + isLocalSystem bool + isLocalAdmin bool +} + +func (u *testActor) UserID() ipn.WindowsUserID { return u.uid } + +func (u *testActor) Username() (string, error) { return u.name, nil } + +func (u *testActor) IsLocalSystem() bool { return u.isLocalSystem } + +func (u *testActor) IsLocalAdmin(operatorUID string) bool { return u.isLocalAdmin } + func TestValidHost(t *testing.T) { tests := []struct { host string @@ -189,7 +207,7 @@ func TestWhoIsArgTypes(t *testing.T) { func TestShouldDenyServeConfigForGOOSAndUserContext(t *testing.T) { newHandler := func(connIsLocalAdmin bool) *Handler { - return &Handler{testConnIsLocalAdmin: &connIsLocalAdmin} + return &Handler{Actor: &testActor{isLocalAdmin: connIsLocalAdmin}, b: newTestLocalBackend(t)} } tests := []struct { name string