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,22 +670,71 @@ 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 { - 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 + // 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) } - case ipproto.UDP: - if port := p.Dst.Port(); port != 53 { + 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 } var pn tcpip.NetworkProtocolNumber @@ -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 @@ var userPingSem = syncs.NewSemaphore(20) // 20 child ping processes at once 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 @@ var isSynology = runtime.GOOS == "linux" && distro.Get() == distro.Synology // 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 51292c649..64c167229 100644 --- a/wgengine/netstack/netstack_test.go +++ b/wgengine/netstack/netstack_test.go @@ -704,3 +704,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) + } + }) +}