From 59f7d2f519b9f484e92cc52dfe82c740c8f3f510 Mon Sep 17 00:00:00 2001 From: Grant Spence Date: Fri, 10 Nov 2023 07:00:47 -0800 Subject: [PATCH] UPSTREAM: 6354: openshift: Fix OCPBUGS-15755 plugin/cache: key cache on Checking Disabled (CD) bit (#6354) * plugin/cache: key cache on Checking Disabled (CD) bit Key the cache on CD bit, which effectively separates the entries for queries with CD disabled or enabled. Signed-off-by: Grant Spence --- plugin/cache/cache.go | 16 +- plugin/cache/cache_test.go | 655 +++++++++++------- plugin/cache/handler.go | 9 +- .../{prefech_test.go => prefetch_test.go} | 83 ++- plugin/test/helpers.go | 22 +- 5 files changed, 509 insertions(+), 276 deletions(-) rename plugin/cache/{prefech_test.go => prefetch_test.go} (67%) diff --git a/plugin/cache/cache.go b/plugin/cache/cache.go index 563b2abbe6d..1378263b828 100644 --- a/plugin/cache/cache.go +++ b/plugin/cache/cache.go @@ -79,7 +79,7 @@ func New() *Cache { // key returns key under which we store the item, -1 will be returned if we don't store the message. // Currently we do not cache Truncated, errors zone transfers or dynamic update messages. // qname holds the already lowercased qname. -func key(qname string, m *dns.Msg, t response.Type, do bool) (bool, uint64) { +func key(qname string, m *dns.Msg, t response.Type, do, cd bool) (bool, uint64) { // We don't store truncated responses. if m.Truncated { return false, 0 @@ -89,13 +89,13 @@ func key(qname string, m *dns.Msg, t response.Type, do bool) (bool, uint64) { return false, 0 } - return true, hash(qname, m.Question[0].Qtype, do) + return true, hash(qname, m.Question[0].Qtype, do, cd) } var one = []byte("1") var zero = []byte("0") -func hash(qname string, qtype uint16, do bool) uint64 { +func hash(qname string, qtype uint16, do, cd bool) uint64 { h := fnv.New64() if do { @@ -104,6 +104,12 @@ func hash(qname string, qtype uint16, do bool) uint64 { h.Write(zero) } + if cd { + h.Write(one) + } else { + h.Write(zero) + } + h.Write([]byte{byte(qtype >> 8)}) h.Write([]byte{byte(qtype)}) h.Write([]byte(qname)) @@ -129,6 +135,7 @@ type ResponseWriter struct { server string // Server handling the request. do bool // When true the original request had the DO bit set. + cd bool // When true the original request had the CD bit set. ad bool // When true the original request had the AD bit set. prefetch bool // When true write nothing back to the client. remoteAddr net.Addr @@ -159,6 +166,7 @@ func newPrefetchResponseWriter(server string, state request.Request, c *Cache) * state: state, server: server, do: state.Do(), + cd: state.Req.CheckingDisabled, prefetch: true, remoteAddr: addr, } @@ -177,7 +185,7 @@ func (w *ResponseWriter) WriteMsg(res *dns.Msg) error { mt, _ := response.Typify(res, w.now().UTC()) // key returns empty string for anything we don't want to cache. - hasKey, key := key(w.state.Name(), res, mt, w.do) + hasKey, key := key(w.state.Name(), res, mt, w.do, w.cd) msgTTL := dnsutil.MinimalTTL(res, mt) var duration time.Duration diff --git a/plugin/cache/cache_test.go b/plugin/cache/cache_test.go index 629a83e3423..947c67516c0 100644 --- a/plugin/cache/cache_test.go +++ b/plugin/cache/cache_test.go @@ -16,297 +16,330 @@ import ( "github.com/miekg/dns" ) -type cacheTestCase struct { - test.Case // the expected message coming "out" of cache - in test.Case // the test message going "in" to cache - AuthenticatedData bool - RecursionAvailable bool - Truncated bool - shouldCache bool +func cacheMsg(m *dns.Msg, tc test.Case) *dns.Msg { + m.RecursionAvailable = tc.RecursionAvailable + m.AuthenticatedData = tc.AuthenticatedData + m.CheckingDisabled = tc.CheckingDisabled + m.Authoritative = tc.Authoritative + m.Rcode = tc.Rcode + m.Truncated = tc.Truncated + m.Answer = tc.Answer + m.Ns = tc.Ns + // m.Extra = tc.in.Extra don't copy Extra, because we don't care and fake EDNS0 DO with tc.Do. + return m +} + +func newTestCache(ttl time.Duration) (*Cache, *ResponseWriter) { + c := New() + c.pttl = ttl + c.nttl = ttl + + crr := &ResponseWriter{ResponseWriter: nil, Cache: c} + crr.nexcept = []string{"neg-disabled.example.org."} + crr.pexcept = []string{"pos-disabled.example.org."} + + return c, crr } -var cacheTestCases = []cacheTestCase{ - { - // Test with Authenticated Data bit - RecursionAvailable: true, AuthenticatedData: true, - Case: test.Case{ - Qname: "miek.nl.", Qtype: dns.TypeMX, - Answer: []dns.RR{ - test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."), - test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."), +// TestCacheInsertion verifies the insertion of items to the cache. +func TestCacheInsertion(t *testing.T) { + cacheTestCases := []struct { + name string + out test.Case // the expected message coming "out" of cache + in test.Case // the test message going "in" to cache + shouldCache bool + }{ + { + name: "test ad bit cache", + out: test.Case{ + Qname: "miek.nl.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."), + test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."), + }, + RecursionAvailable: true, + AuthenticatedData: true, }, - }, - in: test.Case{ - Qname: "miek.nl.", Qtype: dns.TypeMX, - Answer: []dns.RR{ - test.MX("miek.nl. 3601 IN MX 1 aspmx.l.google.com."), - test.MX("miek.nl. 3601 IN MX 10 aspmx2.googlemail.com."), + in: test.Case{ + Qname: "miek.nl.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.MX("miek.nl. 3601 IN MX 1 aspmx.l.google.com."), + test.MX("miek.nl. 3601 IN MX 10 aspmx2.googlemail.com."), + }, + RecursionAvailable: true, + AuthenticatedData: true, }, + shouldCache: true, }, - shouldCache: true, - }, - { - // Test case sensitivity - RecursionAvailable: true, AuthenticatedData: true, - Case: test.Case{ - Qname: "miek.nl.", Qtype: dns.TypeMX, - Answer: []dns.RR{ - test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."), - test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."), + { + name: "test case sensitivity cache", + out: test.Case{ + Qname: "miek.nl.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."), + test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."), + }, + RecursionAvailable: true, + AuthenticatedData: true, }, - }, - in: test.Case{ - Qname: "mIEK.nL.", Qtype: dns.TypeMX, - Answer: []dns.RR{ - test.MX("miek.nl. 3601 IN MX 1 aspmx.l.google.com."), - test.MX("miek.nl. 3601 IN MX 10 aspmx2.googlemail.com."), + in: test.Case{ + Qname: "mIEK.nL.", Qtype: dns.TypeMX, + Answer: []dns.RR{ + test.MX("miek.nl. 3601 IN MX 1 aspmx.l.google.com."), + test.MX("miek.nl. 3601 IN MX 10 aspmx2.googlemail.com."), + }, + RecursionAvailable: true, + AuthenticatedData: true, }, + shouldCache: true, }, - shouldCache: true, - }, - { - // Test truncated responses don't get cached - Truncated: true, - Case: test.Case{ - Qname: "miek.nl.", Qtype: dns.TypeMX, - Answer: []dns.RR{test.MX("miek.nl. 1800 IN MX 1 aspmx.l.google.com.")}, - }, - in: test.Case{}, - shouldCache: false, - }, - { - // Test dns.RcodeNameError cache - RecursionAvailable: true, - Case: test.Case{ - Rcode: dns.RcodeNameError, - Qname: "example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{ - test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + { + name: "test truncated responses shouldn't cache", + in: test.Case{ + Qname: "miek.nl.", Qtype: dns.TypeMX, + Answer: []dns.RR{test.MX("miek.nl. 1800 IN MX 1 aspmx.l.google.com.")}, + Truncated: true, }, + shouldCache: false, }, - in: test.Case{ - Rcode: dns.RcodeNameError, - Qname: "example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{ - test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + { + name: "test dns.RcodeNameError cache", + out: test.Case{ + Rcode: dns.RcodeNameError, + Qname: "example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{ + test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + }, + RecursionAvailable: true, }, - }, - shouldCache: true, - }, - { - // Test dns.RcodeServerFailure cache - RecursionAvailable: true, - Case: test.Case{ - Rcode: dns.RcodeServerFailure, - Qname: "example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{}, - }, - in: test.Case{ - Rcode: dns.RcodeServerFailure, - Qname: "example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{}, - }, - shouldCache: true, - }, - { - // Test dns.RcodeNotImplemented cache - RecursionAvailable: true, - Case: test.Case{ - Rcode: dns.RcodeNotImplemented, - Qname: "example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{}, - }, - in: test.Case{ - Rcode: dns.RcodeNotImplemented, - Qname: "example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{}, - }, - shouldCache: true, - }, - { - // Test cache has separate items for query DO Bit value - // This doesn't cache because this RRSIG is expired and previous tests used DO Bit false - RecursionAvailable: true, - Case: test.Case{ - Qname: "miek.nl.", Qtype: dns.TypeMX, - Do: true, - Answer: []dns.RR{ - test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."), - test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."), - test.RRSIG("miek.nl. 3600 IN RRSIG MX 8 2 1800 20160521031301 20160421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + in: test.Case{ + Rcode: dns.RcodeNameError, + Qname: "example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{ + test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + }, + RecursionAvailable: true, }, + shouldCache: true, }, - in: test.Case{ - Qname: "miek.nl.", Qtype: dns.TypeMX, - Do: true, - Answer: []dns.RR{ - test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."), - test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."), - test.RRSIG("miek.nl. 1800 IN RRSIG MX 8 2 1800 20160521031301 20160421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + { + name: "test dns.RcodeServerFailure cache", + out: test.Case{ + Rcode: dns.RcodeServerFailure, + Qname: "example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{}, + RecursionAvailable: true, + }, + in: test.Case{ + Rcode: dns.RcodeServerFailure, + Qname: "example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{}, + RecursionAvailable: true, }, + shouldCache: true, }, - shouldCache: false, - }, - { - // Test DO Bit with a RRSIG that is not expired - RecursionAvailable: true, - Case: test.Case{ - Qname: "example.org.", Qtype: dns.TypeMX, - Do: true, - Answer: []dns.RR{ - test.MX("example.org. 3600 IN MX 1 aspmx.l.google.com."), - test.MX("example.org. 3600 IN MX 10 aspmx2.googlemail.com."), - test.RRSIG("example.org. 3600 IN RRSIG MX 8 2 1800 20170521031301 20170421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + { + name: "test dns.RcodeNotImplemented cache", + out: test.Case{ + Rcode: dns.RcodeNotImplemented, + Qname: "example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{}, + RecursionAvailable: true, }, + in: test.Case{ + Rcode: dns.RcodeNotImplemented, + Qname: "example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{}, + RecursionAvailable: true, + }, + shouldCache: true, }, - in: test.Case{ - Qname: "example.org.", Qtype: dns.TypeMX, - Do: true, - Answer: []dns.RR{ - test.MX("example.org. 3600 IN MX 1 aspmx.l.google.com."), - test.MX("example.org. 3600 IN MX 10 aspmx2.googlemail.com."), - test.RRSIG("example.org. 1800 IN RRSIG MX 8 2 1800 20170521031301 20170421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + { + name: "test expired RRSIG doesn't cache", + in: test.Case{ + Qname: "miek.nl.", Qtype: dns.TypeMX, + Do: true, + Answer: []dns.RR{ + test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."), + test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."), + test.RRSIG("miek.nl. 1800 IN RRSIG MX 8 2 1800 20160521031301 20160421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + }, + RecursionAvailable: true, }, + shouldCache: false, }, - shouldCache: true, - }, - { - // Test negative zone exception - in: test.Case{ - Rcode: dns.RcodeNameError, - Qname: "neg-disabled.example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{ - test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + { + name: "test DO bit with RRSIG not expired cache", + out: test.Case{ + Qname: "example.org.", Qtype: dns.TypeMX, + Do: true, + Answer: []dns.RR{ + test.MX("example.org. 3600 IN MX 1 aspmx.l.google.com."), + test.MX("example.org. 3600 IN MX 10 aspmx2.googlemail.com."), + test.RRSIG("example.org. 3600 IN RRSIG MX 8 2 1800 20170521031301 20170421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + }, + RecursionAvailable: true, + }, + in: test.Case{ + Qname: "example.org.", Qtype: dns.TypeMX, + Do: true, + Answer: []dns.RR{ + test.MX("example.org. 3600 IN MX 1 aspmx.l.google.com."), + test.MX("example.org. 3600 IN MX 10 aspmx2.googlemail.com."), + test.RRSIG("example.org. 1800 IN RRSIG MX 8 2 1800 20170521031301 20170421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + }, + RecursionAvailable: true, }, + shouldCache: true, }, - Case: test.Case{}, - shouldCache: false, - }, - { - // Test positive zone exception - in: test.Case{ - Rcode: dns.RcodeSuccess, - Qname: "pos-disabled.example.org.", Qtype: dns.TypeA, - Answer: []dns.RR{ - test.A("pos-disabled.example.org. 3600 IN A 127.0.0.1"), + { + name: "test CD bit cache", + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "dnssec-failed.org.", + Qtype: dns.TypeA, + Answer: []dns.RR{ + test.A("dnssec-failed.org. 3600 IN A 127.0.0.1"), + }, + CheckingDisabled: true, }, + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "dnssec-failed.org.", + Answer: []dns.RR{ + test.A("dnssec-failed.org. 3600 IN A 127.0.0.1"), + }, + Qtype: dns.TypeA, + CheckingDisabled: true, + }, + shouldCache: true, }, - Case: test.Case{}, - shouldCache: false, - }, - { - // Test positive zone exception with negative answer - in: test.Case{ - Rcode: dns.RcodeNameError, - Qname: "pos-disabled.example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{ - test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + { + name: "test negative zone exception shouldn't cache", + in: test.Case{ + Rcode: dns.RcodeNameError, + Qname: "neg-disabled.example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{ + test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + }, }, + shouldCache: false, }, - Case: test.Case{ - Rcode: dns.RcodeNameError, - Qname: "pos-disabled.example.org.", Qtype: dns.TypeA, - Ns: []dns.RR{ - test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + { + name: "test positive zone exception shouldn't cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "pos-disabled.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.A("pos-disabled.example.org. 3600 IN A 127.0.0.1"), + }, }, + shouldCache: false, }, - shouldCache: true, - }, - { - // Test negative zone exception with positive answer - in: test.Case{ - Rcode: dns.RcodeSuccess, - Qname: "neg-disabled.example.org.", Qtype: dns.TypeA, - Answer: []dns.RR{ - test.A("neg-disabled.example.org. 3600 IN A 127.0.0.1"), + { + name: "test positive zone exception with negative answer cache", + in: test.Case{ + Rcode: dns.RcodeNameError, + Qname: "pos-disabled.example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{ + test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + }, + }, + out: test.Case{ + Rcode: dns.RcodeNameError, + Qname: "pos-disabled.example.org.", Qtype: dns.TypeA, + Ns: []dns.RR{ + test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), + }, }, + shouldCache: true, }, - Case: test.Case{ - Rcode: dns.RcodeSuccess, - Qname: "neg-disabled.example.org.", Qtype: dns.TypeA, - Answer: []dns.RR{ - test.A("neg-disabled.example.org. 3600 IN A 127.0.0.1"), + { + name: "test negative zone exception with positive answer cache", + in: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "neg-disabled.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.A("neg-disabled.example.org. 3600 IN A 127.0.0.1"), + }, + }, + out: test.Case{ + Rcode: dns.RcodeSuccess, + Qname: "neg-disabled.example.org.", Qtype: dns.TypeA, + Answer: []dns.RR{ + test.A("neg-disabled.example.org. 3600 IN A 127.0.0.1"), + }, }, + shouldCache: true, }, - shouldCache: true, - }, -} - -func cacheMsg(m *dns.Msg, tc cacheTestCase) *dns.Msg { - m.RecursionAvailable = tc.RecursionAvailable - m.AuthenticatedData = tc.AuthenticatedData - m.Authoritative = true - m.Rcode = tc.Rcode - m.Truncated = tc.Truncated - m.Answer = tc.in.Answer - m.Ns = tc.in.Ns - // m.Extra = tc.in.Extra don't copy Extra, because we don't care and fake EDNS0 DO with tc.Do. - return m -} - -func newTestCache(ttl time.Duration) (*Cache, *ResponseWriter) { - c := New() - c.pttl = ttl - c.nttl = ttl - - crr := &ResponseWriter{ResponseWriter: nil, Cache: c} - crr.nexcept = []string{"neg-disabled.example.org."} - crr.pexcept = []string{"pos-disabled.example.org."} - - return c, crr -} - -func TestCache(t *testing.T) { + } now, _ := time.Parse(time.UnixDate, "Fri Apr 21 10:51:21 BST 2017") utc := now.UTC() - c, crr := newTestCache(maxTTL) - - for n, tc := range cacheTestCases { - m := tc.in.Msg() - m = cacheMsg(m, tc) - - state := request.Request{W: &test.ResponseWriter{}, Req: m} + for _, tc := range cacheTestCases { + t.Run(tc.name, func(t *testing.T) { + // Create a new cache every time to prevent accidental comparison with a previous item. + c, crr := newTestCache(maxTTL) - mt, _ := response.Typify(m, utc) - valid, k := key(state.Name(), m, mt, state.Do()) + m := tc.in.Msg() + m = cacheMsg(m, tc.in) - if valid { - crr.set(m, k, mt, c.pttl) - } - - i := c.getIgnoreTTL(time.Now().UTC(), state, "dns://:53") - ok := i != nil - - if !tc.shouldCache && ok { - t.Errorf("Test %d: Cached message that should not have been cached: %s", n, state.Name()) - continue - } - if tc.shouldCache && !ok { - t.Errorf("Test %d: Did not cache message that should have been cached: %s", n, state.Name()) - continue - } + state := request.Request{W: &test.ResponseWriter{}, Req: m} - if ok { - resp := i.toMsg(m, time.Now().UTC(), state.Do(), m.AuthenticatedData) + mt, _ := response.Typify(m, utc) + valid, k := key(state.Name(), m, mt, state.Do(), state.Req.CheckingDisabled) - if err := test.Header(tc.Case, resp); err != nil { - t.Logf("Cache %v", resp) - t.Error(err) - continue + if valid { + // Insert cache entry + crr.set(m, k, mt, c.pttl) } - if err := test.Section(tc.Case, test.Answer, resp.Answer); err != nil { - t.Logf("Cache %v -- %v", test.Answer, resp.Answer) - t.Error(err) + // Attempt to retrieve cache entry + i := c.getIgnoreTTL(time.Now().UTC(), state, "dns://:53") + found := i != nil + + if !tc.shouldCache && found { + t.Fatalf("Cached message that should not have been cached: %s", state.Name()) } - if err := test.Section(tc.Case, test.Ns, resp.Ns); err != nil { - t.Error(err) + if tc.shouldCache && !found { + t.Fatalf("Did not cache message that should have been cached: %s", state.Name()) } - if err := test.Section(tc.Case, test.Extra, resp.Extra); err != nil { - t.Error(err) + + if found { + resp := i.toMsg(m, time.Now().UTC(), state.Do(), m.AuthenticatedData) + + // TODO: If we incorporate these individual checks into the + // test.Header function, we can eliminate them from here. + // Cache entries are always Authoritative. + if resp.Authoritative != true { + t.Error("Expected Authoritative Answer bit to be true, but was false") + } + if resp.AuthenticatedData != tc.out.AuthenticatedData { + t.Errorf("Expected Authenticated Data bit to be %t, but got %t", tc.out.AuthenticatedData, resp.AuthenticatedData) + } + if resp.RecursionAvailable != tc.out.RecursionAvailable { + t.Errorf("Expected Recursion Available bit to be %t, but got %t", tc.out.RecursionAvailable, resp.RecursionAvailable) + } + if resp.CheckingDisabled != tc.out.CheckingDisabled { + t.Errorf("Expected Checking Disabled bit to be %t, but got %t", tc.out.CheckingDisabled, resp.CheckingDisabled) + } + + if err := test.Header(tc.out, resp); err != nil { + t.Logf("Cache %v", resp) + t.Error(err) + } + if err := test.Section(tc.out, test.Answer, resp.Answer); err != nil { + t.Logf("Cache %v -- %v", test.Answer, resp.Answer) + t.Error(err) + } + if err := test.Section(tc.out, test.Ns, resp.Ns); err != nil { + t.Error(err) + } + if err := test.Section(tc.out, test.Extra, resp.Extra); err != nil { + t.Error(err) + } } - } + }) } } @@ -668,7 +701,7 @@ func TestCacheWildcardMetadata(t *testing.T) { if c.pcache.Len() != 1 { t.Errorf("Msg should have been cached") } - _, k := key(qname, w.Msg, response.NoError, state.Do()) + _, k := key(qname, w.Msg, response.NoError, state.Do(), state.Req.CheckingDisabled) i, _ := c.pcache.Get(k) if i.(*item).wildcard != wildcard { t.Errorf("expected wildcard response to enter cache with cache item's wildcard = %q, got %q", wildcard, i.(*item).wildcard) @@ -728,7 +761,129 @@ func TestCacheKeepTTL(t *testing.T) { } } -// wildcardMetadataBackend mocks a backend that reponds with a response for qname synthesized by wildcard +// TestCacheSeparation verifies whether the cache maintains separation for specific DNS query types and options. +func TestCacheSeparation(t *testing.T) { + now, _ := time.Parse(time.UnixDate, "Fri Apr 21 10:51:21 BST 2017") + utc := now.UTC() + + testCases := []struct { + name string + initial test.Case + query test.Case + expectCached bool // if a cache entry should be found before inserting + }{ + { + name: "query type should be unique", + initial: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + }, + query: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeAAAA, + }, + }, + { + name: "DO bit should be unique", + initial: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + }, + query: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + Do: true, + }, + }, + { + name: "CD bit should be unique", + initial: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + }, + query: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + CheckingDisabled: true, + }, + }, + { + name: "CD bit and DO bit should be unique", + initial: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + }, + query: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + CheckingDisabled: true, + Do: true, + }, + }, + { + name: "CD bit, DO bit, and query type should be unique", + initial: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + }, + query: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeMX, + CheckingDisabled: true, + Do: true, + }, + }, + { + name: "authoritative answer bit should NOT be unique", + initial: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + }, + query: test.Case{ + Qname: "example.org.", + Qtype: dns.TypeA, + Authoritative: true, + }, + expectCached: true, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + c := New() + crr := &ResponseWriter{ResponseWriter: nil, Cache: c} + + // Insert initial cache entry + m := tc.initial.Msg() + m = cacheMsg(m, tc.initial) + state := request.Request{W: &test.ResponseWriter{}, Req: m} + + mt, _ := response.Typify(m, utc) + valid, k := key(state.Name(), m, mt, state.Do(), state.Req.CheckingDisabled) + + if valid { + // Insert cache entry + crr.set(m, k, mt, c.pttl) + } + + // Attempt to retrieve cache entry + m = tc.query.Msg() + m = cacheMsg(m, tc.query) + state = request.Request{W: &test.ResponseWriter{}, Req: m} + + item := c.getIgnoreTTL(time.Now().UTC(), state, "dns://:53") + found := item != nil + + if !tc.expectCached && found { + t.Fatal("Found cache message should that should not exist prior to inserting") + } + if tc.expectCached && !found { + t.Fatal("Did not find cache message that should exist prior to inserting") + } + }) + } +} + +// wildcardMetadataBackend mocks a backend that responds with a response for qname synthesized by wildcard // and sets the zone/wildcard metadata value func wildcardMetadataBackend(qname, wildcard string) plugin.Handler { return plugin.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { diff --git a/plugin/cache/handler.go b/plugin/cache/handler.go index de7cae4567c..38a8bfebc5e 100644 --- a/plugin/cache/handler.go +++ b/plugin/cache/handler.go @@ -18,6 +18,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) rc := r.Copy() // We potentially modify r, to prevent other plugins from seeing this (r is a pointer), copy r into rc. state := request.Request{W: w, Req: rc} do := state.Do() + cd := r.CheckingDisabled ad := r.AuthenticatedData zone := plugin.Zones(c.Zones).Matches(state.Name()) @@ -36,7 +37,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) ttl := 0 i := c.getIgnoreTTL(now, state, server) if i == nil { - crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do, ad: ad, + crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do, ad: ad, cd: cd, nexcept: c.nexcept, pexcept: c.pexcept, wildcardFunc: wildcardFunc(ctx)} return c.doRefresh(ctx, state, crr) } @@ -44,7 +45,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) if ttl < 0 { // serve stale behavior if c.verifyStale { - crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do} + crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do, cd: cd} cw := newVerifyStaleResponseWriter(crr) ret, err := c.doRefresh(ctx, state, cw) if cw.refreshed { @@ -121,7 +122,7 @@ func (c *Cache) Name() string { return "cache" } // getIgnoreTTL unconditionally returns an item if it exists in the cache. func (c *Cache) getIgnoreTTL(now time.Time, state request.Request, server string) *item { - k := hash(state.Name(), state.QType(), state.Do()) + k := hash(state.Name(), state.QType(), state.Do(), state.Req.CheckingDisabled) cacheRequests.WithLabelValues(server, c.zonesMetricLabel, c.viewMetricLabel).Inc() if i, ok := c.ncache.Get(k); ok { @@ -145,7 +146,7 @@ func (c *Cache) getIgnoreTTL(now time.Time, state request.Request, server string } func (c *Cache) exists(state request.Request) *item { - k := hash(state.Name(), state.QType(), state.Do()) + k := hash(state.Name(), state.QType(), state.Do(), state.Req.CheckingDisabled) if i, ok := c.ncache.Get(k); ok { return i.(*item) } diff --git a/plugin/cache/prefech_test.go b/plugin/cache/prefetch_test.go similarity index 67% rename from plugin/cache/prefech_test.go rename to plugin/cache/prefetch_test.go index 609956ee928..3085fe0f9bd 100644 --- a/plugin/cache/prefech_test.go +++ b/plugin/cache/prefetch_test.go @@ -28,12 +28,12 @@ func TestPrefetch(t *testing.T) { { after: 0 * time.Second, answer: "hits.reset.example.org. 80 IN A 127.0.0.1", - fetch: true, + fetch: true, // Initial fetch }, { after: 73 * time.Second, answer: "hits.reset.example.org. 7 IN A 127.0.0.1", - fetch: true, + fetch: true, // Triggers prefetch with 7 TTL (10% of 80 = 8 TTL threshold) }, { after: 80 * time.Second, @@ -91,6 +91,68 @@ func TestPrefetch(t *testing.T) { }, }, }, + { + // tests whether cache prefetches with the do bit + qname: "do.prefetch.example.org.", + ttl: 80, + prefetch: 1, + verifications: []verification{ + { + after: 0 * time.Second, + answer: "do.prefetch.example.org. 80 IN A 127.0.0.1", + do: true, + fetch: true, + }, + { + after: 73 * time.Second, + answer: "do.prefetch.example.org. 7 IN A 127.0.0.1", + do: true, + fetch: true, + }, + { + after: 80 * time.Second, + answer: "do.prefetch.example.org. 73 IN A 127.0.0.2", + do: true, + }, + { + // Should be 127.0.0.3 as 127.0.0.2 was the prefetch WITH do bit + after: 80 * time.Second, + answer: "do.prefetch.example.org. 80 IN A 127.0.0.3", + fetch: true, + }, + }, + }, + { + // tests whether cache prefetches with the cd bit + qname: "cd.prefetch.example.org.", + ttl: 80, + prefetch: 1, + verifications: []verification{ + { + after: 0 * time.Second, + answer: "cd.prefetch.example.org. 80 IN A 127.0.0.1", + cd: true, + fetch: true, + }, + { + after: 73 * time.Second, + answer: "cd.prefetch.example.org. 7 IN A 127.0.0.1", + cd: true, + fetch: true, + }, + { + after: 80 * time.Second, + answer: "cd.prefetch.example.org. 73 IN A 127.0.0.2", + cd: true, + }, + { + // Should be 127.0.0.3 as 127.0.0.2 was the prefetch WITH cd bit + after: 80 * time.Second, + answer: "cd.prefetch.example.org. 80 IN A 127.0.0.3", + fetch: true, + }, + }, + }, } t0, err := time.Parse(time.RFC3339, "2018-01-01T14:00:00+00:00") @@ -102,28 +164,29 @@ func TestPrefetch(t *testing.T) { fetchc := make(chan struct{}, 1) c := New() - c.prefetch = tt.prefetch c.Next = prefetchHandler(tt.qname, tt.ttl, fetchc) + c.prefetch = tt.prefetch - req := new(dns.Msg) - req.SetQuestion(tt.qname, dns.TypeA) rec := dnstest.NewRecorder(&test.ResponseWriter{}) for _, v := range tt.verifications { c.now = func() time.Time { return t0.Add(v.after) } + req := new(dns.Msg) + req.SetQuestion(tt.qname, dns.TypeA) + req.CheckingDisabled = v.cd + req.SetEdns0(512, v.do) + c.ServeDNS(context.TODO(), rec, req) if v.fetch { select { case <-fetchc: - if !v.fetch { - t.Fatalf("After %s: want request to trigger a prefetch", v.after) - } + // Prefetch handler was called. case <-time.After(time.Second): t.Fatalf("After %s: want request to trigger a prefetch", v.after) } } - if want, got := rec.Rcode, dns.RcodeSuccess; want != got { + if want, got := dns.RcodeSuccess, rec.Rcode; want != got { t.Errorf("After %s: want rcode %d, got %d", v.after, want, got) } if want, got := 1, len(rec.Msg.Answer); want != got { @@ -140,6 +203,8 @@ func TestPrefetch(t *testing.T) { type verification struct { after time.Duration answer string + do bool + cd bool // fetch defines whether a request is sent to the next handler. fetch bool } diff --git a/plugin/test/helpers.go b/plugin/test/helpers.go index 8145b605ae5..f99790a2360 100644 --- a/plugin/test/helpers.go +++ b/plugin/test/helpers.go @@ -29,15 +29,19 @@ func (p RRSet) Less(i, j int) bool { return p[i].String() < p[j].String() } // Case represents a test case that encapsulates various data from a query and response. // Note that is the TTL of a record is 303 we don't compare it with the TTL. type Case struct { - Qname string - Qtype uint16 - Rcode int - Do bool - AuthenticatedData bool - Answer []dns.RR - Ns []dns.RR - Extra []dns.RR - Error error + Qname string + Qtype uint16 + Rcode int + Do bool + CheckingDisabled bool + RecursionAvailable bool + AuthenticatedData bool + Authoritative bool + Truncated bool + Answer []dns.RR + Ns []dns.RR + Extra []dns.RR + Error error } // Msg returns a *dns.Msg embedded in c.