diff --git a/ipn/backend.go b/ipn/backend.go index fe591a0ab..8042b6625 100644 --- a/ipn/backend.go +++ b/ipn/backend.go @@ -21,6 +21,7 @@ const ( NoState = State(iota) + InUseOtherUser NeedsLogin NeedsMachineAuth Stopped @@ -33,8 +34,14 @@ const GoogleIDTokenType = "ts_android_google_login" func (s State) String() string { - return [...]string{"NoState", "NeedsLogin", "NeedsMachineAuth", - "Stopped", "Starting", "Running"}[s] + return [...]string{ + "NoState", + "InUseOtherUser", + "NeedsLogin", + "NeedsMachineAuth", + "Stopped", + "Starting", + "Running"}[s] } // EngineStatus contains WireGuard engine stats. @@ -53,7 +60,7 @@ type EngineStatus struct { type Notify struct { _ structs.Incomparable Version string // version number of IPN backend - ErrMessage *string // critical error message, if any + ErrMessage *string // critical error message, if any; for InUseOtherUser, the details LoginFinished *empty.Message // event: non-nil when login process succeeded State *State // current IPN state has changed Prefs *Prefs // preferences were changed diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go index f572fcd22..43acd9ef1 100644 --- a/ipn/ipnserver/server.go +++ b/ipn/ipnserver/server.go @@ -10,6 +10,7 @@ "errors" "fmt" "io" + "io/ioutil" "log" "net" "net/http" @@ -102,10 +103,11 @@ type server struct { bs *ipn.BackendServer mu sync.Mutex - serverModeUser *user.User // or nil if not in server mode - lastUserID string // tracks last userid; on change, Reset state for paranoia - allClients map[net.Conn]connIdentity // HTTP or IPN - clients map[net.Conn]bool // subset of allClients; only IPN protocol + serverModeUser *user.User // or nil if not in server mode + lastUserID string // tracks last userid; on change, Reset state for paranoia + allClients map[net.Conn]connIdentity // HTTP or IPN + clients map[net.Conn]bool // subset of allClients; only IPN protocol + disconnectSub map[chan<- struct{}]struct{} // keys are subscribers of disconnects } // connIdentity represents the owner of a localhost TCP connection. @@ -177,6 +179,42 @@ func (s *server) lookupUserFromID(uid string) (*user.User, error) { return u, err } +// blockWhileInUse blocks while until either a Read from conn fails +// (i.e. it's closed) or until the server is able to accept ci as a +// user. +func (s *server) blockWhileInUse(conn io.Reader, ci connIdentity) { + s.logf("blocking client while server in use; connIdentity=%v", ci) + connDone := make(chan struct{}) + go func() { + io.Copy(ioutil.Discard, conn) + close(connDone) + }() + ch := make(chan struct{}, 1) + s.registerDisconnectSub(ch, true) + defer s.registerDisconnectSub(ch, false) + for { + select { + case <-connDone: + s.logf("blocked client Read completed; connIdentity=%v", ci) + return + case <-ch: + s.mu.Lock() + err := s.checkConnIdentityLocked(ci) + s.mu.Unlock() + if err == nil { + s.logf("unblocking client, server is free; connIdentity=%v", ci) + // Server is now available again for a new user. + // TODO(bradfitz): keep this connection alive. But for + // now just return and have our caller close the connection + // (which unblocks the io.Copy goroutine we started above) + // and then the client (e.g. Windows) will reconnect and + // discover that it works. + return + } + } + } +} + func (s *server) serveConn(ctx context.Context, c net.Conn, logf logger.Logf) { // First see if it's an HTTP request. br := bufio.NewReader(c) @@ -195,8 +233,14 @@ func (s *server) serveConn(ctx context.Context, c net.Conn, logf logger.Logf) { defer c.Close() serverToClient := func(b []byte) { ipn.WriteMsg(c, b) } bs := ipn.NewBackendServer(logf, nil, serverToClient) - bs.SendErrorMessage(err.Error()) - time.Sleep(time.Second) + _, occupied := err.(inUseOtherUserError) + if occupied { + bs.SendInUseOtherUserErrorMessage(err.Error()) + s.blockWhileInUse(c, ci) + } else { + bs.SendErrorMessage(err.Error()) + time.Sleep(time.Second) + } return } @@ -243,6 +287,58 @@ func (s *server) serveConn(ctx context.Context, c net.Conn, logf logger.Logf) { } } +// inUseOtherUserError is the error type for when the server is in use +// by a different local user. +type inUseOtherUserError struct{ error } + +func (e inUseOtherUserError) Unwrap() error { return e.error } + +// checkConnIdentityLocked checks whether the provided identity is +// allowed to connect to the server. +// +// The returned error, when non-nil, will be of type inUseOtherUserError. +// +// s.mu must be held. +func (s *server) checkConnIdentityLocked(ci connIdentity) error { + // If clients are already connected, verify they're the same user. + // This mostly matters on Windows at the moment. + if len(s.allClients) > 0 { + var active connIdentity + for _, active = range s.allClients { + break + } + if ci.UserID != active.UserID { + //lint:ignore ST1005 we want to capitalize Tailscale here + return inUseOtherUserError{fmt.Errorf("Tailscale already in use by %s, pid %d", active.User.Username, active.Pid)} + } + } + if su := s.serverModeUser; su != nil && ci.UserID != su.Uid { + //lint:ignore ST1005 we want to capitalize Tailscale here + return inUseOtherUserError{fmt.Errorf("Tailscale running as %s. Access denied.", su.Username)} + } + return nil +} + +// registerDisconnectSub adds ch as a subscribe to connection disconnect +// events. If add is false, the subscriber is removed. +func (s *server) registerDisconnectSub(ch chan<- struct{}, add bool) { + s.mu.Lock() + defer s.mu.Unlock() + if add { + if s.disconnectSub == nil { + s.disconnectSub = make(map[chan<- struct{}]struct{}) + } + s.disconnectSub[ch] = struct{}{} + } else { + delete(s.disconnectSub, ch) + } + +} + +// addConn adds c to the server's list of clients. +// +// If the returned error is of type inUseOtherUserError then the +// returned connIdentity is also valid. func (s *server) addConn(c net.Conn, isHTTP bool) (ci connIdentity, err error) { ci, err = s.getConnIdentity(c) if err != nil { @@ -271,21 +367,8 @@ func (s *server) addConn(c net.Conn, isHTTP bool) (ci connIdentity, err error) { s.allClients = map[net.Conn]connIdentity{} } - // If clients are already connected, verify they're the same user. - // This mostly matters on Windows at the moment. - if len(s.allClients) > 0 { - var active connIdentity - for _, active = range s.allClients { - break - } - if ci.UserID != active.UserID { - //lint:ignore ST1005 we want to capitalize Tailscale here - return ci, fmt.Errorf("Tailscale already in use by %s, pid %d", active.User.Username, active.Pid) - } - } - if su := s.serverModeUser; su != nil && ci.UserID != su.Uid { - //lint:ignore ST1005 we want to capitalize Tailscale here - return ci, fmt.Errorf("Tailscale running as %s. Access denied.", su.Username) + if err := s.checkConnIdentityLocked(ci); err != nil { + return ci, err } if !isHTTP { @@ -307,10 +390,16 @@ func (s *server) removeAndCloseConn(c net.Conn) { delete(s.clients, c) delete(s.allClients, c) remain := len(s.allClients) + for sub := range s.disconnectSub { + select { + case sub <- struct{}{}: + default: + } + } s.mu.Unlock() if remain == 0 && s.resetOnZero { - if s.b.RunningAndDaemonForced() { + if s.b.InServerMode() { s.logf("client disconnected; staying alive in server mode") } else { s.logf("client disconnected; stopping server") @@ -358,7 +447,7 @@ func (s *server) setServerModeUserLocked() { } func (s *server) writeToClients(b []byte) { - inServerMode := s.b.RunningAndDaemonForced() + inServerMode := s.b.InServerMode() s.mu.Lock() defer s.mu.Unlock() @@ -456,7 +545,7 @@ func Run(ctx context.Context, logf logger.Logf, logid string, getEngine func() ( key := string(autoStartKey) if strings.HasPrefix(key, "user-") { uid := strings.TrimPrefix(key, "user-") - u, err := user.LookupId(uid) + u, err := server.lookupUserFromID(uid) if err != nil { logf("ipnserver: found server mode auto-start key %q; failed to load user: %v", key, err) } else { diff --git a/ipn/local.go b/ipn/local.go index 6a8ff8406..8dabe1b79 100644 --- a/ipn/local.go +++ b/ipn/local.go @@ -81,6 +81,7 @@ type LocalBackend struct { stateKey StateKey // computed in part from user-provided value userID string // current controlling user ID (for Windows, primarily) prefs *Prefs + inServerMode bool machinePrivKey wgcfg.PrivateKey state State // hostinfo is mutated in-place while mu is held. @@ -414,6 +415,7 @@ func (b *LocalBackend) Start(opts Options) error { return fmt.Errorf("loading requested state: %v", err) } + b.inServerMode = b.stateKey != "" b.serverURL = b.prefs.ControlURL hostinfo.RoutableIPs = append(hostinfo.RoutableIPs, b.prefs.AdvertiseRoutes...) hostinfo.RequestTags = append(hostinfo.RequestTags, b.prefs.AdvertiseTags...) @@ -766,6 +768,35 @@ func (b *LocalBackend) initMachineKeyLocked() (err error) { return nil } +// writeServerModeStartState stores the ServerModeStartKey value based on the current +// user and prefs. If userID is blank or prefs is blank, no work is done. +// +// b.mu may either be held or not. +func (b *LocalBackend) writeServerModeStartState(userID string, prefs *Prefs) { + if userID == "" || prefs == nil { + return + } + + if prefs.ForceDaemon { + stateKey := StateKey("user-" + userID) + if err := b.store.WriteState(ServerModeStartKey, []byte(stateKey)); err != nil { + b.logf("WriteState error: %v", err) + } + // It's important we do this here too, even if it looks + // redundant with the one in the 'if stateKey != ""' + // check block above. That one won't fire in the case + // where the Windows client started up in client mode. + // This happens when we transition into server mode: + if err := b.store.WriteState(stateKey, prefs.ToBytes()); err != nil { + b.logf("WriteState error: %v", err) + } + } else { + if err := b.store.WriteState(ServerModeStartKey, nil); err != nil { + b.logf("WriteState error: %v", err) + } + } +} + // loadStateLocked sets b.prefs and b.stateKey based on a complex // combination of key, prefs, and legacyPath. b.mu must be held when // calling. @@ -786,6 +817,7 @@ func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath st return fmt.Errorf("initMachineKeyLocked: %w", err) } b.stateKey = "" + b.writeServerModeStartState(b.userID, b.prefs) return nil } @@ -843,13 +875,10 @@ func (b *LocalBackend) State() State { return b.state } -// RunningAndDaemonForced reports whether the backend is currently -// running and the preferences say that Tailscale should run in -// "server mode" (ForceDaemon). -func (b *LocalBackend) RunningAndDaemonForced() bool { +func (b *LocalBackend) InServerMode() bool { b.mu.Lock() defer b.mu.Unlock() - return b.state == Running && b.prefs != nil && b.prefs.ForceDaemon + return b.inServerMode } // getEngineStatus returns a copy of b.engineStatus. @@ -1001,6 +1030,7 @@ func (b *LocalBackend) SetPrefs(newp *Prefs) { oldp := b.prefs newp.Persist = oldp.Persist // caller isn't allowed to override this b.prefs = newp + b.inServerMode = newp.ForceDaemon // We do this to avoid holding the lock while doing everything else. newp = b.prefs.Clone() @@ -1019,26 +1049,7 @@ func (b *LocalBackend) SetPrefs(newp *Prefs) { b.logf("Failed to save new controlclient state: %v", err) } } - if userID != "" { // e.g. on Windows - if newp.ForceDaemon { - stateKey := StateKey("user-" + userID) - if err := b.store.WriteState(ServerModeStartKey, []byte(stateKey)); err != nil { - b.logf("WriteState error: %v", err) - } - // It's important we do this here too, even if it looks - // redundant with the one in the 'if stateKey != ""' - // check block above. That one won't fire in the case - // where the Windows client started up in client mode. - // This happens when we transition into server mode: - if err := b.store.WriteState(stateKey, newp.ToBytes()); err != nil { - b.logf("WriteState error: %v", err) - } - } else { - if err := b.store.WriteState(ServerModeStartKey, nil); err != nil { - b.logf("WriteState error: %v", err) - } - } - } + b.writeServerModeStartState(userID, newp) // [GRINDER STATS LINE] - please don't remove (used for log parsing) b.logf("SetPrefs: %v", newp.Pretty()) diff --git a/ipn/message.go b/ipn/message.go index 77bb9deaa..2ff7f7432 100644 --- a/ipn/message.go +++ b/ipn/message.go @@ -92,6 +92,17 @@ func (bs *BackendServer) SendErrorMessage(msg string) { bs.send(Notify{ErrMessage: &msg}) } +// SendInUseOtherUserErrorMessage sends a Notify message to the client that +// both sets the state to 'InUseOtherUser' and sets the associated reason +// to msg. +func (bs *BackendServer) SendInUseOtherUserErrorMessage(msg string) { + inUse := InUseOtherUser + bs.send(Notify{ + State: &inUse, + ErrMessage: &msg, + }) +} + // GotCommandMsg parses the incoming message b as a JSON Command and // calls GotCommand with it. func (bs *BackendServer) GotCommandMsg(b []byte) error {