From b48b7d82d0c2875ee452dab95b63ae56df716e0f Mon Sep 17 00:00:00 2001 From: James Tucker Date: Fri, 27 Oct 2023 14:20:10 -0700 Subject: [PATCH] appc,ipn/ipnlocal,net/dns/resolver: add App Connector wiring when enabled in prefs An EmbeddedAppConnector is added that when configured observes DNS responses from the PeerAPI. If a response is found matching a configured domain, routes are advertised when necessary. The wiring from a configuration in the netmap capmap is not yet done, so while the connector can be enabled, no domains can yet be added. Updates tailscale/corp#15437 Signed-off-by: James Tucker --- appc/appc.go | 2 +- appc/embedded.go | 165 +++++++++++++++++++++++++++++++++++ appc/embedded_test.go | 118 +++++++++++++++++++++++++ cmd/tailscaled/depaware.txt | 5 +- ipn/ipnlocal/local.go | 56 +++++++++++- ipn/ipnlocal/local_test.go | 92 +++++++++++++++++++ ipn/ipnlocal/peerapi.go | 23 +++-- ipn/ipnlocal/peerapi_test.go | 64 ++++++++++++++ net/dns/resolver/tsdns.go | 8 +- 9 files changed, 518 insertions(+), 15 deletions(-) create mode 100644 appc/embedded.go create mode 100644 appc/embedded_test.go diff --git a/appc/appc.go b/appc/appc.go index d66ece8a3..6da47c3c7 100644 --- a/appc/appc.go +++ b/appc/appc.go @@ -31,7 +31,7 @@ type target struct { Matching tailcfg.ProtoPortRange } -// Server implements an App Connector. +// Server implements an App Connector as expressed in sniproxy. type Server struct { mu sync.RWMutex // mu guards following fields connectors map[appctype.ConfigID]connector diff --git a/appc/embedded.go b/appc/embedded.go new file mode 100644 index 000000000..167377431 --- /dev/null +++ b/appc/embedded.go @@ -0,0 +1,165 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +// Package appc implements App Connectors. An AppConnector provides domain +// oriented routing of traffic. +package appc + +import ( + "net/netip" + "slices" + "strings" + "sync" + + "golang.org/x/net/dns/dnsmessage" + "tailscale.com/types/logger" +) + +/* + * TODO(raggi): the sniproxy servicing portions of this package will be moved + * into the sniproxy or deprecated at some point, when doing so is not + * disruptive. At that time EmbeddedAppConnector can be renamed to AppConnector. + */ + +// RouteAdvertiser is an interface that allows the AppConnector to advertise +// newly discovered routes that need to be served through the AppConnector. +type RouteAdvertiser interface { + // AdvertiseRoute adds a new route advertisement if the route is not already + // being advertised. + AdvertiseRoute(netip.Prefix) error +} + +// EmbeddedAppConnector is an implementation of an AppConnector that performs +// its function as a subsystem inside of a tailscale node. At the control plane +// side App Connector routing is configured in terms of domains rather than IP +// addresses. +// The AppConnectors responsibility inside tailscaled is to apply the routing +// and domain configuration as supplied in the map response. +// DNS requests for configured domains are observed. If the domains resolve to +// routes not yet served by the AppConnector the local node configuration is +// updated to advertise the new route. +type EmbeddedAppConnector struct { + logf logger.Logf + routeAdvertiser RouteAdvertiser + + // mu guards the fields that follow + mu sync.Mutex + // domains is a map of lower case domain names with no trailing dot, to a + // list of resolved IP addresses. + domains map[string][]netip.Addr +} + +// NewEmbeddedAppConnector creates a new EmbeddedAppConnector. +func NewEmbeddedAppConnector(logf logger.Logf, routeAdvertiser RouteAdvertiser) *EmbeddedAppConnector { + return &EmbeddedAppConnector{ + logf: logger.WithPrefix(logf, "appc: "), + routeAdvertiser: routeAdvertiser, + } +} + +// UpdateDomains replaces the current set of configured domains with the +// supplied set of domains. Domains must not contain a trailing dot, and should +// be lower case. +func (e *EmbeddedAppConnector) UpdateDomains(domains []string) { + e.mu.Lock() + defer e.mu.Unlock() + + var old map[string][]netip.Addr + old, e.domains = e.domains, make(map[string][]netip.Addr, len(domains)) + for _, d := range domains { + d = strings.ToLower(d) + e.domains[d] = old[d] + } +} + +// ObserveDNSResponse is a callback invoked by the DNS resolver when a DNS +// response is being returned over the PeerAPI. The response is parsed and +// matched against the configured domains, if matched the routeAdvertiser is +// advised to advertise the discovered route. +func (e *EmbeddedAppConnector) ObserveDNSResponse(res []byte) { + var p dnsmessage.Parser + if _, err := p.Start(res); err != nil { + return + } + if err := p.SkipAllQuestions(); err != nil { + return + } + + for { + h, err := p.AnswerHeader() + if err == dnsmessage.ErrSectionDone { + break + } + if err != nil { + return + } + + if h.Class != dnsmessage.ClassINET { + if err := p.SkipAnswer(); err != nil { + return + } + continue + } + if h.Type != dnsmessage.TypeA && h.Type != dnsmessage.TypeAAAA { + if err := p.SkipAnswer(); err != nil { + return + } + continue + } + + domain := h.Name.String() + if len(domain) == 0 { + return + } + if domain[len(domain)-1] == '.' { + domain = domain[:len(domain)-1] + } + domain = strings.ToLower(domain) + e.logf("[v2] observed DNS response for %s", domain) + + e.mu.Lock() + addrs, ok := e.domains[domain] + e.mu.Unlock() + if !ok { + if err := p.SkipAnswer(); err != nil { + return + } + continue + } + + var addr netip.Addr + switch h.Type { + case dnsmessage.TypeA: + r, err := p.AResource() + if err != nil { + return + } + addr = netip.AddrFrom4(r.A) + case dnsmessage.TypeAAAA: + r, err := p.AAAAResource() + if err != nil { + return + } + addr = netip.AddrFrom16(r.AAAA) + default: + if err := p.SkipAnswer(); err != nil { + return + } + continue + } + if slices.Contains(addrs, addr) { + continue + } + // TODO(raggi): check for existing prefixes + if err := e.routeAdvertiser.AdvertiseRoute(netip.PrefixFrom(addr, addr.BitLen())); err != nil { + e.logf("failed to advertise route for %v: %v", addr, err) + continue + } + e.logf("[v2] advertised route for %v: %v", domain, addr) + + e.mu.Lock() + e.domains[domain] = append(addrs, addr) + e.mu.Unlock() + } + +} diff --git a/appc/embedded_test.go b/appc/embedded_test.go new file mode 100644 index 000000000..fbf29f998 --- /dev/null +++ b/appc/embedded_test.go @@ -0,0 +1,118 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +package appc + +import ( + "net/netip" + "slices" + "testing" + + xmaps "golang.org/x/exp/maps" + "golang.org/x/net/dns/dnsmessage" + "tailscale.com/util/must" +) + +func TestUpdateDomains(t *testing.T) { + a := NewEmbeddedAppConnector(t.Logf, nil) + a.UpdateDomains([]string{"example.com"}) + if got, want := xmaps.Keys(a.domains), []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"}) + + 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"}) + if got, want := xmaps.Keys(a.domains), []string{"up.example.com"}; !slices.Equal(got, want) { + t.Errorf("got %v; want %v", got, want) + } +} + +func TestObserveDNSResponse(t *testing.T) { + rc := &routeCollector{} + a := NewEmbeddedAppConnector(t.Logf, rc) + + // 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")} + + a.UpdateDomains([]string{"example.com"}) + a.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + 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")) + + a.ObserveDNSResponse(dnsResponse("example.com.", "2001:db8::1")) + 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")) + if !slices.Equal(rc.routes, wantRoutes) { + t.Errorf("got %v; want %v", rc.routes, wantRoutes) + } +} + +// dnsResponse is a test helper that creates a DNS response buffer for the given domain and address +func dnsResponse(domain, address string) []byte { + addr := netip.MustParseAddr(address) + b := dnsmessage.NewBuilder(nil, dnsmessage.Header{}) + b.EnableCompression() + b.StartAnswers() + switch addr.BitLen() { + case 32: + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(domain), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: addr.As4(), + }, + ) + case 128: + b.AAAAResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(domain), + Type: dnsmessage.TypeAAAA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AAAAResource{ + AAAA: addr.As16(), + }, + ) + default: + panic("invalid address length") + } + return must.Get(b.Finish()) +} + +// routeCollector is a test helper that collects the list of routes advertised +type routeCollector struct { + routes []netip.Prefix +} + +// routeCollector implements RouteAdvertiser +var _ RouteAdvertiser = (*routeCollector)(nil) + +func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error { + rc.routes = append(rc.routes, pfx) + return nil +} diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 40d4ab0fb..92c59b1ff 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -216,11 +216,13 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de gvisor.dev/gvisor/pkg/tcpip/transport/udp from tailscale.com/net/tstun+ gvisor.dev/gvisor/pkg/waiter from gvisor.dev/gvisor/pkg/context+ inet.af/peercred from tailscale.com/ipn/ipnauth + inet.af/tcpproxy from tailscale.com/appc W 💣 inet.af/wf from tailscale.com/wf nhooyr.io/websocket from tailscale.com/derp/derphttp+ nhooyr.io/websocket/internal/errd from nhooyr.io/websocket nhooyr.io/websocket/internal/xsync from nhooyr.io/websocket tailscale.com from tailscale.com/version + tailscale.com/appc from tailscale.com/ipn/ipnlocal tailscale.com/atomicfile from tailscale.com/ipn+ LD tailscale.com/chirp from tailscale.com/cmd/tailscaled tailscale.com/client/tailscale from tailscale.com/derp+ @@ -269,7 +271,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/net/dns/publicdns from tailscale.com/net/dns/resolver+ tailscale.com/net/dns/recursive from tailscale.com/net/dnsfallback tailscale.com/net/dns/resolvconffile from tailscale.com/net/dns+ - tailscale.com/net/dns/resolver from tailscale.com/ipn/ipnlocal+ + tailscale.com/net/dns/resolver from tailscale.com/net/dns tailscale.com/net/dnscache from tailscale.com/control/controlclient+ tailscale.com/net/dnsfallback from tailscale.com/control/controlclient+ tailscale.com/net/flowtrack from tailscale.com/net/packet+ @@ -319,6 +321,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/tstime/mono from tailscale.com/net/tstun+ tailscale.com/tstime/rate from tailscale.com/wgengine/filter+ tailscale.com/tsweb/varz from tailscale.com/cmd/tailscaled + tailscale.com/types/appctype from tailscale.com/appc tailscale.com/types/dnstype from tailscale.com/ipn/ipnlocal+ tailscale.com/types/empty from tailscale.com/ipn+ tailscale.com/types/flagtype from tailscale.com/cmd/tailscaled diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 074b813fa..0d46a19c0 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -32,6 +32,7 @@ "go4.org/netipx" xmaps "golang.org/x/exp/maps" "gvisor.dev/gvisor/pkg/tcpip" + "tailscale.com/appc" "tailscale.com/client/tailscale/apitype" "tailscale.com/control/controlclient" "tailscale.com/control/controlknobs" @@ -203,9 +204,10 @@ type LocalBackend struct { conf *conffile.Config // latest parsed config, or nil if not in declarative mode pm *profileManager // mu guards access filterHash deephash.Sum - httpTestClient *http.Client // for controlclient. nil by default, used by tests. - ccGen clientGen // function for producing controlclient; lazily populated - sshServer SSHServer // or nil, initialized lazily. + httpTestClient *http.Client // for controlclient. nil by default, used by tests. + ccGen clientGen // function for producing controlclient; lazily populated + sshServer SSHServer // or nil, initialized lazily. + appConnector *appc.EmbeddedAppConnector // or nil, initialized when configured. webClient webClient notify func(ipn.Notify) cc controlclient.Client @@ -2995,6 +2997,9 @@ func (b *LocalBackend) setPrefsLockedOnEntry(caller string, newp *ipn.Prefs) ipn oldHi := b.hostinfo newHi := oldHi.Clone() + if newHi == nil { + newHi = new(tailcfg.Hostinfo) + } b.applyPrefsToHostinfoLocked(newHi, newp.View()) b.hostinfo = newHi hostInfoChanged := !oldHi.Equal(newHi) @@ -3240,6 +3245,10 @@ func (b *LocalBackend) authReconfig() { disableSubnetsIfPAC := hasCapability(nm, tailcfg.NodeAttrDisableSubnetsIfPAC) dohURL, dohURLOK := exitNodeCanProxyDNS(nm, b.peers, prefs.ExitNodeID()) dcfg := dnsConfigForNetmap(nm, b.peers, prefs, b.logf, version.OS()) + // If the current node is an app connector, ensure the app connector machine is started + if prefs.AppConnector().Advertise && b.appConnector == nil { + b.appConnector = appc.NewEmbeddedAppConnector(b.logf, b) + } b.mu.Unlock() if blocked { @@ -4812,6 +4821,14 @@ func (b *LocalBackend) OfferingExitNode() bool { return def4 && def6 } +// OfferingAppConnector reports whether b is currently offering app +// connector services. +func (b *LocalBackend) OfferingAppConnector() bool { + b.mu.Lock() + defer b.mu.Unlock() + return b.appConnector != nil +} + // allowExitNodeDNSProxyToServeName reports whether the Exit Node DNS // proxy is allowed to serve responses for the provided DNS name. func (b *LocalBackend) allowExitNodeDNSProxyToServeName(name string) bool { @@ -5398,6 +5415,39 @@ func (b *LocalBackend) DebugBreakDERPConns() error { return b.magicConn().DebugBreakDERPConns() } +// ObserveDNSResponse passes a DNS response from the PeerAPI DNS server to the +// App Connector to enable route discovery. +func (b *LocalBackend) ObserveDNSResponse(res []byte) { + var appConnector *appc.EmbeddedAppConnector + b.mu.Lock() + if b.appConnector == nil { + b.mu.Unlock() + return + } + appConnector = b.appConnector + b.mu.Unlock() + + appConnector.ObserveDNSResponse(res) +} + +// AdvertiseRoute implements the appc.RouteAdvertiser interface. It sets a new +// route advertisement if one is not already present in the existing routes. +func (b *LocalBackend) AdvertiseRoute(ipp netip.Prefix) error { + currentRoutes := b.Prefs().AdvertiseRoutes() + // TODO(raggi): check if the new route is a subset of an existing route. + if currentRoutes.ContainsFunc(func(r netip.Prefix) bool { return r == ipp }) { + return nil + } + routes := append(currentRoutes.AsSlice(), ipp) + _, err := b.EditPrefs(&ipn.MaskedPrefs{ + Prefs: ipn.Prefs{ + AdvertiseRoutes: routes, + }, + AdvertiseRoutesSet: true, + }) + return err +} + // mayDeref dereferences p if non-nil, otherwise it returns the zero value. func mayDeref[T any](p *T) (v T) { if p == nil { diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 35007cf2b..79f58c2c4 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -10,10 +10,13 @@ "net/http" "net/netip" "reflect" + "slices" "testing" "time" "go4.org/netipx" + "golang.org/x/net/dns/dnsmessage" + "tailscale.com/appc" "tailscale.com/control/controlclient" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" @@ -30,6 +33,7 @@ "tailscale.com/types/ptr" "tailscale.com/util/dnsname" "tailscale.com/util/mak" + "tailscale.com/util/must" "tailscale.com/util/set" "tailscale.com/wgengine" "tailscale.com/wgengine/filter" @@ -1142,6 +1146,47 @@ type tc struct { } } +func TestOfferingAppConnector(t *testing.T) { + b := newTestBackend(t) + if b.OfferingAppConnector() { + t.Fatal("unexpected offering app connector") + } + b.appConnector = appc.NewEmbeddedAppConnector(t.Logf, nil) + if !b.OfferingAppConnector() { + t.Fatal("unexpected not offering app connector") + } +} + +func TestRouteAdvertiser(t *testing.T) { + b := newTestBackend(t) + testPrefix := netip.MustParsePrefix("192.0.0.8/32") + + ra := appc.RouteAdvertiser(b) + must.Do(ra.AdvertiseRoute(testPrefix)) + + routes := b.Prefs().AdvertiseRoutes() + if routes.Len() != 1 || routes.At(0) != testPrefix { + t.Fatalf("got routes %v, want %v", routes, []netip.Prefix{testPrefix}) + } +} + +func TestObserveDNSResponse(t *testing.T) { + b := newTestBackend(t) + + // ensure no error when no app connector is configured + b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + + rc := &routeCollector{} + b.appConnector = appc.NewEmbeddedAppConnector(t.Logf, rc) + b.appConnector.UpdateDomains([]string{"example.com"}) + + b.ObserveDNSResponse(dnsResponse("example.com.", "192.0.0.8")) + 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) + } +} + func resolversEqual(t *testing.T, a, b []*dnstype.Resolver) bool { if a == nil && b == nil { return true @@ -1176,3 +1221,50 @@ func routesEqual(t *testing.T, a, b map[dnsname.FQDN][]*dnstype.Resolver) bool { } return true } + +// dnsResponse is a test helper that creates a DNS response buffer for the given domain and address +func dnsResponse(domain, address string) []byte { + addr := netip.MustParseAddr(address) + b := dnsmessage.NewBuilder(nil, dnsmessage.Header{}) + b.EnableCompression() + b.StartAnswers() + switch addr.BitLen() { + case 32: + b.AResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(domain), + Type: dnsmessage.TypeA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AResource{ + A: addr.As4(), + }, + ) + case 128: + b.AAAAResource( + dnsmessage.ResourceHeader{ + Name: dnsmessage.MustNewName(domain), + Type: dnsmessage.TypeAAAA, + Class: dnsmessage.ClassINET, + TTL: 0, + }, + dnsmessage.AAAAResource{ + AAAA: addr.As16(), + }, + ) + default: + panic("invalid address length") + } + return must.Get(b.Finish()) +} + +// routeCollector is a test helper that collects the list of routes advertised +type routeCollector struct { + routes []netip.Prefix +} + +func (rc *routeCollector) AdvertiseRoute(pfx netip.Prefix) error { + rc.routes = append(rc.routes, pfx) + return nil +} diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index 6ae5ed530..3f5b0beaf 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -32,7 +32,6 @@ "tailscale.com/health" "tailscale.com/hostinfo" "tailscale.com/ipn" - "tailscale.com/net/dns/resolver" "tailscale.com/net/interfaces" "tailscale.com/net/netaddr" "tailscale.com/net/netutil" @@ -51,9 +50,14 @@ // ("cleartext" HTTP/2) support to the peerAPI. var addH2C func(*http.Server) +// peerDNSQueryHandler is implemented by tsdns.Resolver. +type peerDNSQueryHandler interface { + HandlePeerDNSQuery(context.Context, []byte, netip.AddrPort, func(name string) bool) (res []byte, err error) +} + type peerAPIServer struct { b *LocalBackend - resolver *resolver.Resolver + resolver peerDNSQueryHandler taildrop *taildrop.Manager } @@ -861,9 +865,9 @@ func (h *peerAPIHandler) replyToDNSQueries() bool { return true } b := h.ps.b - if !b.OfferingExitNode() { - // If we're not an exit node, there's no point to - // being a DNS server for somebody. + if !b.OfferingExitNode() && !b.OfferingAppConnector() { + // If we're not an exit node or app connector, there's + // no point to being a DNS server for somebody. return false } if !h.remoteAddr.IsValid() { @@ -927,7 +931,7 @@ func (h *peerAPIHandler) handleDNSQuery(w http.ResponseWriter, r *http.Request) ctx, cancel := context.WithTimeout(r.Context(), arbitraryTimeout) defer cancel() - res, err := h.ps.resolver.HandleExitNodeDNSQuery(ctx, q, h.remoteAddr, h.ps.b.allowExitNodeDNSProxyToServeName) + res, err := h.ps.resolver.HandlePeerDNSQuery(ctx, q, h.remoteAddr, h.ps.b.allowExitNodeDNSProxyToServeName) if err != nil { h.logf("handleDNS fwd error: %v", err) if err := ctx.Err(); err != nil { @@ -937,6 +941,13 @@ func (h *peerAPIHandler) handleDNSQuery(w http.ResponseWriter, r *http.Request) } return } + // TODO(raggi): consider pushing the integration down into the resolver + // instead to avoid re-parsing the DNS response for improved performance in + // the future. + if h.ps.b.OfferingAppConnector() { + h.ps.b.ObserveDNSResponse(res) + } + if pretty { // Non-standard response for interactive debugging. w.Header().Set("Content-Type", "application/json") diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index 76069eeba..ebfa858d8 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -5,6 +5,7 @@ import ( "bytes" + "context" "fmt" "io" "io/fs" @@ -14,11 +15,14 @@ "net/netip" "os" "path/filepath" + "slices" "strings" "testing" "github.com/google/go-cmp/cmp" "go4.org/netipx" + "golang.org/x/net/dns/dnsmessage" + "tailscale.com/appc" "tailscale.com/client/tailscale/apitype" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" @@ -680,3 +684,63 @@ func TestPeerAPIReplyToDNSQueries(t *testing.T) { t.Errorf("unexpectedly IPv6 deny; wanted to be a DNS server") } } + +func TestPeerAPIReplyToDNSQueriesAreObserved(t *testing.T) { + var h peerAPIHandler + h.remoteAddr = netip.MustParseAddrPort("100.150.151.152:12345") + + rc := &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.NewEmbeddedAppConnector(t.Logf, rc), + }, + } + h.ps.b.appConnector.UpdateDomains([]string{"example.com"}) + + h.ps.resolver = &fakeResolver{} + f := filter.NewAllowAllForTest(logger.Discard) + h.ps.b.setFilter(f) + + 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") + } + + w := httptest.NewRecorder() + h.handleDNSQuery(w, httptest.NewRequest("GET", "/dns-query?q=true&t=example.com.", nil)) + if w.Code != http.StatusOK { + t.Errorf("unexpected status code: %v", w.Code) + } + + 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) + } +} + +type fakeResolver struct{} + +func (f *fakeResolver) HandlePeerDNSQuery(ctx context.Context, q []byte, from netip.AddrPort, allowName func(name string) bool) (res []byte, err error) { + b := dnsmessage.NewBuilder(nil, dnsmessage.Header{}) + b.EnableCompression() + b.StartAnswers() + 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}, + }, + ) + return b.Finish() +} diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index ddb7b6cdb..97b8434cc 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -314,9 +314,9 @@ func parseExitNodeQuery(q []byte) *response { return p.response() } -// HandleExitNodeDNSQuery handles a DNS query that arrived from a peer -// via the peerapi's DoH server. This is only used when the local -// node is being an exit node. +// HandlePeerDNSQuery handles a DNS query that arrived from a peer +// via the peerapi's DoH server. This is used when the local +// node is being an exit node or an app connector. // // The provided allowName callback is whether a DNS query for a name // (as found by parsing q) is allowed. @@ -325,7 +325,7 @@ func parseExitNodeQuery(q []byte) *response { // still result in a response DNS packet (saying there's a failure) // and a nil error. // TODO: figure out if we even need an error result. -func (r *Resolver) HandleExitNodeDNSQuery(ctx context.Context, q []byte, from netip.AddrPort, allowName func(name string) bool) (res []byte, err error) { +func (r *Resolver) HandlePeerDNSQuery(ctx context.Context, q []byte, from netip.AddrPort, allowName func(name string) bool) (res []byte, err error) { metricDNSExitProxyQuery.Add(1) ch := make(chan packet, 1)