From 39748e956231bd981d359d94db69c5dd73989660 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Wed, 1 Sep 2021 17:25:02 -0700 Subject: [PATCH] net/dns/resolver: authoritatively return NXDOMAIN for reverse zones we own. Fixes #2774 Signed-off-by: David Anderson --- net/dns/resolver/tsdns.go | 52 ++++++++++++++++++---------------- net/dns/resolver/tsdns_test.go | 23 +++++++++------ 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index 7c68f962e..5b7b45456 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -347,14 +347,36 @@ func (r *Resolver) resolveLocal(domain dnsname.FQDN, typ dns.Type) (netaddr.IP, } // resolveReverse returns the unique domain name that maps to the given address. -func (r *Resolver) resolveLocalReverse(ip netaddr.IP) (dnsname.FQDN, dns.RCode) { +func (r *Resolver) resolveLocalReverse(name dnsname.FQDN) (dnsname.FQDN, dns.RCode) { + var ip netaddr.IP + var ok bool + switch { + case strings.HasSuffix(name.WithTrailingDot(), rdnsv4Suffix): + ip, ok = rdnsNameToIPv4(name) + case strings.HasSuffix(name.WithTrailingDot(), rdnsv6Suffix): + ip, ok = rdnsNameToIPv6(name) + } + if !ok { + // This isn't a well-formed in-addr.arpa or ip6.arpa name, but + // who knows what upstreams might do, try kicking it up to + // them. We definitely won't handle it. + return "", dns.RCodeRefused + } + r.mu.Lock() defer r.mu.Unlock() - name, ok := r.ipToHost[ip] + ret, ok := r.ipToHost[ip] if !ok { - return "", dns.RCodeNameError + for _, suffix := range r.localDomains { + if suffix.Contains(name) { + // We are authoritative for this chunk of IP space. + return "", dns.RCodeNameError + } + } + // Not authoritative, signal that forwarding is advisable. + return "", dns.RCodeRefused } - return name, dns.RCodeSuccess + return ret, dns.RCodeSuccess } func (r *Resolver) handleQuery(pkt packet) { @@ -650,26 +672,8 @@ func (r *Resolver) respondReverse(query []byte, name dnsname.FQDN, resp *respons return nil, errNotOurName } - var ip netaddr.IP - var ok bool - switch { - case strings.HasSuffix(name.WithTrailingDot(), rdnsv4Suffix): - ip, ok = rdnsNameToIPv4(name) - case strings.HasSuffix(name.WithTrailingDot(), rdnsv6Suffix): - ip, ok = rdnsNameToIPv6(name) - default: - return nil, errNotOurName - } - - // It is more likely that we failed in parsing the name than that it is actually malformed. - // To avoid frustrating users, just log and delegate. - if !ok { - r.logf("parsing rdns: malformed name: %s", name) - return nil, errNotOurName - } - - resp.Name, resp.Header.RCode = r.resolveLocalReverse(ip) - if resp.Header.RCode == dns.RCodeNameError { + resp.Name, resp.Header.RCode = r.resolveLocalReverse(name) + if resp.Header.RCode == dns.RCodeRefused { return nil, errNotOurName } diff --git a/net/dns/resolver/tsdns_test.go b/net/dns/resolver/tsdns_test.go index 28ec2ef72..248f26072 100644 --- a/net/dns/resolver/tsdns_test.go +++ b/net/dns/resolver/tsdns_test.go @@ -23,15 +23,20 @@ import ( "tailscale.com/wgengine/monitor" ) -var testipv4 = netaddr.MustParseIP("1.2.3.4") -var testipv6 = netaddr.MustParseIP("0001:0203:0405:0607:0809:0a0b:0c0d:0e0f") +var ( + testipv4 = netaddr.MustParseIP("1.2.3.4") + testipv6 = netaddr.MustParseIP("0001:0203:0405:0607:0809:0a0b:0c0d:0e0f") + + 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.") +) var dnsCfg = Config{ Hosts: map[dnsname.FQDN][]netaddr.IP{ "test1.ipn.dev.": []netaddr.IP{testipv4}, "test2.ipn.dev.": []netaddr.IP{testipv6}, }, - LocalDomains: []dnsname.FQDN{"ipn.dev."}, + LocalDomains: []dnsname.FQDN{"ipn.dev.", "3.2.1.in-addr.arpa.", "1.0.0.0.ip6.arpa."}, } const noEdns = 0 @@ -353,18 +358,20 @@ func TestResolveLocalReverse(t *testing.T) { tests := []struct { name string - ip netaddr.IP + q dnsname.FQDN want dnsname.FQDN code dns.RCode }{ - {"ipv4", testipv4, "test1.ipn.dev.", dns.RCodeSuccess}, - {"ipv6", testipv6, "test2.ipn.dev.", dns.RCodeSuccess}, - {"nxdomain", netaddr.IPv4(4, 3, 2, 1), "", dns.RCodeNameError}, + {"ipv4", testipv4Arpa, "test1.ipn.dev.", dns.RCodeSuccess}, + {"ipv6", testipv6Arpa, "test2.ipn.dev.", dns.RCodeSuccess}, + {"ipv4_nxdomain", dnsname.FQDN("5.3.2.1.in-addr.arpa."), "", dns.RCodeNameError}, + {"ipv6_nxdomain", dnsname.FQDN("0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.1.0.0.0.ip6.arpa."), "", dns.RCodeNameError}, + {"nxdomain", dnsname.FQDN("2.3.4.5.in-addr.arpa."), "", dns.RCodeRefused}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - name, code := r.resolveLocalReverse(tt.ip) + name, code := r.resolveLocalReverse(tt.q) if code != tt.code { t.Errorf("code = %v; want %v", code, tt.code) }