Skip to content

Commit 2e5d478

Browse files
authored
fix(resolver): reply to NOTFQDN queries with a well-formed NXDOMAIN (#2250)
With `fqdnOnly` enabled, a query for a dotless name was answered with a dns.Msg carrying only an Rcode: NewEmptyResponse called neither SetReply nor SetRcode, so the message echoed back neither the request id nor the question section. Conforming clients discard such an answer as an id mismatch and go on waiting for a reply that never comes, so users saw a timeout instead of the NXDOMAIN the documentation promises. Build the answer with NewResponseWithRcode instead, whose SetRcode echoes id and question, and drop NewEmptyResponse: FQDNOnlyResolver was its only caller, and every other resolver already replies via SetReply. The e2e test accepted "any DNS error" as a valid rejection, which is why this went unnoticed; it now asserts that the client actually receives NXDOMAIN with its question echoed back. Claude-Session: https://claude.ai/code/session_01QyEFFhrY3j9QtGzR2wUJr9
1 parent ff2aae4 commit 2e5d478

4 files changed

Lines changed: 21 additions & 19 deletions

File tree

e2e/fqdn_only_test.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,17 @@ var _ = Describe("FQDN only mode", func() {
4141
Expect(err).Should(Succeed())
4242
})
4343

44-
It("should reject non-FQDN queries", func(ctx context.Context) {
44+
It("should reject non-FQDN queries with NXDOMAIN", func(ctx context.Context) {
4545
msg := util.NewMsgWithQuestion("myserver.", A)
46+
47+
// the rejection must be a reply the client accepts: one that echoes the request id
48+
// and question. A bare Rcode-only message is discarded as an id mismatch, leaving
49+
// the client to time out instead of seeing the NXDOMAIN.
4650
resp, err := doDNSRequest(ctx, blocky, msg)
47-
if err != nil {
48-
// Any DNS error (connection refused, id mismatch, etc.) is a valid rejection
49-
return
50-
}
51+
Expect(err).Should(Succeed())
5152
Expect(resp.Rcode).Should(Equal(dns.RcodeNameError))
53+
Expect(resp.Question).Should(Equal(msg.Question))
54+
Expect(resp.Answer).Should(BeEmpty())
5255
})
5356

5457
It("should resolve FQDN queries normally", func(ctx context.Context) {

model/response_helpers.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -42,16 +42,3 @@ func NewResponseWithRcode(request *Request, rcode int, rtype ResponseType, reaso
4242
Reason: reason,
4343
}
4444
}
45-
46-
// NewEmptyResponse creates a response with just the Rcode field set (no SetReply or SetRcode).
47-
// This is used for minimal responses where only the return code matters.
48-
func NewEmptyResponse(request *Request, rcode int, rtype ResponseType, reason string) *Response {
49-
response := new(dns.Msg)
50-
response.Rcode = rcode
51-
52-
return &Response{
53-
Res: response,
54-
RType: rtype,
55-
Reason: reason,
56-
}
57-
}

resolver/fqdn_only_resolver.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func (r *FQDNOnlyResolver) Resolve(ctx context.Context, request *model.Request)
2727
if r.IsEnabled() {
2828
domainFromQuestion := util.ExtractDomain(request.Req.Question[0])
2929
if !strings.Contains(domainFromQuestion, ".") {
30-
return model.NewEmptyResponse(request, dns.RcodeNameError, model.ResponseTypeNOTFQDN, "NOTFQDN"), nil
30+
return model.NewResponseWithRcode(request, dns.RcodeNameError, model.ResponseTypeNOTFQDN, "NOTFQDN"), nil
3131
}
3232
}
3333

resolver/fqdn_only_resolver_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,18 @@ var _ = Describe("FqdnOnlyResolver", func() {
8888
// no call of next resolver
8989
Expect(m.Calls).Should(BeZero())
9090
})
91+
It("Should reply to the request, echoing its id and question", func() {
92+
request := newRequest("example", AAAA)
93+
94+
resp, err := sut.Resolve(ctx, request)
95+
Expect(err).Should(Succeed())
96+
97+
// a reply that carries neither the request id nor the question is rejected by the
98+
// client as an id mismatch, so it sees a timeout instead of the NXDOMAIN
99+
Expect(resp.Res.Response).Should(BeTrue())
100+
Expect(resp.Res.Id).Should(Equal(request.Req.Id))
101+
Expect(resp.Res.Question).Should(Equal(request.Req.Question))
102+
})
91103

92104
Describe("IsEnabled", func() {
93105
It("is true", func() {

0 commit comments

Comments
 (0)