net/packet: add some more TSMP packet reject reasons and MaybeBroken bit

Unused for now, but I want to backport this commit to 1.4 so 1.6 can
start sending these and then at least 1.4 logs will stringify nicely.

Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

net/packet/tsmp.go

@@ -23,12 +23,14 @@
 // 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()

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,7 +125,8 @@ 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
@@ -114,6 +134,10 @@ func (e *userspaceEngine) onOpenTimeout(flow flowtrack.Tuple) {
 	delete(e.pendOpen, flow)
 	e.mu.Unlock()
 
+	if !of.problem.IsZero() {
+		e.logf("open-conn-track: timeout opening %v; peer reported problem: %v", flow, of.problem)
+	}
+
 	// Diagnose why it might've timed out.
 	n, ok := e.magicConn.PeerForIP(flow.Dst.IP)
 	if !ok {