cmd/natc: only store v4 addresses

Because we derive v6 addresses from v4 addresses we only need to store
the v4 address, not both.

Updates #14667

Signed-off-by: Fran Bull <fran@tailscale.com>

cmd/natc/ippool/ippool.go

@@ -41,7 +41,7 @@ func (ipp *IPPool) DomainForIP(from tailcfg.NodeID, addr netip.Addr) (string, bo
 	return domain, ok
 }
 
-func (ipp *IPPool) IPForDomain(from tailcfg.NodeID, domain string) ([]netip.Addr, error) {
+func (ipp *IPPool) IPForDomain(from tailcfg.NodeID, domain string) (netip.Addr, error) {
 	npps := &perPeerState{
 		ipset: ipp.IPSet,
 		v6ULA: ipp.V6ULA,
@@ -57,7 +57,7 @@ type perPeerState struct {
 
 	mu           sync.Mutex
 	addrInUse    *big.Int
-	domainToAddr map[string][]netip.Addr
+	domainToAddr map[string]netip.Addr
 	addrToDomain *bart.Table[string]
 }
 
@@ -75,23 +75,23 @@ func (ps *perPeerState) domainForIP(ip netip.Addr) (_ string, ok bool) {
 // ipForDomain assigns a pair of unique IP addresses for the given domain and
 // returns them. The first address is an IPv4 address and the second is an IPv6
 // address. If the domain already has assigned addresses, it returns them.
-func (ps *perPeerState) ipForDomain(domain string) ([]netip.Addr, error) {
+func (ps *perPeerState) ipForDomain(domain string) (netip.Addr, error) {
 	fqdn, err := dnsname.ToFQDN(domain)
 	if err != nil {
-		return nil, err
+		return netip.Addr{}, err
 	}
 	domain = fqdn.WithoutTrailingDot()
 
 	ps.mu.Lock()
 	defer ps.mu.Unlock()
-	if addrs, ok := ps.domainToAddr[domain]; ok {
+	if addr, ok := ps.domainToAddr[domain]; ok {
-		return addrs, nil
+		return addr, nil
 	}
-	addrs := ps.assignAddrsLocked(domain)
+	addr := ps.assignAddrsLocked(domain)
-	if addrs == nil {
+	if !addr.IsValid() {
-		return nil, ErrNoIPsAvailable
+		return netip.Addr{}, ErrNoIPsAvailable
 	}
-	return addrs, nil
+	return addr, nil
 }
 
 // unusedIPv4Locked returns an unused IPv4 address from the available ranges.
@@ -106,22 +106,16 @@ func (ps *perPeerState) unusedIPv4Locked() netip.Addr {
 // and returns them. The first address is an IPv4 address and the second is an
 // IPv6 address. It does not check if the domain already has assigned addresses.
 // ps.mu must be held.
-func (ps *perPeerState) assignAddrsLocked(domain string) []netip.Addr {
+func (ps *perPeerState) assignAddrsLocked(domain string) netip.Addr {
 	if ps.addrToDomain == nil {
 		ps.addrToDomain = &bart.Table[string]{}
 	}
 	v4 := ps.unusedIPv4Locked()
 	if !v4.IsValid() {
-		return nil
+		return netip.Addr{}
 	}
-	as16 := ps.v6ULA.Addr().As16()
+	addr := v4
-	as4 := v4.As4()
+	mak.Set(&ps.domainToAddr, domain, addr)
-	copy(as16[12:], as4[:])
+	ps.addrToDomain.Insert(netip.PrefixFrom(addr, addr.BitLen()), domain)
-	v6 := netip.AddrFrom16(as16)
+	return addr
-	addrs := []netip.Addr{v4, v6}
-	mak.Set(&ps.domainToAddr, domain, addrs)
-	for _, a := range addrs {
-		ps.addrToDomain.Insert(netip.PrefixFrom(a, a.BitLen()), domain)
-	}
-	return addrs
 }

cmd/natc/ippool/ippool_test.go

@@ -7,7 +7,6 @@ import (
 	"errors"
 	"fmt"
 	"net/netip"
-	"slices"
 	"testing"
 
 	"go4.org/netipx"
@@ -33,20 +32,18 @@ func TestIPPoolExhaustion(t *testing.T) {
 
 	for i := 0; i < 5; i++ {
 		for _, domain := range domains {
-			addrs, err := pool.IPForDomain(from, domain)
+			addr, err := pool.IPForDomain(from, domain)
 			if err != nil {
 				errs = append(errs, fmt.Errorf("failed to get IP for domain %q: %w", domain, err))
 				continue
 			}
 
-			for _, addr := range addrs {
+			if d, ok := assignedIPs[addr]; ok {
-				if d, ok := assignedIPs[addr]; ok {
+				if d != domain {
-					if d != domain {
+					t.Errorf("IP %s reused for domain %q, previously assigned to %q", addr, domain, d)
-						t.Errorf("IP %s reused for domain %q, previously assigned to %q", addr, domain, d)
-					}
-				} else {
-					assignedIPs[addr] = domain
 				}
+			} else {
+				assignedIPs[addr] = domain
 			}
 		}
 	}
@@ -80,50 +77,36 @@ func TestIPPool(t *testing.T) {
 		IPSet: addrPool,
 	}
 	from := tailcfg.NodeID(12345)
-	addrs, err := pool.IPForDomain(from, "example.com")
+	addr, err := pool.IPForDomain(from, "example.com")
 	if err != nil {
 		t.Fatalf("ipForDomain() error = %v", err)
 	}
 
-	if len(addrs) != 2 {
+	if !addr.IsValid() {
-		t.Fatalf("ipForDomain() returned %d addresses, want 2", len(addrs))
+		t.Fatal("ipForDomain() returned an invalid address")
 	}
 
-	v4 := addrs[0]
+	if !addr.Is4() {
-	v6 := addrs[1]
+		t.Errorf("Address is not IPv4: %s", addr)
-
-	if !v4.Is4() {
-		t.Errorf("First address is not IPv4: %s", v4)
-	}
-
-	if !v6.Is6() {
-		t.Errorf("Second address is not IPv6: %s", v6)
 	}
 
-	if !addrPool.Contains(v4) {
+	if !addrPool.Contains(addr) {
-		t.Errorf("IPv4 address %s not in range %s", v4, addrPool)
+		t.Errorf("IPv4 address %s not in range %s", addr, addrPool)
-	}
-
-	domain, ok := pool.DomainForIP(from, v4)
-	if !ok {
-		t.Errorf("domainForIP(%s) not found", v4)
-	} else if domain != "example.com" {
-		t.Errorf("domainForIP(%s) = %s, want %s", v4, domain, "example.com")
 	}
 
-	domain, ok = pool.DomainForIP(from, v6)
+	domain, ok := pool.DomainForIP(from, addr)
 	if !ok {
-		t.Errorf("domainForIP(%s) not found", v6)
+		t.Errorf("domainForIP(%s) not found", addr)
 	} else if domain != "example.com" {
-		t.Errorf("domainForIP(%s) = %s, want %s", v6, domain, "example.com")
+		t.Errorf("domainForIP(%s) = %s, want %s", addr, domain, "example.com")
 	}
 
-	addrs2, err := pool.IPForDomain(from, "example.com")
+	addr2, err := pool.IPForDomain(from, "example.com")
 	if err != nil {
 		t.Fatalf("ipForDomain() second call error = %v", err)
 	}
 
-	if !slices.Equal(addrs, addrs2) {
+	if addr.Compare(addr2) != 0 {
-		t.Errorf("ipForDomain() second call = %v, want %v", addrs2, addrs)
+		t.Errorf("ipForDomain() second call = %v, want %v", addr2, addr)
 	}
 }

cmd/natc/natc.go

@@ -317,11 +317,12 @@ func (c *connector) handleDNS(pc net.PacketConn, buf []byte, remoteAddr *net.UDP
 			// ignored and non-ignored addresses, but it's currently the user
 			// preferred behavior.
 			if !c.ignoreDestination(addrs) {
-				addrs, err = c.ipPool.IPForDomain(who.Node.ID, q.Name.String())
+				addr, err := c.ipPool.IPForDomain(who.Node.ID, q.Name.String())
 				if err != nil {
 					log.Printf("HandleDNS(remote=%s): lookup destination failed: %v\n", remoteAddr.String(), err)
 					return
 				}
+				addrs = []netip.Addr{addr, v6ForV4(c.v6ULA.Addr(), addr)}
 			}
 			mak.Set(&resolves, q.Name.String(), addrs)
 		}
@@ -414,6 +415,20 @@ func (c *connector) handleDNS(pc net.PacketConn, buf []byte, remoteAddr *net.UDP
 	}
 }
 
+func v6ForV4(ula netip.Addr, v4 netip.Addr) netip.Addr {
+	as16 := ula.As16()
+	as4 := v4.As4()
+	copy(as16[12:], as4[:])
+	return netip.AddrFrom16(as16)
+}
+
+func v4ForV6(v6 netip.Addr) netip.Addr {
+	as16 := v6.As16()
+	var as4 [4]byte
+	copy(as4[:], as16[12:])
+	return netip.AddrFrom4(as4)
+}
+
 // tsMBox is the mailbox used in SOA records.
 // The convention is to replace the @ symbol with a dot.
 // So in this case, the mailbox is support.tailscale.com. with the trailing dot
@@ -434,7 +449,11 @@ func (c *connector) handleTCPFlow(src, dst netip.AddrPort) (handler func(net.Con
 		log.Printf("HandleTCPFlow: WhoIs failed: %v\n", err)
 		return nil, false
 	}
-	domain, ok := c.ipPool.DomainForIP(who.Node.ID, dst.Addr())
+	dstAddr := dst.Addr()
+	if dstAddr.Is6() {
+		dstAddr = v4ForV6(dstAddr)
+	}
+	domain, ok := c.ipPool.DomainForIP(who.Node.ID, dstAddr)
 	if !ok {
 		return nil, false
 	}

cmd/natc/natc_test.go

@@ -340,50 +340,59 @@ func TestDNSResponse(t *testing.T) {
 								t.Errorf("answer[%d] not an A record", i)
 								continue
 							}
-							resource := ans.Body.(*dnsmessage.AResource)
-							gotIP := netip.AddrFrom4([4]byte(resource.A))
-
-							var ips []netip.Addr
-							if tc.wantIgnored {
-								ips = must.Get(c.resolver.LookupNetIP(t.Context(), "ip4", want.name))
-							} else {
-								ips = must.Get(c.ipPool.IPForDomain(tailcfg.NodeID(123), want.name))
-							}
-							var wantIP netip.Addr
-							for _, ip := range ips {
-								if ip.Is4() {
-									wantIP = ip
-									break
-								}
-							}
-							if gotIP != wantIP {
-								t.Errorf("answer[%d] IP = %s, want %s", i, gotIP, wantIP)
-							}
 						case dnsmessage.TypeAAAA:
 							if ans.Body.(*dnsmessage.AAAAResource) == nil {
 								t.Errorf("answer[%d] not an AAAA record", i)
 								continue
 							}
+						}
+
+						var gotIP netip.Addr
+						switch want.qType {
+						case dnsmessage.TypeA:
+							resource := ans.Body.(*dnsmessage.AResource)
+							gotIP = netip.AddrFrom4([4]byte(resource.A))
+						case dnsmessage.TypeAAAA:
 							resource := ans.Body.(*dnsmessage.AAAAResource)
-							gotIP := netip.AddrFrom16([16]byte(resource.AAAA))
+							gotIP = netip.AddrFrom16([16]byte(resource.AAAA))
+						}
 
-							var ips []netip.Addr
+						var wantIP netip.Addr
-							if tc.wantIgnored {
+						if tc.wantIgnored {
-								ips = must.Get(c.resolver.LookupNetIP(t.Context(), "ip6", want.name))
+							var net string
-							} else {
+							var fxSelectIP func(netip.Addr) bool
-								ips = must.Get(c.ipPool.IPForDomain(tailcfg.NodeID(123), want.name))
+							switch want.qType {
+							case dnsmessage.TypeA:
+								net = "ip4"
+								fxSelectIP = func(a netip.Addr) bool {
+									return a.Is4()
+								}
+							case dnsmessage.TypeAAAA:
+								//TODO(fran) is this branch exercised?
+								net = "ip6"
+								fxSelectIP = func(a netip.Addr) bool {
+									return a.Is6()
+								}
 							}
-							var wantIP netip.Addr
+							ips := must.Get(c.resolver.LookupNetIP(t.Context(), net, want.name))
 							for _, ip := range ips {
-								if ip.Is6() {
+								if fxSelectIP(ip) {
 									wantIP = ip
 									break
 								}
 							}
-							if gotIP != wantIP {
+						} else {
-								t.Errorf("answer[%d] IP = %s, want %s", i, gotIP, wantIP)
+							addr := must.Get(c.ipPool.IPForDomain(tailcfg.NodeID(123), want.name))
+							switch want.qType {
+							case dnsmessage.TypeA:
+								wantIP = addr
+							case dnsmessage.TypeAAAA:
+								wantIP = v6ForV4(v6ULA.Addr(), addr)
 							}
 						}
+						if gotIP != wantIP {
+							t.Errorf("answer[%d] IP = %s, want %s", i, gotIP, wantIP)
+						}
 					}
 				}
 			}
@@ -445,3 +454,29 @@ func TestIgnoreDestination(t *testing.T) {
 		})
 	}
 }
+
+func TestV6V4(t *testing.T) {
+	v6ULA := ula(1)
+
+	tests := [][]string{
+		{"100.64.0.0", "fd7a:115c:a1e0:a99c:1:0:6440:0"},
+		{"0.0.0.0", "fd7a:115c:a1e0:a99c:1::"},
+		{"255.255.255.255", "fd7a:115c:a1e0:a99c:1:0:ffff:ffff"},
+	}
+
+	for i, test := range tests {
+		// to v6
+		v6 := v6ForV4(v6ULA.Addr(), netip.MustParseAddr(test[0]))
+		want := netip.MustParseAddr(test[1])
+		if v6 != want {
+			t.Fatalf("test %d: want: %v, got: %v", i, want, v6)
+		}
+
+		// to v4
+		v4 := v4ForV6(netip.MustParseAddr(test[1]))
+		want = netip.MustParseAddr(test[0])
+		if v4 != want {
+			t.Fatalf("test %d: want: %v, got: %v", i, want, v4)
+		}
+	}
+}