diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index 02bec132d..05f087e21 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -12,7 +12,6 @@ import ( "errors" "net/http" "net/netip" - "strings" "sync" "go4.org/netipx" @@ -310,8 +309,8 @@ const AppConnectorsExperimentalAttrName = "tailscale.com/app-connectors-experime type config struct { isConfigured bool apps []appctype.Conn25Attr - appsByDomain map[string][]string - selfRoutedDomains set.Set[string] + appsByDomain map[dnsname.FQDN][]string + selfRoutedDomains set.Set[dnsname.FQDN] } func configFromNodeView(n tailcfg.NodeView) (config, error) { @@ -326,8 +325,8 @@ func configFromNodeView(n tailcfg.NodeView) (config, error) { cfg := config{ isConfigured: true, apps: apps, - appsByDomain: map[string][]string{}, - selfRoutedDomains: set.Set[string]{}, + appsByDomain: map[dnsname.FQDN][]string{}, + selfRoutedDomains: set.Set[dnsname.FQDN]{}, } for _, app := range apps { selfMatchesTags := false @@ -342,10 +341,9 @@ func configFromNodeView(n tailcfg.NodeView) (config, error) { if err != nil { return config{}, err } - key := fqdn.WithTrailingDot() - mak.Set(&cfg.appsByDomain, key, append(cfg.appsByDomain[key], app.Name)) + mak.Set(&cfg.appsByDomain, fqdn, append(cfg.appsByDomain[fqdn], app.Name)) if selfMatchesTags { - cfg.selfRoutedDomains.Add(key) + cfg.selfRoutedDomains.Add(fqdn) } } } @@ -362,9 +360,8 @@ type client struct { mu sync.Mutex // protects the fields below magicIPPool *ippool transitIPPool *ippool - // map of magic IP -> (transit IP, app) - magicIPs map[netip.Addr]appAddr - config config + assignments addrAssignments + config config } func (c *client) isConfigured() bool { @@ -407,13 +404,7 @@ func (c *client) reconfig(newCfg config) error { return nil } -func (c *client) setMagicIP(magicAddr, transitAddr netip.Addr, app string) { - c.mu.Lock() - defer c.mu.Unlock() - mak.Set(&c.magicIPs, magicAddr, appAddr{addr: transitAddr, app: app}) -} - -func (c *client) isConnectorDomain(domain string) bool { +func (c *client) isConnectorDomain(domain dnsname.FQDN) bool { c.mu.Lock() defer c.mu.Unlock() appNames, ok := c.config.appsByDomain[domain] @@ -424,9 +415,12 @@ func (c *client) isConnectorDomain(domain string) bool { // for this domain+dst address, so that this client can use conn25 connectors. // It checks that this domain should be routed and that this client is not itself a connector for the domain // and generally if it is valid to make the assignment. -func (c *client) reserveAddresses(domain string, dst netip.Addr) (addrs, error) { +func (c *client) reserveAddresses(domain dnsname.FQDN, dst netip.Addr) (addrs, error) { c.mu.Lock() defer c.mu.Unlock() + if existing, ok := c.assignments.lookupByDomainDst(domain, dst); ok { + return existing, nil + } appNames, _ := c.config.appsByDomain[domain] // only reserve for first app app := appNames[0] @@ -438,17 +432,20 @@ func (c *client) reserveAddresses(domain string, dst netip.Addr) (addrs, error) if err != nil { return addrs{}, err } - addrs := addrs{ + as := addrs{ dst: dst, magic: mip, transit: tip, app: app, + domain: domain, } - return addrs, nil + if err := c.assignments.insert(as); err != nil { + return addrs{}, err + } + return as, nil } func (c *client) enqueueAddressAssignment(addrs addrs) { - c.setMagicIP(addrs.magic, addrs.transit, addrs.app) // TODO(fran) 2026-02-03 asynchronously send peerapi req to connector to // allocate these addresses for us. } @@ -483,8 +480,12 @@ func (c *client) mapDNSResponse(buf []byte) []byte { switch h.Type { case dnsmessage.TypeA: - domain := strings.ToLower(h.Name.String()) - if len(domain) == 0 || !c.isConnectorDomain(domain) { + domain, err := dnsname.ToFQDN(h.Name.String()) + if err != nil { + c.logf("bad dnsname: %v", err) + return buf + } + if !c.isConnectorDomain(domain) { if err := p.SkipAnswer(); err != nil { c.logf("error parsing dns response: %v", err) return buf @@ -540,9 +541,44 @@ type addrs struct { dst netip.Addr magic netip.Addr transit netip.Addr + domain dnsname.FQDN app string } func (c addrs) isValid() bool { return c.dst.IsValid() } + +// domainDst is a key for looking up an existing address assignment by the +// DNS response domain and destination IP pair. +type domainDst struct { + domain dnsname.FQDN + dst netip.Addr +} + +// addrAssignments is the collection of addrs assigned by this client +// supporting lookup by magicip or domain+dst +type addrAssignments struct { + byMagicIP map[netip.Addr]addrs + byDomainDst map[domainDst]addrs +} + +func (a *addrAssignments) insert(as addrs) error { + // we likely will want to allow overwriting in the future when we + // have address expiry, but for now this should not happen + if _, ok := a.byMagicIP[as.magic]; ok { + return errors.New("byMagicIP key exists") + } + ddst := domainDst{domain: as.domain, dst: as.dst} + if _, ok := a.byDomainDst[ddst]; ok { + return errors.New("byDomainDst key exists") + } + mak.Set(&a.byMagicIP, as.magic, as) + mak.Set(&a.byDomainDst, ddst, as) + return nil +} + +func (a *addrAssignments) lookupByDomainDst(domain dnsname.FQDN, dst netip.Addr) (addrs, bool) { + v, ok := a.byDomainDst[domainDst{domain: domain, dst: dst}] + return v, ok +} diff --git a/feature/conn25/conn25_test.go b/feature/conn25/conn25_test.go index 0489b22a1..d63e84e02 100644 --- a/feature/conn25/conn25_test.go +++ b/feature/conn25/conn25_test.go @@ -16,6 +16,8 @@ import ( "tailscale.com/tailcfg" "tailscale.com/types/appctype" "tailscale.com/types/logger" + "tailscale.com/util/dnsname" + "tailscale.com/util/must" "tailscale.com/util/set" ) @@ -206,34 +208,16 @@ func TestTransitIPTargetUnknownTIP(t *testing.T) { } } -func TestSetMagicIP(t *testing.T) { - c := newConn25(logger.Discard) - mip := netip.MustParseAddr("0.0.0.1") - tip := netip.MustParseAddr("0.0.0.2") - app := "a" - c.client.setMagicIP(mip, tip, app) - val, ok := c.client.magicIPs[mip] - if !ok { - t.Fatal("expected there to be a value stored for the magic IP") - } - if val.addr != tip { - t.Fatalf("want %v, got %v", tip, val.addr) - } - if val.app != app { - t.Fatalf("want %s, got %s", app, val.app) - } -} - func TestReserveIPs(t *testing.T) { c := newConn25(logger.Discard) c.client.magicIPPool = newIPPool(mustIPSetFromPrefix("100.64.0.0/24")) c.client.transitIPPool = newIPPool(mustIPSetFromPrefix("169.254.0.0/24")) - mbd := map[string][]string{} + mbd := map[dnsname.FQDN][]string{} mbd["example.com."] = []string{"a"} c.client.config.appsByDomain = mbd dst := netip.MustParseAddr("0.0.0.1") - con, err := c.client.reserveAddresses("example.com.", dst) + addrs, err := c.client.reserveAddresses("example.com.", dst) if err != nil { t.Fatal(err) } @@ -242,18 +226,22 @@ func TestReserveIPs(t *testing.T) { wantMagic := netip.MustParseAddr("100.64.0.0") // first from magic pool wantTransit := netip.MustParseAddr("169.254.0.0") // first from transit pool wantApp := "a" // the app name related to example.com. + wantDomain := must.Get(dnsname.ToFQDN("example.com.")) - if wantDst != con.dst { - t.Errorf("want %v, got %v", wantDst, con.dst) + if wantDst != addrs.dst { + t.Errorf("want %v, got %v", wantDst, addrs.dst) + } + if wantMagic != addrs.magic { + t.Errorf("want %v, got %v", wantMagic, addrs.magic) } - if wantMagic != con.magic { - t.Errorf("want %v, got %v", wantMagic, con.magic) + if wantTransit != addrs.transit { + t.Errorf("want %v, got %v", wantTransit, addrs.transit) } - if wantTransit != con.transit { - t.Errorf("want %v, got %v", wantTransit, con.transit) + if wantApp != addrs.app { + t.Errorf("want %s, got %s", wantApp, addrs.app) } - if wantApp != con.app { - t.Errorf("want %s, got %s", wantApp, con.app) + if wantDomain != addrs.domain { + t.Errorf("want %s, got %s", wantDomain, addrs.domain) } } @@ -287,8 +275,8 @@ func TestConfigReconfig(t *testing.T) { cfg []appctype.Conn25Attr tags []string wantErr bool - wantAppsByDomain map[string][]string - wantSelfRoutedDomains set.Set[string] + wantAppsByDomain map[dnsname.FQDN][]string + wantSelfRoutedDomains set.Set[dnsname.FQDN] }{ { name: "bad-config", @@ -302,11 +290,11 @@ func TestConfigReconfig(t *testing.T) { {Name: "two", Domains: []string{"b.example.com"}, Connectors: []string{"tag:two"}}, }, tags: []string{"tag:one"}, - wantAppsByDomain: map[string][]string{ + wantAppsByDomain: map[dnsname.FQDN][]string{ "a.example.com.": {"one"}, "b.example.com.": {"two"}, }, - wantSelfRoutedDomains: set.SetOf([]string{"a.example.com."}), + wantSelfRoutedDomains: set.SetOf([]dnsname.FQDN{"a.example.com."}), }, { name: "more-complex", @@ -317,7 +305,7 @@ func TestConfigReconfig(t *testing.T) { {Name: "four", Domains: []string{"4.b.example.com", "4.d.example.com"}, Connectors: []string{"tag:four"}}, }, tags: []string{"tag:onea", "tag:four", "tag:unrelated"}, - wantAppsByDomain: map[string][]string{ + wantAppsByDomain: map[dnsname.FQDN][]string{ "1.a.example.com.": {"one"}, "1.b.example.com.": {"one", "three"}, "1.c.example.com.": {"three"}, @@ -326,7 +314,7 @@ func TestConfigReconfig(t *testing.T) { "4.b.example.com.": {"four"}, "4.d.example.com.": {"four"}, }, - wantSelfRoutedDomains: set.SetOf([]string{"1.a.example.com.", "1.b.example.com.", "4.b.example.com.", "4.d.example.com."}), + wantSelfRoutedDomains: set.SetOf([]dnsname.FQDN{"1.a.example.com.", "1.b.example.com.", "4.b.example.com.", "4.d.example.com."}), }, } { t.Run(tt.name, func(t *testing.T) { @@ -431,18 +419,24 @@ func TestMapDNSResponse(t *testing.T) { } for _, tt := range []struct { - name string - domain string - addrs []dnsmessage.AResource - wantMagicIPs map[netip.Addr]appAddr + name string + domain string + addrs []dnsmessage.AResource + wantByMagicIP map[netip.Addr]addrs }{ { name: "one-ip-matches", domain: "example.com.", addrs: []dnsmessage.AResource{{A: [4]byte{1, 0, 0, 0}}}, // these are 'expected' because they are the beginning of the provided pools - wantMagicIPs: map[netip.Addr]appAddr{ - netip.MustParseAddr("100.64.0.0"): {app: "app1", addr: netip.MustParseAddr("100.64.0.40")}, + wantByMagicIP: map[netip.Addr]addrs{ + netip.MustParseAddr("100.64.0.0"): { + domain: "example.com.", + dst: netip.MustParseAddr("1.0.0.0"), + magic: netip.MustParseAddr("100.64.0.0"), + transit: netip.MustParseAddr("100.64.0.40"), + app: "app1", + }, }, }, { @@ -452,9 +446,21 @@ func TestMapDNSResponse(t *testing.T) { {A: [4]byte{1, 0, 0, 0}}, {A: [4]byte{2, 0, 0, 0}}, }, - wantMagicIPs: map[netip.Addr]appAddr{ - netip.MustParseAddr("100.64.0.0"): {app: "app1", addr: netip.MustParseAddr("100.64.0.40")}, - netip.MustParseAddr("100.64.0.1"): {app: "app1", addr: netip.MustParseAddr("100.64.0.41")}, + wantByMagicIP: map[netip.Addr]addrs{ + netip.MustParseAddr("100.64.0.0"): { + domain: "example.com.", + dst: netip.MustParseAddr("1.0.0.0"), + magic: netip.MustParseAddr("100.64.0.0"), + transit: netip.MustParseAddr("100.64.0.40"), + app: "app1", + }, + netip.MustParseAddr("100.64.0.1"): { + domain: "example.com.", + dst: netip.MustParseAddr("2.0.0.0"), + magic: netip.MustParseAddr("100.64.0.1"), + transit: netip.MustParseAddr("100.64.0.41"), + app: "app1", + }, }, }, { @@ -482,9 +488,37 @@ func TestMapDNSResponse(t *testing.T) { if !reflect.DeepEqual(dnsResp, bs) { t.Fatal("shouldn't be changing the bytes (yet)") } - if diff := cmp.Diff(tt.wantMagicIPs, c.client.magicIPs, cmpopts.EquateComparable(appAddr{}, netip.Addr{})); diff != "" { - t.Errorf("magicIPs diff (-want, +got):\n%s", diff) + if diff := cmp.Diff(tt.wantByMagicIP, c.client.assignments.byMagicIP, cmpopts.EquateComparable(addrs{}, netip.Addr{})); diff != "" { + t.Errorf("byMagicIP diff (-want, +got):\n%s", diff) } }) } } + +func TestReserveAddressesDeduplicated(t *testing.T) { + c := newConn25(logger.Discard) + c.client.magicIPPool = newIPPool(mustIPSetFromPrefix("100.64.0.0/24")) + c.client.transitIPPool = newIPPool(mustIPSetFromPrefix("169.254.0.0/24")) + c.client.config.appsByDomain = map[dnsname.FQDN][]string{"example.com.": {"a"}} + + dst := netip.MustParseAddr("0.0.0.1") + first, err := c.client.reserveAddresses("example.com.", dst) + if err != nil { + t.Fatal(err) + } + + second, err := c.client.reserveAddresses("example.com.", dst) + if err != nil { + t.Fatal(err) + } + + if first != second { + t.Errorf("expected same addrs on repeated call, got first=%v second=%v", first, second) + } + if got := len(c.client.assignments.byMagicIP); got != 1 { + t.Errorf("want 1 entry in byMagicIP, got %d", got) + } + if got := len(c.client.assignments.byDomainDst); got != 1 { + t.Errorf("want 1 entry in byDomainDst, got %d", got) + } +}