VERSION.txt: this is v1.4.4

Signed-off-by: David Anderson <danderson@tailscale.com>

VERSION.txt

@@ -1 +1 @@
-1.3.0
+1.4.4

cmd/tailscale/cli/status.go

@@ -65,7 +65,17 @@ func runStatus(ctx context.Context, args []string) error {
 			log.Fatal(*n.ErrMessage)
 		}
 		if n.Status != nil {
-			ch <- n.Status
+			select {
+			case ch <- n.Status:
+			default:
+				// A status update from somebody else's request.
+				// Ignoring this matters mostly for "tailscale status -web"
+				// mode, otherwise the channel send would block forever
+				// and pump would stop reading from tailscaled, which
+				// previously caused tailscaled to block (while holding
+				// a mutex), backing up unrelated clients.
+				// See https://github.com/tailscale/tailscale/issues/1234
+			}
 		}
 	})
 	go pump(ctx, bc, c)

cmd/tailscale/cli/up.go

@@ -228,7 +228,16 @@ func runUp(ctx context.Context, args []string) error {
 		AuthKey:  upArgs.authKey,
 		Notify: func(n ipn.Notify) {
 			if n.ErrMessage != nil {
-				fatalf("backend error: %v\n", *n.ErrMessage)
+				msg := *n.ErrMessage
+				if msg == ipn.ErrMsgPermissionDenied {
+					switch runtime.GOOS {
+					case "windows":
+						msg += " (Tailscale service in use by other user?)"
+					default:
+						msg += " (try 'sudo tailscale up [...]')"
+					}
+				}
+				fatalf("backend error: %v\n", msg)
 			}
 			if s := n.State; s != nil {
 				switch *s {

cmd/tailscaled/tailscaled.service

@@ -20,22 +20,5 @@ CacheDirectory=tailscale
 CacheDirectoryMode=0750
 Type=notify
 
-DeviceAllow=/dev/net/tun
-DeviceAllow=/dev/null
-DeviceAllow=/dev/random
-DeviceAllow=/dev/urandom
-DevicePolicy=strict
-LockPersonality=true
-MemoryDenyWriteExecute=true
-PrivateTmp=true
-ProtectClock=true
-ProtectControlGroups=true
-ProtectHome=true
-ProtectKernelTunables=true
-ProtectSystem=strict
-ReadWritePaths=/etc/
-RestrictSUIDSGID=true
-SystemCallArchitectures=native
-
 [Install]
 WantedBy=multi-user.target

control/controlclient/auto.go

@@ -213,7 +213,7 @@ func (c *Client) sendNewMapRequest() {
 	// If we're not already streaming a netmap, or if we're already stuck
 	// in a lite update, then tear down everything and start a new stream
 	// (which starts by sending a new map request)
-	if !c.inPollNetMap || c.inLiteMapUpdate {
+	if !c.inPollNetMap || c.inLiteMapUpdate || !c.loggedIn {
 		c.mu.Unlock()
 		c.cancelMapSafely()
 		return

control/controlclient/direct.go

@@ -550,6 +550,9 @@ func (c *Direct) sendMapRequest(ctx context.Context, maxPolls int, cb func(*Netw
 	everEndpoints := c.everEndpoints
 	c.mu.Unlock()
 
+	if persist.PrivateNodeKey.IsZero() {
+		return errors.New("privateNodeKey is zero")
+	}
 	if backendLogID == "" {
 		return errors.New("hostinfo: BackendLogID missing")
 	}

ipn/message.go

@@ -146,6 +146,10 @@ func (bs *BackendServer) GotFakeCommand(ctx context.Context, cmd *Command) error
 	return bs.GotCommand(ctx, cmd)
 }
 
+// ErrMsgPermissionDenied is the Notify.ErrMessage value used an
+// operation was done from a user/context that didn't have permission.
+const ErrMsgPermissionDenied = "permission denied"
+
 func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error {
 	if cmd.Version != version.Long && !cmd.AllowVersionSkew {
 		vs := fmt.Sprintf("GotCommand: Version mismatch! frontend=%#v backend=%#v",
@@ -178,7 +182,7 @@ func (bs *BackendServer) GotCommand(ctx context.Context, cmd *Command) error {
 	}
 
 	if IsReadonlyContext(ctx) {
-		msg := "permission denied"
+		msg := ErrMsgPermissionDenied
 		bs.send(Notify{ErrMessage: &msg})
 		return nil
 	}

net/interfaces/interfaces_darwin_cgo.go

@@ -15,7 +15,7 @@ package interfaces
 
 // privateGatewayIPFromRoute returns the private gateway ip address from rtm, if it exists.
 // Otherwise, it returns 0.
-int privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
+uint32_t privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
 {
     // sockaddrs are after the message header
     struct sockaddr* dst_sa = (struct sockaddr *)(rtm + 1);
@@ -38,7 +38,7 @@ int privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
         return 0; // gateway not IPv4
 
     struct sockaddr_in* gateway_si= (struct sockaddr_in *)gateway_sa;
-    int ip;
+    uint32_t ip;
     ip = gateway_si->sin_addr.s_addr;
 
     unsigned char a, b;
@@ -62,7 +62,7 @@ int privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
 // If no private gateway IP address was found, it returns 0.
 // On an error, it returns an error code in (0, 255].
 // Any private gateway IP address is > 255.
-int privateGatewayIP()
+uint32_t privateGatewayIP()
 {
     size_t needed;
     int mib[6];
@@ -90,7 +90,7 @@ int privateGatewayIP()
 	struct rt_msghdr2 *rtm;
     for (next = buf; next < lim; next += rtm->rtm_msglen) {
         rtm = (struct rt_msghdr2 *)next;
-		int ip;
+        uint32_t ip;
         ip = privateGatewayIPFromRoute(rtm);
         if (ip) {
             free(buf);

net/packet/tsmp.go

@@ -23,12 +23,14 @@ import (
 // Tailscale node has rejected the connection from another. Unlike a
 // TCP RST, this includes a reason.
 //
-// On the wire, after the IP header, it's currently 7 bytes:
+// On the wire, after the IP header, it's currently 7 or 8 bytes:
 //     * '!'
 //     * IPProto byte (IANA protocol number: TCP or UDP)
 //     * 'A' or 'S' (RejectedDueToACLs, RejectedDueToShieldsUp)
 //     * srcPort big endian uint16
 //     * dstPort big endian uint16
+//     * [optional] byte of flag bits:
+//          lowest bit (0x1): MaybeBroken
 //
 // In the future it might also accept 16 byte IP flow src/dst IPs
 // after the header, if they're different than the IP-level ones.
@@ -39,8 +41,21 @@ type TailscaleRejectedHeader struct {
 	Dst    netaddr.IPPort        // rejected flow's dst
 	Proto  IPProto               // proto that was rejected (TCP or UDP)
 	Reason TailscaleRejectReason // why the connection was rejected
+
+	// MaybeBroken is whether the rejection is non-terminal (the
+	// client should not fail immediately). This is sent by a
+	// target when it's not sure whether it's totally broken, but
+	// it might be. For example, the target tailscaled might think
+	// its host firewall or IP forwarding aren't configured
+	// properly, but tailscaled might be wrong (not having enough
+	// visibility into what the OS is doing). When true, the
+	// message is simply an FYI as a potential reason to use for
+	// later when the pendopen connection tracking timer expires.
+	MaybeBroken bool
 }
 
+const rejectFlagBitMaybeBroken = 0x1
+
 func (rh TailscaleRejectedHeader) Flow() flowtrack.Tuple {
 	return flowtrack.Tuple{Src: rh.Src, Dst: rh.Dst}
 }
@@ -52,14 +67,32 @@ func (rh TailscaleRejectedHeader) String() string {
 type TSMPType uint8
 
 const (
+	// TSMPTypeRejectedConn is the type byte for a TailscaleRejectedHeader.
 	TSMPTypeRejectedConn TSMPType = '!'
 )
 
 type TailscaleRejectReason byte
 
+// IsZero reports whether r is the zero value, representing no rejection.
+func (r TailscaleRejectReason) IsZero() bool { return r == TailscaleRejectReasonNone }
+
 const (
-	RejectedDueToACLs      TailscaleRejectReason = 'A'
+	// TailscaleRejectReasonNone is the TailscaleRejectReason zero value.
+	TailscaleRejectReasonNone TailscaleRejectReason = 0
+
+	// RejectedDueToACLs means that the host rejected the connection due to ACLs.
+	RejectedDueToACLs TailscaleRejectReason = 'A'
+
+	// RejectedDueToShieldsUp means that the host rejected the connection due to shields being up.
 	RejectedDueToShieldsUp TailscaleRejectReason = 'S'
+
+	// RejectedDueToIPForwarding means that the relay node's IP
+	// forwarding is disabled.
+	RejectedDueToIPForwarding TailscaleRejectReason = 'F'
+
+	// RejectedDueToHostFirewall means that the target host's
+	// firewall is blocking the traffic.
+	RejectedDueToHostFirewall TailscaleRejectReason = 'W'
 )
 
 func (r TailscaleRejectReason) String() string {
@@ -68,22 +101,32 @@ func (r TailscaleRejectReason) String() string {
 		return "acl"
 	case RejectedDueToShieldsUp:
 		return "shields"
+	case RejectedDueToIPForwarding:
+		return "host-ip-forwarding-unavailable"
+	case RejectedDueToHostFirewall:
+		return "host-firewall"
 	}
 	return fmt.Sprintf("0x%02x", byte(r))
 }
 
+func (h TailscaleRejectedHeader) hasFlags() bool {
+	return h.MaybeBroken // the only one currently
+}
+
 func (h TailscaleRejectedHeader) Len() int {
-	var ipHeaderLen int
-	if h.IPSrc.Is4() {
-		ipHeaderLen = ip4HeaderLength
-	} else if h.IPSrc.Is6() {
-		ipHeaderLen = ip6HeaderLength
-	}
-	return ipHeaderLen +
-		1 + // TSMPType byte
+	v := 1 + // TSMPType byte
 		1 + // IPProto byte
 		1 + // TailscaleRejectReason byte
 		2*2 // 2 uint16 ports
+	if h.IPSrc.Is4() {
+		v += ip4HeaderLength
+	} else if h.IPSrc.Is6() {
+		v += ip6HeaderLength
+	}
+	if h.hasFlags() {
+		v++
+	}
+	return v
 }
 
 func (h TailscaleRejectedHeader) Marshal(buf []byte) error {
@@ -117,6 +160,14 @@ func (h TailscaleRejectedHeader) Marshal(buf []byte) error {
 	buf[2] = byte(h.Reason)
 	binary.BigEndian.PutUint16(buf[3:5], h.Src.Port)
 	binary.BigEndian.PutUint16(buf[5:7], h.Dst.Port)
+
+	if h.hasFlags() {
+		var flags byte
+		if h.MaybeBroken {
+			flags |= rejectFlagBitMaybeBroken
+		}
+		buf[7] = flags
+	}
 	return nil
 }
 
@@ -129,12 +180,17 @@ func (pp *Parsed) AsTailscaleRejectedHeader() (h TailscaleRejectedHeader, ok boo
 	if len(p) < 7 || p[0] != byte(TSMPTypeRejectedConn) {
 		return
 	}
-	return TailscaleRejectedHeader{
+	h = TailscaleRejectedHeader{
 		Proto:  IPProto(p[1]),
 		Reason: TailscaleRejectReason(p[2]),
 		IPSrc:  pp.Src.IP,
 		IPDst:  pp.Dst.IP,
 		Src:    netaddr.IPPort{IP: pp.Dst.IP, Port: binary.BigEndian.Uint16(p[3:5])},
 		Dst:    netaddr.IPPort{IP: pp.Src.IP, Port: binary.BigEndian.Uint16(p[5:7])},
-	}, true
+	}
+	if len(p) > 7 {
+		flags := p[7]
+		h.MaybeBroken = (flags & rejectFlagBitMaybeBroken) != 0
+	}
+	return h, true
 }

net/packet/tsmp_test.go

@@ -37,6 +37,18 @@ func TestTailscaleRejectedHeader(t *testing.T) {
 			},
 			wantStr: "TSMP-reject-flow{UDP [1::1]:567 > [2::2]:443}: shields",
 		},
+		{
+			h: TailscaleRejectedHeader{
+				IPSrc:       netaddr.MustParseIP("2::2"),
+				IPDst:       netaddr.MustParseIP("1::1"),
+				Src:         netaddr.MustParseIPPort("[1::1]:567"),
+				Dst:         netaddr.MustParseIPPort("[2::2]:443"),
+				Proto:       UDP,
+				Reason:      RejectedDueToIPForwarding,
+				MaybeBroken: true,
+			},
+			wantStr: "TSMP-reject-flow{UDP [1::1]:567 > [2::2]:443}: host-ip-forwarding-unavailable",
+		},
 	}
 	for i, tt := range tests {
 		gotStr := tt.h.String()

types/logger/logger.go

@@ -64,9 +64,9 @@ type limitData struct {
 
 var disableRateLimit = os.Getenv("TS_DEBUG_LOG_RATE") == "all"
 
-// rateFreePrefix are format string prefixes that are exempt from rate limiting.
+// rateFree are format string substrings that are exempt from rate limiting.
 // Things should not be added to this unless they're already limited otherwise.
-var rateFreePrefix = []string{
+var rateFree = []string{
 	"magicsock: disco: ",
 	"magicsock: CreateEndpoint:",
 }
@@ -93,8 +93,8 @@ func RateLimitedFn(logf Logf, f time.Duration, burst int, maxCache int) Logf {
 	)
 
 	judge := func(format string) verdict {
-		for _, pfx := range rateFreePrefix {
-			if strings.HasPrefix(format, pfx) {
+		for _, sub := range rateFree {
+			if strings.Contains(format, sub) {
 				return allow
 			}
 		}

wgengine/magicsock/magicsock.go

@@ -12,6 +12,7 @@ import (
 	crand "crypto/rand"
 	"encoding/binary"
 	"errors"
+	"expvar"
 	"fmt"
 	"hash/fnv"
 	"math"
@@ -153,13 +154,10 @@ type Conn struct {
 	// derpRecvCh is used by ReceiveIPv4 to read DERP messages.
 	derpRecvCh chan derpReadResult
 
-	// derpRecvCountAtomic is atomically incremented by runDerpReader whenever
-	// a DERP message arrives. It's incremented before runDerpReader is interrupted.
+	// derpRecvCountAtomic is how many derpRecvCh sends are pending.
+	// It's incremented by runDerpReader whenever a DERP message
+	// arrives and decremented when they're read.
 	derpRecvCountAtomic int64
-	// derpRecvCountLast is used by ReceiveIPv4 to compare against
-	// its last read value of derpRecvCountAtomic to determine
-	// whether a DERP channel read should be done.
-	derpRecvCountLast int64 // owned by ReceiveIPv4
 
 	// ippEndpoint4 and ippEndpoint6 are owned by ReceiveIPv4 and
 	// ReceiveIPv6, respectively, to cache an IPPort->endpoint for
@@ -304,6 +302,9 @@ type Conn struct {
 	// with IPv4 or IPv6). It's used to suppress log spam and prevent
 	// new connection that'll fail.
 	networkUp syncs.AtomicBool
+
+	// havePrivateKey is whether privateKey is non-zero.
+	havePrivateKey syncs.AtomicBool
 }
 
 // derpRoute is a route entry for a public key, saying that a certain
@@ -960,6 +961,13 @@ func (c *Conn) setNearestDERP(derpNum int) (wantDERP bool) {
 	return true
 }
 
+// startDerpHomeConnectLocked starts connecting to our DERP home, if any.
+//
+// c.mu must be held.
+func (c *Conn) startDerpHomeConnectLocked() {
+	c.goDerpConnect(c.myDerp)
+}
+
 // goDerpConnect starts a goroutine to start connecting to the given
 // DERP node.
 //
@@ -1353,6 +1361,8 @@ type derpReadResult struct {
 	// copyBuf is called to copy the data to dst.  It returns how
 	// much data was copied, which will be n if dst is large
 	// enough. copyBuf can only be called once.
+	// If copyBuf is nil, that's a signal from the sender to ignore
+	// this message.
 	copyBuf func(dst []byte) int
 }
 
@@ -1440,28 +1450,62 @@ func (c *Conn) runDerpReader(ctx context.Context, derpFakeAddr netaddr.IPPort, d
 			continue
 
 		}
-		// Before we wake up ReceiveIPv4 with SetReadDeadline,
-		// note that a DERP packet has arrived.  ReceiveIPv4
-		// will read this field to note that its UDP read
-		// error is due to us.
-		atomic.AddInt64(&c.derpRecvCountAtomic, 1)
-		// Cancel the pconn read goroutine.
-		c.pconn4.SetReadDeadline(aLongTimeAgo)
 
+		if !c.sendDerpReadResult(ctx, res) {
+			return
+		}
 		select {
 		case <-ctx.Done():
 			return
-		case c.derpRecvCh <- res:
-			select {
-			case <-ctx.Done():
-				return
-			case <-didCopy:
-				continue
-			}
+		case <-didCopy:
+			continue
 		}
 	}
 }
 
+var (
+	testCounterZeroDerpReadResultSend expvar.Int
+	testCounterZeroDerpReadResultRecv expvar.Int
+)
+
+// sendDerpReadResult sends res to c.derpRecvCh and reports whether it
+// was sent. (It reports false if ctx was done first.)
+//
+// This includes doing the whole wake-up dance to interrupt
+// ReceiveIPv4's blocking UDP read.
+func (c *Conn) sendDerpReadResult(ctx context.Context, res derpReadResult) (sent bool) {
+	// Before we wake up ReceiveIPv4 with SetReadDeadline,
+	// note that a DERP packet has arrived. ReceiveIPv4
+	// will read this field to note that its UDP read
+	// error is due to us.
+	atomic.AddInt64(&c.derpRecvCountAtomic, 1)
+	// Cancel the pconn read goroutine.
+	c.pconn4.SetReadDeadline(aLongTimeAgo)
+	select {
+	case <-ctx.Done():
+		select {
+		case <-c.donec:
+			// The whole Conn shut down. The reader of
+			// c.derpRecvCh also selects on c.donec, so it's
+			// safe to abort now.
+		case c.derpRecvCh <- (derpReadResult{}):
+			// Just this DERP reader is closing (perhaps
+			// the user is logging out, or the DERP
+			// connection is too idle for sends). Since we
+			// already incremented c.derpRecvCountAtomic,
+			// we need to send on the channel (unless the
+			// conn is going down).
+			// The receiver treats a derpReadResult zero value
+			// message as a skip.
+			testCounterZeroDerpReadResultSend.Add(1)
+
+		}
+		return false
+	case c.derpRecvCh <- res:
+		return true
+	}
+}
+
 type derpWriteRequest struct {
 	addr   netaddr.IPPort
 	pubKey key.Public
@@ -1551,20 +1595,20 @@ func (c *Conn) ReceiveIPv6(b []byte) (int, conn.Endpoint, error) {
 }
 
 func (c *Conn) derpPacketArrived() bool {
-	rc := atomic.LoadInt64(&c.derpRecvCountAtomic)
-	if rc != c.derpRecvCountLast {
-		c.derpRecvCountLast = rc
-		return true
-	}
-	return false
+	return atomic.LoadInt64(&c.derpRecvCountAtomic) > 0
 }
 
 // ReceiveIPv4 is called by wireguard-go to receive an IPv4 packet.
 // In Tailscale's case, that packet might also arrive via DERP. A DERP packet arrival
 // aborts the pconn4 read deadline to make it fail.
 func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, err error) {
+	var pAddr net.Addr
 	for {
-		n, pAddr, err := c.pconn4.ReadFrom(b)
+		// Drain DERP queues before reading new UDP packets.
+		if c.derpPacketArrived() {
+			goto ReadDERP
+		}
+		n, pAddr, err = c.pconn4.ReadFrom(b)
 		if err != nil {
 			// If the pconn4 read failed, the likely reason is a DERP reader received
 			// a packet and interrupted us.
@@ -1572,22 +1616,28 @@ func (c *Conn) ReceiveIPv4(b []byte) (n int, ep conn.Endpoint, err error) {
 			// and for there to have also had a DERP packet arrive, but that's fine:
 			// we'll get the same error from ReadFrom later.
 			if c.derpPacketArrived() {
-				c.pconn4.SetReadDeadline(time.Time{}) // restore
-				n, ep, err = c.receiveIPv4DERP(b)
-				if err == errLoopAgain {
-					continue
-				}
-				return n, ep, err
+				goto ReadDERP
 			}
 			return 0, nil, err
 		}
 		if ep, ok := c.receiveIP(b[:n], pAddr.(*net.UDPAddr), &c.ippEndpoint4); ok {
 			return n, ep, nil
+		} else {
+			continue
 		}
+	ReadDERP:
+		n, ep, err = c.receiveIPv4DERP(b)
+		if err == errLoopAgain {
+			continue
+		}
+		return n, ep, err
 	}
 }
 
 // receiveIP is the shared bits of ReceiveIPv4 and ReceiveIPv6.
+//
+// ok is whether this read should be reported up to wireguard-go (our
+// caller).
 func (c *Conn) receiveIP(b []byte, ua *net.UDPAddr, cache *ippEndpointCache) (ep conn.Endpoint, ok bool) {
 	ipp, ok := netaddr.FromStdAddr(ua.IP, ua.Port, ua.Zone)
 	if !ok {
@@ -1600,6 +1650,13 @@ func (c *Conn) receiveIP(b []byte, ua *net.UDPAddr, cache *ippEndpointCache) (ep
 	if c.handleDiscoMessage(b, ipp) {
 		return nil, false
 	}
+	if !c.havePrivateKey.Get() {
+		// If we have no private key, we're logged out or
+		// stopped.  Don't try to pass these wireguard packets
+		// up to wireguard-go; it'll just complain (Issue
+		// 1167).
+		return nil, false
+	}
 	if cache.ipp == ipp && cache.de != nil && cache.gen == cache.de.numStopAndReset() {
 		ep = cache.de
 	} else {
@@ -1641,6 +1698,13 @@ func (c *Conn) receiveIPv4DERP(b []byte) (n int, ep conn.Endpoint, err error) {
 	case dm = <-c.derpRecvCh:
 		// Below.
 	}
+	if atomic.AddInt64(&c.derpRecvCountAtomic, -1) == 0 {
+		c.pconn4.SetReadDeadline(time.Time{})
+	}
+	if dm.copyBuf == nil {
+		testCounterZeroDerpReadResultRecv.Add(1)
+		return 0, nil, errLoopAgain
+	}
 
 	var regionID int
 	n, regionID = dm.n, dm.regionID
@@ -1750,8 +1814,8 @@ func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey tailcfg.NodeKey, dstD
 	return sent, err
 }
 
-// handleDiscoMessage reports whether msg was a Tailscale inter-node discovery message
-// that was handled.
+// handleDiscoMessage handles a discovery message and reports whether
+// msg was a Tailscale inter-node discovery message.
 //
 // A discovery message has the form:
 //
@@ -1762,11 +1826,18 @@ func (c *Conn) sendDiscoMessage(dst netaddr.IPPort, dstKey tailcfg.NodeKey, dstD
 //
 // For messages received over DERP, the addr will be derpMagicIP (with
 // port being the region)
-func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
+func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) (isDiscoMsg bool) {
 	const headerLen = len(disco.Magic) + len(tailcfg.DiscoKey{}) + disco.NonceLen
 	if len(msg) < headerLen || string(msg[:len(disco.Magic)]) != disco.Magic {
 		return false
 	}
+
+	// If the first four parts are the prefix of disco.Magic
+	// (0x5453f09f) then it's definitely not a valid Wireguard
+	// packet (which starts with little-endian uint32 1, 2, 3, 4).
+	// Use naked returns for all following paths.
+	isDiscoMsg = true
+
 	var sender tailcfg.DiscoKey
 	copy(sender[:], msg[len(disco.Magic):])
 
@@ -1774,20 +1845,21 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 	defer c.mu.Unlock()
 
 	if c.closed {
-		return true
+		return
 	}
 	if debugDisco {
 		c.logf("magicsock: disco: got disco-looking frame from %v", sender.ShortString())
 	}
 	if c.privateKey.IsZero() {
 		// Ignore disco messages when we're stopped.
-		return false
+		// Still return true, to not pass it down to wireguard.
+		return
 	}
 	if c.discoPrivate.IsZero() {
 		if debugDisco {
 			c.logf("magicsock: disco: ignoring disco-looking frame, no local key")
 		}
-		return false
+		return
 	}
 
 	peerNode, ok := c.nodeOfDisco[sender]
@@ -1795,9 +1867,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 		if debugDisco {
 			c.logf("magicsock: disco: ignoring disco-looking frame, don't know node for %v", sender.ShortString())
 		}
-		// Returning false keeps passing it down, to WireGuard.
-		// WireGuard will almost surely reject it, but give it a chance.
-		return false
+		return
 	}
 
 	needsRecvActivityCall := false
@@ -1810,7 +1880,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 		c.logf("magicsock: got disco message from idle peer, starting lazy conf for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
 		if c.noteRecvActivity == nil {
 			c.logf("magicsock: [unexpected] have node without endpoint, without c.noteRecvActivity hook")
-			return false
+			return
 		}
 		needsRecvActivityCall = true
 	} else {
@@ -1829,7 +1899,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 		// Now, recheck invariants that might've changed while we'd
 		// released the lock, which isn't much:
 		if c.closed || c.privateKey.IsZero() {
-			return true
+			return
 		}
 		de, ok = c.endpointOfDisco[sender]
 		if !ok {
@@ -1838,7 +1908,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 				return false
 			}
 			c.logf("magicsock: [unexpected] lazy endpoint not created for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
-			return false
+			return
 		}
 		if !endpointFound0 {
 			c.logf("magicsock: lazy endpoint created via disco message for %v, %v", peerNode.Key.ShortString(), sender.ShortString())
@@ -1865,7 +1935,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 			c.logf("magicsock: disco: failed to open naclbox from %v (wrong rcpt?)", sender)
 		}
 		// TODO(bradfitz): add some counter for this that logs rarely
-		return false
+		return
 	}
 
 	dm, err := disco.Parse(payload)
@@ -1879,7 +1949,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 		// understand. Not even worth logging about, lest it
 		// be too spammy for old clients.
 		// TODO(bradfitz): add some counter for this that logs rarely
-		return true
+		return
 	}
 
 	switch dm := dm.(type) {
@@ -1887,14 +1957,14 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 		c.handlePingLocked(dm, de, src, sender, peerNode)
 	case *disco.Pong:
 		if de == nil {
-			return true
+			return
 		}
 		de.handlePongConnLocked(dm, src)
 	case *disco.CallMeMaybe:
 		if src.IP != derpMagicIPAddr {
 			// CallMeMaybe messages should only come via DERP.
 			c.logf("[unexpected] CallMeMaybe packets should only come via DERP")
-			return true
+			return
 		}
 		if de != nil {
 			c.logf("magicsock: disco: %v<-%v (%v, %v)  got call-me-maybe, %d endpoints",
@@ -1904,8 +1974,7 @@ func (c *Conn) handleDiscoMessage(msg []byte, src netaddr.IPPort) bool {
 			go de.handleCallMeMaybe(dm)
 		}
 	}
-
-	return true
+	return
 }
 
 func (c *Conn) handlePingLocked(dm *disco.Ping, de *discoEndpoint, src netaddr.IPPort, sender tailcfg.DiscoKey, peerNode *tailcfg.Node) {
@@ -2061,7 +2130,9 @@ func (c *Conn) SetNetworkUp(up bool) {
 	c.logf("magicsock: SetNetworkUp(%v)", up)
 	c.networkUp.Set(up)
 
-	if !up {
+	if up {
+		c.startDerpHomeConnectLocked()
+	} else {
 		c.closeAllDerpLocked("network-down")
 	}
 }
@@ -2082,6 +2153,7 @@ func (c *Conn) SetPrivateKey(privateKey wgkey.Private) error {
 		return nil
 	}
 	c.privateKey = newKey
+	c.havePrivateKey.Set(!newKey.IsZero())
 
 	if oldKey.IsZero() {
 		c.everHadKey = true
@@ -2102,7 +2174,7 @@ func (c *Conn) SetPrivateKey(privateKey wgkey.Private) error {
 	// Key changed. Close existing DERP connections and reconnect to home.
 	if c.myDerp != 0 && !newKey.IsZero() {
 		c.logf("magicsock: private key changed, reconnecting to home derp-%d", c.myDerp)
-		c.goDerpConnect(c.myDerp)
+		c.startDerpHomeConnectLocked()
 	}
 
 	if newKey.IsZero() {
@@ -2565,12 +2637,11 @@ func (c *Conn) Rebind() {
 
 	c.mu.Lock()
 	c.closeAllDerpLocked("rebind")
-	haveKey := !c.privateKey.IsZero()
+	if !c.privateKey.IsZero() {
+		c.startDerpHomeConnectLocked()
+	}
 	c.mu.Unlock()
 
-	if haveKey {
-		c.goDerpConnect(c.myDerp)
-	}
 	c.resetEndpointStates()
 }
 

wgengine/magicsock/magicsock_test.go

@@ -17,6 +17,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"runtime"
 	"strconv"
 	"strings"
 	"sync"
@@ -1410,19 +1411,136 @@ func Test32bitAlignment(t *testing.T) {
 	atomic.AddInt64(&c.derpRecvCountAtomic, 1)
 }
 
-func BenchmarkReceiveFrom(b *testing.B) {
-	port := pickPort(b)
+// newNonLegacyTestConn returns a new Conn with DisableLegacyNetworking set true.
+func newNonLegacyTestConn(t testing.TB) *Conn {
+	t.Helper()
+	port := pickPort(t)
 	conn, err := NewConn(Options{
-		Logf: b.Logf,
+		Logf: t.Logf,
 		Port: port,
 		EndpointsFunc: func(eps []string) {
-			b.Logf("endpoints: %q", eps)
+			t.Logf("endpoints: %q", eps)
 		},
 		DisableLegacyNetworking: true,
 	})
 	if err != nil {
-		b.Fatal(err)
+		t.Fatal(err)
 	}
+	return conn
+}
+
+// Tests concurrent DERP readers pushing DERP data into ReceiveIPv4
+// (which should blend all DERP reads into UDP reads).
+func TestDerpReceiveFromIPv4(t *testing.T) {
+	conn := newNonLegacyTestConn(t)
+	defer conn.Close()
+
+	sendConn, err := net.ListenPacket("udp4", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer sendConn.Close()
+	nodeKey, _ := addTestEndpoint(conn, sendConn)
+
+	var sends int = 250e3 // takes about a second
+	if testing.Short() {
+		sends /= 10
+	}
+	senders := runtime.NumCPU()
+	sends -= (sends % senders)
+	var wg sync.WaitGroup
+	defer wg.Wait()
+	t.Logf("doing %v sends over %d senders", sends, senders)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer conn.Close()
+	defer cancel()
+
+	doneCtx, cancelDoneCtx := context.WithCancel(context.Background())
+	cancelDoneCtx()
+
+	for i := 0; i < senders; i++ {
+		wg.Add(1)
+		regionID := i + 1
+		go func() {
+			defer wg.Done()
+			for i := 0; i < sends/senders; i++ {
+				res := derpReadResult{
+					regionID: regionID,
+					n:        123,
+					src:      key.Public(nodeKey),
+					copyBuf:  func(dst []byte) int { return 123 },
+				}
+				// First send with the closed context. ~50% of
+				// these should end up going through the
+				// send-a-zero-derpReadResult path, returning
+				// true, in which case we don't want to send again.
+				// We test later that we hit the other path.
+				if conn.sendDerpReadResult(doneCtx, res) {
+					continue
+				}
+
+				if !conn.sendDerpReadResult(ctx, res) {
+					t.Error("unexpected false")
+					return
+				}
+			}
+		}()
+	}
+
+	zeroSendsStart := testCounterZeroDerpReadResultSend.Value()
+
+	buf := make([]byte, 1500)
+	for i := 0; i < sends; i++ {
+		n, ep, err := conn.ReceiveIPv4(buf)
+		if err != nil {
+			t.Fatal(err)
+		}
+		_ = n
+		_ = ep
+	}
+
+	t.Logf("did %d ReceiveIPv4 calls", sends)
+
+	zeroSends, zeroRecv := testCounterZeroDerpReadResultSend.Value(), testCounterZeroDerpReadResultRecv.Value()
+	if zeroSends != zeroRecv {
+		t.Errorf("did %d zero sends != %d corresponding receives", zeroSends, zeroRecv)
+	}
+	zeroSendDelta := zeroSends - zeroSendsStart
+	if zeroSendDelta == 0 {
+		t.Errorf("didn't see any sends of derpReadResult zero value")
+	}
+	if zeroSendDelta == int64(sends) {
+		t.Errorf("saw %v sends of the derpReadResult zero value which was unexpectedly high (100%% of our %v sends)", zeroSendDelta, sends)
+	}
+}
+
+// addTestEndpoint sets conn's network map to a single peer expected
+// to receive packets from sendConn (or DERP), and returns that peer's
+// nodekey and discokey.
+func addTestEndpoint(conn *Conn, sendConn net.PacketConn) (tailcfg.NodeKey, tailcfg.DiscoKey) {
+	// Give conn just enough state that it'll recognize sendConn as a
+	// valid peer and not fall through to the legacy magicsock
+	// codepath.
+	discoKey := tailcfg.DiscoKey{31: 1}
+	nodeKey := tailcfg.NodeKey{0: 'N', 1: 'K'}
+	conn.SetNetworkMap(&controlclient.NetworkMap{
+		Peers: []*tailcfg.Node{
+			{
+				Key:       nodeKey,
+				DiscoKey:  discoKey,
+				Endpoints: []string{sendConn.LocalAddr().String()},
+			},
+		},
+	})
+	conn.SetPrivateKey(wgkey.Private{0: 1})
+	conn.CreateEndpoint([32]byte(nodeKey), "0000000000000000000000000000000000000000000000000000000000000001.disco.tailscale:12345")
+	conn.addValidDiscoPathForTest(discoKey, netaddr.MustParseIPPort(sendConn.LocalAddr().String()))
+	return nodeKey, discoKey
+}
+
+func BenchmarkReceiveFrom(b *testing.B) {
+	conn := newNonLegacyTestConn(b)
 	defer conn.Close()
 
 	sendConn, err := net.ListenPacket("udp4", "127.0.0.1:0")
@@ -1431,20 +1549,7 @@ func BenchmarkReceiveFrom(b *testing.B) {
 	}
 	defer sendConn.Close()
 
-	// Give conn just enough state that it'll recognize sendConn as a
-	// valid peer and not fall through to the legacy magicsock
-	// codepath.
-	discoKey := tailcfg.DiscoKey{31: 1}
-	conn.SetNetworkMap(&controlclient.NetworkMap{
-		Peers: []*tailcfg.Node{
-			{
-				DiscoKey:  discoKey,
-				Endpoints: []string{sendConn.LocalAddr().String()},
-			},
-		},
-	})
-	conn.CreateEndpoint([32]byte{1: 1}, "0000000000000000000000000000000000000000000000000000000000000001.disco.tailscale:12345")
-	conn.addValidDiscoPathForTest(discoKey, netaddr.MustParseIPPort(sendConn.LocalAddr().String()))
+	addTestEndpoint(conn, sendConn)
 
 	var dstAddr net.Addr = conn.pconn4.LocalAddr()
 	sendBuf := make([]byte, 1<<10)

wgengine/pendopen.go

@@ -30,6 +30,12 @@ func debugConnectFailures() bool {
 
 type pendingOpenFlow struct {
 	timer *time.Timer // until giving up on the flow
+
+	// guarded by userspaceEngine.mu:
+
+	// problem is non-zero if we got a MaybeBroken (non-terminal)
+	// TSMP "reject" header.
+	problem packet.TailscaleRejectReason
 }
 
 func (e *userspaceEngine) removeFlow(f flowtrack.Tuple) (removed bool) {
@@ -45,6 +51,17 @@ func (e *userspaceEngine) removeFlow(f flowtrack.Tuple) (removed bool) {
 	return true
 }
 
+func (e *userspaceEngine) noteFlowProblemFromPeer(f flowtrack.Tuple, problem packet.TailscaleRejectReason) {
+	e.mu.Lock()
+	defer e.mu.Unlock()
+	of, ok := e.pendOpen[f]
+	if !ok {
+		// Not a tracked flow (likely already removed)
+		return
+	}
+	of.problem = problem
+}
+
 func (e *userspaceEngine) trackOpenPreFilterIn(pp *packet.Parsed, t *tstun.TUN) (res filter.Response) {
 	res = filter.Accept // always
 
@@ -54,7 +71,9 @@ func (e *userspaceEngine) trackOpenPreFilterIn(pp *packet.Parsed, t *tstun.TUN)
 		if !ok {
 			return
 		}
-		if f := rh.Flow(); e.removeFlow(f) {
+		if rh.MaybeBroken {
+			e.noteFlowProblemFromPeer(rh.Flow(), rh.Reason)
+		} else if f := rh.Flow(); e.removeFlow(f) {
 			e.logf("open-conn-track: flow %v %v > %v rejected due to %v", rh.Proto, rh.Src, rh.Dst, rh.Reason)
 		}
 		return
@@ -106,14 +125,20 @@ func (e *userspaceEngine) trackOpenPostFilterOut(pp *packet.Parsed, t *tstun.TUN
 
 func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) {
 	e.mu.Lock()
-	if _, ok := e.pendOpen[flow]; !ok {
+	of, ok := e.pendOpen[flow]
+	if !ok {
 		// Not a tracked flow, or already handled & deleted.
 		e.mu.Unlock()
 		return
 	}
 	delete(e.pendOpen, flow)
+	problem := of.problem
 	e.mu.Unlock()
 
+	if !problem.IsZero() {
+		e.logf("open-conn-track: timeout opening %v; peer reported problem: %v", flow, problem)
+	}
+
 	// Diagnose why it might've timed out.
 	n, ok := e.magicConn.PeerForIP(flow.Dst.IP)
 	if !ok {

wgengine/router/router_linux.go

@@ -116,7 +116,7 @@ func newUserspaceRouter(logf logger.Logf, _ *device.Device, tunDev tun.Device) (
 
 	v6err := checkIPv6()
 	if v6err != nil {
-		logf("disabling IPv6 due to system IPv6 config: %v", v6err)
+		logf("disabling tunneled IPv6 due to system IPv6 config: %v", v6err)
 	}
 	supportsV6 := v6err == nil
 	supportsV6NAT := supportsV6 && supportsV6NAT()
@@ -366,7 +366,9 @@ func (r *linuxRouter) setNetfilterMode(mode NetfilterMode) error {
 // address is already assigned to the interface, or if the addition
 // fails.
 func (r *linuxRouter) addAddress(addr netaddr.IPPrefix) error {
-
+	if !r.v6Available && addr.IP.Is6() {
+		return nil
+	}
 	if err := r.cmd.run("ip", "addr", "add", addr.String(), "dev", r.tunname); err != nil {
 		return fmt.Errorf("adding address %q to tunnel interface: %w", addr, err)
 	}
@@ -380,6 +382,9 @@ func (r *linuxRouter) addAddress(addr netaddr.IPPrefix) error {
 // the address is not assigned to the interface, or if the removal
 // fails.
 func (r *linuxRouter) delAddress(addr netaddr.IPPrefix) error {
+	if !r.v6Available && addr.IP.Is6() {
+		return nil
+	}
 	if err := r.delLoopbackRule(addr.IP); err != nil {
 		return err
 	}
@@ -437,6 +442,9 @@ func (r *linuxRouter) delLoopbackRule(addr netaddr.IP) error {
 // interface. Fails if the route already exists, or if adding the
 // route fails.
 func (r *linuxRouter) addRoute(cidr netaddr.IPPrefix) error {
+	if !r.v6Available && cidr.IP.Is6() {
+		return nil
+	}
 	args := []string{
 		"ip", "route", "add",
 		normalizeCIDR(cidr),
@@ -452,6 +460,9 @@ func (r *linuxRouter) addRoute(cidr netaddr.IPPrefix) error {
 // interface. Fails if the route doesn't exist, or if removing the
 // route fails.
 func (r *linuxRouter) delRoute(cidr netaddr.IPPrefix) error {
+	if !r.v6Available && cidr.IP.Is6() {
+		return nil
+	}
 	args := []string{
 		"ip", "route", "del",
 		normalizeCIDR(cidr),
@@ -1034,18 +1045,22 @@ func checkIPv6() error {
 		return errors.New("disable_ipv6 is set")
 	}
 
-	// Older kernels don't support IPv6 policy routing.
+	// Older kernels don't support IPv6 policy routing. Some kernels
+	// support policy routing but don't have this knob, so absence of
+	// the knob is not fatal.
 	bs, err = ioutil.ReadFile("/proc/sys/net/ipv6/conf/all/disable_policy")
-	if err != nil {
-		// Absent knob means policy routing is unsupported.
-		return err
+	if err == nil {
+		disabled, err = strconv.ParseBool(strings.TrimSpace(string(bs)))
+		if err != nil {
+			return errors.New("disable_policy has invalid bool")
+		}
+		if disabled {
+			return errors.New("disable_policy is set")
+		}
 	}
-	disabled, err = strconv.ParseBool(strings.TrimSpace(string(bs)))
-	if err != nil {
-		return errors.New("disable_policy has invalid bool")
-	}
-	if disabled {
-		return errors.New("disable_policy is set")
+
+	if err := checkIPRuleSupportsV6(); err != nil {
+		return fmt.Errorf("kernel doesn't support IPv6 policy routing: %w", err)
 	}
 
 	// Some distros ship ip6tables separately from iptables.
@@ -1053,10 +1068,6 @@ func checkIPv6() error {
 		return err
 	}
 
-	if err := checkIPRuleSupportsV6(); err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -1077,13 +1088,17 @@ func supportsV6NAT() bool {
 }
 
 func checkIPRuleSupportsV6() error {
-	// First add a rule for "ip rule del" to delete.
-	// We ignore the "add" operation's error because it can also
-	// fail if the rule already exists.
-	exec.Command("ip", "-6", "rule", "add",
-		"pref", "123", "fwmark", tailscaleBypassMark, "table", fmt.Sprint(tailscaleRouteTable)).Run()
-	out, err := exec.Command("ip", "-6", "rule", "del",
-		"pref", "123", "fwmark", tailscaleBypassMark, "table", fmt.Sprint(tailscaleRouteTable)).CombinedOutput()
+	add := []string{"-6", "rule", "add", "pref", "1234", "fwmark", tailscaleBypassMark, "table", tailscaleRouteTable}
+	del := []string{"-6", "rule", "del", "pref", "1234", "fwmark", tailscaleBypassMark, "table", tailscaleRouteTable}
+
+	// First delete the rule unconditionally, and don't check for
+	// errors. This is just cleaning up anything that might be already
+	// there.
+	exec.Command("ip", del...).Run()
+
+	// Try adding the rule. This will fail on systems that support
+	// IPv6, but not IPv6 policy routing.
+	out, err := exec.Command("ip", add...).CombinedOutput()
 	if err != nil {
 		out = bytes.TrimSpace(out)
 		var detail interface{} = out
@@ -1092,5 +1107,8 @@ func checkIPRuleSupportsV6() error {
 		}
 		return fmt.Errorf("ip -6 rule failed: %s", detail)
 	}
+
+	// Delete again.
+	exec.Command("ip", del...).Run()
 	return nil
 }

wgengine/router/router_windows.go

@@ -7,6 +7,7 @@ package router
 import (
 	"context"
 	"fmt"
+	"os"
 	"os/exec"
 	"sync"
 	"syscall"
@@ -121,11 +122,12 @@ func cleanup(logf logger.Logf, interfaceName string) {
 type firewallTweaker struct {
 	logf logger.Logf
 
-	mu      sync.Mutex
-	running bool     // doAsyncSet goroutine is running
-	known   bool     // firewall is in known state (in lastVal)
-	want    []string // next value we want, or "" to delete the firewall rule
-	lastVal []string // last set value, if known
+	mu          sync.Mutex
+	didProcRule bool
+	running     bool     // doAsyncSet goroutine is running
+	known       bool     // firewall is in known state (in lastVal)
+	want        []string // next value we want, or "" to delete the firewall rule
+	lastVal     []string // last set value, if known
 }
 
 func (ft *firewallTweaker) clear() { ft.set(nil) }
@@ -177,6 +179,7 @@ func (ft *firewallTweaker) doAsyncSet() {
 			return
 		}
 		needClear := !ft.known || len(ft.lastVal) > 0 || len(val) == 0
+		needProcRule := !ft.didProcRule
 		ft.mu.Unlock()
 
 		if needClear {
@@ -189,6 +192,37 @@ func (ft *firewallTweaker) doAsyncSet() {
 			d, _ := ft.runFirewall("delete", "rule", "name=Tailscale-In", "dir=in")
 			ft.logf("cleared Tailscale-In firewall rules in %v", d)
 		}
+		if needProcRule {
+			ft.logf("deleting any prior Tailscale-Process rule...")
+			d, err := ft.runFirewall("delete", "rule", "name=Tailscale-Process", "dir=in") // best effort
+			if err == nil {
+				ft.logf("removed old Tailscale-Process rule in %v", d)
+			}
+			var exe string
+			exe, err = os.Executable()
+			if err != nil {
+				ft.logf("failed to find Executable for Tailscale-Process rule: %v", err)
+			} else {
+				ft.logf("adding Tailscale-Process rule to allow UDP for %q ...", exe)
+				d, err = ft.runFirewall("add", "rule", "name=Tailscale-Process",
+					"dir=in",
+					"action=allow",
+					"edge=yes",
+					"program="+exe,
+					"protocol=udp",
+					"profile=any",
+					"enable=yes",
+				)
+				if err != nil {
+					ft.logf("error adding Tailscale-Process rule: %v", err)
+				} else {
+					ft.mu.Lock()
+					ft.didProcRule = true
+					ft.mu.Unlock()
+					ft.logf("added Tailscale-Process rule in %v", d)
+				}
+			}
+		}
 		var err error
 		for _, cidr := range val {
 			ft.logf("adding Tailscale-In rule to allow %v ...", cidr)