From acc50d6b672d2d77953eaff0193924d2804b0fa3 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 2 Feb 2021 15:05:51 -0800 Subject: [PATCH] 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 (cherry picked from commit d37058af728c72a4ef29ccb154da4528a9cb9575) --- net/packet/tsmp.go | 80 ++++++++++++++++++++++++++++++++++------- net/packet/tsmp_test.go | 12 +++++++ wgengine/pendopen.go | 28 +++++++++++++-- 3 files changed, 106 insertions(+), 14 deletions(-) diff --git a/net/packet/tsmp.go b/net/packet/tsmp.go index 8b46a6c98..2346c9419 100644 --- a/net/packet/tsmp.go +++ b/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 } diff --git a/net/packet/tsmp_test.go b/net/packet/tsmp_test.go index 71e4f9439..d4a0cf1a0 100644 --- a/net/packet/tsmp_test.go +++ b/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() diff --git a/wgengine/pendopen.go b/wgengine/pendopen.go index a4d0a6f8b..3526fe9bc 100644 --- a/wgengine/pendopen.go +++ b/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 {