Merge remote-tracking branch 'origin/main' into HEAD

* origin/main:
  wgengine/router/dns: run ipconfig /registerdns async, log timing
  net/tshttpproxy: aggressively rate-limit error logs in Transport.Proxy path
  ipn: only use Prefs, not computed stateKey, to determine server mode
  VERSION: rename to version.txt to work around macOS limitations.
  version: greatly simplify redo nonsense, now that we use VERSION.
  ipn, ipn/ipnserver: add IPN state for server in use, handle explicitly
  version: calculate version info without using git tags.
  version: use -g as the "other" suffix, so that `git show` works.
  ipn/ipnserver: remove "Server mode" from a user-visible error message
  ipn: fix crash generating machine key on new installs
  Change some os.IsNotExist to errors.Is(err, os.ErrNotExist) for non-os errors.
  .github/workflows: use cache to speed up Windows tests
  tsweb: add StatusCodeCounters to HandlerOptions
  tsweb: add StdHandlerOpts that accepts an options struct
  ipn: don't temporarilySetMachineKeyInPersist for Android clients

.github/workflows/windows.yml

@@ -24,6 +24,14 @@ jobs:
     - name: Checkout code
       uses: actions/checkout@v2
 
+    - name: Restore Cache
+      uses: actions/cache@v2
+      with:
+        path: ~/go/pkg/mod
+        key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
+        restore-keys: |
+          ${{ runner.os }}-go-
+
     - name: Test
       run: go test ./...
 

VERSION

@@ -1 +0,0 @@
-1.2.0 bb058703ee682490124a8a9f93919c4b3a3c991d

VERSION.txt

@@ -0,0 +1 @@
+1.1.0 f81233524fddeec450940af8dc1a0dd8841bf28c

cmd/derper/derper.go

@@ -63,7 +63,7 @@ func loadConfig() config {
 	}
 	b, err := ioutil.ReadFile(*configPath)
 	switch {
-	case os.IsNotExist(err):
+	case errors.Is(err, os.ErrNotExist):
 		return writeNewConfig()
 	case err != nil:
 		log.Fatal(err)

ipn/backend.go

@@ -21,6 +21,7 @@ type State int
 
 const (
 	NoState = State(iota)
+	InUseOtherUser
 	NeedsLogin
 	NeedsMachineAuth
 	Stopped
@@ -33,8 +34,14 @@ const (
 const GoogleIDTokenType = "ts_android_google_login"
 
 func (s State) String() string {
-	return [...]string{"NoState", "NeedsLogin", "NeedsMachineAuth",
+	return [...]string{
-		"Stopped", "Starting", "Running"}[s]
+		"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

ipn/ipnserver/server.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"log"
 	"net"
 	"net/http"
@@ -106,6 +107,7 @@ type server struct {
 	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)
+		_, 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 already in use by %s", 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.
+	if err := s.checkConnIdentityLocked(ci); err != nil {
-	// This mostly matters on Windows at the moment.
+		return ci, err
-	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 in server mode as %s. Access denied.", su.Username)
 	}
 
 	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 {

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,9 +415,11 @@ func (b *LocalBackend) Start(opts Options) error {
 		return fmt.Errorf("loading requested state: %v", err)
 	}
 
+	b.inServerMode = b.prefs.ForceDaemon
 	b.serverURL = b.prefs.ControlURL
 	hostinfo.RoutableIPs = append(hostinfo.RoutableIPs, b.prefs.AdvertiseRoutes...)
 	hostinfo.RequestTags = append(hostinfo.RequestTags, b.prefs.AdvertiseTags...)
+	b.logf("Start: serverMode=%v; stateKey=%q; tags=%q; routes=%v; url=%v", b.inServerMode, b.stateKey, b.prefs.AdvertiseTags, b.prefs.AdvertiseRoutes, b.prefs.ControlURL)
 	applyPrefsToHostinfo(hostinfo, b.prefs)
 
 	b.notify = opts.Notify
@@ -708,7 +711,9 @@ func (b *LocalBackend) initMachineKeyLocked() (err error) {
 			if err != nil {
 				return
 			}
+			if b.prefs != nil && b.prefs.Persist != nil {
 				b.prefs.Persist.LegacyFrontendPrivateMachineKey = b.machinePrivKey
+			}
 		}()
 	}
 
@@ -764,6 +769,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.
@@ -784,6 +818,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
 	}
 
@@ -803,8 +838,8 @@ func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath st
 			if legacyPath != "" {
 				b.prefs, err = LoadPrefs(legacyPath)
 				if err != nil {
-					if !os.IsNotExist(err) {
+					if !errors.Is(err, os.ErrNotExist) {
-						b.logf("Failed to load legacy prefs: %v", err)
+						b.logf("failed to load legacy prefs: %v", err)
 					}
 					b.prefs = NewPrefs()
 				} else {
@@ -841,13 +876,10 @@ func (b *LocalBackend) State() State {
 	return b.state
 }
 
-// RunningAndDaemonForced reports whether the backend is currently
+func (b *LocalBackend) InServerMode() bool {
-// running and the preferences say that Tailscale should run in
-// "server mode" (ForceDaemon).
-func (b *LocalBackend) RunningAndDaemonForced() 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.
@@ -999,6 +1031,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()
 
@@ -1017,26 +1050,7 @@ func (b *LocalBackend) SetPrefs(newp *Prefs) {
 			b.logf("Failed to save new controlclient state: %v", err)
 		}
 	}
-	if userID != "" { // e.g. on Windows
+	b.writeServerModeStartState(userID, newp)
-		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)
-			}
-		}
-	}
 
 	// [GRINDER STATS LINE] - please don't remove (used for log parsing)
 	b.logf("SetPrefs: %v", newp.Pretty())
@@ -1540,8 +1554,8 @@ func (b *LocalBackend) TestOnlyPublicKeys() (machineKey tailcfg.MachineKey, node
 // clients. We can't do that until 1.0.x is no longer supported.
 func temporarilySetMachineKeyInPersist() bool {
 	//lint:ignore S1008 for comments
-	if runtime.GOOS == "darwin" {
+	if runtime.GOOS == "darwin" || runtime.GOOS == "android" {
-		// iOS and macOS users can't downgrade anyway.
+		// iOS, macOS, Android users can't downgrade anyway.
 		return false
 	}
 	return true

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 {

ipn/prefs_test.go

@@ -5,8 +5,12 @@
 package ipn
 
 import (
+	"errors"
+	"fmt"
+	"os"
 	"reflect"
 	"testing"
+	"time"
 
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"tailscale.com/control/controlclient"
@@ -330,3 +334,14 @@ func TestPrefsPretty(t *testing.T) {
 		}
 	}
 }
+
+func TestLoadPrefsNotExist(t *testing.T) {
+	bogusFile := fmt.Sprintf("/tmp/not-exist-%d", time.Now().UnixNano())
+
+	p, err := LoadPrefs(bogusFile)
+	if errors.Is(err, os.ErrNotExist) {
+		// expected.
+		return
+	}
+	t.Fatalf("unexpected prefs=%#v, err=%v", p, err)
+}

net/tshttpproxy/tshttpproxy_windows.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/alexbrainman/sspi/negotiate"
 	"golang.org/x/sys/windows"
+	"tailscale.com/types/logger"
 )
 
 var (
@@ -38,6 +39,13 @@ var cachedProxy struct {
 	val *url.URL
 }
 
+// proxyErrorf is a rate-limited logger specifically for errors asking
+// WinHTTP for the proxy information. We don't want to log about
+// errors often, otherwise the log message itself will generate a new
+// HTTP request which ultimately will call back into us to log again,
+// forever. So for errors, we only log a bit.
+var proxyErrorf = logger.RateLimitedFn(log.Printf, 10*time.Minute, 2 /* burst*/, 10 /* maxCache */)
+
 func proxyFromWinHTTPOrCache(req *http.Request) (*url.URL, error) {
 	if req.URL == nil {
 		return nil, nil
@@ -79,7 +87,14 @@ func proxyFromWinHTTPOrCache(req *http.Request) (*url.URL, error) {
 			setNoProxyUntil(10 * time.Second)
 			return nil, nil
 		}
-		log.Printf("tshttpproxy: winhttp: GetProxyForURL(%q): %v/%#v", urlStr, err, err)
+		if err == windows.ERROR_INVALID_PARAMETER {
+			// Seen on Windows 8.1. (https://github.com/tailscale/tailscale/issues/879)
+			// TODO(bradfitz): figure this out.
+			setNoProxyUntil(time.Hour)
+			proxyErrorf("tshttpproxy: winhttp: GetProxyForURL(%q): ERROR_INVALID_PARAMETER [unexpected]", urlStr)
+			return nil, nil
+		}
+		proxyErrorf("tshttpproxy: winhttp: GetProxyForURL(%q): %v/%#v", urlStr, err, err)
 		if err == syscall.Errno(ERROR_WINHTTP_UNABLE_TO_DOWNLOAD_SCRIPT) {
 			setNoProxyUntil(10 * time.Second)
 			return nil, nil
@@ -88,7 +103,7 @@ func proxyFromWinHTTPOrCache(req *http.Request) (*url.URL, error) {
 	case <-ctx.Done():
 		cachedProxy.Lock()
 		defer cachedProxy.Unlock()
-		log.Printf("tshttpproxy: winhttp: GetProxyForURL(%q): timeout; using cached proxy %v", urlStr, cachedProxy.val)
+		proxyErrorf("tshttpproxy: winhttp: GetProxyForURL(%q): timeout; using cached proxy %v", urlStr, cachedProxy.val)
 		return cachedProxy.val, nil
 	}
 }
@@ -96,7 +111,7 @@ func proxyFromWinHTTPOrCache(req *http.Request) (*url.URL, error) {
 func proxyFromWinHTTP(ctx context.Context, urlStr string) (proxy *url.URL, err error) {
 	whi, err := winHTTPOpen()
 	if err != nil {
-		log.Printf("winhttp: Open: %v", err)
+		proxyErrorf("winhttp: Open: %v", err)
 		return nil, err
 	}
 	defer whi.Close()

tsweb/tsweb.go

@@ -157,11 +157,22 @@ type ReturnHandler interface {
 	ServeHTTPReturn(http.ResponseWriter, *http.Request) error
 }
 
+type HandlerOptions struct {
+	Quiet200s bool // if set, do not log successfully handled HTTP requests
+	Logf      logger.Logf
+	Now       func() time.Time // if nil, defaults to time.Now
+
+	// If non-nil, StatusCodeCounters maintains counters
+	// of status codes for handled responses.
+	// The keys are "1xx", "2xx", "3xx", "4xx", and "5xx".
+	StatusCodeCounters *expvar.Map
+}
+
 // StdHandler converts a ReturnHandler into a standard http.Handler.
 // Handled requests are logged using logf, as are any errors. Errors
 // are handled as specified by the Handler interface.
 func StdHandler(h ReturnHandler, logf logger.Logf) http.Handler {
-	return stdHandler(h, logf, time.Now, true)
+	return StdHandlerOpts(h, HandlerOptions{Logf: logf, Now: time.Now})
 }
 
 // ReturnHandlerFunc is an adapter to allow the use of ordinary
@@ -178,27 +189,32 @@ func (f ReturnHandlerFunc) ServeHTTPReturn(w http.ResponseWriter, r *http.Reques
 // StdHandlerNo200s is like StdHandler, but successfully handled HTTP
 // requests don't write an access log entry to logf.
 //
-// TODO(danderson): quick stopgap, probably want ...Options on StdHandler instead?
+// TODO(josharian): eliminate this and StdHandler in favor of StdHandlerOpts,
+// rename StdHandlerOpts to StdHandler. Will be a breaking API change.
 func StdHandlerNo200s(h ReturnHandler, logf logger.Logf) http.Handler {
-	return stdHandler(h, logf, time.Now, false)
+	return StdHandlerOpts(h, HandlerOptions{Logf: logf, Now: time.Now, Quiet200s: true})
 }
 
-func stdHandler(h ReturnHandler, logf logger.Logf, now func() time.Time, log200s bool) http.Handler {
+// StdHandlerOpts converts a ReturnHandler into a standard http.Handler.
-	return retHandler{h, logf, now, log200s}
+// Handled requests are logged using opts.Logf, as are any errors.
+// Errors are handled as specified by the Handler interface.
+func StdHandlerOpts(h ReturnHandler, opts HandlerOptions) http.Handler {
+	if opts.Now == nil {
+		opts.Now = time.Now
+	}
+	return retHandler{h, opts}
 }
 
 // retHandler is an http.Handler that wraps a Handler and handles errors.
 type retHandler struct {
 	rh   ReturnHandler
-	logf    logger.Logf
+	opts HandlerOptions
-	timeNow func() time.Time
-	log200s bool
 }
 
 // ServeHTTP implements the http.Handler interface.
 func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	msg := AccessLogRecord{
-		When:       h.timeNow(),
+		When:       h.opts.Now(),
 		RemoteAddr: r.RemoteAddr,
 		Proto:      r.Proto,
 		TLS:        r.TLS != nil,
@@ -209,7 +225,7 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		Referer:    r.Referer(),
 	}
 
-	lw := &loggingResponseWriter{ResponseWriter: w, logf: h.logf}
+	lw := &loggingResponseWriter{ResponseWriter: w, logf: h.opts.Logf}
 	err := h.rh.ServeHTTPReturn(lw, r)
 	hErr, hErrOK := err.(HTTPError)
 
@@ -219,7 +235,7 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		lw.code = 200
 	}
 
-	msg.Seconds = h.timeNow().Sub(msg.When).Seconds()
+	msg.Seconds = h.opts.Now().Sub(msg.When).Seconds()
 	msg.Code = lw.code
 	msg.Bytes = lw.bytes
 
@@ -245,12 +261,12 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			}
 		}
 		if lw.code != 0 {
-			h.logf("[unexpected] handler returned HTTPError %v, but already sent a response with code %d", hErr, lw.code)
+			h.opts.Logf("[unexpected] handler returned HTTPError %v, but already sent a response with code %d", hErr, lw.code)
 			break
 		}
 		msg.Code = hErr.Code
 		if msg.Code == 0 {
-			h.logf("[unexpected] HTTPError %v did not contain an HTTP status code, sending internal server error", hErr)
+			h.opts.Logf("[unexpected] HTTPError %v did not contain an HTTP status code, sending internal server error", hErr)
 			msg.Code = http.StatusInternalServerError
 		}
 		http.Error(lw, hErr.Msg, msg.Code)
@@ -264,8 +280,13 @@ func (h retHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	if msg.Code != 200 || h.log200s {
+	if msg.Code != 200 || !h.opts.Quiet200s {
-		h.logf("%s", msg)
+		h.opts.Logf("%s", msg)
+	}
+
+	if h.opts.StatusCodeCounters != nil {
+		key := fmt.Sprintf("%dxx", msg.Code/100)
+		h.opts.StatusCodeCounters.Add(key, 1)
 	}
 }
 

tsweb/tsweb_test.go

@@ -248,7 +248,7 @@ func TestStdHandler(t *testing.T) {
 			clock.Reset()
 
 			rec := noopHijacker{httptest.NewRecorder(), false}
-			h := stdHandler(test.rh, logf, clock.Now, true)
+			h := StdHandlerOpts(test.rh, HandlerOptions{Logf: logf, Now: clock.Now})
 			h.ServeHTTP(&rec, test.r)
 			res := rec.Result()
 			if res.StatusCode != test.wantCode {

version/.gitignore

@@ -3,6 +3,7 @@ long.txt
 short.txt
 gitcommit.txt
 extragitcommit.txt
+version-info.sh
 version.h
 version.xcconfig
 ver.go

version/all.do

@@ -1,3 +1,2 @@
-
 redo-ifchange ver.go version.xcconfig version.h
 

version/describe.sh

@@ -1,26 +0,0 @@
-#!/bin/sh
-#
-# Constructs a "git describe" compatible version number by using the
-# information in the VERSION file, rather than git tags.
-
-set -eu
-
-dir="$(dirname $0)"
-verfile="$dir/../VERSION"
-
-read -r version hash <"$verfile"
-
-if [ -z "$hash" ]; then
-    # If no explicit hash was given, use the last time the version
-    # file changed as the "origin" hash for this version.
-    hash="$(git rev-list --max-count=1 HEAD -- $verfile)"
-fi
-
-if [ -z "$hash" ]; then
-    echo "Couldn't find base git hash for version '$version'" >2
-    exit 1
-fi
-
-head="$(git rev-parse --short=9 HEAD)"
-changecount="$(git rev-list ${hash}..HEAD | wc -l)"
-echo "v${version}-${changecount}-g${head}"

version/extragitcommit.txt.do

@@ -1,6 +0,0 @@
-# --abbrev=200 is an arbitrary large number to capture the entire git
-# hash without trying to compact it.
-commit=$(cd ../.. && git describe --dirty --exclude "*" --always --abbrev=200)
-echo "$commit" >$3
-redo-always
-redo-stamp <$3

version/gitcommit.txt.do

@@ -1,6 +0,0 @@
-# --abbrev=200 is an arbitrary large number to capture the entire git
-# hash without trying to compact it.
-commit=$(git describe --dirty --exclude "*" --always --abbrev=200)
-echo "$commit" >$3
-redo-always
-redo-stamp <$3

version/long.txt.do

@@ -1,5 +0,0 @@
-redo-ifchange mkversion.sh describe.txt extragitcommit.txt
-read -r describe <describe.txt
-read -r other <extragitcommit.txt
-ver=$(./mkversion.sh long "$describe" "$other")
-echo "$ver" >$3

version/mkversion.sh

@@ -1,139 +0,0 @@
-#!/bin/sh
-
-set -eu
-
-mode=$1
-describe=$2
-other=$3
-
-# Git describe output overall looks like
-# MAJOR.MINOR.PATCH-NUMCOMMITS-GITHASH. Depending on the tag being
-# described and the state of the repo, ver can be missing PATCH,
-# NUMCOMMITS, or both.
-#
-# Valid values look like: 1.2.3-1234-abcdef, 0.98-1234-abcdef,
-# 1.0.0-abcdef, 0.99-abcdef.
-ver="${describe#v}"
-stem="${ver%%-*}" # Just the semver-ish bit e.g. 1.2.3, 0.98
-suffix="${ver#$stem}" # The rest e.g. -23-abcdef, -abcdef
-
-# Normalize the stem into a full major.minor.patch semver. We might
-# not use all those pieces depending on what kind of version we're
-# making, but it's good to have them all on hand.
-case "$stem" in
-    *.*.*)
-        # Full SemVer, nothing to do
-        stem="$stem"
-        ;;
-    *.*)
-        # Old style major.minor, add a .0
-        stem="${stem}.0"
-        ;;
-    *)
-        echo "Unparseable version $stem" >&2
-        exit 1
-        ;;
-esac
-major=$(echo "$stem" | cut -f1 -d.)
-minor=$(echo "$stem" | cut -f2 -d.)
-patch=$(echo "$stem" | cut -f3 -d.)
-
-# Extract the change count and git ID from the suffix.
-case "$suffix" in
-    -*-*)
-        # Has both a change count and a commit hash.
-        changecount=$(echo "$suffix" | cut -f2 -d-)
-        githash=$(echo "$suffix" | cut -f3 -d-)
-        ;;
-    -*)
-        # Git hash only, change count is zero.
-        changecount="0"
-        githash=$(echo "$suffix" | cut -f2 -d-)
-        ;;
-    *)
-        echo "Unparseable version suffix $suffix" >&2
-        exit 1
-        ;;
-esac
-
-# The git hash is of the form "gCOMMITHASH". We want to replace the
-# 'g' with a 't', for "tailscale", to convey that it's specifically
-# the commit hash of the tailscale repo.
-if [ -n "$githash" ]; then
-    # POSIX shell doesn't understand ${foo:1:9} syntax, gaaah.
-    githash="$(echo $githash | cut -c2-10)"
-    githash="t${githash}"
-fi
-
-# "other" is a second git commit hash for another repository used to
-# build the Tailscale code. In practice it's either the commit hash in
-# the Android repository, or the commit hash of Tailscale's
-# proprietary repository (which pins a bunch things like build scripts
-# used and Go toolchain version).
-if [ -n "$other" ]; then
-    other="$(echo $other | cut -c1-9)"
-    other="-o${other}"
-fi
-
-# Validate that the version data makes sense. Rules:
-#  - Odd number minors are unstable. Patch must be 0, and gets
-#    replaced by changecount.
-#  - Even number minors are stable. Changecount must be 0, and
-#    gets removed.
-#
-# After this section, we only use major/minor/patch, which have been
-# tweaked as needed.
-if expr "$minor" : "[0-9]*[13579]$" >/dev/null; then
-    # Unstable
-    if [ "$patch" != "0" ]; then
-        # This is a fatal error, because a non-zero patch number
-        # indicates that we created an unstable git tag in violation
-        # of our versioning policy, and we want to blow up loudly to
-        # get that fixed.
-        echo "Unstable release $describe has a non-zero patch number, which is not allowed" >&2
-        exit 1
-    fi
-    patch="$changecount"
-else
-    # Stable
-    if [ "$changecount" != "0" ]; then
-        # This is a commit that's sitting between two stable
-        # releases. We never want to release such a commit to the
-        # pbulic, but it's useful to be able to build it for
-        # debugging. Just force the version to 0.0.0, so that we're
-        # forced to rely on the git commit hash.
-        major="0"
-        minor="0"
-        patch="0"
-    fi
-fi
-
-if [ "$minor" -eq 1 ]; then
-    # Hack for 1.1: add 1000 to the patch number, so that builds that
-    # use the OSS change count order after the builds that used the
-    # proprietary repo's changecount. Otherwise, the version numbers
-    # would go backwards and things would be unhappy.
-    patch=$((patch + 1000))
-fi
-
-case "$1" in
-    long)
-        echo "${major}.${minor}.${patch}-${githash}${other}"
-        ;;
-    short)
-        echo "${major}.${minor}.${patch}"
-        ;;
-    xcode)
-        # CFBundleShortVersionString: the "short name" used in the App
-        # Store.  eg. 0.92.98
-        echo "VERSION_NAME = ${major}.${minor}.${patch}"
-        # CFBundleVersion: the build number. Needs to be 3 numeric
-        # sections that increment for each release according to SemVer
-        # rules.
-        #
-        # We start counting at 100 because we submitted using raw
-        # build numbers before, and Apple doesn't let you start over.
-        # e.g. 0.98.3 -> 100.98.3
-        echo "VERSION_ID = $((major + 100)).${minor}.${patch}"        
-        ;;
-esac

version/mkversion_test.go

@@ -8,22 +8,21 @@ import (
 	"fmt"
 	"os/exec"
 	"runtime"
+	"strconv"
 	"strings"
 	"testing"
-)
 
-func xcode(short, long string) string {
+	"github.com/google/go-cmp/cmp"
-	return fmt.Sprintf("VERSION_NAME = %s\nVERSION_ID = %s", short, long)
+)
-}
 
-func mkversion(t *testing.T, mode, describe, other string) (string, bool) {
+func mkversion(t *testing.T, gitHash, otherHash string, major, minor, patch, changeCount int) (string, bool) {
 	t.Helper()
-	bs, err := exec.Command("./mkversion.sh", mode, describe, other).CombinedOutput()
+	bs, err := exec.Command("./version.sh", gitHash, otherHash, strconv.Itoa(major), strconv.Itoa(minor), strconv.Itoa(patch), strconv.Itoa(changeCount)).CombinedOutput()
+	out := strings.TrimSpace(string(bs))
 	if err != nil {
-		t.Logf("mkversion.sh output: %s", string(bs))
+		return out, false
-		return "", false
 	}
-	return strings.TrimSpace(string(bs)), true
+	return out, true
 }
 
 func TestMkversion(t *testing.T) {
@@ -31,50 +30,73 @@ func TestMkversion(t *testing.T) {
 		t.Skip("skip test on Windows, because there is no shell to execute mkversion.sh.")
 	}
 	tests := []struct {
-		describe string
+		gitHash, otherHash               string
-		other    string
+		major, minor, patch, changeCount int
-		ok       bool
+		want                             string
-		long     string
-		short    string
-		xcode    string
 	}{
-		{"v0.98-gabcdef", "", true, "0.98.0-tabcdef", "0.98.0", xcode("0.98.0", "100.98.0")},
+		{"abcdef", "", 0, 98, 0, 0, `
-		{"v0.98.1-gabcdef", "", true, "0.98.1-tabcdef", "0.98.1", xcode("0.98.1", "100.98.1")},
+           VERSION_SHORT="0.98.0"
-		{"v1.1.0-37-gabcdef", "", true, "1.1.1037-tabcdef", "1.1.1037", xcode("1.1.1037", "101.1.1037")},
+           VERSION_LONG="0.98.0-tabcdef"
-		{"v1.2.9-gabcdef", "", true, "1.2.9-tabcdef", "1.2.9", xcode("1.2.9", "101.2.9")},
+           VERSION_GIT_HASH="abcdef"
-		{"v1.2.9-0-gabcdef", "", true, "1.2.9-tabcdef", "1.2.9", xcode("1.2.9", "101.2.9")},
+           VERSION_EXTRA_HASH=""
-		{"v1.15.0-129-gabcdef", "", true, "1.15.129-tabcdef", "1.15.129", xcode("1.15.129", "101.15.129")},
+           VERSION_XCODE="100.98.0"
-
+           VERSION_WINRES="0,98,0,0"`},
-		{"v0.98-123-gabcdef", "", true, "0.0.0-tabcdef", "0.0.0", xcode("0.0.0", "100.0.0")},
+		{"abcdef", "", 0, 98, 1, 0, `
-		{"v1.0.0-37-gabcdef", "", true, "0.0.0-tabcdef", "0.0.0", xcode("0.0.0", "100.0.0")},
+           VERSION_SHORT="0.98.1"
-		{"v1.1.0-129-gabcdef", "0123456789abcdef0123456789abcdef", true, "1.1.1129-tabcdef-o012345678", "1.1.1129", xcode("1.1.1129", "101.1.1129")},
+           VERSION_LONG="0.98.1-tabcdef"
-		{"v0.99.5-0-gabcdef", "", false, "", "", ""},   // unstable, patch not allowed
+           VERSION_GIT_HASH="abcdef"
-		{"v0.99.5-123-gabcdef", "", false, "", "", ""}, // unstable, patch not allowed
+           VERSION_EXTRA_HASH=""
-		{"v1-gabcdef", "", false, "", "", ""},          // bad semver
+           VERSION_XCODE="100.98.1"
-		{"v1.0", "", false, "", "", ""},                // missing suffix
+           VERSION_WINRES="0,98,1,0"`},
+		{"abcdef", "", 1, 1, 0, 37, `
+           VERSION_SHORT="1.1.1037"
+           VERSION_LONG="1.1.1037-tabcdef"
+           VERSION_GIT_HASH="abcdef"
+           VERSION_EXTRA_HASH=""
+           VERSION_XCODE="101.1.1037"
+           VERSION_WINRES="1,1,1037,0"`},
+		{"abcdef", "", 1, 2, 9, 0, `
+           VERSION_SHORT="1.2.9"
+           VERSION_LONG="1.2.9-tabcdef"
+           VERSION_GIT_HASH="abcdef"
+           VERSION_EXTRA_HASH=""
+           VERSION_XCODE="101.2.9"
+           VERSION_WINRES="1,2,9,0"`},
+		{"abcdef", "", 1, 15, 0, 129, `
+           VERSION_SHORT="1.15.129"
+           VERSION_LONG="1.15.129-tabcdef"
+           VERSION_GIT_HASH="abcdef"
+           VERSION_EXTRA_HASH=""
+           VERSION_XCODE="101.15.129"
+           VERSION_WINRES="1,15,129,0"`},
+		{"abcdef", "", 1, 2, 0, 17, `
+           VERSION_SHORT="0.0.0"
+           VERSION_LONG="0.0.0-tabcdef"
+           VERSION_GIT_HASH="abcdef"
+           VERSION_EXTRA_HASH=""
+           VERSION_XCODE="100.0.0"
+           VERSION_WINRES="0,0,0,0"`},
+		{"abcdef", "defghi", 1, 15, 0, 129, `
+           VERSION_SHORT="1.15.129"
+           VERSION_LONG="1.15.129-tabcdef-gdefghi"
+           VERSION_GIT_HASH="abcdef"
+           VERSION_EXTRA_HASH="defghi"
+           VERSION_XCODE="101.15.129"
+           VERSION_WINRES="1,15,129,0"`},
+		{"abcdef", "", 0, 99, 5, 0, ""},   // unstable, patch number not allowed
+		{"abcdef", "", 0, 99, 5, 123, ""}, // unstable, patch number not allowed
 	}
 
 	for _, test := range tests {
-		gotlong, longOK := mkversion(t, "long", test.describe, test.other)
+		want := strings.ReplaceAll(strings.TrimSpace(test.want), " ", "")
-		if longOK != test.ok {
+		got, ok := mkversion(t, test.gitHash, test.otherHash, test.major, test.minor, test.patch, test.changeCount)
-			t.Errorf("mkversion.sh long %q ok=%v, want %v", test.describe, longOK, test.ok)
+		invoc := fmt.Sprintf("version.sh %s %s %d %d %d %d", test.gitHash, test.otherHash, test.major, test.minor, test.patch, test.changeCount)
-		}
+		if want == "" && ok {
-		gotshort, shortOK := mkversion(t, "short", test.describe, test.other)
+			t.Errorf("%s ok=true, want false", invoc)
-		if shortOK != test.ok {
+			continue
-			t.Errorf("mkversion.sh short %q ok=%v, want %v", test.describe, shortOK, test.ok)
-		}
-		gotxcode, xcodeOK := mkversion(t, "xcode", test.describe, test.other)
-		if xcodeOK != test.ok {
-			t.Errorf("mkversion.sh xcode %q ok=%v, want %v", test.describe, xcodeOK, test.ok)
-		}
-		if longOK && gotlong != test.long {
-			t.Errorf("mkversion.sh long %q: got %q, want %q", test.describe, gotlong, test.long)
-		}
-		if shortOK && gotshort != test.short {
-			t.Errorf("mkversion.sh short %q: got %q, want %q", test.describe, gotshort, test.short)
 		}
-		if xcodeOK && gotxcode != test.xcode {
+		if diff := cmp.Diff(got, want); want != "" && diff != "" {
-			t.Errorf("mkversion.sh xcode %q: got %q, want %q", test.describe, gotxcode, test.xcode)
+			t.Errorf("%s wrong output (-got+want):\n%s", invoc, diff)
 		}
 	}
 }

version/short.txt.do

@@ -1,5 +0,0 @@
-redo-ifchange mkversion.sh describe.txt extragitcommit.txt
-read -r describe <describe.txt
-read -r other <extragitcommit.txt
-ver=$(./mkversion.sh short "$describe" "$other")
-echo "$ver" >$3

version/ver.go.do

@@ -1,12 +1,9 @@
-redo-ifchange long.txt short.txt gitcommit.txt extragitcommit.txt ver.go.in
+redo-ifchange version-info.sh ver.go.in
 
-read -r LONGVER <long.txt
+. ./version-info.sh
-read -r SHORTVER <short.txt
-read -r GITCOMMIT <gitcommit.txt
-read -r EXTRAGITCOMMIT <extragitcommit.txt
 
-sed -e "s/{LONGVER}/$LONGVER/g" \
+sed -e "s/{LONGVER}/$VERSION_LONG/g" \
-    -e "s/{SHORTVER}/$SHORTVER/g" \
+    -e "s/{SHORTVER}/$VERSION_SHORT/g" \
-    -e "s/{GITCOMMIT}/$GITCOMMIT/g" \
+    -e "s/{GITCOMMIT}/$VERSION_GIT_HASH/g" \
-    -e "s/{EXTRAGITCOMMIT}/$EXTRAGITCOMMIT/g" \
+    -e "s/{EXTRAGITCOMMIT}/$VERSION_EXTRA_HASH/g" \
     <ver.go.in >$3

version/version-info.sh.do

@@ -1,3 +1,3 @@
-./describe.sh >$3
+./version.sh ../.. >$3
 redo-always
 redo-stamp <$3

version/version.h.do

@@ -1,17 +1,9 @@
-redo-ifchange long.txt short.txt
+redo-ifchange version-info.sh
-read -r long <long.txt
-read -r short <short.txt
 
-# get it into "major.minor.patch" format
+. ./version-info.sh
-ver=$(echo "$ver" | sed -e 's/-/./')
 
-# winres is the MAJOR,MINOR,BUILD,REVISION 4-tuple used to identify
+cat >$3 <<EOF
-# the version of Windows binaries. We always set REVISION to 0, which
+#define TAILSCALE_VERSION_LONG "$VERSION_LONG"
-# seems to be how you map SemVer.
+#define TAILSCALE_VERSION_SHORT "$VERSION_SHORT"
-winres=$(echo "$short,0" | sed -e 's/\./,/g')
+#define TAILSCALE_VERSION_WIN_RES $VERSION_WINRES
-
+EOF
-(
-	printf '#define TAILSCALE_VERSION_LONG "%s"\n' "$long"
-	printf '#define TAILSCALE_VERSION_SHORT "%s"\n' "$short"
-	printf '#define TAILSCALE_VERSION_WIN_RES %s\n' "$winres"
-) >$3

version/version.sh

@@ -0,0 +1,109 @@
+#!/bin/sh
+
+set -eu
+
+case $# in
+    0|1)
+        # extra_hash describes a git repository other than the current
+        # one. It gets embedded as an additional commit hash in built
+        # binaries, to help us locate the exact set of tools and code
+        # that were used.
+        extra_hash="${1:-}"
+        if [ -z "$extra_hash" ]; then
+            # Nothing, empty extra hash is fine.
+            extra_hash=""
+        elif [ -d "$extra_hash/.git" ]; then
+            extra_hash=$(cd "$extra_hash" && git describe --always --dirty --exclude '*' --abbrev=200)
+        elif ! expr "$extra_hash" : "^[0-9a-f]*$"; then
+            echo "Invalid extra hash '$extra_hash', must be a git commit hash or path to a git repo" >&2
+            exit 1
+        fi
+
+        # Load the base version and optional corresponding git hash
+        # from the VERSION file. If there is no git hash in the file,
+        # we use the hash of the last change to the VERSION file.
+        version_file="$(dirname $0)/../VERSION.txt"
+        IFS=".$IFS" read -r major minor patch base_git_hash <"$version_file"
+        if [ -z "$base_git_hash" ]; then
+            base_git_hash=$(git rev-list --max-count=1 HEAD -- $version_file)
+        fi
+
+        # The full git has we're currently building at. --abbrev=200 is an
+        # arbitrary large number larger than all currently-known hashes, so
+        # that git displays the full commit hash.
+        git_hash=$(git describe --always --dirty --exclude '*' --abbrev=200)
+        # The number of extra commits between the release base to git_hash.
+        change_count=$(git rev-list ${base_git_hash}..HEAD | wc -l)
+        ;;
+    6)
+        # Test mode: rather than run git commands and whatnot, take in
+        # all the version pieces as arguments.
+        git_hash=$1
+        extra_hash=$2
+        major=$3
+        minor=$4
+        patch=$5
+        change_count=$6
+        ;;
+    *)
+        echo "Usage: $0 [extra-git-hash-or-checkout]"
+        exit 1
+esac
+
+# Shortened versions of git hashes, so that they fit neatly into an
+# "elongated" but still human-readable version number.
+short_git_hash=$(echo $git_hash | cut -c-9)
+short_extra_hash=$(echo $extra_hash | cut -c-9)
+
+# Convert major/minor/patch/change_count into an adjusted
+# major/minor/patch. This block is where all our policies on
+# versioning are.
+if expr "$minor" : "[0-9]*[13579]$" >/dev/null; then
+    # Odd minor numbers are unstable builds.
+    if [ "$patch" != "0" ]; then
+        # This is a fatal error, because a non-zero patch number
+        # indicates that we created an unstable git tag in violation
+        # of our versioning policy, and we want to blow up loudly to
+        # get that fixed.
+        echo "Unstable release $major.$minor.$patch has a non-zero patch number, which is not allowed" >&2
+        exit 1
+    fi
+    patch="$change_count"
+elif [ "$change_count" != "0" ]; then
+    # Even minor numbers are stable builds, but stable builds are
+    # supposed to have a zero change count. Therefore, we're currently
+    # describing a commit that's on a release branch, but hasn't been
+    # tagged as a patch release yet. We allow these commits to build
+    # for testing purposes, but force their version number to 0.0.0,
+    # to reflect that they're an unreleasable build. The git hashes
+    # still completely describe the build commit, so we can still
+    # figure out what this build is if it escapes into the wild.
+    major="0"
+    minor="0"
+    patch="0"
+fi
+
+# Hack for 1.1: add 1000 to the patch number. We switched from using
+# the proprietary repo's change_count over to using the OSS repo's
+# change_count, and this was necessary to avoid a backwards jump in
+# release numbers.
+if [ "$major.$minor" = "1.1" ]; then
+    patch="$((patch + 1000))"
+fi
+
+# At this point, the version number correctly reflects our
+# policies. All that remains is to output the various vars that other
+# code can use to embed version data.
+if [ -z "$extra_hash" ]; then
+    long_version_suffix="-t$short_git_hash"
+else
+    long_version_suffix="-t${short_git_hash}-g${short_extra_hash}"
+fi
+cat <<EOF
+VERSION_SHORT="${major}.${minor}.${patch}"
+VERSION_LONG="${major}.${minor}.${patch}${long_version_suffix}"
+VERSION_GIT_HASH="${git_hash}"
+VERSION_EXTRA_HASH="${extra_hash}"
+VERSION_XCODE="$((major + 100)).${minor}.${patch}"
+VERSION_WINRES="${major},${minor},${patch},0"
+EOF

version/version.xcconfig.do

@@ -1,5 +1,14 @@
-redo-ifchange mkversion.sh describe.txt extragitcommit.txt
+redo-ifchange version-info.sh
-read -r describe <describe.txt
+
-read -r other <extragitcommit.txt
+. ./version-info.sh
-ver=$(./mkversion.sh xcode "$describe" "$other")
+
-echo "$ver" >$3
+# CFBundleShortVersionString: the "short name" used in the App Store.
+# eg. 0.92.98
+echo "VERSION_NAME = $VERSION_SHORT"
+# CFBundleVersion: the build number. Needs to be 3 numeric sections
+# that increment for each release according to SemVer rules.
+#
+# We start counting at 100 because we submitted using raw build
+# numbers before, and Apple doesn't let you start over.  e.g. 0.98.3
+# -> 100.98.3
+echo "VERSION_ID = $VERSION_XCODE"

wgengine/router/dns/direct.go

@@ -9,6 +9,7 @@ package dns
 import (
 	"bufio"
 	"bytes"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -130,7 +131,7 @@ func (m directManager) Up(config Config) error {
 		contents, err := ioutil.ReadFile(resolvConf)
 		// If the original did not exist, still back up an empty file.
 		// The presence of a backup file is the way we know that Up ran.
-		if err != nil && !os.IsNotExist(err) {
+		if err != nil && !errors.Is(err, os.ErrNotExist) {
 			return err
 		}
 		if err := atomicfile.WriteFile(backupConf, contents, 0644); err != nil {

wgengine/router/dns/manager_windows.go

@@ -9,6 +9,7 @@ import (
 	"os/exec"
 	"strings"
 	"syscall"
+	"time"
 
 	"github.com/tailscale/wireguard-go/tun"
 	"golang.org/x/sys/windows/registry"
@@ -96,12 +97,19 @@ func (m windowsManager) Up(config Config) error {
 	// have changed, which makes the DNS settings actually take
 	// effect.
 	//
-	// This command can take a few seconds to run.
+	// This command can take a few seconds to run, so run it async, best effort.
+	go func() {
+		t0 := time.Now()
+		m.logf("running ipconfig /registerdns ...")
 		cmd := exec.Command("ipconfig", "/registerdns")
 		cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
+		d := time.Since(t0).Round(time.Millisecond)
 		if err := cmd.Run(); err != nil {
-		return fmt.Errorf("running ipconfig /registerdns: %w", err)
+			m.logf("error running ipconfig /registerdns after %v: %v", d, err)
+		} else {
+			m.logf("ran ipconfig /registerdns in %v", d)
 		}
+	}()
 
 	return nil
 }