git » dnss » commit 786704d

dnsserver: Be more strict in which requests we cache

author Alberto Bertogli
2026-10-23 18:40:13 UTC
committer Alberto Bertogli
2026-10-23 18:54:18 UTC
parent 969c93325eea0d09e09afb4f87f963a1ad110763

dnsserver: Be more strict in which requests we cache

The cache today is designed to only cache QUERY entries, and also only
EDNS version 0 is supported.

But because we don't explicitly limit this, it could end up answering
NOTIFY queries from the cache, or an EDNS version 1 query could get a
NOERROR instead of BADVERS.

This patch fixes that problem by only allowing requests matching our
expected conditions into the cache.

internal/dnsserver/caching_test.go +46 -0
internal/dnsserver/resolver.go +21 -3

diff --git a/internal/dnsserver/caching_test.go b/internal/dnsserver/caching_test.go
index 179290a..9256461 100644
--- a/internal/dnsserver/caching_test.go
+++ b/internal/dnsserver/caching_test.go
@@ -112,6 +112,52 @@ func TestTTL(t *testing.T) {
 	}
 }
 
+// Test that requests we can't answer from the cache bypass it.
+func TestCacheBypass(t *testing.T) {
+	r := testutil.NewTestResolver()
+	c := NewCachingResolver(r)
+	c.Init()
+	resetStats()
+
+	// Populate the cache, so we can check the bypassed requests don't get
+	// the cached answer.
+	queryA(t, c, "test. A 1.2.3.4", "test.", "1.2.3.4")
+
+	multi := newQuery("test.", dns.TypeA)
+	multi.Question = append(multi.Question, multi.Question[0])
+
+	notify := newQuery("test.", dns.TypeA)
+	notify.Opcode = dns.OpcodeNotify
+
+	ednsV1 := newQuery("test.", dns.TypeA)
+	ednsV1.SetEdns0(4096, false)
+	ednsV1.IsEdns0().SetVersion(1)
+
+	tr := trace.New("test", "TestCacheBypass")
+	defer tr.Finish()
+
+	for _, req := range []*dns.Msg{multi, notify, ednsV1} {
+		resetStats()
+		r.LastQuery = nil
+		if _, err := c.Query(req, tr); err != nil {
+			t.Fatalf("query failed: %v", err)
+		}
+		if r.LastQuery != req {
+			t.Errorf("request was not sent to the back resolver: %v", req)
+		}
+		if !statsEquals(1, 0, 0) || stats.cacheBypassed.Value() != 1 {
+			t.Errorf("bad stats: %v, bypassed %v", dumpStats(),
+				stats.cacheBypassed)
+		}
+	}
+
+	// The answers to the bypassed requests should not have been recorded,
+	// and the cache should still have only the original entry.
+	if len(c.answer) != 1 {
+		t.Errorf("expected 1 entry in the cache, got %d", len(c.answer))
+	}
+}
+
 // Test that we use the minimum TTL of all the RRs in the answer.
 func TestMinTTL(t *testing.T) {
 	r := testutil.NewTestResolver()
diff --git a/internal/dnsserver/resolver.go b/internal/dnsserver/resolver.go
index 21ffb1e..eed5726 100644
--- a/internal/dnsserver/resolver.go
+++ b/internal/dnsserver/resolver.go
@@ -242,6 +242,25 @@ func (c *cachingResolver) Maintain() {
 	}
 }
 
+// canUseCache checks if the request can be answered from (and recorded in)
+// the cache, returning an error with the reason if not.
+func canUseCache(r *dns.Msg) error {
+	if len(r.Question) != 1 {
+		// To keep it simple we only cache single-question queries.
+		return fmt.Errorf("multi-question query")
+	} else if r.Opcode != dns.OpcodeQuery {
+		// Other opcodes (like NOTIFY) are not queries, so the cached
+		// answers don't apply to them.
+		return fmt.Errorf("opcode %d != query", r.Opcode)
+	} else if opt := r.IsEdns0(); opt != nil && opt.Version() != 0 {
+		// We only support EDNS version 0 in our replies. Let the upstream
+		// handle other versions (it should reply with BADVERS).
+		return fmt.Errorf("EDNS version %d != 0", opt.Version())
+	}
+
+	return nil
+}
+
 func wantToCache(question dns.Question, reply *dns.Msg) error {
 	if reply.Rcode != dns.RcodeSuccess {
 		return fmt.Errorf("unsuccessful query")
@@ -325,9 +344,8 @@ func copyRRSlice(a []dns.RR) []dns.RR {
 func (c *cachingResolver) Query(r *dns.Msg, tr *trace.Trace) (*dns.Msg, error) {
 	stats.cacheTotal.Add(1)
 
-	// To keep it simple we only cache single-question queries.
-	if len(r.Question) != 1 {
-		tr.Printf("cache bypass: multi-question query")
+	if err := canUseCache(r); err != nil {
+		tr.Printf("cache bypass: %v", err)
 		stats.cacheBypassed.Add(1)
 		return c.back.Query(r, tr)
 	}