cmd/cli: discard upstream answers whose question mismatches the request

Defense in depth against cache poisoning: a compromised or misbehaving
upstream can return an answer for a different name than was asked (e.g.
records for attacker.example in response to a query for victim.example).
Such an answer would be cached under the legitimate request key and
served to subsequent queries.

Validate that the upstream answer echoes the request's question
(case-insensitive name plus Qtype/Qclass, per RFC 1035 section 4.1.2)
before serving or caching it. A mismatch is logged at debug level and
the upstream is skipped, failing safe to the next upstream or SERVFAIL.

Refs github.com/Control-D-Inc/ctrld/issues/322

cmd/cli/dns_proxy.go

@@ -974,6 +974,17 @@ func (p *prog) processUpstream(ctx context.Context, req *proxyRequest, upstream
 	}
 
 	ctrld.Log(ctx, p.Debug(), "Upstream query successful")
+	// Reject an answer whose question does not match the request before it
+	// can be served or cached. A mismatched question means the upstream
+	// answered a different name/type than asked; caching it would poison
+	// the shared cache with wrong-domain records for the requested name.
+	// See github.com/Control-D-Inc/ctrld/issues/322.
+	if !sameQuestion(req.msg, answer) {
+		ctrld.Log(ctx, p.Debug(),
+			"Discarding answer from %s: question mismatch (asked %q, got %q)",
+			upstream, questionString(req.msg), questionString(answer))
+		return nil
+	}
 	if p.shouldContinueWithNextUpstream(ctx, req, answer, upstream, lastUpstream) {
 		return nil
 	}
@@ -1132,6 +1143,33 @@ func requestID() string {
 	return hex.EncodeToString(b)
 }
 
+// sameQuestion reports whether the upstream answer echoes the request's
+// question. A well-behaved resolver always copies the question section from
+// the query (RFC 1035 section 4.1.2); names are compared case-insensitively
+// because DNS names are case-insensitive. A mismatch means the upstream
+// answered a different name/type than asked - malformed or malicious - and the
+// answer must not be served or cached, or it would poison the shared cache with
+// wrong-domain records. See github.com/Control-D-Inc/ctrld/issues/322.
+func sameQuestion(req, answer *dns.Msg) bool {
+	if req == nil || answer == nil {
+		return false
+	}
+	if len(req.Question) == 0 || len(answer.Question) == 0 {
+		return false
+	}
+	rq, aq := req.Question[0], answer.Question[0]
+	return rq.Qtype == aq.Qtype && rq.Qclass == aq.Qclass && strings.EqualFold(rq.Name, aq.Name)
+}
+
+// questionString renders a message's first question as "name/type" for logging.
+func questionString(msg *dns.Msg) string {
+	if msg == nil || len(msg.Question) == 0 {
+		return "<none>"
+	}
+	q := msg.Question[0]
+	return q.Name + "/" + dns.TypeToString[q.Qtype]
+}
+
 // setCachedAnswerTTL updates the TTL of each DNS record in the provided message based on the current and expiration times.
 func setCachedAnswerTTL(answer *dns.Msg, now, expiredTime time.Time) {
 	ttlSecs := expiredTime.Sub(now).Seconds()

cmd/cli/dns_proxy_test.go

@@ -811,6 +811,36 @@ func Test_prog_queryFromSelf(t *testing.T) {
 	})
 }
 
+func Test_sameQuestion(t *testing.T) {
+	mk := func(name string, qtype uint16) *dns.Msg {
+		m := new(dns.Msg)
+		m.SetQuestion(name, qtype)
+		return m
+	}
+	tests := []struct {
+		name   string
+		req    *dns.Msg
+		answer *dns.Msg
+		want   bool
+	}{
+		{"identical", mk("example.com.", dns.TypeA), mk("example.com.", dns.TypeA), true},
+		{"case insensitive", mk("Example.COM.", dns.TypeA), mk("example.com.", dns.TypeA), true},
+		{"different name", mk("victim.example.", dns.TypeA), mk("attacker.example.", dns.TypeA), false},
+		{"different type", mk("example.com.", dns.TypeA), mk("example.com.", dns.TypeAAAA), false},
+		{"nil req", nil, mk("example.com.", dns.TypeA), false},
+		{"nil answer", mk("example.com.", dns.TypeA), nil, false},
+		{"empty answer question", mk("example.com.", dns.TypeA), new(dns.Msg), false},
+	}
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			if got := sameQuestion(tc.req, tc.answer); got != tc.want {
+				t.Errorf("sameQuestion() = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
 // newTestProg creates a properly initialized *prog for testing.
 func newTestProg(t *testing.T) *prog {
 	p := &prog{cfg: testhelper.SampleConfig(t)}

doq_test.go

@@ -748,7 +748,7 @@ func TestDoQResolve_PreservesRcode(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	pool := newDOQConnPool(uc, []string{"127.0.0.1"})
+	pool := newDOQConnPool(ctx, uc, []string{"127.0.0.1"})
 	t.Cleanup(pool.CloseIdleConnections)
 
 	msg := new(dns.Msg)
@@ -797,7 +797,7 @@ func TestDoQResolve_PreservesWrongQuestion(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
-	pool := newDOQConnPool(uc, []string{"127.0.0.1"})
+	pool := newDOQConnPool(ctx, uc, []string{"127.0.0.1"})
 	t.Cleanup(pool.CloseIdleConnections)
 
 	msg := new(dns.Msg)