From 6ec70a060368263b35eff292660f76c22aac5e59 Mon Sep 17 00:00:00 2001 From: houyuwushang Date: Sat, 11 Jul 2026 06:18:27 +0800 Subject: [PATCH] request/cache: bind responses and entries to QCLASS (#8272) Signed-off-by: houyuwushang --- plugin/cache/ad_bit_test.go | 2 +- plugin/cache/cache.go | 7 +++-- plugin/cache/cache_test.go | 2 +- plugin/cache/handler.go | 10 +++--- plugin/cache/item.go | 4 ++- plugin/cache/qclass_test.go | 63 +++++++++++++++++++++++++++++++++++++ request/request.go | 8 +++-- request/request_test.go | 5 +++ 8 files changed, 89 insertions(+), 12 deletions(-) create mode 100644 plugin/cache/qclass_test.go diff --git a/plugin/cache/ad_bit_test.go b/plugin/cache/ad_bit_test.go index 55286951b..6e85a0f59 100644 --- a/plugin/cache/ad_bit_test.go +++ b/plugin/cache/ad_bit_test.go @@ -12,7 +12,7 @@ import ( // TestCacheADBitNotPartitioned guards the fix for issue #6642. // -// The cache key is hashed on qname, qtype, the DO bit and the CD bit, but +// The cache key is hashed on qname, qtype, qclass, the DO bit and the CD bit, but // deliberately NOT on the AD bit (AD must not partition the cache). This means // a single cache entry serves both AD-requesting and non-AD-requesting clients, // so the AD bit returned to a client must be derived per request from the diff --git a/plugin/cache/cache.go b/plugin/cache/cache.go index cc96162a4..973d78e47 100644 --- a/plugin/cache/cache.go +++ b/plugin/cache/cache.go @@ -95,7 +95,7 @@ func key(qname string, m *dns.Msg, t response.Type, do, cd bool) (bool, uint64) return false, 0 } - return true, hash(qname, m.Question[0].Qtype, do, cd) + return true, hash(qname, m.Question[0].Qtype, m.Question[0].Qclass, do, cd) } func hasSOA(m *dns.Msg) bool { @@ -110,7 +110,7 @@ func hasSOA(m *dns.Msg) bool { var one = []byte("1") var zero = []byte("0") -func hash(qname string, qtype uint16, do, cd bool) uint64 { +func hash(qname string, qtype, qclass uint16, do, cd bool) uint64 { h := fnv.New64() if do { @@ -128,6 +128,9 @@ func hash(qname string, qtype uint16, do, cd bool) uint64 { var qtypeBytes [2]byte binary.BigEndian.PutUint16(qtypeBytes[:], qtype) h.Write(qtypeBytes[:]) + var qclassBytes [2]byte + binary.BigEndian.PutUint16(qclassBytes[:], qclass) + h.Write(qclassBytes[:]) h.Write([]byte(qname)) return h.Sum64() } diff --git a/plugin/cache/cache_test.go b/plugin/cache/cache_test.go index da4c7d7b1..1f60a605c 100644 --- a/plugin/cache/cache_test.go +++ b/plugin/cache/cache_test.go @@ -1072,7 +1072,7 @@ func TestServfailDoesNotShadowPositiveCache(t *testing.T) { posMsg.Response = true posMsg.Answer = []dns.RR{test.A("example.org. 120 IN A 127.0.0.53")} posItem := newItem(posMsg, now.Add(-30*time.Second), 120*time.Second) - k := hash("example.org.", dns.TypeA, false, false) + k := hash("example.org.", dns.TypeA, dns.ClassINET, false, false) c.pcache.Add(k, posItem) // Manually insert a SERVFAIL entry in ncache (stored just now, TTL 5s). diff --git a/plugin/cache/handler.go b/plugin/cache/handler.go index cc0b20bb7..54b4c81b7 100644 --- a/plugin/cache/handler.go +++ b/plugin/cache/handler.go @@ -121,7 +121,7 @@ func (c *Cache) doPrefetch(ctx context.Context, cw *ResponseWriter, i *item, now // When prefetching we loose the item i, and with it the frequency // that we've gathered sofar. See we copy the frequencies info back // into the new item that was stored in the cache. - if i1 := c.exists(cw.state.Name(), cw.state.QType(), cw.do, cw.cd); i1 != nil { + if i1 := c.exists(cw.state.Name(), cw.state.QType(), cw.state.QClass(), cw.do, cw.cd); i1 != nil { i1.Reset(now, i.Hits()) } } @@ -153,7 +153,7 @@ func (c *Cache) verifyWithTimeout(ctx context.Context, state request.Request, w if !cw.refreshed { return false, 0, nil } - fresh := c.exists(state.Name(), state.QType(), state.Do(), state.Req.CheckingDisabled) + fresh := c.exists(state.Name(), state.QType(), state.QClass(), state.Do(), state.Req.CheckingDisabled) if fresh == nil { // Should not happen: refreshed=true means the upstream response was cacheable. return true, res.code, res.err @@ -186,7 +186,7 @@ func (c *Cache) Name() string { return "cache" } // getIfNotStale returns an item if it exists in the cache and has not expired. func (c *Cache) getIfNotStale(now time.Time, state request.Request, server string) *item { - k := hash(state.Name(), state.QType(), state.Do(), state.Req.CheckingDisabled) + k := hash(state.Name(), state.QType(), state.QClass(), state.Do(), state.Req.CheckingDisabled) cacheRequests.WithLabelValues(server, c.zonesMetricLabel, c.viewMetricLabel).Inc() if i, ok := c.ncache.Get(k); ok { @@ -219,8 +219,8 @@ func (c *Cache) getIfNotStale(now time.Time, state request.Request, server strin } // exists unconditionally returns an item if it exists in the cache. -func (c *Cache) exists(name string, qtype uint16, do, cd bool) *item { - k := hash(name, qtype, do, cd) +func (c *Cache) exists(name string, qtype, qclass uint16, do, cd bool) *item { + k := hash(name, qtype, qclass, do, cd) if i, ok := c.ncache.Get(k); ok { return i } diff --git a/plugin/cache/item.go b/plugin/cache/item.go index f1aaeb910..1aa16a5df 100644 --- a/plugin/cache/item.go +++ b/plugin/cache/item.go @@ -14,6 +14,7 @@ import ( type item struct { Name string QType uint16 + QClass uint16 Rcode int AuthenticatedData bool RecursionAvailable bool @@ -41,6 +42,7 @@ func newItem(m *dns.Msg, now time.Time, d time.Duration) *item { if len(m.Question) != 0 { i.Name = m.Question[0].Name i.QType = m.Question[0].Qtype + i.QClass = m.Question[0].Qclass } i.Rcode = m.Rcode i.AuthenticatedData = m.AuthenticatedData @@ -105,7 +107,7 @@ func (i *item) ttl(now time.Time) int { } func (i *item) matches(state request.Request) bool { - if state.QType() == i.QType && strings.EqualFold(state.QName(), i.Name) { + if state.QType() == i.QType && state.QClass() == i.QClass && strings.EqualFold(state.QName(), i.Name) { return true } return false diff --git a/plugin/cache/qclass_test.go b/plugin/cache/qclass_test.go new file mode 100644 index 000000000..71db63021 --- /dev/null +++ b/plugin/cache/qclass_test.go @@ -0,0 +1,63 @@ +package cache + +import ( + "context" + "slices" + "testing" + + "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/pkg/dnstest" + "github.com/coredns/coredns/plugin/test" + + "github.com/miekg/dns" +) + +func TestCacheSeparatesQuestionClasses(t *testing.T) { + const qname = "victim.cache-class.test." + + classes := []uint16{} + c := New() + c.Next = plugin.HandlerFunc(func(_ context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { + qclass := r.Question[0].Qclass + classes = append(classes, qclass) + + m := new(dns.Msg) + m.SetReply(r) + if qclass == dns.ClassCHAOS { + m.Rcode = dns.RcodeNameError + m.Ns = []dns.RR{test.SOA("cache-class.test. 60 CH SOA ns.cache-class.test. hostmaster.cache-class.test. 1 60 60 60 60")} + } else { + m.Answer = []dns.RR{test.A(qname + " 60 IN A 192.0.2.1")} + } + + if err := w.WriteMsg(m); err != nil { + return dns.RcodeServerFailure, err + } + return m.Rcode, nil + }) + + query := func(qclass uint16) *dns.Msg { + t.Helper() + req := new(dns.Msg) + req.SetQuestion(qname, dns.TypeA) + req.Question[0].Qclass = qclass + rec := dnstest.NewRecorder(&test.ResponseWriter{}) + if _, err := c.ServeDNS(context.Background(), rec, req); err != nil { + t.Fatalf("ServeDNS for class %d failed: %v", qclass, err) + } + return rec.Msg + } + + if got := query(dns.ClassCHAOS).Rcode; got != dns.RcodeNameError { + t.Fatalf("CHAOS query rcode = %s, want NXDOMAIN", dns.RcodeToString[got]) + } + + inet := query(dns.ClassINET) + if inet.Rcode != dns.RcodeSuccess || len(inet.Answer) != 1 { + t.Fatalf("IN query returned rcode %s with %d answers, want NOERROR with one answer", dns.RcodeToString[inet.Rcode], len(inet.Answer)) + } + + if want := []uint16{dns.ClassCHAOS, dns.ClassINET}; !slices.Equal(classes, want) { + t.Fatalf("upstream query classes = %v, want %v", classes, want) + } +} diff --git a/request/request.go b/request/request.go index 9188888f5..d11cb0f68 100644 --- a/request/request.go +++ b/request/request.go @@ -340,8 +340,8 @@ func (r *Request) Clear() { r.do = false } -// Match checks if the reply matches the qname and qtype from the request, it returns -// false when they don't match. +// Match checks if the reply matches the qname, qtype and qclass from the request. +// It returns false when they don't match. func (r *Request) Match(reply *dns.Msg) bool { if len(reply.Question) != 1 { return false @@ -359,5 +359,9 @@ func (r *Request) Match(reply *dns.Msg) bool { return false } + if reply.Question[0].Qclass != r.QClass() { + return false + } + return true } diff --git a/request/request_test.go b/request/request_test.go index 7636b56ec..240f02c31 100644 --- a/request/request_test.go +++ b/request/request_test.go @@ -310,6 +310,11 @@ func TestRequestMatch(t *testing.T) { t.Errorf("Failed to match %s %d, got %t, expected %t", "example.com.", dns.TypeA, b, true) } + reply.Question[0].Qclass = dns.ClassCHAOS + if b := st.Match(reply); b { + t.Errorf("Matched response class %d with request class %d", reply.Question[0].Qclass, st.QClass()) + } + reply.SetQuestion("example.org.", dns.TypeA) if b := st.Match(reply); b { t.Errorf("Failed to match %s %d, got %t, expected %t", "example.org.", dns.TypeA, b, false)