From 8f7f9ac17e79ff5e859a814b32b3a463c2905e47 Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Sun, 5 May 2024 15:00:19 -0700 Subject: [PATCH] wgengine/netstack: handle 4via6 routes that are advertised by the same node Previously, a node that was advertising a 4via6 route wouldn't be able to make use of that same route; the packet would be delivered to Tailscale, but since we weren't accepting it in handleLocalPackets, the packet wouldn't be delivered to netstack and would never hit the 4via6 logic. Let's add that support so that usage of 4via6 is consistent regardless of where the connection is initiated from. Updates #11304 Signed-off-by: Andrew Dunham Change-Id: Ic28dc2e58080d76100d73b93360f4698605af7cb --- wgengine/netstack/netstack.go | 141 ++++++++++++++++++++++------- wgengine/netstack/netstack_test.go | 91 +++++++++++++++++++ 2 files changed, 200 insertions(+), 32 deletions(-) diff --git a/wgengine/netstack/netstack.go b/wgengine/netstack/netstack.go index 5bd00bc7b..e6409861f 100644 --- a/wgengine/netstack/netstack.go +++ b/wgengine/netstack/netstack.go @@ -670,23 +670,72 @@ func (ns *Impl) handleLocalPackets(p *packet.Parsed, t *tstun.Wrapper) filter.Re return filter.DropSilently } - // If it's not traffic to the service IP (e.g. magicDNS or Taildrive) we don't - // care; resume processing. - if dst := p.Dst.Addr(); dst != serviceIP && dst != serviceIPv6 { + // Determine if we care about this local packet. + dst := p.Dst.Addr() + switch { + case dst == serviceIP || dst == serviceIPv6: + // We want to intercept some traffic to the "service IP" (e.g. + // 100.100.100.100 for IPv4). However, of traffic to the + // service IP, we only care about UDP 53, and TCP on port 53, + // 80, and 8080. + switch p.IPProto { + case ipproto.TCP: + if port := p.Dst.Port(); port != 53 && port != 80 && port != 8080 { + return filter.Accept + } + case ipproto.UDP: + if port := p.Dst.Port(); port != 53 { + return filter.Accept + } + } + case viaRange.Contains(dst): + // We need to handle 4via6 packets leaving the host if the via + // route is for this host; otherwise the packet will be dropped + // because nothing will translate it. + var shouldHandle bool + if p.IPVersion == 6 && !ns.isLocalIP(dst) { + shouldHandle = ns.lb != nil && ns.lb.ShouldHandleViaIP(dst) + } + if !shouldHandle { + // Unhandled means that we let the regular processing + // occur without doing anything ourselves. + return filter.Accept + } + + if debugNetstack() { + ns.logf("netstack: handling local 4via6 packet: version=%d proto=%v dst=%v src=%v", + p.IPVersion, p.IPProto, p.Dst, p.Src) + } + + // If this is a ping message, handle it and don't pass to + // netstack. + pingIP, handlePing := ns.shouldHandlePing(p) + if handlePing { + ns.logf("netstack: handling local 4via6 ping: dst=%v pingIP=%v", dst, pingIP) + + var pong []byte // the reply to the ping, if our relayed ping works + if dst.Is4() { + h := p.ICMP4Header() + h.ToResponse() + pong = packet.Generate(&h, p.Payload()) + } else if dst.Is6() { + h := p.ICMP6Header() + h.ToResponse() + pong = packet.Generate(&h, p.Payload()) + } + + go ns.userPing(pingIP, pong, userPingDirectionInbound) + return filter.DropSilently + } + + // Fall through to writing inbound so netstack handles the + // 4via6 via connection. + + default: + // Not traffic to the service IP or a 4via6 IP, so we don't + // care about the packet; resume processing. return filter.Accept } - // Of traffic to the service IP, we only care about UDP 53, and TCP - // on port 53, 80, and 8080. - switch p.IPProto { - case ipproto.TCP: - if port := p.Dst.Port(); port != 53 && port != 80 && port != 8080 { - return filter.Accept - } - case ipproto.UDP: - if port := p.Dst.Port(); port != 53 { - return filter.Accept - } - } var pn tcpip.NetworkProtocolNumber switch p.IPVersion { @@ -754,7 +803,7 @@ func (ns *Impl) inject() { } if debugPackets { - ns.logf("[v2] packet Write out: % x", stack.PayloadSince(pkt.NetworkHeader())) + ns.logf("[v2] packet Write out: % x", stack.PayloadSince(pkt.NetworkHeader()).AsSlice()) } // In the normal case, netstack synthesizes the bytes for @@ -767,25 +816,32 @@ func (ns *Impl) inject() { // Determine if the packet is from a service IP, in which case it // needs to go back into the machines network (inbound) instead of // out. - // TODO(tom): Work out a way to avoid parsing packets to determine if - // its from the service IP. Maybe gvisor netstack magic. I - // went through the fields of PacketBuffer, and nop :/ // TODO(tom): Figure out if its safe to modify packet.Parsed to fill in // the IP src/dest even if its missing the rest of the pkt. // That way we dont have to do this twitchy-af byte-yeeting. - if b := pkt.NetworkHeader().Slice(); len(b) >= 20 { // min ipv4 header - switch b[0] >> 4 { // ip proto field - case 4: - if srcIP := netaddr.IPv4(b[12], b[13], b[14], b[15]); serviceIP == srcIP { + hdr := pkt.Network() + switch v := hdr.(type) { + case header.IPv4: + srcIP := netip.AddrFrom4(v.SourceAddress().As4()) + if serviceIP == srcIP { + sendToHost = true + } + case header.IPv6: + srcIP := netip.AddrFrom16(v.SourceAddress().As16()) + if srcIP == serviceIPv6 { + sendToHost = true + } else if viaRange.Contains(srcIP) { + // Only send to the host if this 4via6 route is + // something this node handles. + if ns.lb != nil && ns.lb.ShouldHandleViaIP(srcIP) { sendToHost = true - } - case 6: - if len(b) >= 40 { // min ipv6 header - if srcIP, ok := netip.AddrFromSlice(net.IP(b[8:24])); ok && serviceIPv6 == srcIP { - sendToHost = true + if debugNetstack() { + ns.logf("netstack: sending 4via6 packet to host: %v", srcIP) } } } + default: + // unknown; don't forward to host } // pkt has a non-zero refcount, so injection methods takes @@ -868,6 +924,17 @@ func (ns *Impl) shouldProcessInbound(p *packet.Parsed, t *tstun.Wrapper) bool { var isSynology = runtime.GOOS == "linux" && distro.Get() == distro.Synology +type userPingDirection int + +const ( + // userPingDirectionOutbound is used when the pong packet is to be sent + // "outbound"–i.e. from this node to a peer via WireGuard. + userPingDirectionOutbound userPingDirection = iota + // userPingDirectionInbound is used when the pong packet is to be sent + // "inbound"–i.e. from Tailscale to another process on this host. + userPingDirectionInbound +) + // userPing tried to ping dstIP and if it succeeds, injects pingResPkt // into the tundev. // @@ -878,9 +945,13 @@ func (ns *Impl) shouldProcessInbound(p *packet.Parsed, t *tstun.Wrapper) bool { // people only use ping occasionally to see if their internet's working // so this doesn't need to be great. // +// The 'direction' parameter is used to determine where the response "pong" +// packet should be written, if the ping succeeds. See the documentation on the +// constants for more details. +// // TODO(bradfitz): when we're running on Windows as the system user, use // raw socket APIs instead of ping child processes. -func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte) { +func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte, direction userPingDirection) { if !userPingSem.TryAcquire() { return } @@ -941,8 +1012,14 @@ func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte) { if debugNetstack() { ns.logf("exec pinged %v in %v", dstIP, time.Since(t0)) } - if err := ns.tundev.InjectOutbound(pingResPkt); err != nil { - ns.logf("InjectOutbound ping response: %v", err) + if direction == userPingDirectionOutbound { + if err := ns.tundev.InjectOutbound(pingResPkt); err != nil { + ns.logf("InjectOutbound ping response: %v", err) + } + } else if direction == userPingDirectionInbound { + if err := ns.tundev.InjectInboundCopy(pingResPkt); err != nil { + ns.logf("InjectInboundCopy ping response: %v", err) + } } } @@ -977,7 +1054,7 @@ func (ns *Impl) injectInbound(p *packet.Parsed, t *tstun.Wrapper) filter.Respons h.ToResponse() pong = packet.Generate(&h, p.Payload()) } - go ns.userPing(pingIP, pong) + go ns.userPing(pingIP, pong, userPingDirectionOutbound) return filter.DropSilently } diff --git a/wgengine/netstack/netstack_test.go b/wgengine/netstack/netstack_test.go index a8b180f95..7a3affda7 100644 --- a/wgengine/netstack/netstack_test.go +++ b/wgengine/netstack/netstack_test.go @@ -706,3 +706,94 @@ func TestTCPForwardLimits_PerClient(t *testing.T) { t.Errorf("got clientmetric limit metric=%d, want 1", v) } } + +// TestHandleLocalPackets tests the handleLocalPackets function, ensuring that +// we are properly deciding to handle packets that are destined for "local" +// IPs–addresses that are either for this node, or that it is responsible for. +// +// See, e.g. #11304 +func TestHandleLocalPackets(t *testing.T) { + var ( + selfIP4 = netip.MustParseAddr("100.64.1.2") + selfIP6 = netip.MustParseAddr("fd7a:115c:a1e0::123") + ) + + impl := makeNetstack(t, func(impl *Impl) { + impl.ProcessSubnets = false + impl.ProcessLocalIPs = false + impl.atomicIsLocalIPFunc.Store(func(addr netip.Addr) bool { + return addr == selfIP4 || addr == selfIP6 + }) + }) + + prefs := ipn.NewPrefs() + prefs.AdvertiseRoutes = []netip.Prefix{ + // $ tailscale debug via 7 10.1.1.0/24 + // fd7a:115c:a1e0:b1a:0:7:a01:100/120 + netip.MustParsePrefix("fd7a:115c:a1e0:b1a:0:7:a01:100/120"), + } + _, err := impl.lb.EditPrefs(&ipn.MaskedPrefs{ + Prefs: *prefs, + AdvertiseRoutesSet: true, + }) + if err != nil { + t.Fatalf("EditPrefs: %v", err) + } + + t.Run("ShouldHandleServiceIP", func(t *testing.T) { + pkt := &packet.Parsed{ + IPVersion: 4, + IPProto: ipproto.TCP, + Src: netip.MustParseAddrPort("127.0.0.1:9999"), + Dst: netip.MustParseAddrPort("100.100.100.100:53"), + TCPFlags: packet.TCPSyn, + } + resp := impl.handleLocalPackets(pkt, impl.tundev) + if resp != filter.DropSilently { + t.Errorf("got filter outcome %v, want filter.DropSilently", resp) + } + }) + t.Run("ShouldHandle4via6", func(t *testing.T) { + pkt := &packet.Parsed{ + IPVersion: 6, + IPProto: ipproto.TCP, + Src: netip.MustParseAddrPort("[::1]:1234"), + + // This is an IP in the above 4via6 subnet that this node handles. + // $ tailscale debug via 7 10.1.1.9/24 + // fd7a:115c:a1e0:b1a:0:7:a01:109/120 + Dst: netip.MustParseAddrPort("[fd7a:115c:a1e0:b1a:0:7:a01:109]:5678"), + TCPFlags: packet.TCPSyn, + } + resp := impl.handleLocalPackets(pkt, impl.tundev) + + // DropSilently is the outcome we expected, since we actually + // handled this packet by injecting it into netstack, which + // will handle creating the TCP forwarder. We drop it so we + // don't process the packet outside of netstack. + if resp != filter.DropSilently { + t.Errorf("got filter outcome %v, want filter.DropSilently", resp) + } + }) + t.Run("OtherNonHandled", func(t *testing.T) { + pkt := &packet.Parsed{ + IPVersion: 6, + IPProto: ipproto.TCP, + Src: netip.MustParseAddrPort("[::1]:1234"), + + // This IP is *not* in the above 4via6 route + // $ tailscale debug via 99 10.1.1.9/24 + // fd7a:115c:a1e0:b1a:0:63:a01:109/120 + Dst: netip.MustParseAddrPort("[fd7a:115c:a1e0:b1a:0:63:a01:109]:5678"), + TCPFlags: packet.TCPSyn, + } + resp := impl.handleLocalPackets(pkt, impl.tundev) + + // Accept means that handleLocalPackets does not handle this + // packet, we "accept" it to continue further processing, + // instead of dropping because it was already handled. + if resp != filter.Accept { + t.Errorf("got filter outcome %v, want filter.Accept", resp) + } + }) +}