VERSION.txt: this is 1.2.10.

Fixes #810

(cherry picked from commit 08f94b3b50)

.gitignore

@@ -1,5 +1,6 @@
 # Binaries for programs and plugins
 *~
+*.tmp
 *.exe
 *.dll
 *.so

VERSION.txt

@@ -1 +1 @@
-1.1.0 f81233524fddeec450940af8dc1a0dd8841bf28c
+1.2.10

build_dist.sh

@@ -9,12 +9,8 @@
 # this script, or executing equivalent commands in your
 # distro-specific build system.
 
-set -euo pipefail
+set -eu
 
-describe=$(./version/describe.sh)
-commit=$(git rev-parse --verify --quiet HEAD)
+eval $(./version/version.sh)
 
-long=$(./version/mkversion.sh long "$describe" "")
-short=$(./version/mkversion.sh short "$describe" "")
-
-exec go build -tags xversion -ldflags "-X tailscale.com/version.Long=${long} -X tailscale.com/version.Short=${short} -X tailscale.com/version.GitCommit=${commit}" "$@"
+exec go build -tags xversion -ldflags "-X tailscale.com/version.Long=${VERSION_LONG} -X tailscale.com/version.Short=${VERSION_SHORT} -X tailscale.com/version.GitCommit=${VERSION_GIT_HASH}" "$@"

cmd/tailscale/cli/status.go

@@ -169,6 +169,9 @@ func runStatus(ctx context.Context, args []string) error {
 	}
 	for _, peer := range st.Peers() {
 		ps := st.Peer[peer]
+		if ps.ShareeNode {
+			continue
+		}
 		active := peerActive(ps)
 		if statusArgs.active && !active {
 			continue

cmd/tailscale/cli/up.go

@@ -182,18 +182,11 @@ func runUp(ctx context.Context, args []string) error {
 	var tags []string
 	if upArgs.advertiseTags != "" {
 		tags = strings.Split(upArgs.advertiseTags, ",")
-		for i, tag := range tags {
-			if strings.HasPrefix(tag, "tag:") {
-				// Accept fully-qualified tags (starting with
-				// "tag:"), as we do in the ACL file.
-				err := tailcfg.CheckTag(tag)
-				if err != nil {
-					fatalf("tag: %q: %v", tag, err)
-				}
-			} else if err := tailcfg.CheckTagSuffix(tag); err != nil {
-				fatalf("tag: %q: %v", tag, err)
+		for _, tag := range tags {
+			err := tailcfg.CheckTag(tag)
+			if err != nil {
+				fatalf("tag: %q: %s", tag, err)
 			}
-			tags[i] = "tag:" + tag
 		}
 	}
 

cmd/tailscale/depaware.txt

@@ -56,7 +56,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/net/dnscache                                   from tailscale.com/cmd/tailscale/cli+
      💣 tailscale.com/net/interfaces                                 from tailscale.com/cmd/tailscale/cli+
         tailscale.com/net/netcheck                                   from tailscale.com/cmd/tailscale/cli+
-     💣 tailscale.com/net/netns                                      from tailscale.com/control/controlclient+
+        tailscale.com/net/netns                                      from tailscale.com/control/controlclient+
         tailscale.com/net/stun                                       from tailscale.com/net/netcheck+
         tailscale.com/net/tlsdial                                    from tailscale.com/control/controlclient+
         tailscale.com/net/tsaddr                                     from tailscale.com/ipn+
@@ -75,6 +75,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep
         tailscale.com/types/opt                                      from tailscale.com/control/controlclient+
         tailscale.com/types/strbuilder                               from tailscale.com/wgengine/packet
         tailscale.com/types/structs                                  from tailscale.com/control/controlclient+
+   W    tailscale.com/util/endian                                    from tailscale.com/net/netns
         tailscale.com/util/lineread                                  from tailscale.com/control/controlclient+
         tailscale.com/version                                        from tailscale.com/cmd/tailscale/cli+
         tailscale.com/version/distro                                 from tailscale.com/cmd/tailscale/cli+

cmd/tailscaled/depaware.txt

@@ -61,7 +61,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/net/dnscache                                   from tailscale.com/derp/derphttp+
      💣 tailscale.com/net/interfaces                                 from tailscale.com/ipn+
         tailscale.com/net/netcheck                                   from tailscale.com/wgengine/magicsock
-     💣 tailscale.com/net/netns                                      from tailscale.com/control/controlclient+
+        tailscale.com/net/netns                                      from tailscale.com/control/controlclient+
      💣 tailscale.com/net/netstat                                    from tailscale.com/ipn/ipnserver
         tailscale.com/net/stun                                       from tailscale.com/net/netcheck+
         tailscale.com/net/tlsdial                                    from tailscale.com/control/controlclient+
@@ -83,6 +83,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         tailscale.com/types/opt                                      from tailscale.com/control/controlclient+
         tailscale.com/types/strbuilder                               from tailscale.com/wgengine/packet
         tailscale.com/types/structs                                  from tailscale.com/control/controlclient+
+   W    tailscale.com/util/endian                                    from tailscale.com/net/netns+
         tailscale.com/util/lineread                                  from tailscale.com/control/controlclient+
         tailscale.com/util/pidowner                                  from tailscale.com/ipn/ipnserver
         tailscale.com/version                                        from tailscale.com/cmd/tailscaled+

cmd/tailscaled/tailscaled.service

@@ -6,6 +6,7 @@ After=network-pre.target
 
 [Service]
 EnvironmentFile=/etc/default/tailscaled
+ExecStartPre=/usr/sbin/tailscaled --cleanup
 ExecStart=/usr/sbin/tailscaled --state=/var/lib/tailscale/tailscaled.state --socket=/run/tailscale/tailscaled.sock --port $PORT $FLAGS
 ExecStopPost=/usr/sbin/tailscaled --cleanup
 

control/controlclient/direct.go

@@ -21,6 +21,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"os/exec"
 	"reflect"
 	"runtime"
 	"sort"
@@ -91,9 +92,14 @@ func (p *Persist) Pretty() string {
 	if !p.PrivateNodeKey.IsZero() {
 		nk = p.PrivateNodeKey.Public()
 	}
+	ss := func(k wgcfg.Key) string {
+		if k.IsZero() {
+			return ""
+		}
+		return k.ShortString()
+	}
 	return fmt.Sprintf("Persist{lm=%v, o=%v, n=%v u=%#v}",
-		mk.ShortString(), ok.ShortString(), nk.ShortString(),
-		p.LoginName)
+		ss(mk), ss(ok), ss(nk), p.LoginName)
 }
 
 // Direct is the client that connects to a tailcontrol server for a node.
@@ -539,6 +545,10 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 		Hostinfo:   hostinfo,
 		DebugFlags: c.debugFlags,
 	}
+	if hostinfo != nil && ipForwardingBroken(hostinfo.RoutableIPs) {
+		old := request.DebugFlags
+		request.DebugFlags = append(old[:len(old):len(old)], "warn-ip-forwarding-off")
+	}
 	if c.newDecompressor != nil {
 		request.Compress = "zstd"
 	}
@@ -781,6 +791,8 @@ func decode(res *http.Response, v interface{}, serverKey *wgcfg.Key, mkey *wgcfg
 
 var debugMap, _ = strconv.ParseBool(os.Getenv("TS_DEBUG_MAP"))
 
+var jsonEscapedZero = []byte(`\u0000`)
+
 func (c *Direct) decodeMsg(msg []byte, v interface{}) error {
 	c.mu.Lock()
 	serverKey := c.serverKey
@@ -809,6 +821,10 @@ func (c *Direct) decodeMsg(msg []byte, v interface{}) error {
 		json.Indent(&buf, b, "", "    ")
 		log.Printf("MapResponse: %s", buf.Bytes())
 	}
+
+	if bytes.Contains(b, jsonEscapedZero) {
+		log.Printf("[unexpected] zero byte in controlclient.Direct.decodeMsg into %T: %q", v, b)
+	}
 	if err := json.Unmarshal(b, v); err != nil {
 		return fmt.Errorf("response: %v", err)
 	}
@@ -821,6 +837,9 @@ func decodeMsg(msg []byte, v interface{}, serverKey *wgcfg.Key, mkey *wgcfg.Priv
 	if err != nil {
 		return err
 	}
+	if bytes.Contains(decrypted, jsonEscapedZero) {
+		log.Printf("[unexpected] zero byte in controlclient decodeMsg into %T: %q", v, decrypted)
+	}
 	if err := json.Unmarshal(decrypted, v); err != nil {
 		return fmt.Errorf("response: %v", err)
 	}
@@ -1051,3 +1070,34 @@ func TrimWGConfig() opt.Bool {
 	v, _ := controlTrimWGConfig.Load().(opt.Bool)
 	return v
 }
+
+// ipForwardingBroken reports whether the system's IP forwarding is disabled
+// and will definitely not work for the routes provided.
+//
+// It should not return false positives.
+func ipForwardingBroken(routes []wgcfg.CIDR) bool {
+	if len(routes) == 0 {
+		// Nothing to route, so no need to warn.
+		return false
+	}
+	if runtime.GOOS != "linux" {
+		// We only do subnet routing on Linux for now.
+		// It might work on darwin/macOS when building from source, so
+		// don't return true for other OSes. We can OS-based warnings
+		// already in the admin panel.
+		return false
+	}
+	out, err := ioutil.ReadFile("/proc/sys/net/ipv4/ip_forward")
+	if err != nil {
+		// Try another way.
+		out, err = exec.Command("sysctl", "-n", "net.ipv4.ip_forward").Output()
+	}
+	if err != nil {
+		// Oh well, we tried. This is just for debugging.
+		// We don't want false positives.
+		// TODO: maybe we want a different warning for inability to check?
+		return false
+	}
+	return strings.TrimSpace(string(out)) == "0"
+	// TODO: also check IPv6 if 'routes' contains any IPv6 routes
+}

ipn/ipnstate/ipnstate.go

@@ -64,6 +64,12 @@ type PeerStatus struct {
 	LastHandshake time.Time // with local wireguard
 	KeepAlive     bool
 
+	// ShareeNode indicates this node exists in the netmap because
+	// it's owned by a shared-to user and that node might connect
+	// to us. These nodes should be hidden by "tailscale status"
+	// etc by default.
+	ShareeNode bool `json:",omitempty"`
+
 	// InNetworkMap means that this peer was seen in our latest network map.
 	// In theory, all of InNetworkMap and InMagicSock and InEngine should all be true.
 	InNetworkMap bool
@@ -218,6 +224,9 @@ func (sb *StatusBuilder) AddPeer(peer key.Public, st *PeerStatus) {
 	if st.KeepAlive {
 		e.KeepAlive = true
 	}
+	if st.ShareeNode {
+		e.ShareeNode = true
+	}
 }
 
 type StatusUpdater interface {

ipn/local.go

@@ -221,6 +221,7 @@ func (b *LocalBackend) UpdateStatus(sb *ipnstate.StatusBuilder) {
 				KeepAlive:    p.KeepAlive,
 				Created:      p.Created,
 				LastSeen:     lastSeen,
+				ShareeNode:   p.Hostinfo.ShareeNode,
 			})
 		}
 	}
@@ -419,7 +420,9 @@ func (b *LocalBackend) Start(opts Options) error {
 	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)
+	if b.inServerMode || runtime.GOOS == "windows" {
+		b.logf("Start: serverMode=%v", b.inServerMode)
+	}
 	applyPrefsToHostinfo(hostinfo, b.prefs)
 
 	b.notify = opts.Notify
@@ -704,6 +707,7 @@ func (b *LocalBackend) popBrowserAuthNow() {
 // initMachineKeyLocked is called to initialize b.machinePrivKey.
 //
 // b.prefs must already be initialized.
+// b.stateKey should be set too, but just for nicer log messages.
 // b.mu must be held.
 func (b *LocalBackend) initMachineKeyLocked() (err error) {
 	if temporarilySetMachineKeyInPersist() {
@@ -748,7 +752,11 @@ func (b *LocalBackend) initMachineKeyLocked() (err error) {
 	// have a legacy machine key, use that. Otherwise generate a
 	// new one.
 	if !legacyMachineKey.IsZero() {
-		b.logf("using frontend-provided legacy machine key")
+		if b.stateKey == "" {
+			b.logf("using frontend-provided legacy machine key")
+		} else {
+			b.logf("using legacy machine key from state key %q", b.stateKey)
+		}
 		b.machinePrivKey = legacyMachineKey
 	} else {
 		b.logf("generating new machine key")
@@ -801,23 +809,32 @@ func (b *LocalBackend) writeServerModeStartState(userID string, prefs *Prefs) {
 // loadStateLocked sets b.prefs and b.stateKey based on a complex
 // combination of key, prefs, and legacyPath. b.mu must be held when
 // calling.
-func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath string) error {
+func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath string) (err error) {
 	if prefs == nil && key == "" {
 		panic("state key and prefs are both unset")
 	}
 
+	// Optimistically set stateKey (for initMachineKeyLocked's
+	// logging), but revert it if we return an error so a later SetPrefs
+	// call can't pick it up if it's bogus.
+	b.stateKey = key
+	defer func() {
+		if err != nil {
+			b.stateKey = ""
+		}
+	}()
+
 	if key == "" {
 		// Frontend owns the state, we just need to obey it.
 		//
 		// If the frontend (e.g. on Windows) supplied the
 		// optional/legacy machine key then it's used as the
 		// value instead of making up a new one.
-		b.logf("Using frontend prefs")
+		b.logf("using frontend prefs: %s", prefs.Pretty())
 		b.prefs = prefs.Clone()
 		if err := b.initMachineKeyLocked(); err != nil {
 			return fmt.Errorf("initMachineKeyLocked: %w", err)
 		}
-		b.stateKey = ""
 		b.writeServerModeStartState(b.userID, b.prefs)
 		return nil
 	}
@@ -825,13 +842,13 @@ func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath st
 	if prefs != nil {
 		// Backend owns the state, but frontend is trying to migrate
 		// state into the backend.
-		b.logf("Importing frontend prefs into backend store")
+		b.logf("importing frontend prefs into backend store; frontend prefs: %s", prefs.Pretty())
 		if err := b.store.WriteState(key, prefs.ToBytes()); err != nil {
 			return fmt.Errorf("store.WriteState: %v", err)
 		}
 	}
 
-	b.logf("Using backend prefs")
+	b.logf("using backend prefs")
 	bs, err := b.store.ReadState(key)
 	if err != nil {
 		if errors.Is(err, ErrStateNotExist) {
@@ -843,16 +860,15 @@ func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath st
 					}
 					b.prefs = NewPrefs()
 				} else {
-					b.logf("Imported state from relaynode for %q", key)
+					b.logf("imported prefs from relaynode for %q: %v", key, b.prefs.Pretty())
 				}
 			} else {
 				b.prefs = NewPrefs()
-				b.logf("Created empty state for %q", key)
+				b.logf("created empty state for %q: %s", key, b.prefs.Pretty())
 			}
 			if err := b.initMachineKeyLocked(); err != nil {
 				return fmt.Errorf("initMachineKeyLocked: %w", err)
 			}
-			b.stateKey = key
 			return nil
 		}
 		return fmt.Errorf("store.ReadState(%q): %v", key, err)
@@ -861,7 +877,7 @@ func (b *LocalBackend) loadStateLocked(key StateKey, prefs *Prefs, legacyPath st
 	if err != nil {
 		return fmt.Errorf("PrefsFromBytes: %v", err)
 	}
-	b.stateKey = key
+	b.logf("backend prefs for %q: %s", key, b.prefs.Pretty())
 	if err := b.initMachineKeyLocked(); err != nil {
 		return fmt.Errorf("initMachineKeyLocked: %w", err)
 	}
@@ -1139,6 +1155,7 @@ func (b *LocalBackend) authReconfig() {
 	uc := b.prefs
 	nm := b.netMap
 	hasPAC := b.prevIfState.HasPAC()
+	disableSubnetsIfPAC := nm != nil && nm.Debug != nil && nm.Debug.DisableSubnetsIfPAC.EqualBool(true)
 	b.mu.Unlock()
 
 	if blocked {
@@ -1163,13 +1180,7 @@ func (b *LocalBackend) authReconfig() {
 	if uc.AllowSingleHosts {
 		flags |= controlclient.AllowSingleHosts
 	}
-	if hasPAC {
-		// TODO(bradfitz): make this policy configurable per
-		// domain, flesh out all the edge cases where subnet
-		// routes might shadow corp HTTP proxies, DNS servers,
-		// domain controllers, etc. For now we just want
-		// Tailscale to stay enabled while laptops roam
-		// between corp & non-corp networks.
+	if hasPAC && disableSubnetsIfPAC {
 		if flags&controlclient.AllowSubnetRoutes != 0 {
 			b.logf("authReconfig: have PAC; disabling subnet routes")
 			flags &^= controlclient.AllowSubnetRoutes

ipn/message.go

@@ -5,6 +5,7 @@
 package ipn
 
 import (
+	"bytes"
 	"encoding/binary"
 	"encoding/json"
 	"errors"
@@ -19,6 +20,8 @@ import (
 	"tailscale.com/version"
 )
 
+var jsonEscapedZero = []byte(`\u0000`)
+
 type NoArgs struct{}
 
 type StartArgs struct {
@@ -85,6 +88,9 @@ func (bs *BackendServer) send(n Notify) {
 	if err != nil {
 		log.Fatalf("Failed json.Marshal(notify): %v\n%#v", err, n)
 	}
+	if bytes.Contains(b, jsonEscapedZero) {
+		log.Printf("[unexpected] zero byte in BackendServer.send notify message: %q", b)
+	}
 	bs.sendNotifyMsg(b)
 }
 
@@ -204,6 +210,9 @@ func (bc *BackendClient) GotNotifyMsg(b []byte) {
 		// not interesting
 		return
 	}
+	if bytes.Contains(b, jsonEscapedZero) {
+		log.Printf("[unexpected] zero byte in BackendClient.GotNotifyMsg message: %q", b)
+	}
 	n := Notify{}
 	if err := json.Unmarshal(b, &n); err != nil {
 		log.Fatalf("BackendClient.Notify: cannot decode message (length=%d)\n%#v", len(b), string(b))
@@ -230,6 +239,9 @@ func (bc *BackendClient) send(cmd Command) {
 	if err != nil {
 		log.Fatalf("Failed json.Marshal(cmd): %v\n%#v\n", err, cmd)
 	}
+	if bytes.Contains(b, jsonEscapedZero) {
+		log.Printf("[unexpected] zero byte in BackendClient.send command: %q", b)
+	}
 	bc.sendCommandMsg(b)
 }
 

ipn/prefs.go

@@ -5,6 +5,7 @@
 package ipn
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
@@ -152,9 +153,15 @@ func (p *Prefs) pretty(goos string) string {
 	if len(p.AdvertiseRoutes) > 0 || p.NoSNAT {
 		fmt.Fprintf(&sb, "snat=%v ", !p.NoSNAT)
 	}
+	if len(p.AdvertiseTags) > 0 {
+		fmt.Fprintf(&sb, "tags=%s ", strings.Join(p.AdvertiseTags, ","))
+	}
 	if goos == "linux" {
 		fmt.Fprintf(&sb, "nf=%v ", p.NetfilterMode)
 	}
+	if p.ControlURL != "" && p.ControlURL != "https://login.tailscale.com" {
+		fmt.Fprintf(&sb, "url=%q ", p.ControlURL)
+	}
 	if p.Persist != nil {
 		sb.WriteString(p.Persist.Pretty())
 	} else {
@@ -270,6 +277,14 @@ func LoadPrefs(filename string) (*Prefs, error) {
 	if err != nil {
 		return nil, fmt.Errorf("LoadPrefs open: %w", err) // err includes path
 	}
+	if bytes.Contains(data, jsonEscapedZero) {
+		// Tailscale 1.2.0 - 1.2.8 on Windows had a memory corruption bug
+		// in the backend process that ended up sending NULL bytes over JSON
+		// to the frontend which wrote them out to JSON files on disk.
+		// So if we see one, treat is as corrupt and the user will need
+		// to log in again. (better than crashing)
+		return nil, os.ErrNotExist
+	}
 	p, err := PrefsFromBytes(data, false)
 	if err != nil {
 		return nil, fmt.Errorf("LoadPrefs(%q) decode: %w", filename, err)

ipn/prefs_test.go

@@ -7,6 +7,7 @@ package ipn
 import (
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"reflect"
 	"testing"
@@ -326,6 +327,32 @@ func TestPrefsPretty(t *testing.T) {
 			"windows",
 			"Prefs{ra=false dns=false want=true server=true Persist=nil}",
 		},
+		{
+			Prefs{
+				AllowSingleHosts: true,
+				WantRunning:      true,
+				ControlURL:       "http://localhost:1234",
+				AdvertiseTags:    []string{"tag:foo", "tag:bar"},
+			},
+			"darwin",
+			`Prefs{ra=false dns=false want=true tags=tag:foo,tag:bar url="http://localhost:1234" Persist=nil}`,
+		},
+		{
+			Prefs{
+				Persist: &controlclient.Persist{},
+			},
+			"linux",
+			`Prefs{ra=false mesh=false dns=false want=false routes=[] nf=off Persist{lm=, o=, n= u=""}}`,
+		},
+		{
+			Prefs{
+				Persist: &controlclient.Persist{
+					PrivateNodeKey: wgcfg.PrivateKey{1: 1},
+				},
+			},
+			"linux",
+			`Prefs{ra=false mesh=false dns=false want=false routes=[] nf=off Persist{lm=, o=, n=[B1VKl] u=""}}`,
+		},
 	}
 	for i, tt := range tests {
 		got := tt.p.pretty(tt.os)
@@ -345,3 +372,25 @@ func TestLoadPrefsNotExist(t *testing.T) {
 	}
 	t.Fatalf("unexpected prefs=%#v, err=%v", p, err)
 }
+
+// TestLoadPrefsFileWithZeroInIt verifies that LoadPrefs hanldes corrupted input files.
+// See issue #954 for details.
+func TestLoadPrefsFileWithZeroInIt(t *testing.T) {
+	f, err := ioutil.TempFile("", "TestLoadPrefsFileWithZeroInIt")
+	if err != nil {
+		t.Fatal(err)
+	}
+	path := f.Name()
+	if _, err := f.Write(jsonEscapedZero); err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+	defer os.Remove(path)
+
+	p, err := LoadPrefs(path)
+	if errors.Is(err, os.ErrNotExist) {
+		// expected.
+		return
+	}
+	t.Fatalf("unexpected prefs=%#v, err=%v", p, err)
+}

ipn/store.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"log"
 	"os"
 	"path/filepath"
 	"sync"
@@ -100,6 +101,14 @@ func (s *FileStore) String() string { return fmt.Sprintf("FileStore(%q)", s.path
 // NewFileStore returns a new file store that persists to path.
 func NewFileStore(path string) (*FileStore, error) {
 	bs, err := ioutil.ReadFile(path)
+
+	// Treat an empty file as a missing file.
+	// (https://github.com/tailscale/tailscale/issues/895#issuecomment-723255589)
+	if err == nil && len(bs) == 0 {
+		log.Printf("ipn.NewFileStore(%q): file empty; treating it like a missing file [warning]", path)
+		err = os.ErrNotExist
+	}
+
 	if err != nil {
 		if os.IsNotExist(err) {
 			// Write out an initial file, to verify that we can write

net/interfaces/interfaces_windows.go

@@ -7,6 +7,7 @@ package interfaces
 import (
 	"fmt"
 	"log"
+	"net/url"
 	"os/exec"
 	"syscall"
 	"unsafe"
@@ -176,7 +177,12 @@ func getPACWindows() string {
 			return ""
 		}
 		defer globalFree.Call(uintptr(unsafe.Pointer(res)))
-		return windows.UTF16PtrToString(res)
+		s := windows.UTF16PtrToString(res)
+		if _, err := url.Parse(s); err != nil {
+			log.Printf("getPACWindows: invalid URL %q from winhttp; ignoring", s)
+			return ""
+		}
+		return s
 	}
 	const (
 		ERROR_WINHTTP_AUTODETECTION_FAILED = 12180

net/netcheck/netcheck.go

@@ -715,6 +715,7 @@ func (rs *reportState) probePortMapServices() {
 	uc.WriteTo(pcpPacket(myIP, tempPort, false), port5351)
 
 	res := make([]byte, 1500)
+	sentPCPDelete := false
 	for {
 		n, addr, err := uc.ReadFrom(res)
 		if err != nil {
@@ -732,11 +733,14 @@ func (rs *reportState) probePortMapServices() {
 			if n == 60 && res[0] == 0x02 { // right length and version 2
 				rs.setOptBool(&rs.report.PCP, true)
 
-				// And now delete the mapping.
-				// (PCP is the only protocol of the three that requires
-				// we cause a side effect to detect whether it's present,
-				// so we need to redo that side effect now.)
-				uc.WriteTo(pcpPacket(myIP, tempPort, true), port5351)
+				if !sentPCPDelete {
+					sentPCPDelete = true
+					// And now delete the mapping.
+					// (PCP is the only protocol of the three that requires
+					// we cause a side effect to detect whether it's present,
+					// so we need to redo that side effect now.)
+					uc.WriteTo(pcpPacket(myIP, tempPort, true), port5351)
+				}
 			}
 		}
 	}
@@ -752,6 +756,7 @@ var uPnPPacket = []byte("M-SEARCH * HTTP/1.1\r\n" +
 
 var v4unspec, _ = netaddr.ParseIP("0.0.0.0")
 
+// pcpPacket generates a PCP packet with a MAP opcode.
 func pcpPacket(myIP netaddr.IP, mapToLocalPort int, delete bool) []byte {
 	const udpProtoNumber = 17
 	lifetimeSeconds := uint32(1)
@@ -759,17 +764,24 @@ func pcpPacket(myIP netaddr.IP, mapToLocalPort int, delete bool) []byte {
 		lifetimeSeconds = 0
 	}
 	const opMap = 1
+
+	// 24 byte header + 36 byte map opcode
 	pkt := make([]byte, (32+32+128)/8+(96+8+24+16+16+128)/8)
+
+	// The header (https://tools.ietf.org/html/rfc6887#section-7.1)
 	pkt[0] = 2 // version
 	pkt[1] = opMap
 	binary.BigEndian.PutUint32(pkt[4:8], lifetimeSeconds)
 	myIP16 := myIP.As16()
 	copy(pkt[8:], myIP16[:])
-	rand.Read(pkt[24 : 24+12])
-	pkt[36] = udpProtoNumber
-	binary.BigEndian.PutUint16(pkt[40:], uint16(mapToLocalPort))
+
+	// The map opcode body (https://tools.ietf.org/html/rfc6887#section-11.1)
+	mapOp := pkt[24:]
+	rand.Read(mapOp[:12]) // 96 bit mappping nonce
+	mapOp[12] = udpProtoNumber
+	binary.BigEndian.PutUint16(mapOp[16:], uint16(mapToLocalPort))
 	v4unspec16 := v4unspec.As16()
-	copy(pkt[40:], v4unspec16[:])
+	copy(mapOp[20:], v4unspec16[:])
 	return pkt
 }
 

net/netns/netns_windows.go

@@ -5,14 +5,14 @@
 package netns
 
 import (
-	"encoding/binary"
+	"math/bits"
 	"strings"
 	"syscall"
-	"unsafe"
 
 	"golang.org/x/sys/windows"
 	"golang.zx2c4.com/wireguard/windows/tunnel/winipcfg"
 	"tailscale.com/net/interfaces"
+	"tailscale.com/util/endian"
 )
 
 func interfaceIndex(iface *winipcfg.IPAdapterAddresses) uint32 {
@@ -114,7 +114,8 @@ func bindSocket6(c syscall.RawConn, ifidx uint32) error {
 // representation, suitable for passing to Windows APIs that require a
 // mangled uint32.
 func nativeToBigEndian(i uint32) uint32 {
-	var b [4]byte
-	binary.BigEndian.PutUint32(b[:], i)
-	return *(*uint32)(unsafe.Pointer(&b[0]))
+	if endian.Big {
+		return i
+	}
+	return bits.ReverseBytes32(i)
 }

net/netstat/netstat_windows.go

@@ -6,14 +6,15 @@
 package netstat
 
 import (
-	"encoding/binary"
 	"errors"
 	"fmt"
+	"math/bits"
 	"syscall"
 	"unsafe"
 
 	"golang.org/x/sys/windows"
 	"inet.af/netaddr"
+	"tailscale.com/util/endian"
 )
 
 // See https://docs.microsoft.com/en-us/windows/win32/api/iphlpapi/nf-iphlpapi-getextendedtcptable
@@ -153,9 +154,11 @@ func state(v uint32) string {
 }
 
 func ipport4(addr uint32, port uint16) netaddr.IPPort {
-	a4 := (*[4]byte)(unsafe.Pointer(&addr))
+	if !endian.Big {
+		addr = bits.ReverseBytes32(addr)
+	}
 	return netaddr.IPPort{
-		IP:   netaddr.IPv4(a4[0], a4[1], a4[2], a4[3]),
+		IP:   netaddr.IPv4(byte(addr>>24), byte(addr>>16), byte(addr>>8), byte(addr)),
 		Port: port,
 	}
 }
@@ -173,6 +176,8 @@ func ipport6(addr [16]byte, scope uint32, port uint16) netaddr.IPPort {
 }
 
 func port(v *uint32) uint16 {
-	p := (*[4]byte)(unsafe.Pointer(v))
-	return binary.BigEndian.Uint16(p[:2])
+	if !endian.Big {
+		return uint16(bits.ReverseBytes32(*v) >> 16)
+	}
+	return uint16(*v >> 16)
 }

tailcfg/tailcfg.go

@@ -13,7 +13,6 @@ import (
 	"reflect"
 	"strings"
 	"time"
-	"unicode/utf8"
 
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"go4.org/mem"
@@ -211,8 +210,13 @@ func (m MachineStatus) String() string {
 	}
 }
 
-func isNum(r rune) bool   { return r >= '0' && r <= '9' }
-func isAlpha(r rune) bool { return (r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') }
+func isNum(b byte) bool {
+	return b >= '0' && b <= '9'
+}
+
+func isAlpha(b byte) bool {
+	return (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z')
+}
 
 // CheckTag valids whether a given string can be used as an ACL tag.
 // For now we allow only ascii alphanumeric tags, and they need to start
@@ -227,34 +231,20 @@ func CheckTag(tag string) error {
 	if !strings.HasPrefix(tag, "tag:") {
 		return errors.New("tags must start with 'tag:'")
 	}
-	suffix := tag[len("tag:"):]
-	if err := CheckTagSuffix(suffix); err != nil {
-		return fmt.Errorf("invalid tag %q: %w", tag, err)
-	}
-	return nil
-}
-
-// CheckTagSuffix checks whether tag is a valid tag suffix (the part
-// appearing after "tag:"). The error message does not reference
-// "tag:", so it's suitable for use by the "tailscale up" CLI tool
-// where the "tag:" isn't required. The returned error also does not
-// reference the tag itself, so the caller can wrap it as needed with
-// either the full or short form.
-func CheckTagSuffix(tag string) error {
+	tag = tag[4:]
 	if tag == "" {
 		return errors.New("tag names must not be empty")
 	}
-	if i := strings.IndexFunc(tag, func(r rune) bool { return r >= utf8.RuneSelf }); i != -1 {
-		return errors.New("tag names must only contain ASCII")
+	if !isAlpha(tag[0]) {
+		return errors.New("tag names must start with a letter, after 'tag:'")
 	}
-	if !isAlpha(rune(tag[0])) {
-		return errors.New("tag name must start with a letter")
-	}
-	for _, r := range tag {
-		if !isNum(r) && !isAlpha(r) && r != '-' {
+
+	for _, b := range []byte(tag) {
+		if !isNum(b) && !isAlpha(b) && b != '-' {
 			return errors.New("tag names can only contain numbers, letters, or dashes")
 		}
 	}
+
 	return nil
 }
 
@@ -291,6 +281,7 @@ type Hostinfo struct {
 	OSVersion     string       `json:",omitempty"` // operating system version, with optional distro prefix ("Debian 10.4", "Windows 10 Pro 10.0.19041")
 	DeviceModel   string       `json:",omitempty"` // mobile phone model ("Pixel 3a", "iPhone 11 Pro")
 	Hostname      string       // name of the host the client runs on
+	ShareeNode    bool         `json:",omitempty"` // indicates this node exists in netmap because it's owned by a shared-to user
 	GoArch        string       `json:",omitempty"` // the host's GOARCH value (of the running binary)
 	RoutableIPs   []wgcfg.CIDR `json:",omitempty"` // set of IP ranges this client can route
 	RequestTags   []string     `json:",omitempty"` // set of ACL tags this node wants to claim
@@ -509,6 +500,12 @@ type MapRequest struct {
 	// added and removed all the time during development, and offer no
 	// compatibility promise. To roll out semantic changes, bump
 	// Version instead.
+	//
+	// Current DebugFlags values are:
+	//     * "warn-ip-forwarding-off": client is trying to be a subnet
+	//       router but their IP forwarding is broken.
+	//     * "v6-overlay": IPv6 development flag to have control send
+	//       v6 node addrs
 	DebugFlags []string `json:",omitempty"`
 }
 
@@ -529,9 +526,28 @@ type NetPortRange struct {
 }
 
 // FilterRule represents one rule in a packet filter.
+//
+// A rule is logically a set of source CIDRs to match (described by
+// SrcIPs and SrcBits), and a set of destination targets that are then
+// allowed if a source IP is mathces of those CIDRs.
 type FilterRule struct {
-	SrcIPs   []string // "*" means all
-	SrcBits  []int
+	// SrcIPs are the source IPs/networks to match.
+	// The special value "*" means to match all.
+	SrcIPs []string
+
+	// SrcBits values correspond to the SrcIPs above.
+	//
+	// If present at the same index, it changes the SrcIP above to
+	// be a network with /n CIDR bits. If the slice is nil or
+	// insufficiently long, the default value (for an IPv4
+	// address) for a position is 32, as if the SrcIPs above were
+	// a /32 mask. For a "*" SrcIPs value, the corresponding
+	// SrcBits value is ignored.
+	// TODO: for IPv6, clarify default bits length.
+	SrcBits []int
+
+	// DstPorts are the port ranges to allow once a source IP
+	// matches (is in the CIDR described by SrcIPs & SrcBits).
 	DstPorts []NetPortRange
 }
 
@@ -635,6 +651,10 @@ type Debug struct {
 	// TrimWGConfig controls whether Tailscale does lazy, on-demand
 	// wireguard configuration of peers.
 	TrimWGConfig opt.Bool `json:",omitempty"`
+
+	// DisableSubnetsIfPAC controls whether subnet routers should be
+	// disabled if WPAD is present on the network.
+	DisableSubnetsIfPAC opt.Bool `json:",omitempty"`
 }
 
 func (k MachineKey) String() string                   { return fmt.Sprintf("mkey:%x", k[:]) }

tailcfg/tailcfg_clone.go

@@ -105,6 +105,7 @@ var _HostinfoNeedsRegeneration = Hostinfo(struct {
 	OSVersion     string
 	DeviceModel   string
 	Hostname      string
+	ShareeNode    bool
 	GoArch        string
 	RoutableIPs   []wgcfg.CIDR
 	RequestTags   []string

tailcfg/tailcfg_test.go

@@ -23,9 +23,12 @@ func fieldsOf(t reflect.Type) (fields []string) {
 
 func TestHostinfoEqual(t *testing.T) {
 	hiHandles := []string{
-		"IPNVersion", "FrontendLogID", "BackendLogID", "OS", "OSVersion",
-		"DeviceModel", "Hostname", "GoArch", "RoutableIPs", "RequestTags", "Services",
-		"NetInfo",
+		"IPNVersion", "FrontendLogID", "BackendLogID",
+		"OS", "OSVersion", "DeviceModel", "Hostname",
+		"ShareeNode",
+		"GoArch",
+		"RoutableIPs", "RequestTags",
+		"Services", "NetInfo",
 	}
 	if have := fieldsOf(reflect.TypeOf(Hostinfo{})); !reflect.DeepEqual(have, hiHandles) {
 		t.Errorf("Hostinfo.Equal check might be out of sync\nfields: %q\nhandled: %q\n",
@@ -169,6 +172,11 @@ func TestHostinfoEqual(t *testing.T) {
 			&Hostinfo{Services: []Service{Service{Proto: TCP, Port: 1234, Description: "foo"}}},
 			true,
 		},
+		{
+			&Hostinfo{ShareeNode: true},
+			&Hostinfo{},
+			false,
+		},
 	}
 	for i, tt := range tests {
 		got := tt.a.Equal(tt.b)

util/endian/big.go

@@ -0,0 +1,10 @@
+// Copyright (c) 2020 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.
+
+// +build mips mips64 ppc64 s390x
+
+package endian
+
+// Big is whether the current platform is big endian.
+const Big = true

util/endian/endian.go

@@ -0,0 +1,6 @@
+// Copyright (c) 2020 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 endian exports a constant about whether the machine is big endian.
+package endian

util/endian/little.go

@@ -0,0 +1,10 @@
+// Copyright (c) 2020 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.
+
+// +build 386 amd64 arm arm64 mips64le mipsle ppc64le riscv64 wasm
+
+package endian
+
+// Big is whether the current platform is big endian.
+const Big = false

version/mkversion_test.go

@@ -70,12 +70,12 @@ func TestMkversion(t *testing.T) {
            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_SHORT="1.2.0"
+           VERSION_LONG="1.2.0-17-tabcdef"
            VERSION_GIT_HASH="abcdef"
            VERSION_EXTRA_HASH=""
-           VERSION_XCODE="100.0.0"
-           VERSION_WINRES="0,0,0,0"`},
+           VERSION_XCODE="101.2.0"
+           VERSION_WINRES="1,2,0,0"`},
 		{"abcdef", "defghi", 1, 15, 0, 129, `
            VERSION_SHORT="1.15.129"
            VERSION_LONG="1.15.129-tabcdef-gdefghi"

version/version.sh

@@ -1,22 +1,43 @@
 #!/bin/sh
-
 set -eu
 
+# Return the commitid of the given ref in the given repo dir. If the worktree
+# or index is dirty, also appends -dirty.
+#
+# $ git_hash_dirty ../.. HEAD
+# 1be01ddc6e430ca3aa9beea3587d16750efb3241-dirty
+git_hash_dirty() {
+    (
+        cd "$1"
+        x=$(git rev-parse HEAD)
+        if ! git diff-index --quiet HEAD; then
+            x="$x-dirty"
+        fi
+        echo "$x"
+    )
+}
+
 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
+        # extra_hash_or_dir is either:
+        #   - a git commitid
+        # or
+        #   - the path to a git repo from which to calculate the real hash.
+        #
+        # 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
+        extra_hash_or_dir="${1:-}"
+        if [ -z "$extra_hash_or_dir" ]; 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
+        elif [ -d "$extra_hash_or_dir/.git" ]; then
+            extra_hash=$(git_hash_dirty "$extra_hash_or_dir" HEAD)
+        elif ! expr "$extra_hash_or_dir" : "^[0-9a-f]*$"; then
+            echo "Invalid extra hash '$extra_hash_or_dir', must be a git commit or path to a git repo" >&2
             exit 1
+        else
+            extra_hash="$extra_hash_or_dir"
         fi
 
         # Load the base version and optional corresponding git hash
@@ -25,15 +46,12 @@ case $# in
         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)
+            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)
+        git_hash=$(git_hash_dirty . HEAD)
         # The number of extra commits between the release base to git_hash.
-        change_count=$(git rev-list ${base_git_hash}..HEAD | wc -l)
+        change_count=$(git rev-list --count HEAD "^$base_git_hash")
         ;;
     6)
         # Test mode: rather than run git commands and whatnot, take in
@@ -46,14 +64,14 @@ case $# in
         change_count=$6
         ;;
     *)
-        echo "Usage: $0 [extra-git-hash-or-checkout]"
+        echo "Usage: $0 [extra-git-commitid-or-dir]"
         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)
+short_git_hash=$(echo "$git_hash" | cut -c1-9)
+short_extra_hash=$(echo "$extra_hash" | cut -c1-9)
 
 # Convert major/minor/patch/change_count into an adjusted
 # major/minor/patch. This block is where all our policies on
@@ -62,25 +80,28 @@ 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
+        # indicates that we created an unstable VERSION.txt 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"
+    change_suffix=""
 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"
+    # tagged as a patch release yet.
+    #
+    # We used to change the version number to 0.0.0 in that case, but that
+    # caused some features to get disabled due to the low version number.
+    # Instead, add yet another suffix to the version number, with a change
+    # count.
+    change_suffix="-$change_count"
+else
+    # Even minor number with no extra changes.
+    change_suffix=""
 fi
 
 # Hack for 1.1: add 1000 to the patch number. We switched from using
@@ -95,15 +116,15 @@ fi
 # 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"
+    long_version_suffix="$change_suffix-t$short_git_hash"
 else
-    long_version_suffix="-t${short_git_hash}-g${short_extra_hash}"
+    long_version_suffix="$change_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"
+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

wgengine/filter/filter.go

@@ -371,7 +371,7 @@ func (f *Filter) pre(q *packet.ParsedPacket, rf RunFlags, dir direction) Respons
 		f.logRateLimit(rf, q, dir, Drop, "multicast")
 		return Drop
 	}
-	if q.DstIP.IsLinkLocalUnicast() {
+	if q.DstIP.IsMostLinkLocalUnicast() {
 		f.logRateLimit(rf, q, dir, Drop, "link-local-unicast")
 		return Drop
 	}
@@ -418,7 +418,7 @@ func omitDropLogging(p *packet.ParsedPacket, dir direction) bool {
 			if ipProto == packet.IGMP {
 				return true
 			}
-			if p.DstIP.IsMulticast() || p.DstIP.IsLinkLocalUnicast() {
+			if p.DstIP.IsMulticast() || p.DstIP.IsMostLinkLocalUnicast() {
 				return true
 			}
 		case 6:

wgengine/magicsock/magicsock.go

@@ -2492,6 +2492,9 @@ func (c *Conn) bind1(ruc **RebindingUDPConn, which string) error {
 	host := ""
 	if inTest() && !c.simulatedNetwork {
 		host = "127.0.0.1"
+		if which == "udp6" {
+			host = "::1"
+		}
 	}
 	var pc net.PacketConn
 	var err error

wgengine/monitor/monitor_windows.go

@@ -7,6 +7,8 @@ package monitor
 import (
 	"context"
 	"errors"
+	"fmt"
+	"runtime"
 	"sync"
 	"syscall"
 	"time"
@@ -24,9 +26,15 @@ const (
 )
 
 var (
-	iphlpapi              = syscall.NewLazyDLL("iphlpapi.dll")
-	notifyAddrChangeProc  = iphlpapi.NewProc("NotifyAddrChange")
-	notifyRouteChangeProc = iphlpapi.NewProc("NotifyRouteChange")
+	iphlpapi                 = syscall.NewLazyDLL("iphlpapi.dll")
+	notifyAddrChangeProc     = iphlpapi.NewProc("NotifyAddrChange")
+	notifyRouteChangeProc    = iphlpapi.NewProc("NotifyRouteChange")
+	cancelIPChangeNotifyProc = iphlpapi.NewProc("CancelIPChangeNotify")
+)
+
+const (
+	_STATUS_PENDING = 0x00000103 // 259
+	_STATUS_WAIT_0  = 0
 )
 
 type unspecifiedMessage struct{}
@@ -43,27 +51,33 @@ type messageOrError struct {
 }
 
 type winMon struct {
-	ctx        context.Context
-	cancel     context.CancelFunc
-	messagec   chan messageOrError
-	logf       logger.Logf
-	pollTicker *time.Ticker
-	lastState  *interfaces.State
+	ctx         context.Context
+	cancel      context.CancelFunc
+	messagec    chan messageOrError
+	logf        logger.Logf
+	pollTicker  *time.Ticker
+	lastState   *interfaces.State
+	closeHandle windows.Handle // signaled upon close
 
 	mu            sync.Mutex
-	event         windows.Handle
 	lastNetChange time.Time
 	inFastPoll    bool // recent net change event made us go into fast polling mode (to detect proxy changes)
 }
 
 func newOSMon(logf logger.Logf) (osMon, error) {
+	closeHandle, err := windows.CreateEvent(nil, 1 /* manual reset */, 0 /* unsignaled */, nil /* no name */)
+	if err != nil {
+		return nil, fmt.Errorf("CreateEvent: %w", err)
+	}
+
 	ctx, cancel := context.WithCancel(context.Background())
 	m := &winMon{
-		logf:       logf,
-		ctx:        ctx,
-		cancel:     cancel,
-		messagec:   make(chan messageOrError, 1),
-		pollTicker: time.NewTicker(pollIntervalSlow),
+		logf:        logf,
+		ctx:         ctx,
+		cancel:      cancel,
+		messagec:    make(chan messageOrError, 1),
+		pollTicker:  time.NewTicker(pollIntervalSlow),
+		closeHandle: closeHandle,
 	}
 	go m.awaitIPAndRouteChanges()
 	return m, nil
@@ -72,14 +86,7 @@ func newOSMon(logf logger.Logf) (osMon, error) {
 func (m *winMon) Close() error {
 	m.cancel()
 	m.pollTicker.Stop()
-
-	m.mu.Lock()
-	defer m.mu.Unlock()
-	if h := m.event; h != 0 {
-		// Wake up any reader blocked in Receive.
-		windows.SetEvent(h)
-	}
-
+	windows.SetEvent(m.closeHandle) // wakes up any reader blocked in Receive
 	return nil
 }
 
@@ -136,52 +143,80 @@ func (m *winMon) getIPOrRouteChangeMessage() (message, error) {
 		return nil, errClosed
 	}
 
-	var o windows.Overlapped
-	h, err := windows.CreateEvent(nil, 1 /* true*/, 0 /* unsignaled */, nil /* no name */)
-	if err != nil {
-		m.logf("CreateEvent: %v", err)
-		return nil, err
-	}
-	defer windows.CloseHandle(h)
+	// TODO(bradfitz): locking ourselves to an OS thread here
+	// likely isn't necessary, but also can't really hurt.
+	// We'll be blocked in windows.WaitForMultipleObjects below
+	// anyway, so might as well stay on this thread during the
+	// notify calls and cancel funcs.
+	// Given the past memory corruption from misuse of these APIs,
+	// and my continued lack of understanding of Windows APIs,
+	// I'll be paranoid. But perhaps we can remove this once
+	// we understand more.
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
 
-	m.mu.Lock()
-	m.event = h
-	m.mu.Unlock()
-
-	o.HEvent = h
-
-	err = notifyAddrChange(&h, &o)
+	addrHandle, oaddr, cancel, err := notifyAddrChange()
 	if err != nil {
 		m.logf("notifyAddrChange: %v", err)
 		return nil, err
 	}
-	err = notifyRouteChange(&h, &o)
+	defer cancel()
+
+	routeHandle, oroute, cancel, err := notifyRouteChange()
 	if err != nil {
 		m.logf("notifyRouteChange: %v", err)
 		return nil, err
 	}
+	defer cancel()
 
 	t0 := time.Now()
-	_, err = windows.WaitForSingleObject(o.HEvent, windows.INFINITE)
-	if m.ctx.Err() != nil {
+	eventNum, err := windows.WaitForMultipleObjects([]windows.Handle{
+		m.closeHandle, // eventNum 0
+		addrHandle,    // eventNum 1
+		routeHandle,   // eventNum 2
+	}, false, windows.INFINITE)
+	if m.ctx.Err() != nil || (err == nil && eventNum == 0) {
 		return nil, errClosed
 	}
 	if err != nil {
-		m.logf("waitForSingleObject: %v", err)
+		m.logf("waitForMultipleObjects: %v", err)
 		return nil, err
 	}
+
 	d := time.Since(t0)
-	m.logf("got windows change event after %v", d)
+	var eventStr string
+
+	// notifyAddrChange and notifyRouteChange both seem to return the same
+	// handle value. Determine which fired by looking at the "Internal" (sic)
+	// field of the Ovelapped instead.
+	// TODO(bradfitz): maybe clean this up; see TODO in callNotifyProc.
+	if (eventNum == 1 || eventNum == 2) && addrHandle == routeHandle {
+		if oaddr.Internal == _STATUS_WAIT_0 && oroute.Internal == _STATUS_PENDING {
+			eventStr = "addr-o" // "-o" overlapped suffix to distinguish from "addr" below
+		} else if oroute.Internal == _STATUS_WAIT_0 && oaddr.Internal == _STATUS_PENDING {
+			eventStr = "route-o"
+		} else {
+			eventStr = fmt.Sprintf("[unexpected] addr.internal=%d; route.internal=%d", oaddr.Internal, oroute.Internal)
+		}
+	} else {
+		switch eventNum {
+		case 1:
+			eventStr = "addr"
+		case 2:
+			eventStr = "route"
+		default:
+			eventStr = fmt.Sprintf("%d [unexpected]", eventNum)
+		}
+	}
+	m.logf("got windows change event after %v: evt=%s", d, eventStr)
 
 	m.mu.Lock()
 	{
 		m.lastNetChange = time.Now()
-		m.event = 0
 
 		// Something changed, so assume Windows is about to
 		// discover its new proxy settings from WPAD, which
 		// seems to take a bit. Poll heavily for awhile.
-		m.logf("starting quick poll, waiting for WPAD change")
 		m.inFastPoll = true
 		m.pollTicker.Reset(pollIntervalFast)
 	}
@@ -190,23 +225,46 @@ func (m *winMon) getIPOrRouteChangeMessage() (message, error) {
 	return unspecifiedMessage{}, nil
 }
 
-func notifyAddrChange(h *windows.Handle, o *windows.Overlapped) error {
-	return callNotifyProc(notifyAddrChangeProc, h, o)
+func notifyAddrChange() (h windows.Handle, o *windows.Overlapped, cancel func(), err error) {
+	return callNotifyProc(notifyAddrChangeProc)
 }
 
-func notifyRouteChange(h *windows.Handle, o *windows.Overlapped) error {
-	return callNotifyProc(notifyRouteChangeProc, h, o)
+func notifyRouteChange() (h windows.Handle, o *windows.Overlapped, cancel func(), err error) {
+	return callNotifyProc(notifyRouteChangeProc)
 }
 
-func callNotifyProc(p *syscall.LazyProc, h *windows.Handle, o *windows.Overlapped) error {
-	r1, _, e1 := p.Call(uintptr(unsafe.Pointer(h)), uintptr(unsafe.Pointer(o)))
-	expect := uintptr(0)
-	if h != nil || o != nil {
-		const ERROR_IO_PENDING = 997
-		expect = ERROR_IO_PENDING
+func callNotifyProc(p *syscall.LazyProc) (h windows.Handle, o *windows.Overlapped, cancel func(), err error) {
+	o = new(windows.Overlapped)
+
+	// TODO(bradfitz): understand why this if-false code doesn't
+	// work, even though the docs online suggest we should pass an
+	// event in the overlapped.Hevent field.
+	// The docs at
+	// https://docs.microsoft.com/en-us/windows/win32/api/minwinbase/ns-minwinbase-overlapped
+	// says that o.HEvent can be zero, though, which seems to work.
+	// Note that the returned windows.Handle returns the same value for both
+	// notifyAddrChange and notifyRouteChange, which is why our caller needs
+	// to look at the returned Overlapped's Internal field to see which case
+	// fired. That's also worth understanding more.
+	// See crawshaw's comment at https://github.com/tailscale/tailscale/pull/944#discussion_r526469186
+	// too.
+	if false {
+		evt, err := windows.CreateEvent(nil, 0, 0, nil)
+		if err != nil {
+			return 0, nil, nil, err
+		}
+		o.HEvent = evt
 	}
-	if r1 == expect {
-		return nil
+
+	r1, _, e1 := syscall.Syscall(p.Addr(), 2, uintptr(unsafe.Pointer(&h)), uintptr(unsafe.Pointer(o)), 0)
+
+	// We expect ERROR_IO_PENDING.
+	if syscall.Errno(r1) != windows.ERROR_IO_PENDING {
+		return 0, nil, nil, e1
 	}
-	return e1
+
+	cancel = func() {
+		cancelIPChangeNotifyProc.Call(uintptr(unsafe.Pointer(o)))
+	}
+	return h, o, cancel, nil
 }

wgengine/packet/ip.go

@@ -47,6 +47,10 @@ func (ip IP) IsLinkLocalUnicast() bool {
 	return byte(ip>>24) == 169 && byte(ip>>16) == 254
 }
 
+func (ip IP) IsMostLinkLocalUnicast() bool {
+	return ip.IsLinkLocalUnicast() && ip != 0xA9FEA9FE
+}
+
 // IPProto is either a real IP protocol (ITCP, UDP, ...) or an special value like Unknown.
 // If it is a real IP protocol, its value corresponds to its IP protocol number.
 type IPProto uint8

wgengine/router/dns/config.go

@@ -63,7 +63,8 @@ func (lhs Config) Equal(rhs Config) bool {
 // ManagerConfig is the set of parameters from which
 // a manager implementation is chosen and initialized.
 type ManagerConfig struct {
-	// logf is the logger for the manager to use.
+	// Logf is the logger for the manager to use.
+	// It is wrapped with a "dns: " prefix.
 	Logf logger.Logf
 	// InterfaceNAme is the name of the interface with which DNS settings should be associated.
 	InterfaceName string

wgengine/router/ifconfig_windows.go

@@ -11,6 +11,7 @@ import (
 	"fmt"
 	"log"
 	"net"
+	"runtime"
 	"sort"
 	"time"
 
@@ -165,6 +166,13 @@ func setPrivateNetwork(ifcGUID *windows.GUID) (bool, error) {
 		categoryPrivate = 1
 		categoryDomain  = 2
 	)
+
+	// Lock OS thread when using OLE, which seems to be a requirement
+	// from the Microsoft docs. go-ole doesn't seem to handle it automatically.
+	// https://github.com/tailscale/tailscale/issues/921#issuecomment-727526807
+	runtime.LockOSThread()
+	defer runtime.UnlockOSThread()
+
 	var c ole.Connection
 	if err := c.Initialize(); err != nil {
 		return false, fmt.Errorf("c.Initialize: %v", err)

wgengine/router/router_linux.go

@@ -1023,6 +1023,20 @@ func supportsV6() bool {
 		return false
 	}
 
+	// Older kernels don't support IPv6 policy routing.
+	bs, err = ioutil.ReadFile("/proc/sys/net/ipv6/conf/all/disable_policy")
+	if err != nil {
+		// Absent knob means policy routing is unsupported.
+		return false
+	}
+	disabled, err = strconv.ParseBool(strings.TrimSpace(string(bs)))
+	if err != nil {
+		return false
+	}
+	if disabled {
+		return false
+	}
+
 	// Some distros ship ip6tables separately from iptables.
 	if _, err := exec.LookPath("ip6tables"); err != nil {
 		return false

wgengine/router/router_windows.go

@@ -39,7 +39,7 @@ func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Devic
 	nativeTun := tundev.(*tun.NativeTun)
 	guid := nativeTun.GUID().String()
 	mconfig := dns.ManagerConfig{
-		Logf:          logger.WithPrefix(logf, "dns: "),
+		Logf:          logf,
 		InterfaceName: guid,
 	}
 

wgengine/userspace.go

@@ -669,7 +669,7 @@ func (e *userspaceEngine) noteReceiveActivity(dk tailcfg.DiscoKey) {
 	// couple minutes (just not on every packet).
 	if e.trimmedDisco[dk] {
 		e.logf("wgengine: idle peer %v now active, reconfiguring wireguard", dk.ShortString())
-		e.maybeReconfigWireguardLocked()
+		e.maybeReconfigWireguardLocked(nil)
 	}
 }
 
@@ -707,8 +707,13 @@ func discoKeyFromPeer(p *wgcfg.Peer) tailcfg.DiscoKey {
 	return tailcfg.DiscoKey(k)
 }
 
+// discoChanged are the set of peers whose disco keys have changed, implying they've restarted.
+// If a peer is in this set and was previously in the live wireguard config,
+// it needs to be first removed and then re-added to flush out its wireguard session key.
+// If discoChanged is nil or empty, this extra removal step isn't done.
+//
 // e.wgLock must be held.
-func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
+func (e *userspaceEngine) maybeReconfigWireguardLocked(discoChanged map[key.Public]bool) error {
 	if hook := e.testMaybeReconfigHook; hook != nil {
 		hook()
 		return nil
@@ -738,10 +743,14 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 
 	trimmedDisco := map[tailcfg.DiscoKey]bool{} // TODO: don't re-alloc this map each time
 
+	needRemoveStep := false
 	for i := range full.Peers {
 		p := &full.Peers[i]
 		if !isTrimmablePeer(p, len(full.Peers)) {
 			min.Peers = append(min.Peers, *p)
+			if discoChanged[key.Public(p.PublicKey)] {
+				needRemoveStep = true
+			}
 			continue
 		}
 		tsIP := p.AllowedIPs[0].IP
@@ -750,6 +759,9 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 		trackIPs = append(trackIPs, tsIP)
 		if e.isActiveSince(dk, tsIP, activeCutoff) {
 			min.Peers = append(min.Peers, *p)
+			if discoChanged[key.Public(p.PublicKey)] {
+				needRemoveStep = true
+			}
 		} else {
 			trimmedDisco[dk] = true
 		}
@@ -764,6 +776,26 @@ func (e *userspaceEngine) maybeReconfigWireguardLocked() error {
 
 	e.updateActivityMapsLocked(trackDisco, trackIPs)
 
+	if needRemoveStep {
+		minner := min
+		minner.Peers = nil
+		numRemove := 0
+		for _, p := range min.Peers {
+			if discoChanged[key.Public(p.PublicKey)] {
+				numRemove++
+				continue
+			}
+			minner.Peers = append(minner.Peers, p)
+		}
+		if numRemove > 0 {
+			e.logf("wgengine: Reconfig: removing session keys for %d peers", numRemove)
+			if err := e.wgdev.Reconfig(&minner); err != nil {
+				e.logf("wgdev.Reconfig: %v", err)
+				return err
+			}
+		}
+	}
+
 	e.logf("wgengine: Reconfig: configuring userspace wireguard config (with %d/%d peers)", len(min.Peers), len(full.Peers))
 	if err := e.wgdev.Reconfig(&min); err != nil {
 		e.logf("wgdev.Reconfig: %v", err)
@@ -823,7 +855,7 @@ func (e *userspaceEngine) updateActivityMapsLocked(trackDisco []tailcfg.DiscoKey
 				if elapsedSec >= int64(packetSendRecheckWireguardThreshold/time.Second) {
 					e.wgLock.Lock()
 					defer e.wgLock.Unlock()
-					e.maybeReconfigWireguardLocked()
+					e.maybeReconfigWireguardLocked(nil)
 				}
 			}
 		}
@@ -864,6 +896,32 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	if !engineChanged && !routerChanged {
 		return ErrNoChanges
 	}
+
+	// See if any peers have changed disco keys, which means they've restarted.
+	// If so, we need to update the wireguard-go/device.Device in two phases:
+	// once without the node which has restarted, to clear its wireguard session key,
+	// and a second time with it.
+	discoChanged := make(map[key.Public]bool)
+	{
+		prevEP := make(map[key.Public]wgcfg.Endpoint)
+		for i := range e.lastCfgFull.Peers {
+			if p := &e.lastCfgFull.Peers[i]; len(p.Endpoints) == 1 {
+				prevEP[key.Public(p.PublicKey)] = p.Endpoints[0]
+			}
+		}
+		for i := range cfg.Peers {
+			p := &cfg.Peers[i]
+			if len(p.Endpoints) != 1 {
+				continue
+			}
+			pub := key.Public(p.PublicKey)
+			if old, ok := prevEP[pub]; ok && old != p.Endpoints[0] {
+				discoChanged[pub] = true
+				e.logf("wgengine: Reconfig: %s changed from %s to %s", pub.ShortString(), &old, &p.Endpoints[0])
+			}
+		}
+	}
+
 	e.lastCfgFull = cfg.Copy()
 
 	// Tell magicsock about the new (or initial) private key
@@ -875,7 +933,7 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	}
 	e.magicConn.UpdatePeers(peerSet)
 
-	if err := e.maybeReconfigWireguardLocked(); err != nil {
+	if err := e.maybeReconfigWireguardLocked(discoChanged); err != nil {
 		return err
 	}