net: DNSError.Temporary() is false for DNS server temporary failures #8434

Closed
siebenmann opened this Issue Jul 28, 2014 · 5 comments

5 participants

@siebenmann
version: current Mercurial tip ('devel +ba125b11fd2d')

If you make a DNS query such as net.LookupMX() and it returns what is
actually a net.DNSError result, that result's .Temporary() will only be
true if the error was a timeout talking to your resolving DNS server. It
will not be true if your DNS server returns a variety of 'cannot resolve
this name at the moment' temporary errors, for example if it can't get
a response from any of the domain's NS servers or they respond with
denials of authority.

Part of the problem is this code and comment in src/pkg/net/dnsclient.go's
answer() function:

        if dns.rcode != dnsRcodeSuccess {
                // None of the error codes make sense
                // for the query we sent.  If we didn't get
                // a name error and we didn't get success,
                // the server is behaving incorrectly.

This is in fact not the case. DNS servers may respond with at least
dnsRcodeServerFailure (aka SERVFAIL) when they can't give you an answer
for your request due to problems with other DNS servers. Such failures
aren't uncommon if, for example, you have an MTA that checks whether
the domains of incoming MAIL FROMs are valid.

Depending on your perspective, this is one of two bugs. Either DNSError's
Temporary() method should document that it is simply whether or not the
request timed out or it should return true for temporary server failures.

(I'm aware that Temporary() is not documented at all today, but this
likely leads people to assume that it does what they expect, namely
returns true if this is a 'temporary' DNS failure. This is certainly
what I assumed until I put it in a program and tested.)

I don't have a simple reproduction that is stable over the long term
because I don't know of any testing DNS domains that are guaranteed to
always return SERVFAIL for some queries. If you have logs from an MTA
handy you can trawl its logs for 'temporary failure to resolve domain
<X>' log messages and plug them into a test program to see this in
action.
@ianlancetaylor

Comment 1:

Labels changed: added repo-main, release-go1.4.

@rsc

Comment 2:

Status changed to Thinking.

@rsc

Comment 3:

Setting Temporary to true for SERVFAIL seems reasonable to me.
@rsc

Comment 4:

Leaving alone for this release.

Labels changed: added release-go1.5, removed release-go1.4.

@bradfitz bradfitz modified the milestone: Go1.5 Dec 16, 2014
@bradfitz bradfitz removed the release-go1.5 label Dec 16, 2014
@rsc rsc removed the repo-main label Apr 14, 2015
@rsc rsc modified the milestone: Go1.6Early, Go1.5 Jul 15, 2015
@gopherbot

CL https://golang.org/cl/14169 mentions this issue.

@bradfitz bradfitz added a commit that closed this issue Sep 4, 2015
@dpiddy dpiddy net: make DNSError.Temporary return true on SERVFAIL
Fixes #8434

Change-Id: I323222b4160f3aba35cac1de7f6df93c524b72ec
Reviewed-on: https://go-review.googlesource.com/14169
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
ced0646
@bradfitz bradfitz closed this in ced0646 Sep 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment