mirror of
https://github.com/tailscale/tailscale.git
synced 2026-07-15 01:53:08 -04:00
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 <adrian@tailscale.com>
This commit is contained in:
committed by
Adrian Dewhurst
parent
1c77079fd7
commit
477d5a43df
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user