request/cache: bind responses and entries to QCLASS (#8272)

Signed-off-by: houyuwushang <liuluoqianqiu@outlook.com>

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

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()
 }

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).

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 {
+func (c *Cache) exists(name string, qtype, qclass uint16, do, cd bool) *item {
-	k := hash(name, qtype, do, cd)
+	k := hash(name, qtype, qclass, do, cd)
 	if i, ok := c.ncache.Get(k); ok {
 		return i
 	}

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

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)
+	}
+}

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
+// Match checks if the reply matches the qname, qtype and qclass from the request.
-// false when they don't match.
+// 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
 }

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)