diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index d9bc9e910..d1fd826b7 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -100,6 +100,8 @@ type LocalBackend struct { filterHash deephash.Sum + filterAtomic atomic.Value // of *filter.Filter + // The mutex protects the following elements. mu sync.Mutex httpTestClient *http.Client // for controlclient. nil by default, used by tests. @@ -160,9 +162,6 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, e wge osshare.SetFileSharingEnabled(false, logf) - // Default filter blocks everything and logs nothing, until Start() is called. - e.SetFilter(filter.NewAllowNone(logf, &netaddr.IPSet{})) - ctx, cancel := context.WithCancel(context.Background()) portpoll, err := portlist.NewPoller() if err != nil { @@ -182,6 +181,9 @@ func NewLocalBackend(logf logger.Logf, logid string, store ipn.StateStore, e wge portpoll: portpoll, gotPortPollRes: make(chan struct{}), } + // Default filter blocks everything and logs nothing, until Start() is called. + b.setFilter(filter.NewAllowNone(logf, &netaddr.IPSet{})) + b.statusChanged = sync.NewCond(&b.statusLock) b.e.SetStatusCallback(b.setWgengineStatus) @@ -1011,20 +1013,25 @@ func (b *LocalBackend) updateFilter(netMap *netmap.NetworkMap, prefs *ipn.Prefs) if !haveNetmap { b.logf("netmap packet filter: (not ready yet)") - b.e.SetFilter(filter.NewAllowNone(b.logf, logNets)) + b.setFilter(filter.NewAllowNone(b.logf, logNets)) return } oldFilter := b.e.GetFilter() if shieldsUp { b.logf("netmap packet filter: (shields up)") - b.e.SetFilter(filter.NewShieldsUpFilter(localNets, logNets, oldFilter, b.logf)) + b.setFilter(filter.NewShieldsUpFilter(localNets, logNets, oldFilter, b.logf)) } else { b.logf("netmap packet filter: %v filters", len(packetFilter)) - b.e.SetFilter(filter.New(packetFilter, localNets, logNets, oldFilter, b.logf)) + b.setFilter(filter.New(packetFilter, localNets, logNets, oldFilter, b.logf)) } } +func (b *LocalBackend) setFilter(f *filter.Filter) { + b.filterAtomic.Store(f) + b.e.SetFilter(f) +} + var removeFromDefaultRoute = []netaddr.IPPrefix{ // RFC1918 LAN ranges netaddr.MustParseIPPrefix("192.168.0.0/16"), diff --git a/ipn/ipnlocal/peerapi.go b/ipn/ipnlocal/peerapi.go index 0a7615471..2706872d4 100644 --- a/ipn/ipnlocal/peerapi.go +++ b/ipn/ipnlocal/peerapi.go @@ -41,6 +41,7 @@ import ( "tailscale.com/tailcfg" "tailscale.com/util/clientmetric" "tailscale.com/wgengine" + "tailscale.com/wgengine/filter" ) var initListenConfig func(*net.ListenConfig, netaddr.IP, *interfaces.State, string) error @@ -759,13 +760,45 @@ func (h *peerAPIHandler) handleServeMetrics(w http.ResponseWriter, r *http.Reque } func (h *peerAPIHandler) replyToDNSQueries() bool { - // TODO(bradfitz): maybe lock this down more? what if we're an - // exit node but ACLs don't permit autogroup:internet access - // from h.peerNode via this node? peerapi bypasses ACL checks, - // so we should do additional checks here; but on what? this - // node's UDP port 53? our upstream DNS forwarder IP(s)? - // For now just offer DNS to any peer if we're an exit node. - return h.isSelf || h.ps.b.OfferingExitNode() + if h.isSelf { + // If the peer is owned by the same user, just allow it + // without further checks. + 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. + 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 + } + // 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 + // ourselves. As a proxy for autogroup:internet access, we see + // if we would've accepted a packet to 0.0.0.0:53. We treat + // the IP 0.0.0.0 as being "the internet". + f, ok := b.filterAtomic.Load().(*filter.Filter) + if !ok { + return false + } + // Note: we check TCP here because the Filter type already had + // a CheckTCP method (for unit tests), but it's pretty + // arbitrary. DNS runs over TCP and UDP, so sure... we check + // TCP. + dstIP := netaddr.IPv4(0, 0, 0, 0) + remoteIP := h.remoteAddr.IP() + if remoteIP.Is6() { + // autogroup:internet for IPv6 is defined to start with 2000::/3, + // so use 2000::0 as the probe "the internet" address. + dstIP = netaddr.MustParseIP("2000::") + } + verdict := f.CheckTCP(remoteIP, dstIP, 53) + return verdict == filter.Accept } // handleDNSQuery implements a DoH server (RFC 8484) over the peerapi. diff --git a/ipn/ipnlocal/peerapi_test.go b/ipn/ipnlocal/peerapi_test.go index 1b9c5fb72..457001fed 100644 --- a/ipn/ipnlocal/peerapi_test.go +++ b/ipn/ipnlocal/peerapi_test.go @@ -19,8 +19,13 @@ import ( "strings" "testing" + "inet.af/netaddr" + "tailscale.com/ipn" "tailscale.com/tailcfg" "tailscale.com/tstest" + "tailscale.com/types/logger" + "tailscale.com/wgengine" + "tailscale.com/wgengine/filter" ) type peerAPITestEnv struct { @@ -568,3 +573,55 @@ func TestDeletedMarkers(t *testing.T) { } } + +func TestPeerAPIReplyToDNSQueries(t *testing.T) { + var h peerAPIHandler + + h.isSelf = true + if !h.replyToDNSQueries() { + t.Errorf("for isSelf = false; want true") + } + h.isSelf = false + h.remoteAddr = netaddr.MustParseIPPort("100.150.151.152:12345") + + eng, _ := wgengine.NewFakeUserspaceEngine(logger.Discard, 0) + h.ps = &peerAPIServer{ + b: &LocalBackend{ + e: eng, + }, + } + if h.ps.b.OfferingExitNode() { + t.Fatal("unexpectedly offering exit node") + } + h.ps.b.prefs = &ipn.Prefs{ + AdvertiseRoutes: []netaddr.IPPrefix{ + netaddr.MustParseIPPrefix("0.0.0.0/0"), + netaddr.MustParseIPPrefix("::/0"), + }, + } + if !h.ps.b.OfferingExitNode() { + t.Fatal("unexpectedly not offering exit node") + } + + if h.replyToDNSQueries() { + t.Errorf("unexpectedly doing DNS without filter") + } + + h.ps.b.setFilter(filter.NewAllowNone(logger.Discard, new(netaddr.IPSet))) + if h.replyToDNSQueries() { + t.Errorf("unexpectedly doing DNS without filter") + } + + f := filter.NewAllowAllForTest(logger.Discard) + + h.ps.b.setFilter(f) + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly deny; wanted to be a DNS server") + } + + // Also test IPv6. + h.remoteAddr = netaddr.MustParseIPPort("[fe70::1]:12345") + if !h.replyToDNSQueries() { + t.Errorf("unexpectedly IPv6 deny; wanted to be a DNS server") + } +}