plugin/file/auto: Write CNAME answer to client even if target lookup …

…is SERVFAIL (coredns#4863) * write cname answer to client even if target lookup is servfail Signed-off-by: Chris O'Haver <[email protected]> * fix existing unit test expectations Signed-off-by: Chris O'Haver <[email protected]>
diogenxs · Sep 14, 2021 · 158ad2d · 158ad2d
1 parent 8f7162c
commit 158ad2d
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 4 deletions.
diff --git a/plugin/auto/auto.go b/plugin/auto/auto.go
@@ -82,7 +82,14 @@ func (a Auto) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 	case file.Delegation:
 		m.Authoritative = false
 	case file.ServerFailure:
-		return dns.RcodeServerFailure, nil
+		// If the result is SERVFAIL and the answer is non-empty, then the SERVFAIL came from an
+		// external CNAME lookup and the answer contains the CNAME with no target record. We should
+		// write the CNAME record to the client instead of sending an empty SERVFAIL response.
+		if len(m.Answer) == 0 {
+			return dns.RcodeServerFailure, nil
+		}
+		//  The rcode in the response should be the rcode received from the target lookup. RFC 6604 section 3
+		m.Rcode = dns.RcodeServerFailure
 	}
 
 	w.WriteMsg(m)

diff --git a/plugin/file/file.go b/plugin/file/file.go
@@ -99,7 +99,14 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 	case Delegation:
 		m.Authoritative = false
 	case ServerFailure:
-		return dns.RcodeServerFailure, nil
+		// If the result is SERVFAIL and the answer is non-empty, then the SERVFAIL came from an
+		// external CNAME lookup and the answer contains the CNAME with no target record. We should
+		// write the CNAME record to the client instead of sending an empty SERVFAIL response.
+		if len(m.Answer) == 0 {
+			return dns.RcodeServerFailure, nil
+		}
+		//  The rcode in the response should be the rcode received from the target lookup. RFC 6604 section 3
+		m.Rcode = dns.RcodeServerFailure
 	}
 
 	w.WriteMsg(m)

diff --git a/plugin/file/lookup.go b/plugin/file/lookup.go
@@ -383,7 +383,7 @@ Redo:
 func (z *Zone) doLookup(ctx context.Context, state request.Request, target string, qtype uint16) ([]dns.RR, Result) {
 	m, e := z.Upstream.Lookup(ctx, state, target, qtype)
 	if e != nil {
-		return nil, Success
+		return nil, ServerFailure
 	}
 	if m == nil {
 		return nil, Success

diff --git a/plugin/file/lookup_test.go b/plugin/file/lookup_test.go
@@ -117,6 +117,14 @@ var dnsTestCases = []test.Case{
 		},
 		Ns: miekAuth,
 	},
+	{
+		Qname: "ext-cname.miek.nl.", Qtype: dns.TypeA,
+		Answer: []dns.RR{
+			test.CNAME("ext-cname.miek.nl.	1800	IN	CNAME	example.com."),
+		},
+		Rcode: dns.RcodeServerFailure,
+		Ns:    miekAuth,
+	},
 }
 
 const (
@@ -218,4 +226,6 @@ archive         IN      CNAME   a
 dname           IN      DNAME   x
 
 srv		IN	SRV     10 10 8080 a.miek.nl.
-mx		IN	MX      10 a.miek.nl.`
+mx		IN	MX      10 a.miek.nl.
+
+ext-cname   IN   CNAME  example.com.`
diff --git a/test/file_upstream_test.go b/test/file_upstream_test.go
@@ -77,10 +77,17 @@ func TestFileUpstreamError(t *testing.T) {
 		},
 		"srvfail": {
 			Qname: "srvfail.example.org.", Qtype: dns.TypeA,
+			Answer: []dns.RR{
+				test.CNAME("srvfail.example.org.	3600	IN	CNAME	srvfail.example.net."),
+			},
 			Rcode: dns.RcodeServerFailure,
 		},
 		"srvfail-chain": {
 			Qname: "chain2.example.org.", Qtype: dns.TypeA,
+			Answer: []dns.RR{
+				test.CNAME("chain2.example.org.	3600	IN	CNAME	srvfail.example.org."),
+				test.CNAME("srvfail.example.org.	3600	IN	CNAME	srvfail.example.net."),
+			},
 			Rcode: dns.RcodeServerFailure,
 		},
 		"nodata": {