From 24bdcbe5c780d1d0656d4913aa8026d7f98db67f Mon Sep 17 00:00:00 2001 From: Tom Date: Fri, 8 Apr 2022 12:17:31 -0700 Subject: [PATCH] net/dns, net/dns/resolver, wgengine: refactor DNS request path (#4364) * net/dns, net/dns/resolver, wgengine: refactor DNS request path Previously, method calls into the DNS manager/resolver types handled DNS requests rather than DNS packets. This is fine for UDP as one packet corresponds to one request or response, however will not suit an implementation that supports DNS over TCP. To support PRs implementing this in the future, wgengine delegates all handling/construction of packets to the magic DNS endpoint, to the DNS types themselves. Handling IP packets at this level enables future support for both UDP and TCP. Signed-off-by: Tom DNetto --- net/dns/manager.go | 9 ++-- net/dns/resolver/tsdns.go | 75 ++++++++++++++++++++++++++++++++-- net/dns/resolver/tsdns_test.go | 13 +++--- wgengine/userspace.go | 57 ++++++-------------------- 4 files changed, 97 insertions(+), 57 deletions(-) diff --git a/net/dns/manager.go b/net/dns/manager.go index eb4b46592..d337a5616 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -14,6 +14,7 @@ import ( "tailscale.com/net/dns/resolver" "tailscale.com/net/tsdial" "tailscale.com/types/dnstype" + "tailscale.com/types/ipproto" "tailscale.com/types/logger" "tailscale.com/util/dnsname" "tailscale.com/wgengine/monitor" @@ -204,12 +205,12 @@ func toIPPorts(ips []netaddr.IP) (ret []netaddr.IPPort) { return ret } -func (m *Manager) EnqueueRequest(bs []byte, from netaddr.IPPort) error { - return m.resolver.EnqueueRequest(bs, from) +func (m *Manager) EnqueuePacket(bs []byte, proto ipproto.Proto, from, to netaddr.IPPort) error { + return m.resolver.EnqueuePacket(bs, proto, from, to) } -func (m *Manager) NextResponse() ([]byte, netaddr.IPPort, error) { - return m.resolver.NextResponse() +func (m *Manager) NextPacket() ([]byte, error) { + return m.resolver.NextPacket() } func (m *Manager) Down() error { diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index e8d85d77f..66117a6ef 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -25,9 +25,12 @@ import ( dns "golang.org/x/net/dns/dnsmessage" "inet.af/netaddr" "tailscale.com/net/dns/resolvconffile" + tspacket "tailscale.com/net/packet" "tailscale.com/net/tsaddr" "tailscale.com/net/tsdial" + "tailscale.com/net/tstun" "tailscale.com/types/dnstype" + "tailscale.com/types/ipproto" "tailscale.com/types/logger" "tailscale.com/util/clientmetric" "tailscale.com/util/dnsname" @@ -36,6 +39,13 @@ import ( const dnsSymbolicFQDN = "magicdns.localhost-tailscale-daemon." +var ( + magicDNSIP = tsaddr.TailscaleServiceIP() + magicDNSIPv6 = tsaddr.TailscaleServiceIPv6() +) + +const magicDNSPort = 53 + // maxResponseBytes is the maximum size of a response from a Resolver. The // actual buffer size will be one larger than this so that we can detect // truncation in a platform-agnostic way. @@ -282,10 +292,20 @@ func (r *Resolver) Close() { r.forwarder.Close() } -// EnqueueRequest places the given DNS request in the resolver's queue. +// EnqueuePacket handles a packet to the magicDNS endpoint. // It takes ownership of the payload and does not block. // If the queue is full, the request will be dropped and an error will be returned. -func (r *Resolver) EnqueueRequest(bs []byte, from netaddr.IPPort) error { +func (r *Resolver) EnqueuePacket(bs []byte, proto ipproto.Proto, from, to netaddr.IPPort) error { + if to.Port() != magicDNSPort || proto != ipproto.UDP { + return nil + } + + return r.enqueueRequest(bs, proto, from, to) +} + +// enqueueRequest places the given DNS request in the resolver's queue. +// If the queue is full, the request will be dropped and an error will be returned. +func (r *Resolver) enqueueRequest(bs []byte, proto ipproto.Proto, from, to netaddr.IPPort) error { metricDNSQueryLocal.Add(1) select { case <-r.closed: @@ -302,9 +322,56 @@ func (r *Resolver) EnqueueRequest(bs []byte, from netaddr.IPPort) error { return nil } -// NextResponse returns a DNS response to a previously enqueued request. +// NextPacket returns the next packet to service traffic for magicDNS. The returned +// packet is prefixed with unused space consistent with the semantics of injection +// into tstun.Wrapper. +// It blocks until a response is available and gives up ownership of the response payload. +func (r *Resolver) NextPacket() (ipPacket []byte, err error) { + bs, to, err := r.nextResponse() + if err != nil { + return nil, err + } + + // Unused space is needed further down the stack. To avoid extra + // allocations/copying later on, we allocate such space here. + const offset = tstun.PacketStartOffset + + var buf []byte + switch { + case to.IP().Is4(): + h := tspacket.UDP4Header{ + IP4Header: tspacket.IP4Header{ + Src: magicDNSIP, + Dst: to.IP(), + }, + SrcPort: magicDNSPort, + DstPort: to.Port(), + } + hlen := h.Len() + buf = make([]byte, offset+hlen+len(bs)) + copy(buf[offset+hlen:], bs) + h.Marshal(buf[offset:]) + case to.IP().Is6(): + h := tspacket.UDP6Header{ + IP6Header: tspacket.IP6Header{ + Src: magicDNSIPv6, + Dst: to.IP(), + }, + SrcPort: magicDNSPort, + DstPort: to.Port(), + } + hlen := h.Len() + buf = make([]byte, offset+hlen+len(bs)) + copy(buf[offset+hlen:], bs) + h.Marshal(buf[offset:]) + } + + return buf, nil +} + +// nextResponse returns a DNS response to a previously enqueued request. // It blocks until a response is available and gives up ownership of the response payload. -func (r *Resolver) NextResponse() (packet []byte, to netaddr.IPPort, err error) { +func (r *Resolver) nextResponse() (packet []byte, to netaddr.IPPort, err error) { select { case <-r.closed: return nil, netaddr.IPPort{}, ErrClosed diff --git a/net/dns/resolver/tsdns_test.go b/net/dns/resolver/tsdns_test.go index 9f62eaa68..106c979aa 100644 --- a/net/dns/resolver/tsdns_test.go +++ b/net/dns/resolver/tsdns_test.go @@ -27,6 +27,7 @@ import ( "tailscale.com/net/tsdial" "tailscale.com/tstest" "tailscale.com/types/dnstype" + "tailscale.com/types/ipproto" "tailscale.com/util/dnsname" "tailscale.com/wgengine/monitor" ) @@ -37,6 +38,8 @@ var ( testipv4Arpa = dnsname.FQDN("4.3.2.1.in-addr.arpa.") testipv6Arpa = dnsname.FQDN("f.0.e.0.d.0.c.0.b.0.a.0.9.0.8.0.7.0.6.0.5.0.4.0.3.0.2.0.1.0.0.0.ip6.arpa.") + + magicDNSv4Port = netaddr.MustParseIPPort("100.100.100.100:53") ) var dnsCfg = Config{ @@ -231,10 +234,10 @@ func unpackResponse(payload []byte) (dnsResponse, error) { } func syncRespond(r *Resolver, query []byte) ([]byte, error) { - if err := r.EnqueueRequest(query, netaddr.IPPort{}); err != nil { - return nil, fmt.Errorf("EnqueueRequest: %w", err) + if err := r.enqueueRequest(query, ipproto.UDP, netaddr.IPPort{}, magicDNSv4Port); err != nil { + return nil, fmt.Errorf("enqueueRequest: %w", err) } - payload, _, err := r.NextResponse() + payload, _, err := r.nextResponse() return payload, err } @@ -727,14 +730,14 @@ func TestDelegateCollision(t *testing.T) { // packets will have the same dns txid. for _, p := range packets { payload := dnspacket(p.qname, p.qtype, noEdns) - err := r.EnqueueRequest(payload, p.addr) + err := r.enqueueRequest(payload, ipproto.UDP, p.addr, magicDNSv4Port) if err != nil { t.Error(err) } } // Despite the txid collision, the answer(s) should still match the query. - resp, addr, err := r.NextResponse() + resp, addr, err := r.nextResponse() if err != nil { t.Error(err) } diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 972cc288b..8a392d731 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -487,23 +487,22 @@ func (e *userspaceEngine) handleLocalPackets(p *packet.Parsed, t *tstun.Wrapper) // handleDNS is an outbound pre-filter resolving Tailscale domains. func (e *userspaceEngine) handleDNS(p *packet.Parsed, t *tstun.Wrapper) filter.Response { - if p.Dst.Port() == magicDNSPort && p.IPProto == ipproto.UDP { - switch p.Dst.IP() { - case magicDNSIP, magicDNSIPv6: - err := e.dns.EnqueueRequest(append([]byte(nil), p.Payload()...), p.Src) - if err != nil { - e.logf("dns: enqueue: %v", err) - } - return filter.Drop + switch p.Dst.IP() { + case magicDNSIP, magicDNSIPv6: + err := e.dns.EnqueuePacket(append([]byte(nil), p.Payload()...), p.IPProto, p.Src, p.Dst) + if err != nil { + e.logf("dns: enqueue: %v", err) } + return filter.Drop + default: + return filter.Accept } - return filter.Accept } -// pollResolver reads responses from the DNS resolver and injects them inbound. +// pollResolver reads packets from the DNS resolver and injects them inbound. func (e *userspaceEngine) pollResolver() { for { - bs, to, err := e.dns.NextResponse() + bs, err := e.dns.NextPacket() if err == resolver.ErrClosed { return } @@ -512,39 +511,9 @@ func (e *userspaceEngine) pollResolver() { continue } - var buf []byte - const offset = tstun.PacketStartOffset - switch { - case to.IP().Is4(): - h := packet.UDP4Header{ - IP4Header: packet.IP4Header{ - Src: magicDNSIP, - Dst: to.IP(), - }, - SrcPort: magicDNSPort, - DstPort: to.Port(), - } - hlen := h.Len() - // TODO(dmytro): avoid this allocation without importing tstun quirks into dns. - buf = make([]byte, offset+hlen+len(bs)) - copy(buf[offset+hlen:], bs) - h.Marshal(buf[offset:]) - case to.IP().Is6(): - h := packet.UDP6Header{ - IP6Header: packet.IP6Header{ - Src: magicDNSIPv6, - Dst: to.IP(), - }, - SrcPort: magicDNSPort, - DstPort: to.Port(), - } - hlen := h.Len() - // TODO(dmytro): avoid this allocation without importing tstun quirks into dns. - buf = make([]byte, offset+hlen+len(bs)) - copy(buf[offset+hlen:], bs) - h.Marshal(buf[offset:]) - } - e.tundev.InjectInboundDirect(buf, offset) + // The leading empty space required by the semantics of + // InjectInboundDirect is allocated in NextPacket(). + e.tundev.InjectInboundDirect(bs, tstun.PacketStartOffset) } }