plugin/file: test that out-of-zone CNAME targets keep NOERROR (#8569)

plugin/file/lookup.go

@@ -469,6 +469,9 @@ func delegationFromElem(tr *tree.Tree, elem *tree.Elem, qname string, qtype uint
 }
 
 func (z *Zone) doLookup(ctx context.Context, state request.Request, target string, qtype uint16) ([]dns.RR, Result) {
+	if z.Upstream == nil {
+		return nil, ServerFailure
+	}
 	m, e := z.Upstream.Lookup(ctx, state, target, qtype)
 	if e != nil {
 		return nil, ServerFailure
@@ -485,6 +488,9 @@ func (z *Zone) doLookup(ctx context.Context, state request.Request, target strin
 	if m.Rcode == dns.RcodeSuccess && len(m.Answer) == 0 {
 		return m.Answer, NoData
 	}
+	// Anything else, including the REFUSED returned for a target outside every
+	// zone served here, keeps NOERROR: the CNAME is the answer and the resolver
+	// chases the target itself. See #7346.
 	return m.Answer, Success
 }
 

plugin/file/lookup_test.go

@@ -557,3 +557,55 @@ cname-srv	IN CNAME	srv.example.org.
 dname		IN DNAME	dsub.example.org.
 srv.dsub	IN SRV	10 50 8080 host.example.org.
 `
+
+const dbExampleOrgOutOfZone = `$ORIGIN example.org.
+@	3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. ( 2017042745 7200 3600 1209600 3600 )
+	3600 IN NS  a.iana-servers.net.
+www	3600 IN CNAME www.example.net.
+`
+
+// refusedUpstream answers every lookup with REFUSED, which is what the server
+// returns for a name that matches none of the zones it serves.
+type refusedUpstream struct{}
+
+func (refusedUpstream) Lookup(_ context.Context, _ request.Request, name string, typ uint16) (*dns.Msg, error) {
+	m := new(dns.Msg)
+	m.SetQuestion(name, typ)
+	m.Rcode = dns.RcodeRefused
+	return m, nil
+}
+
+// TestLookupRefusedUpstream checks that a CNAME whose target lies outside every
+// served zone is answered with NOERROR and the CNAME, so the resolver can chase
+// the target itself. The self-lookup for such a target comes back REFUSED, which
+// is not an error for an authoritative answer. See #7346 and #7381.
+func TestLookupRefusedUpstream(t *testing.T) {
+	const origin = "example.org."
+	zone, err := Parse(strings.NewReader(dbExampleOrgOutOfZone), origin, "stdin", 0)
+	if err != nil {
+		t.Fatalf("Expected no error when reading zone, got %q", err)
+	}
+	zone.Upstream = refusedUpstream{}
+
+	fm := File{Next: test.ErrorHandler(), Zones: Zones{Z: map[string]*Zone{origin: zone}, Names: []string{origin}}}
+
+	m := new(dns.Msg)
+	m.SetQuestion("www.example.org.", dns.TypeA)
+
+	rec := dnstest.NewRecorder(&test.ResponseWriter{})
+	if _, err := fm.ServeDNS(context.TODO(), rec, m); err != nil {
+		t.Fatalf("Expected no error, got %v", err)
+	}
+	if rec.Msg == nil {
+		t.Fatal("Expected a response to be written, got none")
+	}
+	if rec.Msg.Rcode != dns.RcodeSuccess {
+		t.Errorf("rcode is %q, expected %q", dns.RcodeToString[rec.Msg.Rcode], dns.RcodeToString[dns.RcodeSuccess])
+	}
+	if len(rec.Msg.Answer) != 1 {
+		t.Fatalf("Expected the CNAME in the answer, got %d records", len(rec.Msg.Answer))
+	}
+	if _, ok := rec.Msg.Answer[0].(*dns.CNAME); !ok {
+		t.Errorf("Expected a CNAME in the answer, got %s", rec.Msg.Answer[0])
+	}
+}

plugin/file/zone.go

@@ -1,6 +1,7 @@
 package file
 
 import (
+	"context"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -8,11 +9,17 @@ import (
 	"time"
 
 	"github.com/coredns/coredns/plugin/file/tree"
-	"github.com/coredns/coredns/plugin/pkg/upstream"
+	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
 )
 
+// Upstreamer is used to resolve CNAME or other external targets during the
+// resolution process.
+type Upstreamer interface {
+	Lookup(ctx context.Context, state request.Request, name string, typ uint16) (*dns.Msg, error)
+}
+
 // Zone is a structure that contains all data related to a DNS zone.
 type Zone struct {
 	origin     string
@@ -32,7 +39,7 @@ type Zone struct {
 	ReloadByMtime  bool
 	reloadShutdown chan bool
 
-	Upstream *upstream.Upstream // Upstream for looking up external names during the resolution process.
+	Upstream Upstreamer // Upstream for looking up external names during the resolution process.
 }
 
 // Apex contains the apex records of a zone: SOA, NS and their potential signatures.