tstest/integration: add testNode.AwaitListening, DERP+STUN, improve proxy trap

Updates #1840

cmd/tailscale/cli/cli_test.go

@@ -41,6 +41,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 		flagSet  map[string]bool
 		curPrefs *ipn.Prefs
 		curUser  string // os.Getenv("USER") on the client side
+		goos     string // empty means "linux"
 		mp       *ipn.MaskedPrefs
 		want     string
 	}{
@@ -144,7 +145,7 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			flagSet:  f("authkey"),
 			curPrefs: ipn.NewPrefs(),
 			mp: &ipn.MaskedPrefs{
-				Prefs:          *defaultPrefsFromUpArgs(t),
+				Prefs:          *defaultPrefsFromUpArgs(t, "linux"),
 				WantRunningSet: true,
 			},
 			want: "",
@@ -348,11 +349,72 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 			},
 			want: accidentalUpPrefix + " --hostname=newhostname --accept-routes --exit-node=100.64.5.6 --accept-dns --shields-up --advertise-tags=tag:foo,tag:bar --unattended --advertise-routes=10.0.0.0/16 --netfilter-mode=nodivert --operator=alice",
 		},
+		{
+			name:    "loggedout_is_implicit",
+			flagSet: f("advertise-exit-node"),
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				LoggedOut:  true,
+			},
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					AdvertiseRoutes: []netaddr.IPPrefix{
+						netaddr.MustParseIPPrefix("0.0.0.0/0"),
+					},
+				},
+				AdvertiseRoutesSet: true,
+			},
+			// not an error. LoggedOut is implicit.
+			want: "",
+		},
+		{
+			// Test that a pre-1.8 version of Tailscale with bogus NoSNAT pref
+			// values is able to enable exit nodes without warnings.
+			name:    "make_windows_exit_node",
+			flagSet: f("advertise-exit-node"),
+			curPrefs: &ipn.Prefs{
+				ControlURL: ipn.DefaultControlURL,
+				NoSNAT:     true, // assume this no-op accidental pre-1.8 value
+			},
+			goos: "windows",
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					AdvertiseRoutes: []netaddr.IPPrefix{
+						netaddr.MustParseIPPrefix("192.168.0.0/16"),
+					},
+				},
+				AdvertiseRoutesSet: true,
+			},
+			want: "", // not an error
+		},
+		{
+			name:    "ignore_netfilter_change_non_linux",
+			flagSet: f("accept-dns"),
+			curPrefs: &ipn.Prefs{
+				ControlURL:    ipn.DefaultControlURL,
+				NetfilterMode: preftype.NetfilterNoDivert, // we never had this bug, but pretend it got set non-zero on Windows somehow
+			},
+			goos: "windows",
+			mp: &ipn.MaskedPrefs{
+				Prefs: ipn.Prefs{
+					ControlURL: ipn.DefaultControlURL,
+					CorpDNS:    false,
+				},
+				CorpDNSSet: true,
+			},
+			want: "", // not an error
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			goos := "linux"
+			if tt.goos != "" {
+				goos = tt.goos
+			}
 			var got string
-			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp, tt.curUser); err != nil {
+			if err := checkForAccidentalSettingReverts(tt.flagSet, tt.curPrefs, tt.mp, goos, tt.curUser); err != nil {
 				got = err.Error()
 			}
 			if strings.TrimSpace(got) != tt.want {
@@ -362,13 +424,8 @@ func TestCheckForAccidentalSettingReverts(t *testing.T) {
 	}
 }
 
-func defaultPrefsFromUpArgs(t testing.TB) *ipn.Prefs {
-	upFlagSet.Parse(nil) // populates upArgs
-	if upFlagSet.Lookup("netfilter-mode") == nil && upArgs.netfilterMode == "" {
-		// This flag is not compiled on on-Linux platforms,
-		// but prefsFromUpArgs requires it be populated.
-		upArgs.netfilterMode = defaultNetfilterMode()
-	}
+func defaultPrefsFromUpArgs(t testing.TB, goos string) *ipn.Prefs {
+	upArgs := defaultUpArgsByGOOS(goos)
 	prefs, err := prefsFromUpArgs(upArgs, logger.Discard, new(ipnstate.Status), "linux")
 	if err != nil {
 		t.Fatalf("defaultPrefsFromUpArgs: %v", err)
@@ -377,6 +434,12 @@ func defaultPrefsFromUpArgs(t testing.TB) *ipn.Prefs {
 	return prefs
 }
 
+func defaultUpArgsByGOOS(goos string) (args upArgsT) {
+	fs := newUpFlagSet(goos, &args)
+	fs.Parse(nil) // populates args
+	return
+}
+
 func TestPrefsFromUpArgs(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -388,13 +451,28 @@ func TestPrefsFromUpArgs(t *testing.T) {
 		wantWarn string
 	}{
 		{
-			name: "zero",
-			goos: "windows",
-			args: upArgsT{},
+			name: "default_linux",
+			goos: "linux",
+			args: defaultUpArgsByGOOS("linux"),
 			want: &ipn.Prefs{
-				WantRunning:   true,
-				NoSNAT:        true,
-				NetfilterMode: preftype.NetfilterOn, // silly, but default from ipn.NewPref currently
+				ControlURL:       ipn.DefaultControlURL,
+				WantRunning:      true,
+				NoSNAT:           false,
+				NetfilterMode:    preftype.NetfilterOn,
+				CorpDNS:          true,
+				AllowSingleHosts: true,
+			},
+		},
+		{
+			name: "default_windows",
+			goos: "windows",
+			args: defaultUpArgsByGOOS("windows"),
+			want: &ipn.Prefs{
+				ControlURL:       ipn.DefaultControlURL,
+				WantRunning:      true,
+				CorpDNS:          true,
+				AllowSingleHosts: true,
+				NetfilterMode:    preftype.NetfilterOn,
 			},
 		},
 		{

cmd/tailscale/cli/file.go

@@ -29,6 +29,7 @@ import (
 	"tailscale.com/client/tailscale"
 	"tailscale.com/client/tailscale/apitype"
 	"tailscale.com/ipn"
+	"tailscale.com/net/tsaddr"
 	"tailscale.com/version"
 )
 
@@ -98,7 +99,7 @@ func runCp(ctx context.Context, args []string) error {
 
 	peerAPIBase, lastSeen, isOffline, err := discoverPeerAPIBase(ctx, ip)
 	if err != nil {
-		return err
+		return fmt.Errorf("can't send to %s: %v", target, err)
 	}
 	if isOffline {
 		fmt.Fprintf(os.Stderr, "# warning: %s is offline\n", target)
@@ -203,7 +204,32 @@ func discoverPeerAPIBase(ctx context.Context, ipStr string) (base string, lastSe
 			return ft.PeerAPIURL, lastSeen, isOffline, nil
 		}
 	}
-	return "", time.Time{}, false, errors.New("target seems to be running an old Tailscale version")
+	return "", time.Time{}, false, fileTargetErrorDetail(ctx, ip)
+}
+
+// fileTargetErrorDetail returns a non-nil error saying why ip is an
+// invalid file sharing target.
+func fileTargetErrorDetail(ctx context.Context, ip netaddr.IP) error {
+	found := false
+	if st, err := tailscale.Status(ctx); err == nil && st.Self != nil {
+		for _, peer := range st.Peer {
+			for _, pip := range peer.TailscaleIPs {
+				if pip == ip {
+					found = true
+					if peer.UserID != st.Self.UserID {
+						return errors.New("owned by different user; can only send files to your own devices")
+					}
+				}
+			}
+		}
+	}
+	if found {
+		return errors.New("target seems to be running an old Tailscale version")
+	}
+	if !tsaddr.IsTailscaleIP(ip) {
+		return fmt.Errorf("unknown target; %v is not a Tailscale IP address", ip)
+	}
+	return errors.New("unknown target; not in your Tailnet")
 }
 
 const maxSniff = 4 << 20

cmd/tailscale/cli/up.go

@@ -52,7 +52,9 @@ flag is also used.
 	Exec:    runUp,
 }
 
-var upFlagSet = (func() *flag.FlagSet {
+var upFlagSet = newUpFlagSet(runtime.GOOS, &upArgs)
+
+func newUpFlagSet(goos string, upArgs *upArgsT) *flag.FlagSet {
 	upf := flag.NewFlagSet("up", flag.ExitOnError)
 
 	upf.BoolVar(&upArgs.forceReauth, "force-reauth", false, "force reauthentication")
@@ -70,18 +72,18 @@ var upFlagSet = (func() *flag.FlagSet {
 	upf.StringVar(&upArgs.hostname, "hostname", "", "hostname to use instead of the one provided by the OS")
 	upf.StringVar(&upArgs.advertiseRoutes, "advertise-routes", "", "routes to advertise to other nodes (comma-separated, e.g. \"10.0.0.0/8,192.168.0.0/24\")")
 	upf.BoolVar(&upArgs.advertiseDefaultRoute, "advertise-exit-node", false, "offer to be an exit node for internet traffic for the tailnet")
-	if safesocket.PlatformUsesPeerCreds() {
+	if safesocket.GOOSUsesPeerCreds(goos) {
 		upf.StringVar(&upArgs.opUser, "operator", "", "Unix username to allow to operate on tailscaled without sudo")
 	}
-	if runtime.GOOS == "linux" {
+	switch goos {
+	case "linux":
 		upf.BoolVar(&upArgs.snat, "snat-subnet-routes", true, "source NAT traffic to local routes advertised with --advertise-routes")
 		upf.StringVar(&upArgs.netfilterMode, "netfilter-mode", defaultNetfilterMode(), "netfilter mode (one of on, nodivert, off)")
-	}
-	if runtime.GOOS == "windows" {
+	case "windows":
 		upf.BoolVar(&upArgs.forceDaemon, "unattended", false, "run in \"Unattended Mode\" where Tailscale keeps running even after the current GUI user logs out (Windows-only)")
 	}
 	return upf
-})()
+}
 
 func defaultNetfilterMode() string {
 	if distro.Get() == distro.Synology {
@@ -214,12 +216,13 @@ func prefsFromUpArgs(upArgs upArgsT, warnf logger.Logf, st *ipnstate.Status, goo
 	prefs.ShieldsUp = upArgs.shieldsUp
 	prefs.AdvertiseRoutes = routes
 	prefs.AdvertiseTags = tags
-	prefs.NoSNAT = !upArgs.snat
 	prefs.Hostname = upArgs.hostname
 	prefs.ForceDaemon = upArgs.forceDaemon
 	prefs.OperatorUser = upArgs.opUser
 
 	if goos == "linux" {
+		prefs.NoSNAT = !upArgs.snat
+
 		switch upArgs.netfilterMode {
 		case "on":
 			prefs.NetfilterMode = preftype.NetfilterOn
@@ -302,7 +305,7 @@ func runUp(ctx context.Context, args []string) error {
 	})
 
 	if !upArgs.reset {
-		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp, os.Getenv("USER")); err != nil {
+		if err := checkForAccidentalSettingReverts(flagSet, curPrefs, mp, runtime.GOOS, os.Getenv("USER")); err != nil {
 			fatalf("%s", err)
 		}
 	}
@@ -528,7 +531,7 @@ const accidentalUpPrefix = "Error: changing settings via 'tailscale up' requires
 //
 // mp is the mask of settings actually set, where mp.Prefs is the new
 // preferences to set, including any values set from implicit flags.
-func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Prefs, mp *ipn.MaskedPrefs, curUser string) error {
+func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Prefs, mp *ipn.MaskedPrefs, goos, curUser string) error {
 	if len(flagSet) == 0 {
 		// A bare "tailscale up" is a special case to just
 		// mean bringing the network up without any changes.
@@ -586,10 +589,21 @@ func checkForAccidentalSettingReverts(flagSet map[string]bool, curPrefs *ipn.Pre
 		if reflect.DeepEqual(exi, imi) {
 			continue
 		}
-		if flagName == "operator" && imi == "" && exi == curUser {
-			// Don't require setting operator if the current user matches
-			// the configured operator.
-			continue
+		switch flagName {
+		case "operator":
+			if imi == "" && exi == curUser {
+				// Don't require setting operator if the current user matches
+				// the configured operator.
+				continue
+			}
+		case "snat-subnet-routes", "netfilter-mode":
+			if goos != "linux" {
+				// Issue 1833: we used to accidentally set the NoSNAT
+				// pref for non-Linux nodes. It only affects Linux, so
+				// ignore it if it changes. Likewise, ignore
+				// Linux-only netfilter-mode on non-Linux.
+				continue
+			}
 		}
 		switch flagName {
 		case "":

cmd/tailscale/cli/web.go

@@ -73,7 +73,11 @@ func runWeb(ctx context.Context, args []string) error {
 	}
 
 	if webArgs.cgi {
-		return cgi.Serve(http.HandlerFunc(webHandler))
+		if err := cgi.Serve(http.HandlerFunc(webHandler)); err != nil {
+			log.Printf("tailscale.cgi: %v", err)
+			return err
+		}
+		return nil
 	}
 	return http.ListenAndServe(webArgs.listen, http.HandlerFunc(webHandler))
 }
@@ -208,7 +212,7 @@ func webHandler(w http.ResponseWriter, r *http.Request) {
 	if r.Method == "POST" {
 		type mi map[string]interface{}
 		w.Header().Set("Content-Type", "application/json")
-		url, err := tailscaleUp(r.Context())
+		url, err := tailscaleUpForceReauth(r.Context())
 		if err != nil {
 			json.NewEncoder(w).Encode(mi{"error": err})
 			return
@@ -244,7 +248,7 @@ func webHandler(w http.ResponseWriter, r *http.Request) {
 }
 
 // TODO(crawshaw): some of this is very similar to the code in 'tailscale up', can we share anything?
-func tailscaleUp(ctx context.Context) (authURL string, retErr error) {
+func tailscaleUpForceReauth(ctx context.Context) (authURL string, retErr error) {
 	prefs := ipn.NewPrefs()
 	prefs.ControlURL = ipn.DefaultControlURL
 	prefs.WantRunning = true
@@ -256,10 +260,31 @@ func tailscaleUp(ctx context.Context) (authURL string, retErr error) {
 		prefs.NetfilterMode = preftype.NetfilterOff
 	}
 
-	c, bc, ctx, cancel := connect(ctx)
+	st, err := tailscale.Status(ctx)
+	if err != nil {
+		return "", fmt.Errorf("can't fetch status: %v", err)
+	}
+	origAuthURL := st.AuthURL
+
+	// printAuthURL reports whether we should print out the
+	// provided auth URL from an IPN notify.
+	printAuthURL := func(url string) bool {
+		return url != origAuthURL
+	}
+
+	c, bc, pumpCtx, cancel := connect(ctx)
 	defer cancel()
 
+	gotEngineUpdate := make(chan bool, 1) // gets value upon an engine update
+	go pump(pumpCtx, bc, c)
+
 	bc.SetNotifyCallback(func(n ipn.Notify) {
+		if n.Engine != nil {
+			select {
+			case gotEngineUpdate <- true:
+			default:
+			}
+		}
 		if n.ErrMessage != nil {
 			msg := *n.ErrMessage
 			if msg == ipn.ErrMsgPermissionDenied {
@@ -272,11 +297,21 @@ func tailscaleUp(ctx context.Context) (authURL string, retErr error) {
 			}
 			retErr = fmt.Errorf("backend error: %v", msg)
 			cancel()
-		} else if url := n.BrowseToURL; url != nil {
+		} else if url := n.BrowseToURL; url != nil && printAuthURL(*url) {
 			authURL = *url
 			cancel()
 		}
 	})
+	// Wait for backend client to be connected so we know
+	// we're subscribed to updates. Otherwise we can miss
+	// an update upon its transition to running. Do so by causing some traffic
+	// back to the bus that we then wait on.
+	bc.RequestEngineStatus()
+	select {
+	case <-gotEngineUpdate:
+	case <-pumpCtx.Done():
+		return authURL, pumpCtx.Err()
+	}
 
 	bc.SetPrefs(prefs)
 
@@ -284,7 +319,6 @@ func tailscaleUp(ctx context.Context) (authURL string, retErr error) {
 		StateKey: ipn.GlobalDaemonStateKey,
 	})
 	bc.StartLoginInteractive()
-	pump(ctx, bc, c)
 
 	if authURL == "" && retErr == nil {
 		return "", fmt.Errorf("login failed with no backend error message")

cmd/tailscale/cli/web.html

@@ -11,140 +11,133 @@
 </head>
 
 <body class="py-14">
-	<main class="container max-w-lg mx-auto py-6 px-8 bg-white rounded-md shadow-2xl" style="width: 95%">
-		<header class="flex justify-between items-center min-width-0 py-2 mb-8">
-			<svg width="26" height="26" viewBox="0 0 23 23" title="Tailscale" fill="none" xmlns="http://www.w3.org/2000/svg"
-				class="flex-shrink-0 mr-4">
-				<circle opacity="0.2" cx="3.4" cy="3.25" r="2.7" fill="currentColor"></circle>
-				<circle cx="3.4" cy="11.3" r="2.7" fill="currentColor"></circle>
-				<circle opacity="0.2" cx="3.4" cy="19.5" r="2.7" fill="currentColor"></circle>
-				<circle cx="11.5" cy="11.3" r="2.7" fill="currentColor"></circle>
-				<circle cx="11.5" cy="19.5" r="2.7" fill="currentColor"></circle>
-				<circle opacity="0.2" cx="11.5" cy="3.25" r="2.7" fill="currentColor"></circle>
-				<circle opacity="0.2" cx="19.5" cy="3.25" r="2.7" fill="currentColor"></circle>
-				<circle cx="19.5" cy="11.3" r="2.7" fill="currentColor"></circle>
-				<circle opacity="0.2" cx="19.5" cy="19.5" r="2.7" fill="currentColor"></circle>
-			</svg>
-			<div class="flex items-center justify-end space-x-2 w-2/3">
-				{{ with .Profile.LoginName }}
-				<div class="text-right truncate leading-4">
-					<h4 class="truncate">{{.}}</h4>
-					<a href="#" class="text-xs text-gray-500 hover:text-gray-700 js-loginButton">Switch account</a>
-				</div>
+<main class="container max-w-lg mx-auto py-6 px-8 bg-white rounded-md shadow-2xl" style="width: 95%">
+	<header class="flex justify-between items-center min-width-0 py-2 mb-8">
+		<svg width="26" height="26" viewBox="0 0 23 23" title="Tailscale" fill="none" xmlns="http://www.w3.org/2000/svg"
+			class="flex-shrink-0 mr-4">
+			<circle opacity="0.2" cx="3.4" cy="3.25" r="2.7" fill="currentColor"></circle>
+			<circle cx="3.4" cy="11.3" r="2.7" fill="currentColor"></circle>
+			<circle opacity="0.2" cx="3.4" cy="19.5" r="2.7" fill="currentColor"></circle>
+			<circle cx="11.5" cy="11.3" r="2.7" fill="currentColor"></circle>
+			<circle cx="11.5" cy="19.5" r="2.7" fill="currentColor"></circle>
+			<circle opacity="0.2" cx="11.5" cy="3.25" r="2.7" fill="currentColor"></circle>
+			<circle opacity="0.2" cx="19.5" cy="3.25" r="2.7" fill="currentColor"></circle>
+			<circle cx="19.5" cy="11.3" r="2.7" fill="currentColor"></circle>
+			<circle opacity="0.2" cx="19.5" cy="19.5" r="2.7" fill="currentColor"></circle>
+		</svg>
+		<div class="flex items-center justify-end space-x-2 w-2/3">
+			{{ with .Profile.LoginName }}
+			<div class="text-right truncate leading-4">
+				<h4 class="truncate">{{.}}</h4>
+				<a href="#" class="text-xs text-gray-500 hover:text-gray-700 js-loginButton">Switch account</a>
+			</div>
+			{{ end }}
+			<div class="relative flex-shrink-0 w-8 h-8 rounded-full overflow-hidden">
+				{{ with .Profile.ProfilePicURL }}
+				<div class="w-8 h-8 flex pointer-events-none rounded-full bg-gray-200"
+					style="background-image: url('{{.}}'); background-size: cover;"></div>
+				{{ else }}
+				<div class="w-8 h-8 flex pointer-events-none rounded-full border border-gray-400 border-dashed"></div>
 				{{ end }}
-				<div class="relative flex-shrink-0 w-8 h-8 rounded-full overflow-hidden">
-					{{ with .Profile.ProfilePicURL }}
-					<div class="w-8 h-8 flex pointer-events-none rounded-full bg-gray-200"
-						style="background-image: url('{{.}}'); background-size: cover;"></div>
-					{{ else }}
-					<div class="w-8 h-8 flex pointer-events-none rounded-full border border-gray-400 border-dashed"></div>
-					{{ end }}
-				</div>
 			</div>
-		</header>
-		{{ if .IP }}
-		<div
-			class="border border-gray-200 bg-gray-0 rounded-lg p-2 pl-3 pr-3 mb-8 width-full flex items-center justify-between">
-			<div class="flex items-center min-width-0">
-				<svg class="flex-shrink-0 text-gray-600 mr-3 ml-1" xmlns="http://www.w3.org/2000/svg" width="20" height="20"
-					viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round"
-					stroke-linejoin="round">
-					<rect x="2" y="2" width="20" height="8" rx="2" ry="2"></rect>
-					<rect x="2" y="14" width="20" height="8" rx="2" ry="2"></rect>
-					<line x1="6" y1="6" x2="6.01" y2="6"></line>
-					<line x1="6" y1="18" x2="6.01" y2="18"></line>
-				</svg>
-				<h4 class="font-semibold truncate mr-2">{{.DeviceName}}</h4>
-			</div>
-			<h5>{{.IP}}</h5>
 		</div>
-		{{ end }}
-		{{ if or (eq .Status "NeedsLogin") (eq .Status "NoState") }}
-		{{ if .IP }}
-		<div class="mb-6">
-			<p class="text-gray-700">Your device's key has expired. Reauthenticate this device by logging in again, or <a
-					href="https://tailscale.com/kb/1028/key-expiry" class="link" target="_blank">learn more</a>.</p>
+	</header>
+	{{ if .IP }}
+	<div
+		class="border border-gray-200 bg-gray-0 rounded-lg p-2 pl-3 pr-3 mb-8 width-full flex items-center justify-between">
+		<div class="flex items-center min-width-0">
+			<svg class="flex-shrink-0 text-gray-600 mr-3 ml-1" xmlns="http://www.w3.org/2000/svg" width="20" height="20"
+				viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" stroke-linecap="round"
+				stroke-linejoin="round">
+				<rect x="2" y="2" width="20" height="8" rx="2" ry="2"></rect>
+				<rect x="2" y="14" width="20" height="8" rx="2" ry="2"></rect>
+				<line x1="6" y1="6" x2="6.01" y2="6"></line>
+				<line x1="6" y1="18" x2="6.01" y2="18"></line>
+			</svg>
+			<h4 class="font-semibold truncate mr-2">{{.DeviceName}}</h4>
 		</div>
-		<a href="#" class="mb-4 js-loginButton" target="_blank">
-			<button class="button button-blue w-full">Reauthenticate</button>
-		</a>
-		{{ else }}
-		<div class="mb-6">
-			<h3 class="text-3xl font-semibold mb-3">Log in</h3>
-			<p class="text-gray-700">Get started by logging in to your Tailscale network. Or,&nbsp;learn&nbsp;more at <a
-					href="https://tailscale.com/" class="link" target="_blank">tailscale.com</a>.</p>
-		</div>
-		<a href="#" class="mb-4 js-loginButton" target="_blank">
-			<button class="button button-blue w-full">Log In</button>
-		</a>
-		{{ end }}
-		{{ else if eq .Status "NeedsMachineAuth" }}
-		<div class="mb-4">
-			This device is authorized, but needs approval from a network admin before it can connect to the network.
-		</div>
-		{{ else }}
-		<div class="mb-4">
-			<p>You are connected! Access this device over Tailscale using the device name or IP address above.</p>
-		</div>
-		<a href="#" class="mb-4 link font-medium js-loginButton" target="_blank">Reauthenticate</a>
-		{{ end }}
-	</main>
-	<script>
-		(function () {
-			let loginButtons = document.querySelectorAll(".js-loginButton");
-			let fetchingUrl = false;
+		<h5>{{.IP}}</h5>
+	</div>
+	{{ end }}
+	{{ if or (eq .Status "NeedsLogin") (eq .Status "NoState") }}
+	{{ if .IP }}
+	<div class="mb-6">
+		<p class="text-gray-700">Your device's key has expired. Reauthenticate this device by logging in again, or <a
+				href="https://tailscale.com/kb/1028/key-expiry" class="link" target="_blank">learn more</a>.</p>
+	</div>
+	<a href="#" class="mb-4 js-loginButton" target="_blank">
+		<button class="button button-blue w-full">Reauthenticate</button>
+	</a>
+	{{ else }}
+	<div class="mb-6">
+		<h3 class="text-3xl font-semibold mb-3">Log in</h3>
+		<p class="text-gray-700">Get started by logging in to your Tailscale network. Or,&nbsp;learn&nbsp;more at <a
+				href="https://tailscale.com/" class="link" target="_blank">tailscale.com</a>.</p>
+	</div>
+	<a href="#" class="mb-4 js-loginButton" target="_blank">
+		<button class="button button-blue w-full">Log In</button>
+	</a>
+	{{ end }}
+	{{ else if eq .Status "NeedsMachineAuth" }}
+	<div class="mb-4">
+		This device is authorized, but needs approval from a network admin before it can connect to the network.
+	</div>
+	{{ else }}
+	<div class="mb-4">
+		<p>You are connected! Access this device over Tailscale using the device name or IP address above.</p>
+	</div>
+	<a href="#" class="mb-4 link font-medium js-loginButton" target="_blank">Reauthenticate</a>
+	{{ end }}
+</main>
+<script>(function () {
+let loginButtons = document.querySelectorAll(".js-loginButton");
+let fetchingUrl = false;
 
-			function handleClick(e) {
-				e.preventDefault();
+function handleClick(e) {
+	e.preventDefault();
 
-				if (fetchingUrl) {
-					return;
-				}
+	if (fetchingUrl) {
+		return;
+	}
 
-				fetchingUrl = true;
-				const urlParams = new URLSearchParams(window.location.search);
-				const token = urlParams.get("SynoToken");
-				const nextParams = new URLSearchParams({ up: true });
-				if (token) {
-					nextParams.set("SynoToken", token)
-				}
-				const nextUrl = new URL(window.location);
-				nextUrl.search = nextParams.toString()
-				const url = nextUrl.toString();
+	fetchingUrl = true;
+	const urlParams = new URLSearchParams(window.location.search);
+	const token = urlParams.get("SynoToken");
+	const nextParams = new URLSearchParams({ up: true });
+	if (token) {
+		nextParams.set("SynoToken", token)
+	}
+	const nextUrl = new URL(window.location);
+	nextUrl.search = nextParams.toString()
+	const url = nextUrl.toString();
 
-				const tab = window.open("/redirect", "_blank");
+	fetch(url, {
+		method: "POST",
+		headers: {
+			"Accept": "application/json",
+			"Content-Type": "application/json",
+		}
+	}).then(res => res.json()).then(res => {
+		fetchingUrl = false;
+		const err = res["error"];
+		if (err) {
+			throw new Error(err);
+		}
+		const url = res["url"];
+		if (url) {
+			document.location.href = url;
+		} else {
+			location.reload();
+		}
+	}).catch(err => {
+		alert("Failed to log in: " + err.message);
+	});
+}
 
-				fetch(url, {
-					method: "POST",
-					headers: {
-						"Accept": "application/json",
-						"Content-Type": "application/json",
-					}
-				}).then(res => res.json()).then(res => {
-					fetchingUrl = false;
-					const err = res["error"];
-					if (err) {
-						throw new Error(err);
-					}
-					const url = res["url"];
-					if (url) {
-						authUrl = url;
-						tab.location = url;
-						tab.focus();
-					} else {
-						location.reload();
-					}
-				}).catch(err => {
-					tab.close();
-					alert("Failed to log in: " + err.message);
-				});
-			}
-
-			Array.from(loginButtons).forEach(el => {
-				el.addEventListener("click", handleClick);
-			})
-		})();
-	</script>
+Array.from(loginButtons).forEach(el => {
+	el.addEventListener("click", handleClick);
+})
+})();</script>
 </body>
 
 </html>

cmd/tailscale/depaware.txt

@@ -33,7 +33,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/net/portmapper                                 from tailscale.com/net/netcheck+
         tailscale.com/net/stun                                       from tailscale.com/net/netcheck
         tailscale.com/net/tlsdial                                    from tailscale.com/derp/derphttp
-        tailscale.com/net/tsaddr                                     from tailscale.com/net/interfaces
+        tailscale.com/net/tsaddr                                     from tailscale.com/net/interfaces+
      💣 tailscale.com/net/tshttpproxy                                from tailscale.com/derp/derphttp+
         tailscale.com/paths                                          from tailscale.com/cmd/tailscale/cli+
         tailscale.com/safesocket                                     from tailscale.com/cmd/tailscale/cli+
@@ -85,7 +85,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         golang.org/x/text/transform                                  from golang.org/x/text/secure/bidirule+
         golang.org/x/text/unicode/bidi                               from golang.org/x/net/idna+
         golang.org/x/text/unicode/norm                               from golang.org/x/net/idna
-        golang.org/x/time/rate                                       from tailscale.com/types/logger+
+        golang.org/x/time/rate                                       from tailscale.com/cmd/tailscale/cli+
         bufio                                                        from compress/flate+
         bytes                                                        from bufio+
         compress/flate                                               from compress/gzip+

cmd/tailscaled/depaware.txt

@@ -188,7 +188,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         golang.org/x/text/transform                                  from golang.org/x/text/secure/bidirule+
         golang.org/x/text/unicode/bidi                               from golang.org/x/net/idna+
         golang.org/x/text/unicode/norm                               from golang.org/x/net/idna
-        golang.org/x/time/rate                                       from tailscale.com/types/logger+
+        golang.org/x/time/rate                                       from inet.af/netstack/tcpip/stack+
         bufio                                                        from compress/flate+
         bytes                                                        from bufio+
         compress/flate                                               from compress/gzip+
@@ -221,7 +221,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         debug/elf                                                    from rsc.io/goversion/version
         debug/macho                                                  from rsc.io/goversion/version
         debug/pe                                                     from rsc.io/goversion/version
-        embed                                                        from tailscale.com/net/dns
+   L    embed                                                        from tailscale.com/net/dns
         encoding                                                     from encoding/json+
         encoding/asn1                                                from crypto/x509+
         encoding/base64                                              from encoding/json+

go.mod

@@ -7,12 +7,13 @@ require (
 	github.com/anmitsu/go-shlex v0.0.0-20161002113705-648efa622239 // indirect
 	github.com/coreos/go-iptables v0.4.5
 	github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 // indirect
+	github.com/frankban/quicktest v1.12.1
 	github.com/github/certstore v0.1.0
 	github.com/gliderlabs/ssh v0.2.2
 	github.com/go-multierror/multierror v1.0.2
 	github.com/go-ole/go-ole v1.2.4
 	github.com/godbus/dbus/v5 v5.0.3
-	github.com/google/go-cmp v0.5.4
+	github.com/google/go-cmp v0.5.5
 	github.com/goreleaser/nfpm v1.1.10
 	github.com/jsimonetti/rtnetlink v0.0.0-20210212075122-66c871082f2b
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51

go.sum

@@ -25,6 +25,8 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs
 github.com/dvyukov/go-fuzz v0.0.0-20201127111758-49e582c6c23d/go.mod h1:11Gm+ccJnvAhCNLlf5+cS9KjtbaD5I5zaZpFMsTHWTw=
 github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568 h1:BHsljHzVlRcyQhjrss6TZTdY2VfCqZPbv5k3iBFa2ZQ=
 github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc=
+github.com/frankban/quicktest v1.12.1 h1:P6vQcHwZYgVGIpUzKB5DXzkEeYJppJOStPLuh9aB89c=
+github.com/frankban/quicktest v1.12.1/go.mod h1:qLE0fzW0VuyUAJgPU19zByoIr0HtCHN/r/VLSOOIySU=
 github.com/github/fakeca v0.1.0 h1:Km/MVOFvclqxPM9dZBC4+QE564nU4gz4iZ0D9pMw28I=
 github.com/github/fakeca v0.1.0/go.mod h1:+bormgoGMMuamOscx7N91aOuUST7wdaJ2rNjeohylyo=
 github.com/gliderlabs/ssh v0.2.2 h1:6zsha5zo/TWhRhwqCD3+EarCAgZ2yN28ipRnGPnwkI0=
@@ -44,6 +46,8 @@ github.com/google/go-cmp v0.5.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M=
 github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.5 h1:Khx7svrCpmxxtHBq5j2mp/xVjsi8hQMfNLvJFAlrGgU=
+github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI=
 github.com/google/rpmpack v0.0.0-20191226140753-aa36bfddb3a0 h1:BW6OvS3kpT5UEPbCZ+KyX/OB4Ks9/MNMhWjqPPkZxsE=
 github.com/google/rpmpack v0.0.0-20191226140753-aa36bfddb3a0/go.mod h1:RaTPr0KUf2K7fnZYLNDrr8rxAamWs3iNywJLtQ2AzBg=
@@ -68,6 +72,8 @@ github.com/klauspost/compress v1.10.10 h1:a/y8CglcM7gLGYmlbP/stPE5sR3hbhFRUjCBfd
 github.com/klauspost/compress v1.10.10/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs=
 github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI=
 github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo=
+github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI=
+github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI=
 github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ=
 github.com/kr/pty v1.1.8 h1:AkaSdXYQOWeaO3neb8EM634ahkXXe3jYbVh/F9lq+GI=
 github.com/kr/pty v1.1.8/go.mod h1:O1sed60cT9XZ5uDucP5qwvh+TE3NnUj51EiZO/lmSfw=

net/dns/debian_resolvconf.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+// +build linux freebsd openbsd
+
 package dns
 
 import (

net/dns/direct.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+// +build linux freebsd openbsd
+
 package dns
 
 import (

net/dns/openresolv.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+// +build linux freebsd openbsd
+
 package dns
 
 import (

net/dns/resolvconf.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+// +build linux freebsd openbsd
+
 package dns
 
 import (

net/stun/stuntest/stuntest.go

@@ -26,11 +26,11 @@ type stunStats struct {
 	readIPv6 int
 }
 
-func Serve(t *testing.T) (addr *net.UDPAddr, cleanupFn func()) {
+func Serve(t testing.TB) (addr *net.UDPAddr, cleanupFn func()) {
 	return ServeWithPacketListener(t, nettype.Std{})
 }
 
-func ServeWithPacketListener(t *testing.T, ln nettype.PacketListener) (addr *net.UDPAddr, cleanupFn func()) {
+func ServeWithPacketListener(t testing.TB, ln nettype.PacketListener) (addr *net.UDPAddr, cleanupFn func()) {
 	t.Helper()
 
 	// TODO(crawshaw): use stats to test re-STUN logic
@@ -52,7 +52,7 @@ func ServeWithPacketListener(t *testing.T, ln nettype.PacketListener) (addr *net
 	}
 }
 
-func runSTUN(t *testing.T, pc net.PacketConn, stats *stunStats, done chan<- struct{}) {
+func runSTUN(t testing.TB, pc net.PacketConn, stats *stunStats, done chan<- struct{}) {
 	defer close(done)
 
 	var buf [64 << 10]byte

paths/paths.go

@@ -26,6 +26,13 @@ func DefaultTailscaledSocket() string {
 	if runtime.GOOS == "darwin" {
 		return "/var/run/tailscaled.socket"
 	}
+	if runtime.GOOS == "linux" {
+		// TODO(crawshaw): does this path change with DSM7?
+		const synologySock = "/volume1/@appstore/Tailscale/var/tailscaled.sock" // SYNOPKG_PKGDEST in scripts/installer
+		if fi, err := os.Stat(filepath.Dir(synologySock)); err == nil && fi.IsDir() {
+			return synologySock
+		}
+	}
 	if fi, err := os.Stat("/var/run"); err == nil && fi.IsDir() {
 		return "/var/run/tailscale/tailscaled.sock"
 	}

safesocket/safesocket.go

@@ -61,8 +61,12 @@ func LocalTCPPortAndToken() (port int, token string, err error) {
 
 // PlatformUsesPeerCreds reports whether the current platform uses peer credentials
 // to authenticate connections.
-func PlatformUsesPeerCreds() bool {
-	switch runtime.GOOS {
+func PlatformUsesPeerCreds() bool { return GOOSUsesPeerCreds(runtime.GOOS) }
+
+// GOOSUsesPeerCreds is like PlatformUsesPeerCreds but takes a
+// runtime.GOOS value instead of using the current one.
+func GOOSUsesPeerCreds(goos string) bool {
+	switch goos {
 	case "linux", "darwin", "freebsd":
 		return true
 	}

tstest/integration/integration_test.go

@@ -7,11 +7,14 @@ package integration
 
 import (
 	"bytes"
+	crand "crypto/rand"
+	"crypto/tls"
 	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"log"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -21,77 +24,94 @@ import (
 	"runtime"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"go4.org/mem"
+	"tailscale.com/derp"
+	"tailscale.com/derp/derphttp"
+	"tailscale.com/net/stun/stuntest"
+	"tailscale.com/safesocket"
 	"tailscale.com/smallzstd"
+	"tailscale.com/tailcfg"
 	"tailscale.com/tstest"
+	"tailscale.com/tstest/integration/testcontrol"
+	"tailscale.com/types/key"
+	"tailscale.com/types/logger"
+	"tailscale.com/types/nettype"
 )
 
+var mainError atomic.Value // of error
+
+func TestMain(m *testing.M) {
+	v := m.Run()
+	if v != 0 {
+		os.Exit(v)
+	}
+	if err, ok := mainError.Load().(error); ok {
+		fmt.Fprintf(os.Stderr, "FAIL: %v\n", err)
+		os.Exit(1)
+	}
+	os.Exit(0)
+}
+
 func TestIntegration(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		t.Skip("not tested/working on Windows yet")
 	}
-	td := t.TempDir()
-	daemonExe := build(t, td, "tailscale.com/cmd/tailscaled")
-	cliExe := build(t, td, "tailscale.com/cmd/tailscale")
 
-	logc := new(logCatcher)
-	ts := httptest.NewServer(logc)
-	defer ts.Close()
+	bins := buildTestBinaries(t)
 
-	httpProxy := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		var got bytes.Buffer
-		r.Write(&got)
-		err := fmt.Errorf("unexpected HTTP proxy via proxy: %s", got.Bytes())
-		t.Error(err)
-		go panic(err)
-	}))
-	defer httpProxy.Close()
+	env := newTestEnv(t, bins)
+	defer env.Close()
 
-	socketPath := filepath.Join(td, "tailscale.sock")
-	dcmd := exec.Command(daemonExe,
-		"--tun=userspace-networking",
-		"--state="+filepath.Join(td, "tailscale.state"),
-		"--socket="+socketPath,
-	)
-	dcmd.Env = append(os.Environ(),
-		"TS_LOG_TARGET="+ts.URL,
-		"HTTP_PROXY="+httpProxy.URL,
-		"HTTPS_PROXY="+httpProxy.URL,
-	)
-	if err := dcmd.Start(); err != nil {
-		t.Fatalf("starting tailscaled: %v", err)
-	}
+	n1 := newTestNode(t, env)
+
+	dcmd := n1.StartDaemon(t)
 	defer dcmd.Process.Kill()
 
-	var json []byte
-	if err := tstest.WaitFor(20*time.Second, func() (err error) {
-		json, err = exec.Command(cliExe, "--socket="+socketPath, "status", "--json").CombinedOutput()
-		if err != nil {
-			return fmt.Errorf("running tailscale status: %v, %s", err, json)
-		}
-		return nil
-	}); err != nil {
-		t.Fatal(err)
-	}
+	n1.AwaitListening(t)
 
-	if os.Getenv("TS_RUN_TEST") == "failing_up" {
-		// Force a connection through the HTTP proxy to panic and fail.
-		exec.Command(cliExe, "--socket="+socketPath, "up").Run()
+	json, err := n1.Tailscale("status", "--json").CombinedOutput()
+	if err != nil {
+		t.Fatalf("running tailscale status: %v, %s", err, json)
 	}
+	t.Logf("Status: %s", json)
 
 	if err := tstest.WaitFor(20*time.Second, func() error {
 		const sub = `Program starting: `
-		if !logc.logsContains(mem.S(sub)) {
-			return fmt.Errorf("log catcher didn't see %#q; got %s", sub, logc.logsString())
+		if !env.LogCatcher.logsContains(mem.S(sub)) {
+			return fmt.Errorf("log catcher didn't see %#q; got %s", sub, env.LogCatcher.logsString())
 		}
 		return nil
 	}); err != nil {
 		t.Error(err)
 	}
 
+	t.Logf("Running up --login-server=%s ...", env.ControlServer.URL)
+	if err := n1.Tailscale("up", "--login-server="+env.ControlServer.URL).Run(); err != nil {
+		t.Fatalf("up: %v", err)
+	}
+
+	if d, _ := time.ParseDuration(os.Getenv("TS_POST_UP_SLEEP")); d > 0 {
+		t.Logf("Sleeping for %v to give 'up' time to misbehave (https://github.com/tailscale/tailscale/issues/1840) ...", d)
+		time.Sleep(d)
+	}
+
+	var ip string
+	if err := tstest.WaitFor(20*time.Second, func() error {
+		out, err := n1.Tailscale("ip").Output()
+		if err != nil {
+			return err
+		}
+		ip = string(out)
+		return nil
+	}); err != nil {
+		t.Error(err)
+	}
+	t.Logf("Got IP: %v", ip)
+
 	dcmd.Process.Signal(os.Interrupt)
 
 	ps, err := dcmd.Process.Wait()
@@ -102,7 +122,149 @@ func TestIntegration(t *testing.T) {
 		t.Errorf("tailscaled ExitCode = %d; want 0", ps.ExitCode())
 	}
 
-	t.Logf("number of HTTP logcatcher requests: %v", logc.numRequests())
+	t.Logf("number of HTTP logcatcher requests: %v", env.LogCatcher.numRequests())
+	if err, ok := mainError.Load().(error); ok {
+		t.Error(err)
+		t.Logf("logs: %s", env.LogCatcher.logsString())
+	}
+}
+
+// testBinaries are the paths to a tailscaled and tailscale binary.
+// These can be shared by multiple nodes.
+type testBinaries struct {
+	dir    string // temp dir for tailscale & tailscaled
+	daemon string // tailscaled
+	cli    string // tailscale
+}
+
+// buildTestBinaries builds tailscale and tailscaled, failing the test
+// if they fail to compile.
+func buildTestBinaries(t testing.TB) *testBinaries {
+	td := t.TempDir()
+	return &testBinaries{
+		dir:    td,
+		daemon: build(t, td, "tailscale.com/cmd/tailscaled"),
+		cli:    build(t, td, "tailscale.com/cmd/tailscale"),
+	}
+}
+
+// testEnv contains the test environment (set of servers) used by one
+// or more nodes.
+type testEnv struct {
+	Binaries *testBinaries
+
+	LogCatcher       *logCatcher
+	LogCatcherServer *httptest.Server
+
+	Control       *testcontrol.Server
+	ControlServer *httptest.Server
+
+	// CatchBadTrafficServer is an HTTP server that panics the process
+	// if it receives any traffic. We point the HTTP_PROXY to this,
+	// so any accidental traffic leaving tailscaled goes here and fails
+	// the test. (localhost traffic bypasses HTTP_PROXY)
+	CatchBadTrafficServer *httptest.Server
+
+	derpShutdown func()
+}
+
+// newTestEnv starts a bunch of services and returns a new test
+// environment.
+//
+// Call Close to shut everything down.
+func newTestEnv(t testing.TB, bins *testBinaries) *testEnv {
+
+	derpMap, derpShutdown := runDERPAndStun(t, t.Logf)
+
+	logc := new(logCatcher)
+	control := &testcontrol.Server{
+		DERPMap: derpMap,
+	}
+	e := &testEnv{
+		Binaries:         bins,
+		LogCatcher:       logc,
+		LogCatcherServer: httptest.NewServer(logc),
+		Control:          control,
+		ControlServer:    httptest.NewServer(control),
+
+		derpShutdown: derpShutdown,
+	}
+	e.CatchBadTrafficServer = httptest.NewServer(http.HandlerFunc(e.catchUnexpectedTraffic))
+	return e
+}
+
+func (e *testEnv) Close() error {
+	e.LogCatcherServer.Close()
+	e.CatchBadTrafficServer.Close()
+	e.ControlServer.Close()
+	e.derpShutdown()
+	return nil
+}
+
+// testNode is a machine with a tailscale & tailscaled.
+// Currently, the test is simplistic and user==node==machine.
+// That may grow complexity later to test more.
+type testNode struct {
+	env *testEnv
+
+	dir       string // temp dir for sock & state
+	sockFile  string
+	stateFile string
+}
+
+// newTestNode allocates a temp directory for a new test node.
+// The node is not started automatically.
+func newTestNode(t *testing.T, env *testEnv) *testNode {
+	dir := t.TempDir()
+	return &testNode{
+		env:       env,
+		dir:       dir,
+		sockFile:  filepath.Join(dir, "tailscale.sock"),
+		stateFile: filepath.Join(dir, "tailscale.state"),
+	}
+}
+
+// StartDaemon starts the node's tailscaled, failing if it fails to
+// start.
+func (n *testNode) StartDaemon(t testing.TB) *exec.Cmd {
+	cmd := exec.Command(n.env.Binaries.daemon,
+		"--tun=userspace-networking",
+		"--state="+n.stateFile,
+		"--socket="+n.sockFile,
+	)
+	cmd.Env = append(os.Environ(),
+		"TS_LOG_TARGET="+n.env.LogCatcherServer.URL,
+		"HTTP_PROXY="+n.env.CatchBadTrafficServer.URL,
+		"HTTPS_PROXY="+n.env.CatchBadTrafficServer.URL,
+	)
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("starting tailscaled: %v", err)
+	}
+	return cmd
+}
+
+// AwaitListening waits for the tailscaled to be serving local clients
+// over its localhost IPC mechanism. (Unix socket, etc)
+func (n *testNode) AwaitListening(t testing.TB) {
+	if err := tstest.WaitFor(20*time.Second, func() (err error) {
+		c, err := safesocket.Connect(n.sockFile, 41112)
+		if err != nil {
+			return err
+		}
+		c.Close()
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// Tailscale returns a command that runs the tailscale CLI with the provided arguments.
+// It does not start the process.
+func (n *testNode) Tailscale(arg ...string) *exec.Cmd {
+	cmd := exec.Command(n.env.Binaries.cli, "--socket="+n.sockFile)
+	cmd.Args = append(cmd.Args, arg...)
+	cmd.Dir = n.dir
+	return cmd
 }
 
 func exe() string {
@@ -112,7 +274,7 @@ func exe() string {
 	return ""
 }
 
-func findGo(t *testing.T) string {
+func findGo(t testing.TB) string {
 	goBin := filepath.Join(runtime.GOROOT(), "bin", "go"+exe())
 	if fi, err := os.Stat(goBin); err != nil {
 		if os.IsNotExist(err) {
@@ -126,7 +288,7 @@ func findGo(t *testing.T) string {
 	return goBin
 }
 
-func build(t *testing.T, outDir, target string) string {
+func build(t testing.TB, outDir, target string) string {
 	exe := ""
 	if runtime.GOOS == "windows" {
 		exe = ".exe"
@@ -139,6 +301,7 @@ func build(t *testing.T, outDir, target string) string {
 	return bin
 }
 
+// logCatcher is a minimal logcatcher for the logtail upload client.
 type logCatcher struct {
 	mu     sync.Mutex
 	buf    bytes.Buffer
@@ -212,3 +375,59 @@ func (lc *logCatcher) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 	w.WriteHeader(200) // must have no content, but not a 204
 }
+
+// catchUnexpectedTraffic is an HTTP proxy handler to note whether any
+// HTTP traffic tries to leave localhost from tailscaled. We don't
+// expect any, so any request triggers a failure.
+func (e *testEnv) catchUnexpectedTraffic(w http.ResponseWriter, r *http.Request) {
+	var got bytes.Buffer
+	r.Write(&got)
+	err := fmt.Errorf("unexpected HTTP proxy via proxy: %s", got.Bytes())
+	mainError.Store(err)
+	log.Printf("Error: %v", err)
+}
+
+func runDERPAndStun(t testing.TB, logf logger.Logf) (derpMap *tailcfg.DERPMap, cleanup func()) {
+	var serverPrivateKey key.Private
+	if _, err := crand.Read(serverPrivateKey[:]); err != nil {
+		t.Fatal(err)
+	}
+	d := derp.NewServer(serverPrivateKey, logf)
+
+	httpsrv := httptest.NewUnstartedServer(derphttp.Handler(d))
+	httpsrv.Config.ErrorLog = logger.StdLogger(logf)
+	httpsrv.Config.TLSNextProto = make(map[string]func(*http.Server, *tls.Conn, http.Handler))
+	httpsrv.StartTLS()
+
+	stunAddr, stunCleanup := stuntest.ServeWithPacketListener(t, nettype.Std{})
+
+	m := &tailcfg.DERPMap{
+		Regions: map[int]*tailcfg.DERPRegion{
+			1: {
+				RegionID:   1,
+				RegionCode: "test",
+				Nodes: []*tailcfg.DERPNode{
+					{
+						Name:         "t1",
+						RegionID:     1,
+						HostName:     "127.0.0.1", // to bypass HTTP proxy
+						IPv4:         "127.0.0.1",
+						IPv6:         "none",
+						STUNPort:     stunAddr.Port,
+						DERPTestPort: httpsrv.Listener.Addr().(*net.TCPAddr).Port,
+						STUNTestIP:   stunAddr.IP.String(),
+					},
+				},
+			},
+		},
+	}
+
+	cleanup = func() {
+		httpsrv.CloseClientConnections()
+		httpsrv.Close()
+		d.Close()
+		stunCleanup()
+	}
+
+	return m, cleanup
+}

tstest/integration/testcontrol/testcontrol.go

@@ -0,0 +1,545 @@
+// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package testcontrol contains a minimal control plane server for testing purposes.
+package testcontrol
+
+import (
+	"bytes"
+	crand "crypto/rand"
+	"encoding/binary"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+	"io/ioutil"
+	"log"
+	"math/rand"
+	"net/http"
+	"strings"
+	"sync"
+	"time"
+
+	"github.com/klauspost/compress/zstd"
+	"golang.org/x/crypto/nacl/box"
+	"inet.af/netaddr"
+	"tailscale.com/derp/derpmap"
+	"tailscale.com/smallzstd"
+	"tailscale.com/tailcfg"
+	"tailscale.com/types/logger"
+	"tailscale.com/types/wgkey"
+)
+
+// Server is a control plane server. Its zero value is ready for use.
+// Everything is stored in-memory in one tailnet.
+type Server struct {
+	Logf    logger.Logf      // nil means to use the log package
+	DERPMap *tailcfg.DERPMap // nil means to use prod DERP map
+
+	initMuxOnce sync.Once
+	mux         *http.ServeMux
+
+	mu      sync.Mutex
+	pubKey  wgkey.Key
+	privKey wgkey.Private
+	nodes   map[tailcfg.NodeKey]*tailcfg.Node
+	users   map[tailcfg.NodeKey]*tailcfg.User
+	logins  map[tailcfg.NodeKey]*tailcfg.Login
+	updates map[tailcfg.NodeID]chan updateType
+}
+
+func (s *Server) logf(format string, a ...interface{}) {
+	if s.Logf != nil {
+		s.Logf(format, a...)
+	} else {
+		log.Printf(format, a...)
+	}
+}
+
+func (s *Server) initMux() {
+	s.mux = http.NewServeMux()
+	s.mux.HandleFunc("/", s.serveUnhandled)
+	s.mux.HandleFunc("/key", s.serveKey)
+	s.mux.HandleFunc("/machine/", s.serveMachine)
+}
+
+func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	s.initMuxOnce.Do(s.initMux)
+	s.mux.ServeHTTP(w, r)
+}
+
+func (s *Server) serveUnhandled(w http.ResponseWriter, r *http.Request) {
+	var got bytes.Buffer
+	r.Write(&got)
+	go panic(fmt.Sprintf("testcontrol.Server received unhandled request: %s", got.Bytes()))
+}
+
+func (s *Server) publicKey() wgkey.Key {
+	pub, _ := s.keyPair()
+	return pub
+}
+
+func (s *Server) privateKey() wgkey.Private {
+	_, priv := s.keyPair()
+	return priv
+}
+
+func (s *Server) keyPair() (pub wgkey.Key, priv wgkey.Private) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.pubKey.IsZero() {
+		var err error
+		s.privKey, err = wgkey.NewPrivate()
+		if err != nil {
+			go panic(err) // bring down test, even if in http.Handler
+		}
+		s.pubKey = s.privKey.Public()
+	}
+	return s.pubKey, s.privKey
+}
+
+func (s *Server) serveKey(w http.ResponseWriter, r *http.Request) {
+	w.Header().Set("Content-Type", "text/plain")
+	w.WriteHeader(200)
+	io.WriteString(w, s.publicKey().HexString())
+}
+
+func (s *Server) serveMachine(w http.ResponseWriter, r *http.Request) {
+	mkeyStr := strings.TrimPrefix(r.URL.Path, "/machine/")
+	rem := ""
+	if i := strings.IndexByte(mkeyStr, '/'); i != -1 {
+		rem = mkeyStr[i:]
+		mkeyStr = mkeyStr[:i]
+	}
+
+	key, err := wgkey.ParseHex(mkeyStr)
+	if err != nil {
+		http.Error(w, "bad machine key hex", 400)
+		return
+	}
+	mkey := tailcfg.MachineKey(key)
+
+	if r.Method != "POST" {
+		http.Error(w, "POST required", 400)
+		return
+	}
+
+	switch rem {
+	case "":
+		s.serveRegister(w, r, mkey)
+	case "/map":
+		s.serveMap(w, r, mkey)
+	default:
+		s.serveUnhandled(w, r)
+	}
+}
+
+// Node returns the node for nodeKey. It's always nil or cloned memory.
+func (s *Server) Node(nodeKey tailcfg.NodeKey) *tailcfg.Node {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.nodes[nodeKey].Clone()
+}
+
+func (s *Server) getUser(nodeKey tailcfg.NodeKey) (*tailcfg.User, *tailcfg.Login) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if s.users == nil {
+		s.users = map[tailcfg.NodeKey]*tailcfg.User{}
+	}
+	if s.logins == nil {
+		s.logins = map[tailcfg.NodeKey]*tailcfg.Login{}
+	}
+	if u, ok := s.users[nodeKey]; ok {
+		return u, s.logins[nodeKey]
+	}
+	id := tailcfg.UserID(len(s.users) + 1)
+	domain := "fake-control.example.net"
+	loginName := fmt.Sprintf("user-%d@%s", id, domain)
+	displayName := fmt.Sprintf("User %d", id)
+	login := &tailcfg.Login{
+		ID:            tailcfg.LoginID(id),
+		Provider:      "testcontrol",
+		LoginName:     loginName,
+		DisplayName:   displayName,
+		ProfilePicURL: "https://tailscale.com/static/images/marketing/team-carney.jpg",
+		Domain:        domain,
+	}
+	user := &tailcfg.User{
+		ID:          id,
+		LoginName:   loginName,
+		DisplayName: displayName,
+		Domain:      domain,
+		Logins:      []tailcfg.LoginID{login.ID},
+	}
+	s.users[nodeKey] = user
+	s.logins[nodeKey] = login
+	return user, login
+}
+
+func (s *Server) serveRegister(w http.ResponseWriter, r *http.Request, mkey tailcfg.MachineKey) {
+	var req tailcfg.RegisterRequest
+	if err := s.decode(mkey, r.Body, &req); err != nil {
+		panic(fmt.Sprintf("serveRegister: decode: %v", err))
+	}
+	if req.Version != 1 {
+		panic(fmt.Sprintf("serveRegister: unsupported version: %d", req.Version))
+	}
+	if req.NodeKey.IsZero() {
+		panic("serveRegister: request has zero node key")
+	}
+
+	user, login := s.getUser(req.NodeKey)
+	s.mu.Lock()
+	if s.nodes == nil {
+		s.nodes = map[tailcfg.NodeKey]*tailcfg.Node{}
+	}
+	s.nodes[req.NodeKey] = &tailcfg.Node{
+		ID:                tailcfg.NodeID(user.ID),
+		StableID:          tailcfg.StableNodeID(fmt.Sprintf("TESTCTRL%08x", int(user.ID))),
+		User:              user.ID,
+		Machine:           mkey,
+		Key:               req.NodeKey,
+		MachineAuthorized: true,
+	}
+	s.mu.Unlock()
+
+	res, err := s.encode(mkey, false, tailcfg.RegisterResponse{
+		User:              *user,
+		Login:             *login,
+		NodeKeyExpired:    false,
+		MachineAuthorized: true,
+		AuthURL:           "", // all good; TODO(bradfitz): add ways to not start all good.
+	})
+	if err != nil {
+		go panic(fmt.Sprintf("serveRegister: encode: %v", err))
+	}
+	w.WriteHeader(200)
+	w.Write(res)
+}
+
+// updateType indicates why a long-polling map request is being woken
+// up for an update.
+type updateType int
+
+const (
+	// updatePeerChanged is an update that a peer has changed.
+	updatePeerChanged updateType = iota + 1
+
+	// updateSelfChanged is an update that the node changed itself
+	// via a lite endpoint update. These ones are never dup-suppressed,
+	// as the client is expecting an answer regardless.
+	updateSelfChanged
+)
+
+func (s *Server) updateLocked(source string, peers []tailcfg.NodeID) {
+	for _, peer := range peers {
+		sendUpdate(s.updates[peer], updatePeerChanged)
+	}
+}
+
+// sendUpdate sends updateType to dst if dst is non-nil and
+// has capacity.
+func sendUpdate(dst chan<- updateType, updateType updateType) {
+	if dst == nil {
+		return
+	}
+	// The dst channel has a buffer size of 1.
+	// If we fail to insert an update into the buffer that
+	// means there is already an update pending.
+	select {
+	case dst <- updateType:
+	default:
+	}
+}
+
+func (s *Server) serveMap(w http.ResponseWriter, r *http.Request, mkey tailcfg.MachineKey) {
+	ctx := r.Context()
+
+	req := new(tailcfg.MapRequest)
+	if err := s.decode(mkey, r.Body, req); err != nil {
+		go panic(fmt.Sprintf("bad map request: %v", err))
+	}
+
+	jitter := time.Duration(rand.Intn(8000)) * time.Millisecond
+	keepAlive := 50*time.Second + jitter
+
+	node := s.Node(req.NodeKey)
+	if node == nil {
+		http.Error(w, "node not found", 400)
+		return
+	}
+	if node.Machine != mkey {
+		http.Error(w, "node doesn't match machine key", 400)
+		return
+	}
+
+	var peersToUpdate []tailcfg.NodeID
+	if !req.ReadOnly {
+		endpoints := filterInvalidIPv6Endpoints(req.Endpoints)
+		node.Endpoints = endpoints
+		// TODO: more
+		// TODO: register node,
+		//s.UpdateEndpoint(mkey, req.NodeKey,
+		// XXX
+	}
+
+	nodeID := node.ID
+
+	s.mu.Lock()
+	updatesCh := make(chan updateType, 1)
+	oldUpdatesCh := s.updates[nodeID]
+	if breakSameNodeMapResponseStreams(req) {
+		if oldUpdatesCh != nil {
+			close(oldUpdatesCh)
+		}
+		if s.updates == nil {
+			s.updates = map[tailcfg.NodeID]chan updateType{}
+		}
+		s.updates[nodeID] = updatesCh
+	} else {
+		sendUpdate(oldUpdatesCh, updateSelfChanged)
+	}
+	s.updateLocked("serveMap", peersToUpdate)
+	s.mu.Unlock()
+
+	// ReadOnly implies no streaming, as it doesn't
+	// register an updatesCh to get updates.
+	streaming := req.Stream && !req.ReadOnly
+	compress := req.Compress != ""
+
+	w.WriteHeader(200)
+	for {
+		res, err := s.MapResponse(req)
+		if err != nil {
+			// TODO: log
+			return
+		}
+		if res == nil {
+			return // done
+		}
+		// TODO: add minner if/when needed
+		resBytes, err := json.Marshal(res)
+		if err != nil {
+			s.logf("json.Marshal: %v", err)
+			return
+		}
+		if err := s.sendMapMsg(w, mkey, compress, resBytes); err != nil {
+			return
+		}
+		if !streaming {
+			return
+		}
+	keepAliveLoop:
+		for {
+			var keepAliveTimer *time.Timer
+			var keepAliveTimerCh <-chan time.Time
+			if keepAlive > 0 {
+				keepAliveTimer = time.NewTimer(keepAlive)
+				keepAliveTimerCh = keepAliveTimer.C
+			}
+			select {
+			case <-ctx.Done():
+				if keepAliveTimer != nil {
+					keepAliveTimer.Stop()
+				}
+				return
+			case _, ok := <-updatesCh:
+				if !ok {
+					// replaced by new poll request
+					return
+				}
+				break keepAliveLoop
+			case <-keepAliveTimerCh:
+				if err := s.sendMapMsg(w, mkey, compress, keepAliveMsg); err != nil {
+					return
+				}
+			}
+		}
+	}
+}
+
+var keepAliveMsg = &struct {
+	KeepAlive bool
+}{
+	KeepAlive: true,
+}
+
+var prodDERPMap = derpmap.Prod()
+
+// MapResponse generates a MapResponse for a MapRequest.
+//
+// No updates to s are done here.
+func (s *Server) MapResponse(req *tailcfg.MapRequest) (res *tailcfg.MapResponse, err error) {
+	node := s.Node(req.NodeKey)
+	if node == nil {
+		// node key rotated away (once test server supports that)
+		return nil, nil
+	}
+	derpMap := s.DERPMap
+	if derpMap == nil {
+		derpMap = prodDERPMap
+	}
+	user, _ := s.getUser(req.NodeKey)
+	res = &tailcfg.MapResponse{
+		Node:            node,
+		DERPMap:         derpMap,
+		Domain:          string(user.Domain),
+		CollectServices: "true",
+		PacketFilter:    tailcfg.FilterAllowAll,
+	}
+	res.Node.Addresses = []netaddr.IPPrefix{
+		netaddr.MustParseIPPrefix(fmt.Sprintf("100.64.%d.%d/32", uint8(node.ID>>8), uint8(node.ID))),
+	}
+	res.Node.AllowedIPs = res.Node.Addresses
+	return res, nil
+}
+
+func (s *Server) sendMapMsg(w http.ResponseWriter, mkey tailcfg.MachineKey, compress bool, msg interface{}) error {
+	resBytes, err := s.encode(mkey, compress, msg)
+	if err != nil {
+		return err
+	}
+	if len(resBytes) > 16<<20 {
+		return fmt.Errorf("map message too big: %d", len(resBytes))
+	}
+	var siz [4]byte
+	binary.LittleEndian.PutUint32(siz[:], uint32(len(resBytes)))
+	if _, err := w.Write(siz[:]); err != nil {
+		return err
+	}
+	if _, err := w.Write(resBytes); err != nil {
+		return err
+	}
+	if f, ok := w.(http.Flusher); ok {
+		f.Flush()
+	} else {
+		s.logf("[unexpected] ResponseWriter %T is not a Flusher", w)
+	}
+	return nil
+}
+
+func (s *Server) decode(mkey tailcfg.MachineKey, r io.Reader, v interface{}) error {
+	if c, _ := r.(io.Closer); c != nil {
+		defer c.Close()
+	}
+	const msgLimit = 1 << 20
+	msg, err := ioutil.ReadAll(io.LimitReader(r, msgLimit))
+	if err != nil {
+		return err
+	}
+	if len(msg) == msgLimit {
+		return errors.New("encrypted message too long")
+	}
+
+	var nonce [24]byte
+	if len(msg) < len(nonce)+1 {
+		return errors.New("missing nonce")
+	}
+	copy(nonce[:], msg)
+	msg = msg[len(nonce):]
+
+	priv := s.privateKey()
+	pub, pri := (*[32]byte)(&mkey), (*[32]byte)(&priv)
+	decrypted, ok := box.Open(nil, msg, &nonce, pub, pri)
+	if !ok {
+		return errors.New("can't decrypt request")
+	}
+	return json.Unmarshal(decrypted, v)
+}
+
+var zstdEncoderPool = &sync.Pool{
+	New: func() interface{} {
+		encoder, err := smallzstd.NewEncoder(nil, zstd.WithEncoderLevel(zstd.SpeedFastest))
+		if err != nil {
+			panic(err)
+		}
+		return encoder
+	},
+}
+
+func (s *Server) encode(mkey tailcfg.MachineKey, compress bool, v interface{}) (b []byte, err error) {
+	var isBytes bool
+	if b, isBytes = v.([]byte); !isBytes {
+		b, err = json.Marshal(v)
+		if err != nil {
+			return nil, err
+		}
+	}
+	if compress {
+		encoder := zstdEncoderPool.Get().(*zstd.Encoder)
+		b = encoder.EncodeAll(b, nil)
+		encoder.Close()
+		zstdEncoderPool.Put(encoder)
+	}
+	var nonce [24]byte
+	if _, err := io.ReadFull(crand.Reader, nonce[:]); err != nil {
+		panic(err)
+	}
+	priv := s.privateKey()
+	pub, pri := (*[32]byte)(&mkey), (*[32]byte)(&priv)
+	msgData := box.Seal(nonce[:], b, &nonce, pub, pri)
+	return msgData, nil
+}
+
+// filterInvalidIPv6Endpoints removes invalid IPv6 endpoints from eps,
+// modify the slice in place, returning the potentially smaller subset (aliasing
+// the original memory).
+//
+// Two types of IPv6 endpoints are considered invalid: link-local
+// addresses, and anything with a zone.
+func filterInvalidIPv6Endpoints(eps []string) []string {
+	clean := eps[:0]
+	for _, ep := range eps {
+		if keepClientEndpoint(ep) {
+			clean = append(clean, ep)
+		}
+	}
+	return clean
+}
+
+func keepClientEndpoint(ep string) bool {
+	ipp, err := netaddr.ParseIPPort(ep)
+	if err != nil {
+		// Shouldn't have made it this far if we unmarshalled
+		// the incoming JSON response.
+		return false
+	}
+	ip := ipp.IP
+	if ip.Zone() != "" {
+		return false
+	}
+	if ip.Is6() && ip.IsLinkLocalUnicast() {
+		// We let clients send these for now, but
+		// tailscaled doesn't know how to use them yet
+		// so we filter them out for now. A future
+		// MapRequest.Version might signal that
+		// clients know how to use them (e.g. try all
+		// local scopes).
+		return false
+	}
+	return true
+}
+
+// breakSameNodeMapResponseStreams reports whether req should break a
+// prior long-polling MapResponse stream (if active) from the same
+// node ID.
+func breakSameNodeMapResponseStreams(req *tailcfg.MapRequest) bool {
+	if req.ReadOnly {
+		// Don't register our updatesCh for closability
+		// nor close another peer's if we're a read-only request.
+		return false
+	}
+	if !req.Stream && req.OmitPeers {
+		// Likewise, if we're not streaming and not asking for peers,
+		// (but still mutable, without Readonly set), consider this an endpoint
+		// update request only, and don't close any existing map response
+		// for this nodeID. It's likely the same client with a built-up
+		// compression context. We want to let them update their
+		// new endpoints with us without breaking that other long-running
+		// map response.
+		return false
+	}
+	return true
+}

types/logger/logger.go

@@ -18,8 +18,6 @@ import (
 	"strings"
 	"sync"
 	"time"
-
-	"golang.org/x/time/rate"
 )
 
 // Logf is the basic Tailscale logger type: a printf-like func.
@@ -57,9 +55,9 @@ func Discard(string, ...interface{}) {}
 // limitData is used to keep track of each format string's associated
 // rate-limiting data.
 type limitData struct {
-	lim        *rate.Limiter // the token bucket associated with this string
-	msgBlocked bool          // whether a "duplicate error" message has already been logged
-	ele        *list.Element // list element used to access this string in the cache
+	bucket   *tokenBucket  // the token bucket associated with this string
+	nBlocked int           // number of messages skipped
+	ele      *list.Element // list element used to access this string in the cache
 }
 
 var disableRateLimit = os.Getenv("TS_DEBUG_LOG_RATE") == "all"
@@ -71,31 +69,34 @@ var rateFree = []string{
 	"magicsock: CreateEndpoint:",
 }
 
-// RateLimitedFn returns a rate-limiting Logf wrapping the given logf.
-// Messages are allowed through at a maximum of one message every f (where f is a time.Duration), in
-// bursts of up to burst messages at a time. Up to maxCache strings will be held at a time.
+// RateLimitedFn is a wrapper for RateLimitedFnWithClock that includes the
+// current time automatically. This is mainly for backward compatibility.
 func RateLimitedFn(logf Logf, f time.Duration, burst int, maxCache int) Logf {
+	return RateLimitedFnWithClock(logf, f, burst, maxCache, time.Now)
+}
+
+// RateLimitedFnWithClock returns a rate-limiting Logf wrapping the given
+// logf. Messages are allowed through at a maximum of one message every f
+// (where f is a time.Duration), in bursts of up to burst messages at a
+// time. Up to maxCache format strings will be tracked separately.
+// timeNow is a function that returns the current time, used for calculating
+// rate limits.
+func RateLimitedFnWithClock(logf Logf, f time.Duration, burst int, maxCache int, timeNow func() time.Time) Logf {
 	if disableRateLimit {
 		return logf
 	}
-	r := rate.Every(f)
 	var (
 		mu       sync.Mutex
 		msgLim   = make(map[string]*limitData) // keyed by logf format
 		msgCache = list.New()                  // a rudimentary LRU that limits the size of the map
 	)
 
-	type verdict int
-	const (
-		allow verdict = iota
-		warn
-		block
-	)
-
-	judge := func(format string) verdict {
+	return func(format string, args ...interface{}) {
+		// Shortcut for formats with no rate limit
 		for _, sub := range rateFree {
 			if strings.Contains(format, sub) {
-				return allow
+				logf(format, args...)
+				return
 			}
 		}
 
@@ -106,8 +107,8 @@ func RateLimitedFn(logf Logf, f time.Duration, burst int, maxCache int) Logf {
 			msgCache.MoveToFront(rl.ele)
 		} else {
 			rl = &limitData{
-				lim: rate.NewLimiter(r, burst),
-				ele: msgCache.PushFront(format),
+				bucket: newTokenBucket(f, burst, timeNow()),
+				ele:    msgCache.PushFront(format),
 			}
 			msgLim[format] = rl
 			if msgCache.Len() > maxCache {
@@ -115,24 +116,39 @@ func RateLimitedFn(logf Logf, f time.Duration, burst int, maxCache int) Logf {
 				msgCache.Remove(msgCache.Back())
 			}
 		}
-		if rl.lim.Allow() {
-			rl.msgBlocked = false
-			return allow
-		}
-		if !rl.msgBlocked {
-			rl.msgBlocked = true
-			return warn
-		}
-		return block
-	}
 
-	return func(format string, args ...interface{}) {
-		switch judge(format) {
-		case allow:
+		rl.bucket.AdvanceTo(timeNow())
+
+		// Make sure there's enough room for at least a few
+		// more logs before we unblock, so we don't alternate
+		// between blocking and unblocking.
+		if rl.nBlocked > 0 && rl.bucket.remaining >= 2 {
+			// Only print this if we dropped more than 1
+			// message. Otherwise we'd *increase* the total
+			// number of log lines printed.
+			if rl.nBlocked > 1 {
+				logf("[RATELIMIT] format(%q) (%d dropped)",
+					format, rl.nBlocked-1)
+			}
+			rl.nBlocked = 0
+		}
+		if rl.nBlocked == 0 && rl.bucket.Get() {
 			logf(format, args...)
-		case warn:
-			// For the warning, log the specific format string
-			logf("[RATE LIMITED] format string \"%s\" (example: \"%s\")", format, strings.TrimSpace(fmt.Sprintf(format, args...)))
+			if rl.bucket.remaining == 0 {
+				// Enter "blocked" mode immediately after
+				// reaching the burst limit. We want to
+				// always accompany the format() message
+				// with an example of the format, which is
+				// effectively the same as printing the
+				// message anyway. But this way they can
+				// be on two separate lines and we don't
+				// corrupt the original message.
+				logf("[RATELIMIT] format(%q)", format)
+				rl.nBlocked = 1
+			}
+			return
+		} else {
+			rl.nBlocked++
 		}
 	}
 }

types/logger/logger_test.go

@@ -44,16 +44,27 @@ func TestRateLimiter(t *testing.T) {
 		"boring string with constant formatting (constant)",
 		"templated format string no. 0",
 		"boring string with constant formatting (constant)",
+		"[RATELIMIT] format(\"boring string with constant formatting %s\")",
 		"templated format string no. 1",
-		"[RATE LIMITED] format string \"boring string with constant formatting %s\" (example: \"boring string with constant formatting (constant)\")",
-		"[RATE LIMITED] format string \"templated format string no. %d\" (example: \"templated format string no. 2\")",
+		"[RATELIMIT] format(\"templated format string no. %d\")",
 		"Make sure this string makes it through the rest (that are blocked) 4",
 		"4 shouldn't get filtered.",
+		"hello 1",
+		"hello 2",
+		"[RATELIMIT] format(\"hello %v\")",
+		"[RATELIMIT] format(\"hello %v\") (2 dropped)",
+		"hello 5",
+		"hello 6",
+		"[RATELIMIT] format(\"hello %v\")",
+		"hello 7",
 	}
 
+	var now time.Time
+	nowf := func() time.Time { return now }
+
 	testsRun := 0
 	lgtest := logTester(want, t, &testsRun)
-	lg := RateLimitedFn(lgtest, 1*time.Minute, 2, 50)
+	lg := RateLimitedFnWithClock(lgtest, 1*time.Minute, 2, 50, nowf)
 	var prefixed Logf
 	for i := 0; i < 10; i++ {
 		lg("boring string with constant formatting %s", "(constant)")
@@ -64,6 +75,19 @@ func TestRateLimiter(t *testing.T) {
 			prefixed(" shouldn't get filtered.")
 		}
 	}
+
+	lg("hello %v", 1)
+	lg("hello %v", 2) // printed, but rate limit starts
+	lg("hello %v", 3) // rate limited (not printed)
+	now = now.Add(1 * time.Minute)
+	lg("hello %v", 4) // still limited (not printed)
+	now = now.Add(1 * time.Minute)
+	lg("hello %v", 5) // restriction lifted; prints drop count + message
+
+	lg("hello %v", 6) // printed, but rate limit starts
+	now = now.Add(2 * time.Minute)
+	lg("hello %v", 7) // restriction lifted; no drop count needed
+
 	if testsRun < len(want) {
 		t.Fatalf("Tests after %s weren't logged.", want[testsRun])
 	}

types/logger/tokenbucket.go

@@ -0,0 +1,63 @@
+// Copyright (c) 2021 Tailscale Inc & AUTHORS All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package logger
+
+import (
+	"time"
+)
+
+// tokenBucket is a simple token bucket style rate limiter.
+
+// It's similar in function to golang.org/x/time/rate.Limiter, which we
+// can't use because:
+// - It doesn't give access to the number of accumulated tokens, which we
+//   need for implementing hysteresis;
+// - It doesn't let us provide our own time function, which we need for
+//   implementing proper unit tests.
+// rate.Limiter is also much more complex than necessary, but that wouldn't
+// be enough to disqualify it on its own.
+//
+// Unlike rate.Limiter, this token bucket does not attempt to
+// do any locking of its own. Don't try to access it re-entrantly.
+// That's fine inside this types/logger package because we already have
+// locking at a higher level.
+type tokenBucket struct {
+	remaining int
+	max       int
+	tick      time.Duration
+	t         time.Time
+}
+
+func newTokenBucket(tick time.Duration, max int, now time.Time) *tokenBucket {
+	return &tokenBucket{max, max, tick, now}
+}
+
+func (tb *tokenBucket) Get() bool {
+	if tb.remaining > 0 {
+		tb.remaining--
+		return true
+	}
+	return false
+}
+
+func (tb *tokenBucket) Refund(n int) {
+	b := tb.remaining + n
+	if b > tb.max {
+		tb.remaining = tb.max
+	} else {
+		tb.remaining = b
+	}
+}
+
+func (tb *tokenBucket) AdvanceTo(t time.Time) {
+	diff := t.Sub(tb.t)
+
+	// only use up whole ticks. The remainder will be used up
+	// next time.
+	ticks := int(diff / tb.tick)
+	tb.t = tb.t.Add(time.Duration(ticks) * tb.tick)
+
+	tb.Refund(ticks)
+}

wgengine/magicsock/magicsock_test.go

@@ -10,7 +10,6 @@ import (
 	crand "crypto/rand"
 	"crypto/tls"
 	"encoding/binary"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io/ioutil"
@@ -26,7 +25,6 @@ import (
 	"time"
 	"unsafe"
 
-	"github.com/google/go-cmp/cmp"
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun/tuntest"
 	"golang.org/x/crypto/nacl/box"
@@ -996,148 +994,6 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 		ping1(t)
 		ping2(t)
 	})
-
-	// TODO: Remove this once the following tests are reliable.
-	if run, _ := strconv.ParseBool(os.Getenv("RUN_CURSED_TESTS")); !run {
-		t.Skip("skipping following tests because RUN_CURSED_TESTS is not set.")
-	}
-
-	pingSeq := func(t *testing.T, count int, totalTime time.Duration, strict bool) {
-		msg := func(i int) []byte {
-			b := tuntest.Ping(net.ParseIP("1.0.0.2"), net.ParseIP("1.0.0.1"))
-			b[len(b)-1] = byte(i) // set seq num
-			return b
-		}
-
-		// Space out ping transmissions so that the overall
-		// transmission happens in totalTime.
-		//
-		// We do this because the packet spray logic in magicsock is
-		// time-based to allow for reliable NAT traversal. However,
-		// for the packet spraying test further down, there needs to
-		// be at least 1 sprayed packet that is not the handshake, in
-		// case the handshake gets eaten by the race resolution logic.
-		//
-		// This is an inherent "race by design" in our current
-		// magicsock+wireguard-go codebase: sometimes, racing
-		// handshakes will result in a sub-optimal path for a few
-		// hundred milliseconds, until a subsequent spray corrects the
-		// issue. In order for the test to reflect that magicsock
-		// works as designed, we have to space out packet transmission
-		// here.
-		interPacketGap := totalTime / time.Duration(count)
-		if interPacketGap < 1*time.Millisecond {
-			interPacketGap = 0
-		}
-
-		for i := 0; i < count; i++ {
-			b := msg(i)
-			m1.tun.Outbound <- b
-			time.Sleep(interPacketGap)
-		}
-
-		for i := 0; i < count; i++ {
-			b := msg(i)
-			select {
-			case msgRecv := <-m2.tun.Inbound:
-				if !bytes.Equal(b, msgRecv) {
-					if strict {
-						t.Errorf("return ping %d did not transit correctly: %s", i, cmp.Diff(b, msgRecv))
-					}
-				}
-			case <-time.After(pingTimeout):
-				if strict {
-					t.Errorf("return ping %d did not transit", i)
-				}
-			}
-		}
-	}
-
-	t.Run("ping 1.0.0.1 x50", func(t *testing.T) {
-		setT(t)
-		defer setT(outerT)
-		pingSeq(t, 50, 0, true)
-	})
-
-	// Add DERP relay.
-	derpEp := "127.3.3.40:1"
-	ep0 := cfgs[0].Peers[0].Endpoints
-	ep0 = derpEp + "," + ep0
-	cfgs[0].Peers[0].Endpoints = ep0
-	ep1 := cfgs[1].Peers[0].Endpoints
-	ep1 = derpEp + "," + ep1
-	cfgs[1].Peers[0].Endpoints = ep1
-	if err := m1.Reconfig(&cfgs[0]); err != nil {
-		t.Fatal(err)
-	}
-	if err := m2.Reconfig(&cfgs[1]); err != nil {
-		t.Fatal(err)
-	}
-
-	t.Run("add DERP", func(t *testing.T) {
-		setT(t)
-		defer setT(outerT)
-		pingSeq(t, 20, 0, true)
-	})
-
-	// Disable real route.
-	cfgs[0].Peers[0].Endpoints = derpEp
-	cfgs[1].Peers[0].Endpoints = derpEp
-	if err := m1.Reconfig(&cfgs[0]); err != nil {
-		t.Fatal(err)
-	}
-	if err := m2.Reconfig(&cfgs[1]); err != nil {
-		t.Fatal(err)
-	}
-	time.Sleep(250 * time.Millisecond) // TODO remove
-
-	t.Run("all traffic over DERP", func(t *testing.T) {
-		setT(t)
-		defer setT(outerT)
-		defer func() {
-			if t.Failed() || true {
-				logf("cfg0: %v", stringifyConfig(cfgs[0]))
-				logf("cfg1: %v", stringifyConfig(cfgs[1]))
-			}
-		}()
-		pingSeq(t, 20, 0, true)
-	})
-
-	m1.dev.RemoveAllPeers()
-	m2.dev.RemoveAllPeers()
-
-	// Give one peer a non-DERP endpoint. We expect the other to
-	// accept it via roamAddr.
-	cfgs[0].Peers[0].Endpoints = ep0
-	if ep2 := cfgs[1].Peers[0].Endpoints; len(ep2) != 1 {
-		t.Errorf("unexpected peer endpoints in dev2: %v", ep2)
-	}
-	if err := m2.Reconfig(&cfgs[1]); err != nil {
-		t.Fatal(err)
-	}
-	if err := m1.Reconfig(&cfgs[0]); err != nil {
-		t.Fatal(err)
-	}
-	// Dear future human debugging a test failure here: this test is
-	// flaky, and very infrequently will drop 1-2 of the 50 ping
-	// packets. This does not affect normal operation of tailscaled,
-	// but makes this test fail.
-	//
-	// TODO(danderson): finish root-causing and de-flake this test.
-	t.Run("one real route is enough thanks to spray", func(t *testing.T) {
-		setT(t)
-		defer setT(outerT)
-		pingSeq(t, 50, 700*time.Millisecond, false)
-
-		cfg, err := wgcfg.DeviceConfig(m2.dev)
-		if err != nil {
-			t.Fatal(err)
-		}
-		ep2 := cfg.Peers[0].Endpoints
-		if len(ep2) != 2 {
-			t.Error("handshake spray failed to find real route")
-		}
-	})
 }
 
 // TestAddrSet tests addrSet appendDests and updateDst.
@@ -1362,14 +1218,6 @@ func TestDiscoStringLogRace(t *testing.T) {
 	wg.Wait()
 }
 
-func stringifyConfig(cfg wgcfg.Config) string {
-	j, err := json.Marshal(cfg)
-	if err != nil {
-		panic(err)
-	}
-	return string(j)
-}
-
 func Test32bitAlignment(t *testing.T) {
 	var de discoEndpoint
 

wgengine/router/router_test.go

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
+// +build linux windows
+
 package router
 
 import "inet.af/netaddr"

wgengine/wglog/wglog.go

@@ -21,7 +21,7 @@ import (
 // It can be modified at run time to adjust to new wireguard-go configurations.
 type Logger struct {
 	DeviceLogger *device.Logger
-	replacer     atomic.Value // of *strings.Replacer
+	replace      atomic.Value // of map[string]string
 }
 
 // NewLogger creates a new logger for use with wireguard-go.
@@ -29,41 +29,48 @@ type Logger struct {
 // and rewrites peer keys from wireguard-go into Tailscale format.
 func NewLogger(logf logger.Logf) *Logger {
 	ret := new(Logger)
-
 	wrapper := func(format string, args ...interface{}) {
-		msg := fmt.Sprintf(format, args...)
-		if strings.Contains(msg, "Routine:") && !strings.Contains(msg, "receive incoming") {
+		if strings.Contains(format, "Routine:") && !strings.Contains(format, "receive incoming") {
 			// wireguard-go logs as it starts and stops routines.
 			// Drop those; there are a lot of them, and they're just noise.
 			return
 		}
-		if strings.Contains(msg, "Failed to send data packet") {
+		if strings.Contains(format, "Failed to send data packet") {
 			// Drop. See https://github.com/tailscale/tailscale/issues/1239.
 			return
 		}
-		if strings.Contains(msg, "Interface up requested") || strings.Contains(msg, "Interface down requested") {
+		if strings.Contains(format, "Interface up requested") || strings.Contains(format, "Interface down requested") {
 			// Drop. Logs 1/s constantly while the tun device is open.
 			// See https://github.com/tailscale/tailscale/issues/1388.
 			return
 		}
-		r := ret.replacer.Load()
-		if r == nil {
+		replace, _ := ret.replace.Load().(map[string]string)
+		if replace == nil {
 			// No replacements specified; log as originally planned.
 			logf(format, args...)
 			return
 		}
-		// Do the replacements.
-		new := r.(*strings.Replacer).Replace(msg)
-		if new == msg {
-			// No replacements. Log as originally planned.
-			logf(format, args...)
-			return
+		// Duplicate the args slice so that we can modify it.
+		// This is not always required, but the code required to avoid it is not worth the complexity.
+		newargs := make([]interface{}, len(args))
+		copy(newargs, args)
+		for i, arg := range newargs {
+			// We want to replace *device.Peer args with the Tailscale-formatted version of themselves.
+			// Using *device.Peer directly makes this hard to test, so we string any fmt.Stringers,
+			// and if the string ends up looking exactly like a known Peer, we replace it.
+			// This is slightly imprecise, in that we don't check the formatting verb. Oh well.
+			s, ok := arg.(fmt.Stringer)
+			if !ok {
+				continue
+			}
+			wgStr := s.String()
+			tsStr, ok := replace[wgStr]
+			if !ok {
+				continue
+			}
+			newargs[i] = tsStr
 		}
-		// We made some replacements. Log the new version.
-		// This changes the format string,
-		// which is somewhat unfortunate as it impacts rate limiting,
-		// but there's not much we can do about that.
-		logf("%s", new)
+		logf(format, newargs...)
 	}
 	ret.DeviceLogger = &device.Logger{
 		Verbosef: logger.WithPrefix(wrapper, "[v2] "),
@@ -76,14 +83,13 @@ func NewLogger(logf logger.Logf) *Logger {
 // SetPeers is safe for concurrent use.
 func (x *Logger) SetPeers(peers []wgcfg.Peer) {
 	// Construct a new peer public key log rewriter.
-	var replace []string
+	replace := make(map[string]string)
 	for _, peer := range peers {
 		old := "peer(" + wireguardGoString(peer.PublicKey) + ")"
 		new := peer.PublicKey.ShortString()
-		replace = append(replace, old, new)
+		replace[old] = new
 	}
-	r := strings.NewReplacer(replace...)
-	x.replacer.Store(r)
+	x.replace.Store(replace)
 }
 
 // wireguardGoString prints p in the same format used by wireguard-go.

wgengine/wglog/wglog_test.go

@@ -15,22 +15,27 @@ import (
 
 func TestLogger(t *testing.T) {
 	tests := []struct {
-		in   string
-		want string
-		omit bool
+		format string
+		args   []interface{}
+		want   string
+		omit   bool
 	}{
-		{"hi", "hi", false},
-		{"Routine: starting", "", true},
-		{"peer(IMTB…r7lM) says it misses you", "[IMTBr] says it misses you", false},
+		{"hi", nil, "hi", false},
+		{"Routine: starting", nil, "", true},
+		{"%v says it misses you", []interface{}{stringer("peer(IMTB…r7lM)")}, "[IMTBr] says it misses you", false},
 	}
 
-	c := make(chan string, 1)
+	type log struct {
+		format string
+		args   []interface{}
+	}
+
+	c := make(chan log, 1)
 	logf := func(format string, args ...interface{}) {
-		s := fmt.Sprintf(format, args...)
 		select {
-		case c <- s:
+		case c <- log{format, args}:
 		default:
-			t.Errorf("wrote %q, but shouldn't have", s)
+			t.Errorf("wrote %q, but shouldn't have", fmt.Sprintf(format, args...))
 		}
 	}
 
@@ -45,15 +50,23 @@ func TestLogger(t *testing.T) {
 		if tt.omit {
 			// Write a message ourselves into the channel.
 			// Then if logf also attempts to write into the channel, it'll fail.
-			c <- ""
+			c <- log{}
 		}
-		x.DeviceLogger.Errorf(tt.in)
-		got := <-c
+		x.DeviceLogger.Errorf(tt.format, tt.args...)
+		gotLog := <-c
 		if tt.omit {
 			continue
 		}
-		if got != tt.want {
-			t.Errorf("Println(%q) = %q want %q", tt.in, got, tt.want)
+		if got := fmt.Sprintf(gotLog.format, gotLog.args...); got != tt.want {
+			t.Errorf("Printf(%q, %v) = %q want %q", tt.format, tt.args, got, tt.want)
 		}
 	}
 }
+
+func stringer(s string) stringerString {
+	return stringerString(s)
+}
+
+type stringerString string
+
+func (s stringerString) String() string { return string(s) }