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 <andrew@du.nham.ca>
Change-Id: Ic28dc2e58080d76100d73b93360f4698605af7cb

wgengine/netstack/netstack.go

@@ -670,13 +670,14 @@ 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
+	// Determine if we care about this local packet.
-	// care; resume processing.
+	dst := p.Dst.Addr()
-	if dst := p.Dst.Addr(); dst != serviceIP && dst != serviceIPv6 {
+	switch {
-		return filter.Accept
+	case dst == serviceIP || dst == serviceIPv6:
-	}
+		// We want to intercept some traffic to the "service IP" (e.g.
-	// Of traffic to the service IP, we only care about UDP 53, and TCP
+		// 100.100.100.100 for IPv4). However, of traffic to the
-	// on port 53, 80, and 8080.
+		// 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 {
@@ -687,6 +688,54 @@ func (ns *Impl) handleLocalPackets(p *packet.Parsed, t *tstun.Wrapper) filter.Re
 				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
+	}
 
 	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
+		hdr := pkt.Network()
-			switch b[0] >> 4 { // ip proto field
+		switch v := hdr.(type) {
-			case 4:
+		case header.IPv4:
-				if srcIP := netaddr.IPv4(b[12], b[13], b[14], b[15]); serviceIP == srcIP {
+			srcIP := netip.AddrFrom4(v.SourceAddress().As4())
+			if serviceIP == srcIP {
 				sendToHost = true
 			}
-			case 6:
+		case header.IPv6:
-				if len(b) >= 40 { // min ipv6 header
+			srcIP := netip.AddrFrom16(v.SourceAddress().As16())
-					if srcIP, ok := netip.AddrFromSlice(net.IP(b[8:24])); ok && serviceIPv6 == srcIP {
+			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
+					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,9 +1012,15 @@ func (ns *Impl) userPing(dstIP netip.Addr, pingResPkt []byte) {
 	if debugNetstack() {
 		ns.logf("exec pinged %v in %v", dstIP, time.Since(t0))
 	}
+	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)
+		}
+	}
 }
 
 // injectInbound is installed as a packet hook on the 'inbound' (from a
@@ -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
 	}
 

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)
+		}
+	})
+}