Coalesce identical misses, refuse non-QUERY opcodes, refresh hot entries early, check the answered question

Concurrent cache misses for one name each went upstream. After a restart, a
cache flush or a cluster-object change that is a burst of identical queries
exactly when the cache is empty. Identical exchanges (same name, type, class,
DO bit, pinned upstream set and forwarded client subnet) now share one: the
first query runs it on its own goroutine and context, the rest wait and get
private copies, and the leader alone writes the cache. If the leader's client
leaves, waiters take over instead of failing with its cancellation. Shared
answers are counted in fancydns_upstream_coalesced_total.

validate() accepted OpcodeUpdate, so a dynamic update was forwarded upstream as
an ordinary lookup, and every other opcode got FORMERR. A recursive resolver
implements QUERY only: anything else is now NOTIMP (RFC 1035 4.1.1), checked
before the question count since STATUS carries no question.

Prefetch only fired when a stale entry was served, so the first client after a
hot name expired got a TTL-1 stale answer. A fresh entry is now refreshed in
the background when it has been served at least twice and under a tenth of its
lifetime remains; the hit counter restarts with each stored entry, so a name
nobody comes back for stops being refreshed. Counted in
fancydns_cache_prefetch_total.

miekg/dns matches only the message ID of an upstream reply, so a forged reply
with the right ID for a different question was accepted and cached under the
name that was asked. exchangeOne now requires the reply to carry the question
that was sent (an empty section only for FORMERR, SERVFAIL, NOTIMP and
REFUSED). A mismatch fails the attempt without counting against the upstream,
so a spoofer cannot trip the circuit breaker.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

internal/cache/manager.go

@@ -9,6 +9,8 @@ import (
 	"time"
 
 	"github.com/miekg/dns"
+
+	"git.k8s-git-k6n.net/mats/fancy-dns/internal/metrics"
 )
 
 var (
@@ -153,8 +155,11 @@ func (m *Manager) Get(k Key) Result {
 	}
 	if kind == Stale {
 		res.TTL = 0
-		if p := m.prefetchFn.Load(); p != nil && m.opt.Prefetch && m.lru.TryPrefetch(k) {
+	}
+	if m.opt.Prefetch && (kind == Stale || m.nearExpiry(e, now)) {
+		if p := m.prefetchFn.Load(); p != nil && m.lru.TryPrefetch(k) {
 			fn := *p
+			metrics.RecordPrefetch()
 			go func() {
 				defer m.lru.ReleasePrefetch(k)
 				fn(k)
@@ -164,6 +169,29 @@ func (m *Manager) Get(k Key) Result {
 	return res
 }
 
+// Prefetch thresholds. An entry is refreshed ahead of its expiry when it is
+// popular (it has been served at least prefetchMinHits times since it was
+// stored) and has under a tenth of its lifetime left, so a hot name is replaced
+// before a client ever sees it expire. The hit counter starts again with each
+// stored entry, so a name that stops being asked for stops being refreshed
+// after one more cycle.
+const (
+	prefetchMinHits     = 2
+	prefetchRemaining   = 10 // refresh when remaining < lifetime/prefetchRemaining
+	prefetchMinLifetime = 10 * time.Second
+)
+
+func (m *Manager) nearExpiry(e *Entry, now time.Time) bool {
+	if e.Neg || e.Hits.Load() < prefetchMinHits {
+		return false
+	}
+	life := e.ExpiresAt.Sub(e.CreatedAt)
+	if life < prefetchMinLifetime {
+		return false
+	}
+	return e.ExpiresAt.Sub(now) < life/prefetchRemaining
+}
+
 // unpack rebuilds a private message from stored wire bytes and ages the TTLs
 // down to what is left of the entry's lifetime.
 func (e *Entry) unpack(k Key, now time.Time) (*dns.Msg, error) {

internal/engine/coalesce_test.go

@@ -0,0 +1,217 @@
+package engine
+
+import (
+	"context"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/miekg/dns"
+
+	"git.k8s-git-k6n.net/mats/fancy-dns/internal/config"
+)
+
+func waitInflight(t *testing.T, h *harness, n int64) {
+	t.Helper()
+	deadline := time.Now().Add(5 * time.Second)
+	for h.eng.Inflight() < n {
+		if time.Now().After(deadline) {
+			t.Fatalf("only %d of %d queries reached the engine", h.eng.Inflight(), n)
+		}
+		time.Sleep(2 * time.Millisecond)
+	}
+}
+
+func TestIdenticalMissesShareOneUpstreamExchange(t *testing.T) {
+	h := newHarness(t, hopt{})
+	release := h.up.hold()
+	const n = 25
+	results := make([]Response, n)
+	var wg sync.WaitGroup
+	for i := 0; i < n; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			results[i] = h.ask("burst.test.", dns.TypeA)
+		}()
+	}
+	h.up.waitArrived(t)
+	waitInflight(t, h, n)
+	release()
+	wg.Wait()
+
+	if got := h.up.hitCount("burst.test."); got != 1 {
+		t.Fatalf("upstream saw %d queries for %d identical misses, want 1", got, n)
+	}
+	seen := map[*dns.Msg]bool{}
+	for i, r := range results {
+		requireRcode(t, r, dns.RcodeSuccess)
+		if len(r.Msg.Answer) != 1 {
+			t.Fatalf("client %d got %v", i, r.Msg)
+		}
+		if seen[r.Msg] {
+			t.Fatal("two clients were handed the same message; one's edits would reach the other")
+		}
+		seen[r.Msg] = true
+	}
+	// The answer was cached once, by the leader.
+	h.ask("burst.test.", dns.TypeA)
+	if got := h.up.hitCount("burst.test."); got != 1 {
+		t.Fatalf("a later query went upstream again: %d", got)
+	}
+}
+
+func TestDifferentQuestionsDoNotShare(t *testing.T) {
+	h := newHarness(t, hopt{})
+	release := h.up.hold()
+	var wg sync.WaitGroup
+	for _, qt := range []uint16{dns.TypeA, dns.TypeAAAA} {
+		for _, name := range []string{"one.test.", "two.test."} {
+			wg.Add(1)
+			go func() { defer wg.Done(); h.ask(name, qt) }()
+		}
+	}
+	waitInflight(t, h, 4)
+	release()
+	wg.Wait()
+	if got := h.up.totalHits(); got != 4 {
+		t.Fatalf("upstream saw %d queries, want 4 (distinct names and types must not share)", got)
+	}
+}
+
+// TestWaitersSurviveTheLeaderLeaving: the client whose query started the
+// exchange hanging up says nothing about the upstream, so the others must not
+// be failed with its cancellation.
+func TestWaitersSurviveTheLeaderLeaving(t *testing.T) {
+	h := newHarness(t, hopt{})
+	release := h.up.hold()
+	leaderCtx, cancelLeader := context.WithCancel(context.Background())
+	done := make(chan Response, 1)
+	go func() { done <- h.eng.Handle(leaderCtx, h.request("leave.test.", dns.TypeA)) }()
+	h.up.waitArrived(t)
+
+	var follower atomic.Pointer[Response]
+	var wg sync.WaitGroup
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		r := h.ask("leave.test.", dns.TypeA)
+		follower.Store(&r)
+	}()
+	waitInflight(t, h, 2)
+	cancelLeader()
+	<-done
+	release()
+	wg.Wait()
+	requireRcode(t, *follower.Load(), dns.RcodeSuccess)
+}
+
+func TestFlightGroupCopiesAndIsolatesKeys(t *testing.T) {
+	var g flightGroup
+	var calls atomic.Int32
+	gate := make(chan struct{})
+	mk := func() (*dns.Msg, string, error) {
+		calls.Add(1)
+		<-gate
+		m := new(dns.Msg)
+		m.SetQuestion("x.test.", dns.TypeA)
+		return m, "up", nil
+	}
+	k := flightKey{name: "x.test.", qtype: dns.TypeA}
+	msgs := make(chan *dns.Msg, 3)
+	for i := 0; i < 3; i++ {
+		go func() {
+			m, _, _, _ := g.do(context.Background(), k, mk)
+			msgs <- m
+		}()
+	}
+	time.Sleep(50 * time.Millisecond)
+	close(gate)
+	a, b, c := <-msgs, <-msgs, <-msgs
+	if calls.Load() != 1 {
+		t.Fatalf("fn ran %d times", calls.Load())
+	}
+	if a == b || b == c || a == c {
+		t.Fatal("callers share a message")
+	}
+	// A different key (another subnet, say) is its own exchange.
+	k2 := k
+	k2.ecs = "192.0.2.0/24/0"
+	if _, _, shared, _ := g.do(context.Background(), k2, func() (*dns.Msg, string, error) { return new(dns.Msg), "up", nil }); shared {
+		t.Fatal("a different ECS shared an exchange")
+	}
+}
+
+func TestNonQueryOpcodesAreNotImplemented(t *testing.T) {
+	h := newHarness(t, hopt{})
+	for name, mut := range map[string]func(*dns.Msg){
+		"update": func(m *dns.Msg) { m.Opcode = dns.OpcodeUpdate },
+		"notify": func(m *dns.Msg) { m.Opcode = dns.OpcodeNotify },
+		"status": func(m *dns.Msg) { m.Opcode = dns.OpcodeStatus; m.Question = nil },
+	} {
+		t.Run(name, func(t *testing.T) {
+			req := h.request("zone.test.", dns.TypeSOA)
+			mut(req.Msg)
+			res := h.askWith(req)
+			requireRcode(t, res, dns.RcodeNotImplemented)
+			if n := h.up.totalHits(); n != 0 {
+				t.Fatalf("%d queries reached the upstream", n)
+			}
+		})
+	}
+}
+
+// TestPopularEntryIsRefreshedBeforeItExpires: a name that is being asked for
+// is replaced in the cache before its TTL runs out, so no client is the one
+// that finds it expired.
+func TestPopularEntryIsRefreshedBeforeItExpires(t *testing.T) {
+	h := newHarness(t, hopt{
+		clock:      true,
+		serveStale: time.Minute,
+		cacheCfg:   func(c *config.Cache) { c.Prefetch = true },
+	})
+	h.ask("hot.test.", dns.TypeA) // miss, stored with a 60s TTL
+	h.ask("hot.test.", dns.TypeA)
+	h.ask("hot.test.", dns.TypeA)
+	if got := h.up.hitCount("hot.test."); got != 1 {
+		t.Fatalf("setup: %d upstream queries", got)
+	}
+
+	h.clock.Advance(30 * time.Second) // half way: nothing yet
+	h.ask("hot.test.", dns.TypeA)
+	time.Sleep(50 * time.Millisecond)
+	if got := h.up.hitCount("hot.test."); got != 1 {
+		t.Fatalf("refreshed too early: %d upstream queries", got)
+	}
+
+	h.clock.Advance(25 * time.Second) // 5s left of 60: under a tenth
+	res := h.ask("hot.test.", dns.TypeA)
+	if !res.CacheHit || res.CacheKind != "hit" {
+		t.Fatalf("the query that triggers the refresh must still be served fresh: %+v", res)
+	}
+	deadline := time.Now().Add(5 * time.Second)
+	for h.up.hitCount("hot.test.") < 2 {
+		if time.Now().After(deadline) {
+			t.Fatal("the entry was not refreshed ahead of its expiry")
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+}
+
+// TestUnpopularEntryIsNotRefreshed: an entry nobody came back for is allowed
+// to expire; refreshing everything would turn the cache into a poller.
+func TestUnpopularEntryIsNotRefreshed(t *testing.T) {
+	h := newHarness(t, hopt{
+		clock:      true,
+		serveStale: time.Minute,
+		cacheCfg:   func(c *config.Cache) { c.Prefetch = true },
+	})
+	h.ask("once.test.", dns.TypeA)
+	h.clock.Advance(57 * time.Second)
+	h.ask("once.test.", dns.TypeA) // a single hit, near expiry
+	time.Sleep(100 * time.Millisecond)
+	if got := h.up.hitCount("once.test."); got != 1 {
+		t.Fatalf("a once-asked name was refreshed: %d upstream queries", got)
+	}
+}

internal/engine/engine.go

@@ -130,6 +130,8 @@ type Engine struct {
 
 	// inflight is the current in-flight count for the gauge.
 	inflight atomic.Int64
+	// flights coalesces identical concurrent upstream exchanges.
+	flights flightGroup
 	// dropped counts queries rejected by the concurrency cap.
 	dropped atomic.Uint64
 
@@ -283,6 +285,9 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response {
 	// else looks at it.
 	if err := validate(q); err != nil {
 		res.Msg = formErr(q)
+		if err == errNotImplemented {
+			res.Msg = notImpl(q)
+		}
 		res.Action = qlog.ActionError
 		e.finish(req, res, start, decision)
 		return res
@@ -433,7 +438,14 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response {
 	}
 	fwd := e.prepareUpstream(q, req)
 	upStart := time.Now()
-	up, addr, err := pool.Exchange(ctx, fwd, decision.Upstreams, time.Duration(e.upstreamTimeout.Load()))
+	// Identical misses in flight share one upstream exchange. The leader does
+	// the work (and the cache write below, via the shared path); the rest wait.
+	up, addr, shared, err := e.flights.do(ctx, upstreamFlightKey(fwd, decision.Upstreams), func() (*dns.Msg, string, error) {
+		return pool.Exchange(ctx, fwd, decision.Upstreams, time.Duration(e.upstreamTimeout.Load()))
+	})
+	if shared {
+		metrics.RecordCoalesced()
+	}
 	res.Duration = time.Since(start)
 	if err != nil || up == nil {
 		metrics.RecordUpstreamError()
@@ -442,7 +454,9 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response {
 		e.finish(req, res, start, decision)
 		return res
 	}
-	metrics.RecordUpstreamLatency(addr, time.Since(upStart))
+	if !shared {
+		metrics.RecordUpstreamLatency(addr, time.Since(upStart))
+	}
 	// finalize is what makes the answer safe to put on the wire: it echoes the
 	// client's question and id verbatim and truncates to the client's
 	// advertised buffer. Without it an uncached multi-kilobyte answer left
@@ -464,7 +478,7 @@ func (e *Engine) Handle(ctx context.Context, req Request) Response {
 		key := cache.QueryKey(qname, qtype, q.Question[0].Qclass, cache.Do(q), 0)
 		// Cache the untruncated upstream answer so a small client later does
 		// not poison the entry with a truncated copy.
-		if !up.Truncated {
+		if !up.Truncated && !shared {
 			e.cache.PutResponse(key, up, addr)
 		}
 	}
@@ -912,10 +926,14 @@ func validate(q *dns.Msg) error {
 	if q.Response {
 		return errMalformed
 	}
-	if len(q.Question) != 1 {
-		return errMalformed
+	// A recursive resolver implements QUERY and nothing else. UPDATE and
+	// NOTIFY are for authoritative servers: forwarding them upstream as lookups
+	// is wrong, and so is answering FORMERR (RFC 1035 §4.1.1: NOTIMP).
+	// Checked before the question count: STATUS carries no question.
+	if q.Opcode != dns.OpcodeQuery {
+		return errNotImplemented
 	}
-	if q.Opcode != dns.OpcodeQuery && q.Opcode != dns.OpcodeUpdate {
+	if len(q.Question) != 1 {
 		return errMalformed
 	}
 	name := q.Question[0].Name
@@ -925,7 +943,10 @@ func validate(q *dns.Msg) error {
 	return nil
 }
 
-var errMalformed = errString("malformed query")
+var (
+	errMalformed      = errString("malformed query")
+	errNotImplemented = errString("opcode not implemented")
+)
 
 type errString string
 
@@ -937,6 +958,12 @@ func formErr(q *dns.Msg) *dns.Msg {
 	return m
 }
 
+func notImpl(q *dns.Msg) *dns.Msg {
+	m := new(dns.Msg)
+	m.SetRcode(q, dns.RcodeNotImplemented)
+	return m
+}
+
 func servFail(q *dns.Msg, reason string) *dns.Msg {
 	m := new(dns.Msg)
 	m.SetRcode(q, dns.RcodeServerFailure)
@@ -1200,3 +1227,22 @@ func (e *Engine) SetFlusher(fn func()) {
 	}
 	e.flusher.Store(&fn)
 }
+
+// upstreamFlightKey names the exchange fwd would make, so two queries share an
+// upstream round trip only when the upstream would give them the same answer.
+func upstreamFlightKey(fwd *dns.Msg, ups []string) flightKey {
+	qn := fwd.Question[0]
+	k := flightKey{
+		name: strings.ToLower(qn.Name), qtype: qn.Qtype, qclass: qn.Qclass,
+		ups: strings.Join(ups, ","),
+	}
+	if o := fwd.IsEdns0(); o != nil {
+		k.do = o.Do()
+		for _, opt := range o.Option {
+			if sub, ok := opt.(*dns.EDNS0_SUBNET); ok {
+				k.ecs = sub.String()
+			}
+		}
+	}
+	return k
+}

internal/engine/flight.go

@@ -0,0 +1,83 @@
+package engine
+
+import (
+	"context"
+	"sync"
+
+	"github.com/miekg/dns"
+)
+
+// flightKey identifies exchanges that would produce the same upstream answer:
+// the cache key, the upstream set an ACL group pinned, and the client subnet
+// that was forwarded (an ECS-aware upstream answers by subnet).
+type flightKey struct {
+	name, ups, ecs string
+	qtype, qclass  uint16
+	do             bool
+}
+
+// flight is one upstream exchange other queries may wait on.
+type flight struct {
+	done chan struct{}
+	up   *dns.Msg
+	addr string
+	err  error
+	// leaderGaveUp is true when the leader's own context ended the exchange.
+	// That says nothing about the upstream, so waiters try again themselves.
+	leaderGaveUp bool
+}
+
+// flightGroup coalesces identical concurrent upstream exchanges, so a burst of
+// cache misses for one name (a restart, a cache flush, a viral link) costs the
+// upstream one query rather than one per client.
+//
+// The leader runs the exchange on its own goroutine and its own context;
+// there is no detached work, so a shutdown that waits for in-flight queries
+// still waits for every exchange.
+type flightGroup struct {
+	mu sync.Mutex
+	m  map[flightKey]*flight
+}
+
+// do runs fn once per key among concurrent callers. shared is true for a
+// caller that received another caller's answer; its message is a private copy.
+func (g *flightGroup) do(ctx context.Context, key flightKey, fn func() (*dns.Msg, string, error)) (up *dns.Msg, addr string, shared bool, err error) {
+	for {
+		g.mu.Lock()
+		if g.m == nil {
+			g.m = make(map[flightKey]*flight)
+		}
+		if f, ok := g.m[key]; ok {
+			g.mu.Unlock()
+			select {
+			case <-f.done:
+			case <-ctx.Done():
+				return nil, "", true, ctx.Err()
+			}
+			if f.leaderGaveUp {
+				continue // the leader's client left; take over
+			}
+			if f.err != nil {
+				return nil, f.addr, true, f.err
+			}
+			return f.up.Copy(), f.addr, true, nil
+		}
+		f := &flight{done: make(chan struct{})}
+		g.m[key] = f
+		g.mu.Unlock()
+
+		f.up, f.addr, f.err = fn()
+		f.leaderGaveUp = f.err != nil && ctx.Err() != nil
+		g.mu.Lock()
+		delete(g.m, key)
+		g.mu.Unlock()
+		close(f.done)
+		// The leader keeps the original; waiters get copies. f.up is read by
+		// waiters only after done is closed, and the leader's later changes to
+		// its message must not reach them, so hand the leader a copy instead.
+		if f.err == nil && f.up != nil {
+			return f.up.Copy(), f.addr, false, nil
+		}
+		return f.up, f.addr, false, f.err
+	}
+}

internal/metrics/metrics.go

@@ -28,6 +28,8 @@ var (
 	cacheHits      prometheus.Counter
 	cacheMisses    prometheus.Counter
 	cacheStaleHits prometheus.Counter
+	coalesced      prometheus.Counter
+	prefetches     prometheus.Counter
 	upstreamErrors prometheus.Counter
 	dnsErrors      prometheus.Counter
 	dropsTotal     prometheus.Counter
@@ -82,6 +84,14 @@ func initCollectors() {
 			Namespace: Namespace, Name: "cache_stale_hits_total",
 			Help: "Answers served from the serve-stale window.",
 		})
+		coalesced = prometheus.NewCounter(prometheus.CounterOpts{
+			Namespace: Namespace, Name: "upstream_coalesced_total",
+			Help: "Cache misses answered by sharing another query's in-flight upstream exchange.",
+		})
+		prefetches = prometheus.NewCounter(prometheus.CounterOpts{
+			Namespace: Namespace, Name: "cache_prefetch_total",
+			Help: "Background refreshes started for an entry that was stale or close to expiry.",
+		})
 		upstreamErrors = prometheus.NewCounter(prometheus.CounterOpts{
 			Namespace: Namespace, Name: "upstream_errors_total",
 			Help: "Upstream exchange failures.",
@@ -143,7 +153,7 @@ func initCollectors() {
 		})
 
 		Registry.MustRegister(
-			queriesTotal, blockedTotal, cacheHits, cacheMisses, cacheStaleHits,
+			queriesTotal, blockedTotal, cacheHits, cacheMisses, cacheStaleHits, coalesced, prefetches,
 			upstreamErrors, dnsErrors, dropsTotal, refusedTotal, upstreamHealth,
 			concurrentNow, listCount, listBytes, aclGroups, localRecords, up,
 			queryDuration, upstreamDur, parseDuration,
@@ -208,6 +218,12 @@ func RecordMiss() { initCollectors(); cacheMisses.Inc(); misses.Add(1) }
 // RecordStaleHit counts a serve-stale hit.
 func RecordStaleHit() { initCollectors(); cacheStaleHits.Inc(); staleHits.Add(1) }
 
+// RecordCoalesced counts a query that shared an in-flight upstream exchange.
+func RecordCoalesced() { initCollectors(); coalesced.Inc() }
+
+// RecordPrefetch counts a background refresh.
+func RecordPrefetch() { initCollectors(); prefetches.Inc() }
+
 // RecordUpstreamError counts an upstream failure.
 func RecordUpstreamError() { initCollectors(); upstreamErrors.Inc() }
 

internal/resolver/pool.go

@@ -637,6 +637,12 @@ func (p *Pool) exchangeOne(ctx context.Context, cfg config.Upstream, up *Upstrea
 	client := *p.clientFor(up)
 	client.Timeout = timeout
 	resp, rtt, err := exchangeContext(ctx, &client, q.Copy(), up.addr)
+	if err == nil && !answersQuestion(q, resp) {
+		// Not a failure of the upstream: an off-path attacker who guessed the
+		// ID and port can send this, and must not be able to trip the breaker
+		// by doing it. The attempt fails and the next upstream is tried.
+		return nil, up.addr, errQuestionMismatch
+	}
 	if err != nil {
 		// Cancellation by the caller (including losing a parallel race) says
 		// nothing about upstream health. Only an attempt failure counts.
@@ -653,6 +659,9 @@ func (p *Pool) exchangeOne(ctx context.Context, cfg config.Upstream, up *Upstrea
 	// the budget expires, retain the truncated answer for the client to retry.
 	tcpClient := &dns.Client{Net: "tcp", Timeout: timeout}
 	resp2, rtt2, err2 := exchangeContext(ctx, tcpClient, q.Copy(), up.addr)
+	if err2 == nil && !answersQuestion(q, resp2) {
+		err2 = errQuestionMismatch
+	}
 	if err := ctx.Err(); err != nil {
 		return nil, up.addr, err
 	}
@@ -671,6 +680,30 @@ func (p *Pool) exchangeOne(ctx context.Context, cfg config.Upstream, up *Upstrea
 	return resp2, up.addr, nil
 }
 
+var errQuestionMismatch = errors.New("response does not answer the question that was asked")
+
+// answersQuestion reports whether resp is a reply to q. The DNS client library
+// matches only the message ID, which is 16 bits; without this check anyone who
+// lands the ID and the source port can have any name's answer cached under the
+// name that was asked. A resolver that refuses before it can answer may leave
+// the question out (RFC 1035 §4.1.1), so an empty section is accepted for those
+// error codes and nothing else.
+func answersQuestion(q, resp *dns.Msg) bool {
+	if resp == nil || len(q.Question) != 1 {
+		return false
+	}
+	if len(resp.Question) == 0 {
+		switch resp.Rcode {
+		case dns.RcodeFormatError, dns.RcodeServerFailure, dns.RcodeNotImplemented, dns.RcodeRefused:
+			return true
+		}
+		return false
+	}
+	a, b := q.Question[0], resp.Question[0]
+	return len(resp.Question) == 1 && a.Qtype == b.Qtype && a.Qclass == b.Qclass &&
+		strings.EqualFold(dns.Fqdn(a.Name), dns.Fqdn(b.Name))
+}
+
 // exchangeContext also interrupts active socket I/O on caller cancellation;
 // miekg/dns applies context deadlines but only checks cancellation while dialing.
 func exchangeContext(ctx context.Context, client *dns.Client, q *dns.Msg, addr string) (*dns.Msg, time.Duration, error) {

internal/resolver/question_test.go

@@ -0,0 +1,60 @@
+package resolver
+
+import (
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/miekg/dns"
+)
+
+// TestAnswerForAnotherQuestionIsRejected pins the check the DNS library does
+// not make: it matches the message ID and nothing else, so a reply with the
+// right ID for a different question would otherwise be cached under the name
+// that was asked.
+func TestAnswerForAnotherQuestionIsRejected(t *testing.T) {
+	spoof := newFakeUpstream(t, fakeOpt{wrongQuestion: true})
+	p := NewPool(poolCfg(udpServer(spoof.addr)))
+	for i := 0; i < 5; i++ { // more than the breaker threshold
+		resp, _, err := p.Exchange(testCtx(t), query("www.example.test"), nil, time.Second)
+		if !errors.Is(err, errQuestionMismatch) || resp != nil {
+			t.Fatalf("attempt %d: resp=%v err=%v, want the mismatch error", i, resp, err)
+		}
+	}
+	// A forged reply must not be able to take the upstream out of rotation.
+	if up := p.All()[0]; up.BreakerOpen() {
+		t.Fatal("spoofed replies opened the circuit breaker")
+	}
+}
+
+func TestAnswersQuestion(t *testing.T) {
+	q := query("WWW.Example.Test")
+	reply := func(name string, qtype uint16, rcode int, drop bool) *dns.Msg {
+		m := new(dns.Msg)
+		m.SetReply(q)
+		m.Question[0].Name, m.Question[0].Qtype = name, qtype
+		m.Rcode = rcode
+		if drop {
+			m.Question = nil
+		}
+		return m
+	}
+	for name, tc := range map[string]struct {
+		resp *dns.Msg
+		want bool
+	}{
+		"same":                  {reply("www.example.test.", dns.TypeA, 0, false), true},
+		"case differs":          {reply("WWW.EXAMPLE.TEST.", dns.TypeA, 0, false), true},
+		"other name":            {reply("evil.example.", dns.TypeA, 0, false), false},
+		"other type":            {reply("www.example.test.", dns.TypeAAAA, 0, false), false},
+		"formerr, no question":  {reply("", 0, dns.RcodeFormatError, true), true},
+		"refused, no question":  {reply("", 0, dns.RcodeRefused, true), true},
+		"success, no question":  {reply("", 0, dns.RcodeSuccess, true), false},
+		"nxdomain, no question": {reply("", 0, dns.RcodeNameError, true), false},
+		"nil":                   {nil, false},
+	} {
+		if got := answersQuestion(q, tc.resp); got != tc.want {
+			t.Errorf("%s: got %v, want %v", name, got, tc.want)
+		}
+	}
+}

internal/resolver/resolver_test.go

@@ -21,6 +21,9 @@ type fakeOpt struct {
 	ip          string        // A record handed out in the answer
 	delay       time.Duration // how long the handler pretends to think
 	truncateUDP bool          // answer TC=1 over UDP, forcing the client onto TCP
+	// wrongQuestion answers with the right ID but another name's question and
+	// records: what an off-path spoofer who guessed the ID would send.
+	wrongQuestion bool
 }
 
 // fakeUpstream is a real dns.Server on 127.0.0.1, listening for UDP and TCP on
@@ -125,6 +128,9 @@ func (f *fakeUpstream) handle(w dns.ResponseWriter, q *dns.Msg) {
 	m := new(dns.Msg)
 	m.SetReply(q)
 	m.Truncated = !tcp && f.opt.truncateUDP
+	if f.opt.wrongQuestion {
+		m.Question[0].Name = "evil.example."
+	}
 	if q.Question[0].Qtype == dns.TypeA && !m.Truncated {
 		m.Answer = []dns.RR{&dns.A{
 			Hdr: dns.RR_Header{Name: q.Question[0].Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},