diff --git a/appc/appconnector.go b/appc/appconnector.go index 4df696b0d..6d06656f9 100644 --- a/appc/appconnector.go +++ b/appc/appconnector.go @@ -62,6 +62,9 @@ type AppConnector struct { logf logger.Logf routeAdvertiser RouteAdvertiser + // storeRoutesFunc will be called to persist routes if it is not nil. + storeRoutesFunc func(*RouteInfo) error + // mu guards the fields that follow mu sync.Mutex @@ -80,11 +83,24 @@ type AppConnector struct { } // NewAppConnector creates a new AppConnector. -func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *AppConnector { - return &AppConnector{ +func NewAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser, routeInfo *RouteInfo, storeRoutesFunc func(*RouteInfo) error) *AppConnector { + ac := &AppConnector{ logf: logger.WithPrefix(logf, "appc: "), routeAdvertiser: routeAdvertiser, + storeRoutesFunc: storeRoutesFunc, } + if routeInfo != nil { + ac.domains = routeInfo.Domains + ac.wildcards = routeInfo.Wildcards + ac.controlRoutes = routeInfo.Control + } + return ac +} + +// ShouldStoreRoutes returns true if the appconnector was created with the controlknob on +// and is storing its discovered routes persistently. +func (e *AppConnector) ShouldStoreRoutes() bool { + return e.storeRoutesFunc != nil } // UpdateDomainsAndRoutes starts an asynchronous update of the configuration diff --git a/appc/appconnector_test.go b/appc/appconnector_test.go index 5b739b097..c95683335 100644 --- a/appc/appconnector_test.go +++ b/appc/appconnector_test.go @@ -17,194 +17,238 @@ "tailscale.com/util/must" ) +func fakeStoreRoutes(*RouteInfo) error { return nil } + func TestUpdateDomains(t *testing.T) { - ctx := context.Background() - a := NewAppConnector(t.Logf, nil) - a.UpdateDomains([]string{"example.com"}) + for _, shouldStore := range []bool{false, true} { + ctx := context.Background() + var a *AppConnector + if shouldStore { + a = NewAppConnector(t.Logf, nil, &RouteInfo{}, fakeStoreRoutes) + } else { + a = NewAppConnector(t.Logf, nil, nil, nil) + } + a.UpdateDomains([]string{"example.com"}) - a.Wait(ctx) - if got, want := a.Domains().AsSlice(), []string{"example.com"}; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + a.Wait(ctx) + if got, want := a.Domains().AsSlice(), []string{"example.com"}; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - addr := netip.MustParseAddr("192.0.0.8") - a.domains["example.com"] = append(a.domains["example.com"], addr) - a.UpdateDomains([]string{"example.com"}) - a.Wait(ctx) + addr := netip.MustParseAddr("192.0.0.8") + a.domains["example.com"] = append(a.domains["example.com"], addr) + a.UpdateDomains([]string{"example.com"}) + a.Wait(ctx) - if got, want := a.domains["example.com"], []netip.Addr{addr}; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + if got, want := a.domains["example.com"], []netip.Addr{addr}; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // domains are explicitly downcased on set. - a.UpdateDomains([]string{"UP.EXAMPLE.COM"}) - a.Wait(ctx) - if got, want := xmaps.Keys(a.domains), []string{"up.example.com"}; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) + // domains are explicitly downcased on set. + a.UpdateDomains([]string{"UP.EXAMPLE.COM"}) + a.Wait(ctx) + if got, want := xmaps.Keys(a.domains), []string{"up.example.com"}; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } } } func TestUpdateRoutes(t *testing.T) { - ctx := context.Background() - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) - a.updateDomains([]string{"*.example.com"}) + for _, shouldStore := range []bool{false, true} { + ctx := context.Background() + rc := &appctest.RouteCollector{} + var a *AppConnector + if shouldStore { + a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + } else { + a = NewAppConnector(t.Logf, rc, nil, nil) + } + a.updateDomains([]string{"*.example.com"}) - // This route should be collapsed into the range - a.ObserveDNSResponse(dnsResponse("a.example.com.", "192.0.2.1")) - a.Wait(ctx) + // This route should be collapsed into the range + a.ObserveDNSResponse(dnsResponse("a.example.com.", "192.0.2.1")) + a.Wait(ctx) - if !slices.Equal(rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) { - t.Fatalf("got %v, want %v", rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) - } + if !slices.Equal(rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) { + t.Fatalf("got %v, want %v", rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) + } - // This route should not be collapsed or removed - a.ObserveDNSResponse(dnsResponse("b.example.com.", "192.0.0.1")) - a.Wait(ctx) + // This route should not be collapsed or removed + a.ObserveDNSResponse(dnsResponse("b.example.com.", "192.0.0.1")) + a.Wait(ctx) - routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")} - a.updateRoutes(routes) + routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24"), netip.MustParsePrefix("192.0.0.1/32")} + a.updateRoutes(routes) - slices.SortFunc(rc.Routes(), prefixCompare) - rc.SetRoutes(slices.Compact(rc.Routes())) - slices.SortFunc(routes, prefixCompare) + slices.SortFunc(rc.Routes(), prefixCompare) + rc.SetRoutes(slices.Compact(rc.Routes())) + slices.SortFunc(routes, prefixCompare) - // Ensure that the non-matching /32 is preserved, even though it's in the domains table. - if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) { - t.Errorf("added routes: got %v, want %v", rc.Routes(), routes) - } + // Ensure that the non-matching /32 is preserved, even though it's in the domains table. + if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) { + t.Errorf("added routes: got %v, want %v", rc.Routes(), routes) + } - // Ensure that the contained /32 is removed, replaced by the /24. - wantRemoved := []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")} - if !slices.EqualFunc(rc.RemovedRoutes(), wantRemoved, prefixEqual) { - t.Fatalf("unexpected removed routes: %v", rc.RemovedRoutes()) + // Ensure that the contained /32 is removed, replaced by the /24. + wantRemoved := []netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")} + if !slices.EqualFunc(rc.RemovedRoutes(), wantRemoved, prefixEqual) { + t.Fatalf("unexpected removed routes: %v", rc.RemovedRoutes()) + } } } func TestUpdateRoutesUnadvertisesContainedRoutes(t *testing.T) { - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) - mak.Set(&a.domains, "example.com", []netip.Addr{netip.MustParseAddr("192.0.2.1")}) - rc.SetRoutes([]netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) - routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")} - a.updateRoutes(routes) + for _, shouldStore := range []bool{false, true} { + rc := &appctest.RouteCollector{} + var a *AppConnector + if shouldStore { + a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + } else { + a = NewAppConnector(t.Logf, rc, nil, nil) + } + mak.Set(&a.domains, "example.com", []netip.Addr{netip.MustParseAddr("192.0.2.1")}) + rc.SetRoutes([]netip.Prefix{netip.MustParsePrefix("192.0.2.1/32")}) + routes := []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")} + a.updateRoutes(routes) - if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) { - t.Fatalf("got %v, want %v", rc.Routes(), routes) + if !slices.EqualFunc(routes, rc.Routes(), prefixEqual) { + t.Fatalf("got %v, want %v", rc.Routes(), routes) + } } } func TestDomainRoutes(t *testing.T) { - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) - a.updateDomains([]string{"example.com"}) - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - a.Wait(context.Background()) + for _, shouldStore := range []bool{false, true} { + rc := &appctest.RouteCollector{} + var a *AppConnector + if shouldStore { + a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + } else { + a = NewAppConnector(t.Logf, rc, nil, nil) + } + a.updateDomains([]string{"example.com"}) + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + a.Wait(context.Background()) - want := map[string][]netip.Addr{ - "example.com": {netip.MustParseAddr("192.0.0.8")}, - } + want := map[string][]netip.Addr{ + "example.com": {netip.MustParseAddr("192.0.0.8")}, + } - if got := a.DomainRoutes(); !reflect.DeepEqual(got, want) { - t.Fatalf("DomainRoutes: got %v, want %v", got, want) + if got := a.DomainRoutes(); !reflect.DeepEqual(got, want) { + t.Fatalf("DomainRoutes: got %v, want %v", got, want) + } } } func TestObserveDNSResponse(t *testing.T) { - ctx := context.Background() - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) + for _, shouldStore := range []bool{false, true} { + ctx := context.Background() + rc := &appctest.RouteCollector{} + var a *AppConnector + if shouldStore { + a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + } else { + a = NewAppConnector(t.Logf, rc, nil, nil) + } - // a has no domains configured, so it should not advertise any routes - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - if got, want := rc.Routes(), ([]netip.Prefix)(nil); !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + // a has no domains configured, so it should not advertise any routes + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + if got, want := rc.Routes(), ([]netip.Prefix)(nil); !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - a.updateDomains([]string{"example.com"}) - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - a.Wait(ctx) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + a.updateDomains([]string{"example.com"}) + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + a.Wait(ctx) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // a CNAME record chain should result in a route being added if the chain - // matches a routed domain. - a.updateDomains([]string{"www.example.com", "example.com"}) - a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.9", "www.example.com.", "chain.example.com.", "example.com.")) - a.Wait(ctx) - wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.9/32")) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + // a CNAME record chain should result in a route being added if the chain + // matches a routed domain. + a.updateDomains([]string{"www.example.com", "example.com"}) + a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.9", "www.example.com.", "chain.example.com.", "example.com.")) + a.Wait(ctx) + wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.9/32")) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // a CNAME record chain should result in a route being added if the chain - // even if only found in the middle of the chain - a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.10", "outside.example.org.", "www.example.com.", "example.org.")) - a.Wait(ctx) - wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.10/32")) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + // a CNAME record chain should result in a route being added if the chain + // even if only found in the middle of the chain + a.ObserveDNSResponse(dnsCNAMEResponse("192.0.0.10", "outside.example.org.", "www.example.com.", "example.org.")) + a.Wait(ctx) + wantRoutes = append(wantRoutes, netip.MustParsePrefix("192.0.0.10/32")) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - wantRoutes = append(wantRoutes, netip.MustParsePrefix("2001:db8::1/128")) + wantRoutes = append(wantRoutes, netip.MustParsePrefix("2001:db8::1/128")) - a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) - a.Wait(ctx) - if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { - t.Errorf("got %v; want %v", got, want) - } + a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) + a.Wait(ctx) + if got, want := rc.Routes(), wantRoutes; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } - // don't re-advertise routes that have already been advertised - a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) - a.Wait(ctx) - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) - } + // don't re-advertise routes that have already been advertised + a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) + a.Wait(ctx) + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) + } - // don't advertise addresses that are already in a control provided route - pfx := netip.MustParsePrefix("192.0.2.0/24") - a.updateRoutes([]netip.Prefix{pfx}) - wantRoutes = append(wantRoutes, pfx) - a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1")) - a.Wait(ctx) - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) - } - if !slices.Contains(a.domains["example.com"], netip.MustParseAddr("192.0.2.1")) { - t.Errorf("missing %v from %v", "192.0.2.1", a.domains["exmaple.com"]) + // don't advertise addresses that are already in a control provided route + pfx := netip.MustParsePrefix("192.0.2.0/24") + a.updateRoutes([]netip.Prefix{pfx}) + wantRoutes = append(wantRoutes, pfx) + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.2.1")) + a.Wait(ctx) + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("rc.Routes(): got %v; want %v", rc.Routes(), wantRoutes) + } + if !slices.Contains(a.domains["example.com"], netip.MustParseAddr("192.0.2.1")) { + t.Errorf("missing %v from %v", "192.0.2.1", a.domains["exmaple.com"]) + } } } func TestWildcardDomains(t *testing.T) { - ctx := context.Background() - rc := &appctest.RouteCollector{} - a := NewAppConnector(t.Logf, rc) + for _, shouldStore := range []bool{false, true} { + ctx := context.Background() + rc := &appctest.RouteCollector{} + var a *AppConnector + if shouldStore { + a = NewAppConnector(t.Logf, rc, &RouteInfo{}, fakeStoreRoutes) + } else { + a = NewAppConnector(t.Logf, rc, nil, nil) + } - a.updateDomains([]string{"*.example.com"}) - a.ObserveDNSResponse(dnsResponse("foo.example.com.", "192.0.0.8")) - a.Wait(ctx) - if got, want := rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) { - t.Errorf("routes: got %v; want %v", got, want) - } - if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { - t.Errorf("wildcards: got %v; want %v", got, want) - } + a.updateDomains([]string{"*.example.com"}) + a.ObserveDNSResponse(dnsResponse("foo.example.com.", "192.0.0.8")) + a.Wait(ctx) + if got, want := rc.Routes(), []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")}; !slices.Equal(got, want) { + t.Errorf("routes: got %v; want %v", got, want) + } + if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { + t.Errorf("wildcards: got %v; want %v", got, want) + } - a.updateDomains([]string{"*.example.com", "example.com"}) - if _, ok := a.domains["foo.example.com"]; !ok { - t.Errorf("expected foo.example.com to be preserved in domains due to wildcard") - } - if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { - t.Errorf("wildcards: got %v; want %v", got, want) - } + a.updateDomains([]string{"*.example.com", "example.com"}) + if _, ok := a.domains["foo.example.com"]; !ok { + t.Errorf("expected foo.example.com to be preserved in domains due to wildcard") + } + if got, want := a.wildcards, []string{"example.com"}; !slices.Equal(got, want) { + t.Errorf("wildcards: got %v; want %v", got, want) + } - // There was an early regression where the wildcard domain was added repeatedly, this guards against that. - a.updateDomains([]string{"*.example.com", "example.com"}) - if len(a.wildcards) != 1 { - t.Errorf("expected only one wildcard domain, got %v", a.wildcards) + // There was an early regression where the wildcard domain was added repeatedly, this guards against that. + a.updateDomains([]string{"*.example.com", "example.com"}) + if len(a.wildcards) != 1 { + t.Errorf("expected only one wildcard domain, got %v", a.wildcards) + } } } diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 6e3a62967..2f80ba38c 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -72,6 +72,10 @@ type Knobs struct { // ProbeUDPLifetime is whether the node should probe UDP path lifetime on // the tail end of an active direct connection in magicsock. ProbeUDPLifetime atomic.Bool + + // AppCStoreRoutes is whether the node should store RouteInfo to StateStore + // if it's an app connector. + AppCStoreRoutes atomic.Bool } // UpdateFromNodeAttributes updates k (if non-nil) based on the provided self @@ -96,6 +100,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { forceNfTables = has(tailcfg.NodeAttrLinuxMustUseNfTables) seamlessKeyRenewal = has(tailcfg.NodeAttrSeamlessKeyRenewal) probeUDPLifetime = has(tailcfg.NodeAttrProbeUDPLifetime) + appCStoreRoutes = has(tailcfg.NodeAttrStoreAppCRoutes) ) if has(tailcfg.NodeAttrOneCGNATEnable) { @@ -118,6 +123,7 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { k.LinuxForceNfTables.Store(forceNfTables) k.SeamlessKeyRenewal.Store(seamlessKeyRenewal) k.ProbeUDPLifetime.Store(probeUDPLifetime) + k.AppCStoreRoutes.Store(appCStoreRoutes) } // AsDebugJSON returns k as something that can be marshalled with json.Marshal @@ -141,5 +147,6 @@ func (k *Knobs) AsDebugJSON() map[string]any { "LinuxForceNfTables": k.LinuxForceNfTables.Load(), "SeamlessKeyRenewal": k.SeamlessKeyRenewal.Load(), "ProbeUDPLifetime": k.ProbeUDPLifetime.Load(), + "AppCStoreRoutes": k.AppCStoreRoutes.Load(), } } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 73453603d..c3e92cdad 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3516,8 +3516,22 @@ func (b *LocalBackend) reconfigAppConnectorLocked(nm *netmap.NetworkMap, prefs i return } - if b.appConnector == nil { - b.appConnector = appc.NewAppConnector(b.logf, b) + shouldAppCStoreRoutes := b.ControlKnobs().AppCStoreRoutes.Load() + if b.appConnector == nil || b.appConnector.ShouldStoreRoutes() != shouldAppCStoreRoutes { + var ri *appc.RouteInfo + var storeFunc func(*appc.RouteInfo) error + if shouldAppCStoreRoutes { + var err error + ri, err = b.readRouteInfoLocked() + if err != nil { + ri = &appc.RouteInfo{} + if err != ipn.ErrStateNotExist { + b.logf("Unsuccessful Read RouteInfo: ", err) + } + } + storeFunc = b.storeRouteInfo + } + b.appConnector = appc.NewAppConnector(b.logf, b, ri, storeFunc) } if nm == nil { return diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index acb1c3f0a..61efc2fb0 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -55,6 +55,8 @@ "tailscale.com/wgengine/wgcfg" ) +func fakeStoreRoutes(*appc.RouteInfo) error { return nil } + func inRemove(ip netip.Addr) bool { for _, pfx := range removeFromDefaultRoute { if pfx.Contains(ip) { @@ -1291,13 +1293,19 @@ type tc struct { } func TestOfferingAppConnector(t *testing.T) { - b := newTestBackend(t) - if b.OfferingAppConnector() { - t.Fatal("unexpected offering app connector") - } - b.appConnector = appc.NewAppConnector(t.Logf, nil) - if !b.OfferingAppConnector() { - t.Fatal("unexpected not offering app connector") + for _, shouldStore := range []bool{false, true} { + b := newTestBackend(t) + if b.OfferingAppConnector() { + t.Fatal("unexpected offering app connector") + } + if shouldStore { + b.appConnector = appc.NewAppConnector(t.Logf, nil, &appc.RouteInfo{}, fakeStoreRoutes) + } else { + b.appConnector = appc.NewAppConnector(t.Logf, nil, nil, nil) + } + if !b.OfferingAppConnector() { + t.Fatal("unexpected not offering app connector") + } } } @@ -1342,21 +1350,27 @@ func TestRouterAdvertiserIgnoresContainedRoutes(t *testing.T) { } func TestObserveDNSResponse(t *testing.T) { - b := newTestBackend(t) + for _, shouldStore := range []bool{false, true} { + b := newTestBackend(t) - // ensure no error when no app connector is configured - b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + // ensure no error when no app connector is configured + b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - rc := &appctest.RouteCollector{} - b.appConnector = appc.NewAppConnector(t.Logf, rc) - b.appConnector.UpdateDomains([]string{"example.com"}) - b.appConnector.Wait(context.Background()) + rc := &appctest.RouteCollector{} + if shouldStore { + b.appConnector = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes) + } else { + b.appConnector = appc.NewAppConnector(t.Logf, rc, nil, nil) + } + b.appConnector.UpdateDomains([]string{"example.com"}) + b.appConnector.Wait(context.Background()) - b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) - b.appConnector.Wait(context.Background()) - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Fatalf("got routes %v, want %v", rc.Routes(), wantRoutes) + b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + b.appConnector.Wait(context.Background()) + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Fatalf("got routes %v, want %v", rc.Routes(), wantRoutes) + } } } diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index bffbb1076..1e63a396e 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -687,185 +687,209 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) { } func TestPeerAPIPrettyReplyCNAME(t *testing.T) { - var h peerAPIHandler - h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") + for _, shouldStore := range []bool{false, true} { + var h peerAPIHandler + h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") - eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) - h.ps = &peerAPIServer{ - b: &LocalBackend{ - e: eng, - pm: pm, - store: pm.Store(), - // configure as an app connector just to enable the API. - appConnector: appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}), - }, - } + eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + var a *appc.AppConnector + if shouldStore { + a = appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}, &appc.RouteInfo{}, fakeStoreRoutes) + } else { + a = appc.NewAppConnector(t.Logf, &appctest.RouteCollector{}, nil, nil) + } + h.ps = &peerAPIServer{ + b: &LocalBackend{ + e: eng, + pm: pm, + store: pm.Store(), + // configure as an app connector just to enable the API. + appConnector: a, + }, + } - h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { - b.CNAMEResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("www.example.com."), - Type: dnsmessage.TypeCNAME, - Class: dnsmessage.ClassINET, - TTL: 0, - }, - dnsmessage.CNAMEResource{ - CNAME: dnsmessage.MustNewName("example.com."), - }, - ) - b.AResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - TTL: 0, - }, - dnsmessage.AResource{ - A: [4]byte{192, 0, 0, 8}, - }, - ) - }} - f := filter.NewAllowAllForTest(logger.Discard) - h.ps.b.setFilter(f) + h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { + b.CNAMEResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("www.example.com."), + Type: dnsmessage.TypeCNAME, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.CNAMEResource{ + CNAME: dnsmessage.MustNewName("example.com."), + }, + ) + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("example.com."), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: [4]byte{192, 0, 0, 8}, + }, + ) + }} + f := filter.NewAllowAllForTest(logger.Discard) + h.ps.b.setFilter(f) - if !h.replyToDNSQueries() { - t.Errorf("unexpectedly deny; wanted to be a DNS server") - } + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly deny; wanted to be a DNS server") + } - w := httptest.NewRecorder() - h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) - if w.Code != http.StatusOK { - t.Errorf("unexpected status code: %v", w.Code) - } - var addrs []string - json.NewDecoder(w.Body).Decode(&addrs) - if len(addrs) == 0 { - t.Fatalf("no addresses returned") - } - for _, addr := range addrs { - netip.MustParseAddr(addr) + w := httptest.NewRecorder() + h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: %v", w.Code) + } + var addrs []string + json.NewDecoder(w.Body).Decode(&addrs) + if len(addrs) == 0 { + t.Fatalf("no addresses returned") + } + for _, addr := range addrs { + netip.MustParseAddr(addr) + } } } func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) { - ctx := context.Background() - var h peerAPIHandler - h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") + for _, shouldStore := range []bool{false, true} { + ctx := context.Background() + var h peerAPIHandler + h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") - rc := &appctest.RouteCollector{} - eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) - h.ps = &peerAPIServer{ - b: &LocalBackend{ - e: eng, - pm: pm, - store: pm.Store(), - appConnector: appc.NewAppConnector(t.Logf, rc), - }, - } - h.ps.b.appConnector.UpdateDomains([]string{"example.com"}) - h.ps.b.appConnector.Wait(ctx) - - h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { - b.AResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - TTL: 0, + rc := &appctest.RouteCollector{} + eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + var a *appc.AppConnector + if shouldStore { + a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes) + } else { + a = appc.NewAppConnector(t.Logf, rc, nil, nil) + } + h.ps = &peerAPIServer{ + b: &LocalBackend{ + e: eng, + pm: pm, + store: pm.Store(), + appConnector: a, }, - dnsmessage.AResource{ - A: [4]byte{192, 0, 0, 8}, - }, - ) - }} - f := filter.NewAllowAllForTest(logger.Discard) - h.ps.b.setFilter(f) + } + h.ps.b.appConnector.UpdateDomains([]string{"example.com"}) + h.ps.b.appConnector.Wait(ctx) - if !h.ps.b.OfferingAppConnector() { - t.Fatal("expecting to be offering app connector") - } - if !h.replyToDNSQueries() { - t.Errorf("unexpectedly deny; wanted to be a DNS server") - } + h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("example.com."), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: [4]byte{192, 0, 0, 8}, + }, + ) + }} + f := filter.NewAllowAllForTest(logger.Discard) + h.ps.b.setFilter(f) - w := httptest.NewRecorder() - h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=example.com.", nil)) - if w.Code != http.StatusOK { - t.Errorf("unexpected status code: %v", w.Code) - } - h.ps.b.appConnector.Wait(ctx) + if !h.ps.b.OfferingAppConnector() { + t.Fatal("expecting to be offering app connector") + } + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly deny; wanted to be a DNS server") + } - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + w := httptest.NewRecorder() + h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=example.com.", nil)) + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: %v", w.Code) + } + h.ps.b.appConnector.Wait(ctx) + + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + } } } func TestPeerAPIReplyToDNSQueriesAreObservedWithCNAMEFlattening(t *testing.T) { - ctx := context.Background() - var h peerAPIHandler - h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") + for _, shouldStore := range []bool{false, true} { + ctx := context.Background() + var h peerAPIHandler + h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") - rc := &appctest.RouteCollector{} - eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) - h.ps = &peerAPIServer{ - b: &LocalBackend{ - e: eng, - pm: pm, - store: pm.Store(), - appConnector: appc.NewAppConnector(t.Logf, rc), - }, - } - h.ps.b.appConnector.UpdateDomains([]string{"www.example.com"}) - h.ps.b.appConnector.Wait(ctx) - - h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { - b.CNAMEResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("www.example.com."), - Type: dnsmessage.TypeCNAME, - Class: dnsmessage.ClassINET, - TTL: 0, + rc := &appctest.RouteCollector{} + eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) + pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + var a *appc.AppConnector + if shouldStore { + a = appc.NewAppConnector(t.Logf, rc, &appc.RouteInfo{}, fakeStoreRoutes) + } else { + a = appc.NewAppConnector(t.Logf, rc, nil, nil) + } + h.ps = &peerAPIServer{ + b: &LocalBackend{ + e: eng, + pm: pm, + store: pm.Store(), + appConnector: a, }, - dnsmessage.CNAMEResource{ - CNAME: dnsmessage.MustNewName("example.com."), - }, - ) - b.AResource( - dnsmessage.ResourceHeader{ - Name: dnsmessage.MustNewName("example.com."), - Type: dnsmessage.TypeA, - Class: dnsmessage.ClassINET, - TTL: 0, - }, - dnsmessage.AResource{ - A: [4]byte{192, 0, 0, 8}, - }, - ) - }} - f := filter.NewAllowAllForTest(logger.Discard) - h.ps.b.setFilter(f) + } + h.ps.b.appConnector.UpdateDomains([]string{"www.example.com"}) + h.ps.b.appConnector.Wait(ctx) - if !h.ps.b.OfferingAppConnector() { - t.Fatal("expecting to be offering app connector") - } - if !h.replyToDNSQueries() { - t.Errorf("unexpectedly deny; wanted to be a DNS server") - } + h.ps.resolver = &fakeResolver{build: func(b *dnsmessage.Builder) { + b.CNAMEResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("www.example.com."), + Type: dnsmessage.TypeCNAME, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.CNAMEResource{ + CNAME: dnsmessage.MustNewName("example.com."), + }, + ) + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName("example.com."), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: [4]byte{192, 0, 0, 8}, + }, + ) + }} + f := filter.NewAllowAllForTest(logger.Discard) + h.ps.b.setFilter(f) - w := httptest.NewRecorder() - h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) - if w.Code != http.StatusOK { - t.Errorf("unexpected status code: %v", w.Code) - } - h.ps.b.appConnector.Wait(ctx) + if !h.ps.b.OfferingAppConnector() { + t.Fatal("expecting to be offering app connector") + } + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly deny; wanted to be a DNS server") + } - wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} - if !slices.Equal(rc.Routes(), wantRoutes) { - t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + w := httptest.NewRecorder() + h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=www.example.com.", nil)) + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: %v", w.Code) + } + h.ps.b.appConnector.Wait(ctx) + + wantRoutes := []netip.Prefix{netip.MustParsePrefix("192.0.0.8/32")} + if !slices.Equal(rc.Routes(), wantRoutes) { + t.Errorf("got %v; want %v", rc.Routes(), wantRoutes) + } } } diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index 39ef0d7f6..5308a04ed 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2253,6 +2253,9 @@ type Oauth2Token struct { // NodeAttrAutoExitNode permits the automatic exit nodes feature. NodeAttrAutoExitNode NodeCapability = "auto-exit-node" + + // NodeAttrStoreAppCRoutes configures the node to store app connector routes persistently. + NodeAttrStoreAppCRoutes NodeCapability = "store-appc-routes" ) // SetDNSRequest is a request to add a DNS record.