Permalink
Browse files

[release-branch.go1.11] net: fail fast for DNS rcode success with no …

…answers of requested type

DNS responses which do not contain answers of the requested type return
errNoSuchHost, the same error as rcode name error. Prior to
golang.org/cl/37879, both cases resulted in no additional name servers
being consulted for the question. That CL changed the behavior for both
cases. Issue #25336 was filed about the rcode name error case and
golang.org/cl/113815 fixed it. This CL fixes the no answers of requested
type case as well.

Updates #27525
Fixes #27537

Change-Id: I52fadedcd195f16adf62646b76bea2ab3b15d117
Reviewed-on: https://go-review.googlesource.com/133675
Run-TryBot: Ian Gudger <igudger@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
(cherry picked from commit 94f48dd)
Reviewed-on: https://go-review.googlesource.com/138175
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
  • Loading branch information...
iangudger authored and bradfitz committed Sep 6, 2018
1 parent e535c71 commit 05a0c7b4c60e240e521b0840c786cacd69221e68
Showing with 158 additions and 100 deletions.
  1. +70 −45 src/net/dnsclient_unix.go
  2. +88 −55 src/net/dnsclient_unix_test.go
@@ -27,6 +27,20 @@ import (
"golang_org/x/net/dns/dnsmessage"
)
var (
errLameReferral = errors.New("lame referral")
errCannotUnmarshalDNSMessage = errors.New("cannot unmarshal DNS message")
errCannotMarshalDNSMessage = errors.New("cannot marshal DNS message")
errServerMisbehaving = errors.New("server misbehaving")
errInvalidDNSResponse = errors.New("invalid DNS response")
errNoAnswerFromDNSServer = errors.New("no answer from DNS server")
// errServerTemporarlyMisbehaving is like errServerMisbehaving, except
// that when it gets translated to a DNSError, the IsTemporary field
// gets set to true.
errServerTemporarlyMisbehaving = errors.New("server misbehaving")
)
func newRequest(q dnsmessage.Question) (id uint16, udpReq, tcpReq []byte, err error) {
id = uint16(rand.Int()) ^ uint16(time.Now().UnixNano())
b := dnsmessage.NewBuilder(make([]byte, 2, 514), dnsmessage.Header{ID: id, RecursionDesired: true})
@@ -105,14 +119,14 @@ func dnsStreamRoundTrip(c Conn, id uint16, query dnsmessage.Question, b []byte)
var p dnsmessage.Parser
h, err := p.Start(b[:n])
if err != nil {
return dnsmessage.Parser{}, dnsmessage.Header{}, errors.New("cannot unmarshal DNS message")
return dnsmessage.Parser{}, dnsmessage.Header{}, errCannotUnmarshalDNSMessage
}
q, err := p.Question()
if err != nil {
return dnsmessage.Parser{}, dnsmessage.Header{}, errors.New("cannot unmarshal DNS message")
return dnsmessage.Parser{}, dnsmessage.Header{}, errCannotUnmarshalDNSMessage
}
if !checkResponse(id, query, h, q) {
return dnsmessage.Parser{}, dnsmessage.Header{}, errors.New("invalid DNS response")
return dnsmessage.Parser{}, dnsmessage.Header{}, errInvalidDNSResponse
}
return p, h, nil
}
@@ -122,7 +136,7 @@ func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Que
q.Class = dnsmessage.ClassINET
id, udpReq, tcpReq, err := newRequest(q)
if err != nil {
return dnsmessage.Parser{}, dnsmessage.Header{}, errors.New("cannot marshal DNS message")
return dnsmessage.Parser{}, dnsmessage.Header{}, errCannotMarshalDNSMessage
}
for _, network := range []string{"udp", "tcp"} {
ctx, cancel := context.WithDeadline(ctx, time.Now().Add(timeout))
@@ -147,31 +161,31 @@ func (r *Resolver) exchange(ctx context.Context, server string, q dnsmessage.Que
return dnsmessage.Parser{}, dnsmessage.Header{}, mapErr(err)
}
if err := p.SkipQuestion(); err != dnsmessage.ErrSectionDone {
return dnsmessage.Parser{}, dnsmessage.Header{}, errors.New("invalid DNS response")
return dnsmessage.Parser{}, dnsmessage.Header{}, errInvalidDNSResponse
}
if h.Truncated { // see RFC 5966
continue
}
return p, h, nil
}
return dnsmessage.Parser{}, dnsmessage.Header{}, errors.New("no answer from DNS server")
return dnsmessage.Parser{}, dnsmessage.Header{}, errNoAnswerFromDNSServer
}
// checkHeader performs basic sanity checks on the header.
func checkHeader(p *dnsmessage.Parser, h dnsmessage.Header, name, server string) error {
if h.RCode == dnsmessage.RCodeNameError {
return errNoSuchHost
}
_, err := p.AnswerHeader()
if err != nil && err != dnsmessage.ErrSectionDone {
return &DNSError{
Err: "cannot unmarshal DNS message",
Name: name,
Server: server,
}
return errCannotUnmarshalDNSMessage
}
// libresolv continues to the next server when it receives
// an invalid referral response. See golang.org/issue/15434.
if h.RCode == dnsmessage.RCodeSuccess && !h.Authoritative && !h.RecursionAvailable && err == dnsmessage.ErrSectionDone {
return &DNSError{Err: "lame referral", Name: name, Server: server}
return errLameReferral
}
if h.RCode != dnsmessage.RCodeSuccess && h.RCode != dnsmessage.RCodeNameError {
@@ -180,11 +194,10 @@ func checkHeader(p *dnsmessage.Parser, h dnsmessage.Header, name, server string)
// a name error and we didn't get success,
// the server is behaving incorrectly or
// having temporary trouble.
err := &DNSError{Err: "server misbehaving", Name: name, Server: server}
if h.RCode == dnsmessage.RCodeServerFailure {
err.IsTemporary = true
return errServerTemporarlyMisbehaving
}
return err
return errServerMisbehaving
}
return nil
@@ -194,28 +207,16 @@ func skipToAnswer(p *dnsmessage.Parser, qtype dnsmessage.Type, name, server stri
for {
h, err := p.AnswerHeader()
if err == dnsmessage.ErrSectionDone {
return &DNSError{
Err: errNoSuchHost.Error(),
Name: name,
Server: server,
}
return errNoSuchHost
}
if err != nil {
return &DNSError{
Err: "cannot unmarshal DNS message",
Name: name,
Server: server,
}
return errCannotUnmarshalDNSMessage
}
if h.Type == qtype {
return nil
}
if err := p.SkipAnswer(); err != nil {
return &DNSError{
Err: "cannot unmarshal DNS message",
Name: name,
Server: server,
}
return errCannotUnmarshalDNSMessage
}
}
}
@@ -229,7 +230,7 @@ func (r *Resolver) tryOneName(ctx context.Context, cfg *dnsConfig, name string,
n, err := dnsmessage.NewName(name)
if err != nil {
return dnsmessage.Parser{}, "", errors.New("cannot marshal DNS message")
return dnsmessage.Parser{}, "", errCannotMarshalDNSMessage
}
q := dnsmessage.Question{
Name: n,
@@ -243,38 +244,62 @@ func (r *Resolver) tryOneName(ctx context.Context, cfg *dnsConfig, name string,
p, h, err := r.exchange(ctx, server, q, cfg.timeout)
if err != nil {
lastErr = &DNSError{
dnsErr := &DNSError{
Err: err.Error(),
Name: name,
Server: server,
}
if nerr, ok := err.(Error); ok && nerr.Timeout() {
lastErr.(*DNSError).IsTimeout = true
dnsErr.IsTimeout = true
}
// Set IsTemporary for socket-level errors. Note that this flag
// may also be used to indicate a SERVFAIL response.
if _, ok := err.(*OpError); ok {
lastErr.(*DNSError).IsTemporary = true
dnsErr.IsTemporary = true
}
lastErr = dnsErr
continue
}
// The name does not exist, so trying another server won't help.
//
// TODO: indicate this in a more obvious way, such as a field on DNSError?
if h.RCode == dnsmessage.RCodeNameError {
return dnsmessage.Parser{}, "", &DNSError{Err: errNoSuchHost.Error(), Name: name, Server: server}
}
lastErr = checkHeader(&p, h, name, server)
if lastErr != nil {
if err := checkHeader(&p, h, name, server); err != nil {
dnsErr := &DNSError{
Err: err.Error(),
Name: name,
Server: server,
}
if err == errServerTemporarlyMisbehaving {
dnsErr.IsTemporary = true
}
if err == errNoSuchHost {
// The name does not exist, so trying
// another server won't help.
//
// TODO: indicate this in a more
// obvious way, such as a field on
// DNSError?
return p, server, dnsErr
}
lastErr = dnsErr
continue
}
lastErr = skipToAnswer(&p, qtype, name, server)
if lastErr == nil {
err = skipToAnswer(&p, qtype, name, server)
if err == nil {
return p, server, nil
}
lastErr = &DNSError{
Err: err.Error(),
Name: name,
Server: server,
}
if err == errNoSuchHost {
// The name does not exist, so trying another
// server won't help.
//
// TODO: indicate this in a more obvious way,
// such as a field on DNSError?
return p, server, lastErr
}
}
}
return dnsmessage.Parser{}, "", lastErr
@@ -1427,28 +1427,35 @@ func TestDNSGoroutineRace(t *testing.T) {
}
}
func lookupWithFake(fake fakeDNSServer, name string, typ dnsmessage.Type) error {
r := Resolver{PreferGo: true, Dial: fake.DialContext}
resolvConf.mu.RLock()
conf := resolvConf.dnsConfig
resolvConf.mu.RUnlock()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
_, _, err := r.tryOneName(ctx, conf, name, typ)
return err
}
// Issue 8434: verify that Temporary returns true on an error when rcode
// is SERVFAIL
func TestIssue8434(t *testing.T) {
msg := dnsmessage.Message{
Header: dnsmessage.Header{
RCode: dnsmessage.RCodeServerFailure,
err := lookupWithFake(fakeDNSServer{
rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
return dnsmessage.Message{
Header: dnsmessage.Header{
ID: q.ID,
Response: true,
RCode: dnsmessage.RCodeServerFailure,
},
Questions: q.Questions,
}, nil
},
}
b, err := msg.Pack()
if err != nil {
t.Fatal("Pack failed:", err)
}
var p dnsmessage.Parser
h, err := p.Start(b)
if err != nil {
t.Fatal("Start failed:", err)
}
if err := p.SkipAllQuestions(); err != nil {
t.Fatal("SkipAllQuestions failed:", err)
}
err = checkHeader(&p, h, "golang.org", "foo:53")
}, "golang.org.", dnsmessage.TypeALL)
if err == nil {
t.Fatal("expected an error")
}
@@ -1464,50 +1471,76 @@ func TestIssue8434(t *testing.T) {
}
}
// Issue 12778: verify that NXDOMAIN without RA bit errors as
// "no such host" and not "server misbehaving"
// TestNoSuchHost verifies that tryOneName works correctly when the domain does
// not exist.
//
// Issue 12778: verify that NXDOMAIN without RA bit errors as "no such host"
// and not "server misbehaving"
//
// Issue 25336: verify that NXDOMAIN errors fail fast.
func TestIssue12778(t *testing.T) {
lookups := 0
fake := fakeDNSServer{
rh: func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
lookups++
return dnsmessage.Message{
Header: dnsmessage.Header{
ID: q.ID,
Response: true,
RCode: dnsmessage.RCodeNameError,
RecursionAvailable: false,
},
Questions: q.Questions,
}, nil
//
// Issue 27525: verify that empty answers fail fast.
func TestNoSuchHost(t *testing.T) {
tests := []struct {
name string
f func(string, string, dnsmessage.Message, time.Time) (dnsmessage.Message, error)
}{
{
"NXDOMAIN",
func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
return dnsmessage.Message{
Header: dnsmessage.Header{
ID: q.ID,
Response: true,
RCode: dnsmessage.RCodeNameError,
RecursionAvailable: false,
},
Questions: q.Questions,
}, nil
},
},
{
"no answers",
func(n, _ string, q dnsmessage.Message, _ time.Time) (dnsmessage.Message, error) {
return dnsmessage.Message{
Header: dnsmessage.Header{
ID: q.ID,
Response: true,
RCode: dnsmessage.RCodeSuccess,
RecursionAvailable: false,
Authoritative: true,
},
Questions: q.Questions,
}, nil
},
},
}
r := Resolver{PreferGo: true, Dial: fake.DialContext}
resolvConf.mu.RLock()
conf := resolvConf.dnsConfig
resolvConf.mu.RUnlock()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
_, _, err := r.tryOneName(ctx, conf, ".", dnsmessage.TypeALL)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
lookups := 0
err := lookupWithFake(fakeDNSServer{
rh: func(n, s string, q dnsmessage.Message, d time.Time) (dnsmessage.Message, error) {
lookups++
return test.f(n, s, q, d)
},
}, ".", dnsmessage.TypeALL)
if lookups != 1 {
t.Errorf("got %d lookups, wanted 1", lookups)
}
if lookups != 1 {
t.Errorf("got %d lookups, wanted 1", lookups)
}
if err == nil {
t.Fatal("expected an error")
}
de, ok := err.(*DNSError)
if !ok {
t.Fatalf("err = %#v; wanted a *net.DNSError", err)
}
if de.Err != errNoSuchHost.Error() {
t.Fatalf("Err = %#v; wanted %q", de.Err, errNoSuchHost.Error())
if err == nil {
t.Fatal("expected an error")
}
de, ok := err.(*DNSError)
if !ok {
t.Fatalf("err = %#v; wanted a *net.DNSError", err)
}
if de.Err != errNoSuchHost.Error() {
t.Fatalf("Err = %#v; wanted %q", de.Err, errNoSuchHost.Error())
}
})
}
}

0 comments on commit 05a0c7b

Please sign in to comment.