net/dns/resolver: authoritatively return NXDOMAIN for reverse zones we own.

Fixes #2774

Signed-off-by: David Anderson <danderson@tailscale.com>

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 {
+		for _, suffix := range r.localDomains {
+			if suffix.Contains(name) {
+				// We are authoritative for this chunk of IP space.
 				return "", dns.RCodeNameError
 			}
-	return name, dns.RCodeSuccess
+		}
+		// Not authoritative, signal that forwarding is advisable.
+		return "", dns.RCodeRefused
+	}
+	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
+	resp.Name, resp.Header.RCode = r.resolveLocalReverse(name)
-	var ok bool
+	if resp.Header.RCode == dns.RCodeRefused {
-	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 {
 		return nil, errNotOurName
 	}
 

net/dns/resolver/tsdns_test.go

@@ -23,15 +23,20 @@ import (
 	"tailscale.com/wgengine/monitor"
 )
 
-var testipv4 = netaddr.MustParseIP("1.2.3.4")
+var (
-var testipv6 = netaddr.MustParseIP("0001:0203:0405:0607:0809:0a0b:0c0d:0e0f")
+	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},
+		{"ipv4", testipv4Arpa, "test1.ipn.dev.", dns.RCodeSuccess},
-		{"ipv6", testipv6, "test2.ipn.dev.", dns.RCodeSuccess},
+		{"ipv6", testipv6Arpa, "test2.ipn.dev.", dns.RCodeSuccess},
-		{"nxdomain", netaddr.IPv4(4, 3, 2, 1), "", dns.RCodeNameError},
+		{"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)
 			}