From 0157000cab9f6d2d233bd540df4824e1d9dd8d0a Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Sun, 25 Aug 2024 20:43:20 -0700 Subject: [PATCH] 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 #13038 Change-Id: I85e973aaa38b43c14611943ff45c7c825ee9200a Signed-off-by: Brad Fitzpatrick --- tstest/integration/nat/nat_test.go | 48 +++++++++++++++++++----------- tstest/natlab/vnet/vnet.go | 30 +++++++++++++++++-- 2 files changed, 57 insertions(+), 21 deletions(-) diff --git a/tstest/integration/nat/nat_test.go b/tstest/integration/nat/nat_test.go index 9d3f16a56..0a5dc7828 100644 --- a/tstest/integration/nat/nat_test.go +++ b/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) diff --git a/tstest/natlab/vnet/vnet.go b/tstest/natlab/vnet/vnet.go index ef9b04411..1c4e22c71 100644 --- a/tstest/natlab/vnet/vnet.go +++ b/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,10 +2003,13 @@ 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() @@ -2000,7 +2017,9 @@ func (s *Server) takeAgentConn(ctx context.Context, n *node) (_ *agentConn, ok b 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 }