diff --git a/plugin/cache/cache.go b/plugin/cache/cache.go index 973d78e47..76a73ff40 100644 --- a/plugin/cache/cache.go +++ b/plugin/cache/cache.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "hash/fnv" "net" + "strings" "time" "github.com/coredns/coredns/plugin" @@ -94,6 +95,20 @@ func key(qname string, m *dns.Msg, t response.Type, do, cd bool) (bool, uint64) if t == response.NameError && !hasSOA(m) { return false, 0 } + // An upstream may return NOERROR with a non-empty answer that still does not + // resolve the question and without an SOA to bound a negative TTL: a CNAME + // chain that does not terminate in the queried type (an incomplete recursion + // result from a forwarder). This is effectively an SOA-less NODATA response, + // which per RFC 2308 section 5 SHOULD NOT be cached. response.Typify classifies + // it as NoError because the answer section is non-empty, so caching it in the + // positive cache would replay the non-answer to clients until it expires. Skip + // caching so the next query is resolved upstream again. An empty answer section + // is deliberately left cacheable: it is indistinguishable from a legitimate + // NOERROR positive response that carries its data outside the answer section + // (for example the whoami plugin, which answers in the additional section). + if t == response.NoError && !hasSOA(m) && isNODATA(m) { + return false, 0 + } return true, hash(qname, m.Question[0].Qtype, m.Question[0].Qclass, do, cd) } @@ -107,6 +122,121 @@ func hasSOA(m *dns.Msg) bool { return false } +// isNODATA reports whether a NOERROR response with a non-empty answer section +// does not answer the question. Following RFC 1034 section 3.6.2 and RFC 2308 +// sections 1 and 2.2, a query of any type other than CNAME (and ANY) is +// restarted along the CNAME chain, so the effective owner name is the target at +// the end of the CNAME chain that starts at the question name. The response +// answers the question only if it carries a record of the queried type at that +// terminal name (records at any other owner name are irrelevant, and per RFC +// 1034 a CNAME's owner never co-locates other data). This rule is independent of +// the queried type: an MX, TXT, SRV, etc. chain that does not reach the queried +// type is NODATA just like an A or AAAA one. When the chain is malformed (an +// owner with more than one distinct CNAME target, or a loop) it has no +// well-defined terminal name, so the response is treated as NODATA, which errs +// toward re-querying upstream rather than caching a non-answer. An empty answer +// section returns false so that legitimate positive responses carrying data +// outside the answer section (for example the whoami plugin) remain cacheable. +// ANY queries are excluded because any record answers them. Note: a bare DNAME +// (RFC 6672) without its synthesized CNAME is treated as NODATA; standard +// responses include the synthesized CNAME, which the chain walk follows. +func isNODATA(m *dns.Msg) bool { + if len(m.Answer) == 0 { + return false + } + qtype := m.Question[0].Qtype + if qtype == dns.TypeANY { + return false + } + // A CNAME query is answered by the CNAME itself, so the chain is not + // followed; otherwise resolve it to the terminal owner name. + name := m.Question[0].Name + if qtype != dns.TypeCNAME { + terminal, ok := canonicalName(m.Answer, name) + if !ok { + // The CNAME chain is malformed (an owner with more than one + // distinct target, or a loop) and therefore has no well-defined + // QNAME per RFC 2181 section 10.1 and RFC 1034 section 3.6.2. Such + // a response cannot be shown to answer the question, so treat it as + // NODATA and (being SOA-less) leave it uncacheable. + return true + } + name = terminal + } + for _, r := range m.Answer { + h := r.Header() + if h.Rrtype == qtype && strings.EqualFold(h.Name, name) { + return false + } + } + return true +} + +// canonicalName follows the owner-linked CNAME chain in answer starting at name +// and returns the terminal target name together with a validity flag. Records +// whose owner is not on the chain are ignored. The chain is invalid (ok=false) +// when it is not a single unambiguous path to a terminal name: an owner that has +// more than one distinct CNAME target violates RFC 2181 section 10.1 (an alias +// has exactly one canonical name), and a revisited owner is a CNAME loop, which +// RFC 1034 section 3.6.2 says must be signalled as an error. Reporting validity +// rather than silently stopping keeps the classification order-independent and +// fail-closed: callers treat a malformed chain as a non-answer. Duplicate CNAME +// records that name the same target are tolerated, since they still describe a +// single canonical name. +func canonicalName(answer []dns.RR, name string) (string, bool) { + visited := nameSet{} + for { + if visited.contains(name) { + // Revisited owner: the chain contains a CNAME loop. + return name, false + } + visited.add(name) + + target, ok := uniqueCNAMETarget(answer, name) + if !ok { + // Owner has more than one distinct canonical name. + return name, false + } + if target == "" { + // Terminal owner reached: no CNAME continues the chain. + return name, true + } + name = target + } +} + +// uniqueCNAMETarget returns the canonical name that owner is aliased to by a +// CNAME record in answer. ok is false when owner carries more than one distinct +// CNAME target, which violates RFC 2181 section 10.1. When owner has no CNAME the +// returned target is empty and ok is true, marking a terminal owner. Duplicate +// CNAME records naming the same target are tolerated. +func uniqueCNAMETarget(answer []dns.RR, owner string) (target string, ok bool) { + for _, r := range answer { + c, isCNAME := r.(*dns.CNAME) + if !isCNAME || !strings.EqualFold(c.Header().Name, owner) { + continue + } + if target != "" && !strings.EqualFold(target, c.Target) { + return "", false + } + target = c.Target + } + return target, true +} + +// nameSet is a set of domain names compared case-insensitively, used to detect +// revisited owners (loops) while walking a CNAME chain. +type nameSet map[string]struct{} + +func (s nameSet) contains(name string) bool { + _, ok := s[strings.ToLower(name)] + return ok +} + +func (s nameSet) add(name string) { + s[strings.ToLower(name)] = struct{}{} +} + var one = []byte("1") var zero = []byte("0") diff --git a/plugin/cache/cache_test.go b/plugin/cache/cache_test.go index 1f60a605c..848eaf7f8 100644 --- a/plugin/cache/cache_test.go +++ b/plugin/cache/cache_test.go @@ -282,6 +282,243 @@ func TestCacheInsertion(t *testing.T) { }, shouldCache: true, }, + { + name: "test NOERROR dangling CNAME chain without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target1.example.net."), + test.CNAME("target1.example.net. 3600 IN CNAME target2.example.net."), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR CNAME chain ending in a different type without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.AAAA("target.example.net. 3600 IN AAAA ::1"), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR MX query CNAME chain without terminal MX without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR MX query CNAME chain terminating in MX caches", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.MX("target.example.net. 3600 IN MX 10 mail.example.net."), + }, + RecursionAvailable: true, + }, + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.MX("target.example.net. 3600 IN MX 10 mail.example.net."), + }, + RecursionAvailable: true, + }, + shouldCache: true, + }, + { + name: "test NOERROR DNSSEC dangling CNAME chain without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Do: true, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.RRSIG("alias.example.org. 3600 IN RRSIG CNAME 8 2 3600 20170521031301 20170421031301 12051 example.org. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR CNAME chain with unrelated A off the chain without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.A("unrelated.example.net. 3600 IN A 192.0.2.1"), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR lone CNAME answer to an ANY query caches", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeANY, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + }, + RecursionAvailable: true, + }, + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeANY, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + }, + RecursionAvailable: true, + }, + shouldCache: true, + }, + { + name: "test NOERROR CNAME chain terminating in A record caches", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.A("target.example.net. 3600 IN A 127.0.0.1"), + }, + RecursionAvailable: true, + }, + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.A("target.example.net. 3600 IN A 127.0.0.1"), + }, + RecursionAvailable: true, + }, + shouldCache: true, + }, + { + name: "test NOERROR CNAME chain terminating in A record out of order caches", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.A("target.example.net. 3600 IN A 127.0.0.1"), + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + }, + RecursionAvailable: true, + }, + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.A("target.example.net. 3600 IN A 127.0.0.1"), + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + }, + RecursionAvailable: true, + }, + shouldCache: true, + }, + { + name: "test NOERROR CNAME answer to a CNAME query caches", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeCNAME, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + }, + RecursionAvailable: true, + }, + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeCNAME, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + }, + RecursionAvailable: true, + }, + shouldCache: true, + }, + { + name: "test NOERROR CNAME chain with two distinct targets at one owner without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target1.example.net."), + test.CNAME("alias.example.org. 3600 IN CNAME target2.example.net."), + test.A("target1.example.net. 3600 IN A 192.0.2.1"), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR CNAME chain with two distinct targets at one owner reversed order without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target2.example.net."), + test.CNAME("alias.example.org. 3600 IN CNAME target1.example.net."), + test.A("target1.example.net. 3600 IN A 192.0.2.1"), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR CNAME loop with co-located A without SOA does not cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.CNAME("target.example.net. 3600 IN CNAME alias.example.org."), + test.A("alias.example.org. 3600 IN A 192.0.2.1"), + }, + RecursionAvailable: true, + }, + shouldCache: false, + }, + { + name: "test NOERROR CNAME chain with duplicate identical CNAME terminating in A caches", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.A("target.example.net. 3600 IN A 127.0.0.1"), + }, + RecursionAvailable: true, + }, + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "alias.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.CNAME("alias.example.org. 3600 IN CNAME target.example.net."), + test.A("target.example.net. 3600 IN A 127.0.0.1"), + }, + RecursionAvailable: true, + }, + shouldCache: true, + }, } now, _ := time.Parse(time.UnixDate, "Fri Apr 21 10:51:21 BST 2017") utc := now.UTC() @@ -780,7 +1017,7 @@ func ttlBackend(ttl int) plugin.Handler { m.SetReply(r) m.Response, m.RecursionAvailable = true, true - m.Answer = []dns.RR{test.A(fmt.Sprintf("example.org. %d IN A 127.0.0.53", ttl))} + m.Answer = []dns.RR{test.A(fmt.Sprintf("%s %d IN A 127.0.0.53", r.Question[0].Name, ttl))} w.WriteMsg(m) return dns.RcodeSuccess, nil }) @@ -813,7 +1050,7 @@ func slowTTLBackend(ttl int, delay time.Duration, done chan<- struct{}) plugin.H m := new(dns.Msg) m.SetReply(r) m.Response, m.RecursionAvailable = true, true - m.Answer = []dns.RR{test.A(fmt.Sprintf("example.org. %d IN A 127.0.0.53", ttl))} + m.Answer = []dns.RR{test.A(fmt.Sprintf("%s %d IN A 127.0.0.53", r.Question[0].Name, ttl))} w.WriteMsg(m) if done != nil { close(done)