From 477d5a43df073f3dae17cca7fd86a6487b8bb7d3 Mon Sep 17 00:00:00 2001 From: Adrian Dewhurst Date: Mon, 29 Jun 2026 12:12:10 -0400 Subject: [PATCH] ipn/ipnlocal, feature/conn25: add hook for accepting PeerAPI DNS Currently, PeerAPI DNS is only allowed if 1. The peer is owned by the same user as this device, or 2. The node is an exit node or app connector a. and the peer has access to a hypothetical DNS server at 0.0.0.0:53 (which approximately means "the peer has access to autogroup:internet") None of this is useful for conn25. This adds the most basic of hooks (and converts the existing logic to a hook, which should improve clarity and lead to the possibility of moving the existing checks into feature packages in future). There is an extra filter based on the name being queried that is performed later. It refuses names in tailcfg.DNSConfig.ExitNodeFilteredSet. That filter is not modified by this change. With this change, if conn25 is configured as a connector, then all PeerAPI DNS queries are permitted (still subject to the ExitNodeFilteredSet as noted above). More work is required: the goal before release (i.e. the WIPCode check is removed) is that each query should be checked against the list of domains in the requested conn25 app. For now, this only verifies that conn25 is configured (and does not include the autogroup:internet check, which is not how conn25 grants will operate when implemented, soon). This change has been manually tested against the scenario outlined in tailscale/corp#40117; unfortunately the code's structure makes writing a unit test difficult. The more comprehensive changes needed for tailscale/corp#40076 should include an integration test that covers this case. The hook must go in the ipnlocal package rather than the usual extension host to prevent a circular dependency on the ipnlocal.PeerAPIHandler interface. Registering PeerAPI handlers uses a similar strategy, likely because of, at least in part, this same problem. Updates tailscale/corp#40076 Fixes tailscale/corp#40117 Change-Id: I367714170b509d7a421f62672e5824b3590c2b9c Signed-off-by: Adrian Dewhurst --- feature/conn25/conn25.go | 22 ++++++++++++++++++++++ ipn/ipnlocal/peerapi.go | 39 ++++++++++++++++++++++++++++++++------- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/feature/conn25/conn25.go b/feature/conn25/conn25.go index 73b25fcd9..504698871 100644 --- a/feature/conn25/conn25.go +++ b/feature/conn25/conn25.go @@ -81,6 +81,7 @@ func init() { }, nil }) ipnlocal.RegisterPeerAPIHandler("/v0/connector/transit-ip", handleConnectorTransitIP) + ipnlocal.HookReplyToDNSQueries.Add(handleHookReplyToDNSQueries) } func handleConnectorTransitIP(h ipnlocal.PeerAPIHandler, w http.ResponseWriter, r *http.Request) { @@ -97,6 +98,18 @@ func handleConnectorTransitIP(h ipnlocal.PeerAPIHandler, w http.ResponseWriter, e.handleConnectorTransitIP(h, w, r) } +func handleHookReplyToDNSQueries(h ipnlocal.PeerAPIHandler) bool { + // TODO(tailscale/corp#39033): Remove for alpha release. + if !envknob.UseWIPCode() && !testenv.InTest() { + return false + } + e, ok := ipnlocal.GetExt[*extension](h.LocalBackend()) + if !ok { + return false + } + return e.handleHookReplyToDNSQueries(h) +} + // extension is an [ipnext.Extension] managing the connector on platforms // that import this package. type extension struct { @@ -343,6 +356,15 @@ func (e *extension) handleConnectorTransitIP(h ipnlocal.PeerAPIHandler, w http.R w.Write(bs) } +func (e *extension) handleHookReplyToDNSQueries(h ipnlocal.PeerAPIHandler) bool { + if !e.conn25.isConfigured() { + return false + } + // TODO(tailscale/corp#40076): verify the peer has access to the query's + // app (if any) domain. + return true +} + // onSelfChange implements the [ipnext.Hooks.OnSelfChange] hook. func (e *extension) onSelfChange(selfNode tailcfg.NodeView) { cfg, err := configFromNodeView(selfNode) diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index d72a519ab..53d1e6d35 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -674,6 +674,12 @@ func (h *peerAPIHandler) handleServeDNSFwd(w http.ResponseWriter, r *http.Reques dh.ServeHTTP(w, r) } +// HookReplyToDNSQueries allows extensions to register a willingness to allow +// handling PeerAPI DNS queries for the peer making this request. +var HookReplyToDNSQueries = feature.Hooks[func(PeerAPIHandler) bool]{ + offersExitNodeOrAppConnectorAndPeerHasAutogroupInternet, +} + func (h *peerAPIHandler) replyToDNSQueries() bool { if !buildfeatures.HasDNS { return false @@ -683,17 +689,36 @@ func (h *peerAPIHandler) replyToDNSQueries() bool { // without further checks. return true } - b := h.ps.b - 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() { // This should never be the case if the peerAPIHandler // was wired up correctly, but just in case. return false } + + for _, hook := range HookReplyToDNSQueries { + if hook(h) { + return true + } + } + return false +} + +// offersExitNodeOrAppConnectorAndPeerHasAutogroupInternet is run as part of +// [HookReplyToDNSQueries] and handles our legacy PeerAPI DNS acceptance +// criteria: +// - When a node is advertising an exit node it will accept DNS queries +// from peers that have access to autogroup:internet. +// - When a node is advertising an app connector, it will accept DNS queries +// to peers that have access to a relevant app. +// +// Further details about how these are accomplished are in inline comments. +func offersExitNodeOrAppConnectorAndPeerHasAutogroupInternet(h PeerAPIHandler) bool { + b := h.LocalBackend() + if !b.OfferingExitNode() && !b.OfferingAppConnector() { + // If we're not an exit node or app connector, this hook + // doesn't apply. + return false + } // Otherwise, we're an exit node but the peer is not us, so // we need to check if they're allowed access to the internet. // As peerapi bypasses wgengine/filter checks, we need to check @@ -716,7 +741,7 @@ func (h *peerAPIHandler) replyToDNSQueries() bool { // arbitrary. DNS runs over TCP and UDP, so sure... we check // TCP. dstIP := netaddr.IPv4(0, 0, 0, 0) - remoteIP := h.remoteAddr.Addr() + remoteIP := h.RemoteAddr().Addr() if remoteIP.Is6() { // autogroup:internet for IPv6 is defined to start with 2000::/3, // so use 2000::0 as the probe "the internet" address.