tstest/natlab: fix IPv6 tests, remove TODOs

The reason they weren't working was because the cmd/tta agent in the
guest was dialing out to the test and the vnet couldn't map its global
unicast IPv6 address to a node as it was just using a
map[netip.Addr]*node and blindly trusting the *node was
populated. Instead, it was nil, so the agent connection fetching
didn't work for its RoundTripper and the test could never drive the
node. That map worked for IPv4 but for IPv6 we need to use the method
that takes into account the node's IPv6 SLAAC address. Most call sites
had been converted but I'd missed that one.

Also clean up some debug, and prohibit nodes' link-local unicast
addresses from dialing 2000::/3 directly for now. We can allow that to
be configured opt-in later (some sort of IPv6 NAT mode. Whatever it's
called.) That mode was working on accident, but was confusing: Linux
would do source address selection from link local for the first few
seconds and then after SLAAC and DAD, switch to using the global
unicast source address. Be consistent for now and force it to use the
global unicast.

Updates tailscale#13038

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

tstest/integration/nat/nat_test.go

@@ -210,21 +210,24 @@ func hardPMP(c *vnet.Config) *vnet.Node {
 		fmt.Sprintf("10.7.%d.1/24", n), vnet.HardNAT, vnet.NATPMP))
 }
 
-func (nt *natTest) runTest(node1, node2 addNodeFunc) pingRoute {
+func (nt *natTest) runTest(addNode ...addNodeFunc) pingRoute {
+	if len(addNode) < 1 || len(addNode) > 2 {
+		nt.tb.Fatalf("runTest: invalid number of nodes %v; want 1 or 2", len(addNode))
+	}
 	t := nt.tb
 
 	var c vnet.Config
 	c.SetPCAPFile(*pcapFile)
-	nodes := []*vnet.Node{
-		node1(&c),
-		node2(&c),
-	}
-	if nodes[0] == nil || nodes[1] == nil {
-		t.Skip("skipping test; not applicable combination")
-	}
-	if *logTailscaled {
-		nodes[0].SetVerboseSyslog(true)
-		nodes[1].SetVerboseSyslog(true)
+	nodes := []*vnet.Node{}
+	for _, fn := range addNode {
+		node := fn(&c)
+		if node == nil {
+			t.Skip("skipping test; not applicable combination")
+		}
+		nodes = append(nodes, node)
+		if *logTailscaled {
+			node.SetVerboseSyslog(true)
+		}
 	}
 
 	var err error
@@ -310,16 +313,18 @@ func (nt *natTest) runTest(node1, node2 addNodeFunc) pingRoute {
 	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
 	defer cancel()
 
-	lc1 := nt.vnet.NodeAgentClient(nodes[0])
-	lc2 := nt.vnet.NodeAgentClient(nodes[1])
-	clients := []*vnet.NodeAgentClient{lc1, lc2}
+	var clients []*vnet.NodeAgentClient
+	for _, n := range nodes {
+		clients = append(clients, nt.vnet.NodeAgentClient(n))
+	}
+	sts := make([]*ipnstate.Status, len(nodes))
 
 	var eg errgroup.Group
-	var sts [2]*ipnstate.Status
 	for i, c := range clients {
 		i, c := i, c
 		eg.Go(func() error {
 			node := nodes[i]
+			t.Logf("%v calling Status...", node)
 			st, err := c.Status(ctx)
 			if err != nil {
 				return fmt.Errorf("%v status: %w", node, err)
@@ -357,7 +362,11 @@ func (nt *natTest) runTest(node1, node2 addNodeFunc) pingRoute {
 
 	defer nt.vnet.Close()
 
-	pingRes, err := ping(ctx, lc1, sts[1].Self.TailscaleIPs[0])
+	if len(nodes) < 2 {
+		return ""
+	}
+
+	pingRes, err := ping(ctx, clients[0], sts[1].Self.TailscaleIPs[0])
 	if err != nil {
 		t.Fatalf("ping failure: %v", err)
 	}
@@ -468,15 +477,18 @@ func TestEasyEasy(t *testing.T) {
 	nt.want(routeDirect)
 }
 
+func TestSingleJustIPv6(t *testing.T) {
+	nt := newNatTest(t)
+	nt.runTest(just6)
+}
+
 func TestJustIPv6(t *testing.T) {
-	t.Skip("TODO: get this working")
 	nt := newNatTest(t)
 	nt.runTest(just6, just6)
 	nt.want(routeDirect)
 }
 
 func TestEasy4AndJust6(t *testing.T) {
-	t.Skip("TODO: get this working")
 	nt := newNatTest(t)
 	nt.runTest(easyAnd6, just6)
 	nt.want(routeDirect)

tstest/natlab/vnet/vnet.go

@@ -327,9 +327,14 @@ func (n *network) acceptTCP(r *tcp.ForwarderRequest) {
 	}
 
 	if destPort == 8008 && fakeTestAgent.Match(destIP) {
+		node, ok := n.nodeForDestIP(clientRemoteIP)
+		if !ok {
+			n.logf("unknown client IP %v trying to connect to test driver", clientRemoteIP)
+			r.Complete(true)
+			return
+		}
 		r.Complete(false)
 		tc := gonet.NewTCPConn(&wq, ep)
-		node := n.nodesByIP[clientRemoteIP]
 		ac := &agentConn{node, tc}
 		n.s.addIdleAgentConn(ac)
 		return
@@ -1177,6 +1182,11 @@ func (n *network) HandleEthernetPacketForRouter(ep EthernetPacket) {
 		return
 	}
 
+	if flow.src.Is6() && flow.src.IsLinkLocalUnicast() && !flow.dst.IsLinkLocalUnicast() {
+		// Don't log.
+		return
+	}
+
 	n.logf("router got unknown packet: %v", packet)
 }
 
@@ -1539,6 +1549,10 @@ func (s *Server) shouldInterceptTCP(pkt gopacket.Packet) bool {
 	if !ok {
 		return false
 	}
+	if flow.src.Is6() && flow.src.IsLinkLocalUnicast() {
+		return false
+	}
+
 	if tcp.DstPort == 80 || tcp.DstPort == 443 {
 		for _, v := range []virtualIP{fakeControl, fakeDERP1, fakeDERP2, fakeLogCatcher} {
 			if v.Match(flow.dst) {
@@ -1989,18 +2003,23 @@ func (s *Server) addIdleAgentConn(ac *agentConn) {
 }
 
 func (s *Server) takeAgentConn(ctx context.Context, n *node) (_ *agentConn, ok bool) {
+	const debug = false
 	for {
 		ac, ok := s.takeAgentConnOne(n)
 		if ok {
-			//log.Printf("got agent conn for %v", n.mac)
+			if debug {
+				log.Printf("takeAgentConn: got agent conn for %v", n.mac)
+			}
 			return ac, true
 		}
 		s.mu.Lock()
 		ready := make(chan struct{})
 		mak.Set(&s.agentConnWaiter, n, ready)
 		s.mu.Unlock()
 
-		//log.Printf("waiting for agent conn for %v", n.mac)
+		if debug {
+			log.Printf("takeAgentConn: waiting for agent conn for %v", n.mac)
+		}
 		select {
 		case <-ctx.Done():
 			return nil, false
@@ -2016,11 +2035,16 @@ func (s *Server) takeAgentConn(ctx context.Context, n *node) (_ *agentConn, ok b
 func (s *Server) takeAgentConnOne(n *node) (_ *agentConn, ok bool) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
+	miss := 0
 	for ac := range s.agentConns {
 		if ac.node == n {
 			s.agentConns.Delete(ac)
 			return ac, true
 		}
+		miss++
+	}
+	if miss > 0 {
+		log.Printf("takeAgentConnOne: missed %d times for %v", miss, n.mac)
 	}
 	return nil, false
 }