net/dns/resolver: forward SERVFAIL responses over PeerDNS

As per the docstring, (*forwarder).forwardWithDestChan should either send to responseChan
and returns nil, or returns a non-nil error (without sending to the channel).
However, this does not hold when all upstream DNS servers replied with an error.

We've been handling this special error path in (*Resolver).Query but not in (*Resolver).HandlePeerDNSQuery.
As a result, SERVFAIL responses from upstream servers were being converted into HTTP 503 responses,
instead of being properly forwarded as SERVFAIL within a successful HTTP response, as per RFC 8484, section 4.2.1:
A successful HTTP response with a 2xx status code (see Section 6.3 of [RFC7231]) is used for any valid DNS response,
regardless of the DNS response code. For example, a successful 2xx HTTP status code is used even with a DNS message
whose DNS response code indicates failure, such as SERVFAIL or NXDOMAIN.

In this PR we fix (*forwarder).forwardWithDestChan to no longer return an error when it sends a response to responseChan,
and remove the special handling in (*Resolver).Query, as it is no longer necessary.

Updates #13571

Signed-off-by: Nick Hill <mykola.khyl@gmail.com>
This commit is contained in:
Nick Hill 2024-10-04 12:23:34 -05:00 committed by Nick Khyl
parent f9949cde8b
commit 17335d2104
2 changed files with 2 additions and 9 deletions

View File

@ -1053,6 +1053,7 @@ func (f *forwarder) forwardWithDestChan(ctx context.Context, query packet, respo
if verboseDNSForward() {
f.logf("forwarder response(%d, %v, %d) = %d, %v", fq.txid, typ, len(domain), len(res.bs), firstErr)
}
return nil
}
}
return firstErr

View File

@ -321,15 +321,7 @@ func (r *Resolver) Query(ctx context.Context, bs []byte, family string, from net
defer cancel()
err = r.forwarder.forwardWithDestChan(ctx, packet{bs, family, from}, responses)
if err != nil {
select {
// Best effort: use any error response sent by forwardWithDestChan.
// This is present in some errors paths, such as when all upstream
// DNS servers replied with an error.
case resp := <-responses:
return resp.bs, err
default:
return nil, err
}
return nil, err
}
return (<-responses).bs, nil
}