From 4e14439112b89e05e454d66331833b59540e715c Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 25 Sep 2020 10:13:10 +0200 Subject: [PATCH] Handle DNS Rcodes --- nameserver/nameserver.go | 11 ++++++++--- resolver/namerecord.go | 1 + resolver/resolve.go | 14 +++++++------- resolver/resolver-env.go | 1 + resolver/resolver-mdns.go | 3 +++ resolver/resolver-plain.go | 1 + resolver/resolver-tcp.go | 5 +++++ resolver/rrcache.go | 39 ++++++++++++++++++++++++++------------ 8 files changed, 53 insertions(+), 22 deletions(-) diff --git a/nameserver/nameserver.go b/nameserver/nameserver.go index 9deec933..21b81151 100644 --- a/nameserver/nameserver.go +++ b/nameserver/nameserver.go @@ -98,7 +98,12 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) // Start context tracer for context-aware logging. ctx, tracer := log.AddTracer(ctx) defer tracer.Submit() - tracer.Tracef("nameserver: handling new request for %s%s from %s:%d", q.FQDN, q.QType, remoteAddr.IP, remoteAddr.Port) + tracer.Tracef("nameserver: handling new request for %s from %s:%d", q.ID(), remoteAddr.IP, remoteAddr.Port) + + // Check if there are more than one question. + if len(request.Question) > 1 { + tracer.Warningf("nameserver: received more than one question from (%s:%d), first question is %s", remoteAddr.IP, remoteAddr.Port, q.ID()) + } // Setup quick reply function. reply := func(responder nsutil.Responder, rrProviders ...nsutil.RRProvider) error { @@ -197,10 +202,10 @@ func handleRequest(ctx context.Context, w dns.ResponseWriter, request *dns.Msg) // React to special errors. switch { case errors.Is(err, resolver.ErrNotFound): - tracer.Tracef("nameserver: NXDomain via error: %s", err) + tracer.Tracef("nameserver: %s", err) return reply(nsutil.NxDomain("nxdomain: " + err.Error())) case errors.Is(err, resolver.ErrBlocked): - tracer.Tracef("nameserver: block via error: %s", err) + tracer.Tracef("nameserver: %s", err) return reply(nsutil.ZeroIP("blocked: " + err.Error())) case errors.Is(err, resolver.ErrLocalhost): tracer.Tracef("nameserver: returning localhost records") diff --git a/resolver/namerecord.go b/resolver/namerecord.go index f277c435..45deae30 100644 --- a/resolver/namerecord.go +++ b/resolver/namerecord.go @@ -28,6 +28,7 @@ type NameRecord struct { Domain string Question string + RCode int Answer []string Ns []string Extra []string diff --git a/resolver/resolve.go b/resolver/resolve.go index 566ba63d..526c6931 100644 --- a/resolver/resolve.go +++ b/resolver/resolve.go @@ -206,10 +206,10 @@ func checkCache(ctx context.Context, q *Query) *RRCache { // We still return the cache, if it isn't NXDomain, as it will be used if the // new query fails. if rrCache.Expired() { - if rrCache.IsNXDomain() { - return nil + if rrCache.RCode == dns.RcodeSuccess { + return rrCache } - return rrCache + return nil } // Check if the cache will expire soon and start an async request. @@ -377,8 +377,8 @@ resolveLoop: // Defensive: This should normally not happen. continue } - // Check if we got NXDomain and whether we should try another resolver. - if rrCache.IsNXDomain() && tryAll { + // Check if request suceeded and whether we should try another resolver. + if rrCache.RCode != dns.RcodeSuccess && tryAll { continue } break resolveLoop @@ -404,7 +404,7 @@ resolveLoop: // There was an error during resolving, return the old cache entry instead. log.Tracer(ctx).Debugf("resolver: serving backup cache of %s because query failed: %s", q.ID(), err) return oldCache, nil - case rrCache.IsNXDomain(): + case !rrCache.Cacheable(): // The new result is NXDomain, return the old cache entry instead. log.Tracer(ctx).Debugf("resolver: serving backup cache of %s because fresh response is NXDomain", q.ID()) return oldCache, nil @@ -417,7 +417,7 @@ resolveLoop: } // Save the new entry if cache is enabled. - if !q.NoCaching { + if !q.NoCaching && rrCache.Cacheable() { rrCache.Clean(minTTL) err = rrCache.Save() if err != nil { diff --git a/resolver/resolver-env.go b/resolver/resolver-env.go index 7cdbc008..2d4ad230 100644 --- a/resolver/resolver-env.go +++ b/resolver/resolver-env.go @@ -111,6 +111,7 @@ func (er *envResolverConn) makeRRCache(q *Query, answers []dns.RR) *RRCache { return &RRCache{ Domain: q.FQDN, Question: q.QType, + RCode: dns.RcodeSuccess, Answer: answers, Extra: []dns.RR{internalSpecialUseComment}, // Always add comment about this TLD. Server: envResolver.Server, diff --git a/resolver/resolver-mdns.go b/resolver/resolver-mdns.go index bd965258..21a91a87 100644 --- a/resolver/resolver-mdns.go +++ b/resolver/resolver-mdns.go @@ -202,6 +202,7 @@ func handleMDNSMessages(ctx context.Context, messages chan *dns.Msg) error { rrCache = &RRCache{ Domain: question.Name, Question: dns.Type(question.Qtype), + RCode: dns.RcodeSuccess, Server: mDNSResolver.Server, ServerScope: mDNSResolver.ServerIPScope, ServerInfo: mDNSResolver.ServerInfo, @@ -303,6 +304,7 @@ func handleMDNSMessages(ctx context.Context, messages chan *dns.Msg) error { rrCache = &RRCache{ Domain: v.Header().Name, Question: dns.Type(v.Header().Class), + RCode: dns.RcodeSuccess, Answer: []dns.RR{v}, Server: mDNSResolver.Server, ServerScope: mDNSResolver.ServerIPScope, @@ -423,6 +425,7 @@ func queryMulticastDNS(ctx context.Context, q *Query) (*RRCache, error) { return &RRCache{ Domain: q.FQDN, Question: q.QType, + RCode: dns.RcodeNameError, Server: mDNSResolver.Server, ServerScope: mDNSResolver.ServerIPScope, ServerInfo: mDNSResolver.ServerInfo, diff --git a/resolver/resolver-plain.go b/resolver/resolver-plain.go index a64dc314..3892ab91 100644 --- a/resolver/resolver-plain.go +++ b/resolver/resolver-plain.go @@ -81,6 +81,7 @@ func (pr *PlainResolver) Query(ctx context.Context, q *Query) (*RRCache, error) newRecord := &RRCache{ Domain: q.FQDN, Question: q.QType, + RCode: reply.Rcode, Answer: reply.Answer, Ns: reply.Ns, Extra: reply.Extra, diff --git a/resolver/resolver-tcp.go b/resolver/resolver-tcp.go index ff915b97..76c54cb8 100644 --- a/resolver/resolver-tcp.go +++ b/resolver/resolver-tcp.go @@ -50,6 +50,7 @@ func (ifq *InFlightQuery) MakeCacheRecord(reply *dns.Msg) *RRCache { return &RRCache{ Domain: ifq.Query.FQDN, Question: ifq.Query.QType, + RCode: reply.Rcode, Answer: reply.Answer, Ns: reply.Ns, Extra: reply.Extra, @@ -477,6 +478,10 @@ func (mgr *tcpResolverConnMgr) handleQueryResponse(conn *dns.Conn, msg *dns.Msg) // persist to database rrCache := inFlight.MakeCacheRecord(msg) + if !rrCache.Cacheable() { + return + } + rrCache.Clean(minTTL) err := rrCache.Save() if err != nil { diff --git a/resolver/rrcache.go b/resolver/rrcache.go index b6a103a8..9411e9db 100644 --- a/resolver/rrcache.go +++ b/resolver/rrcache.go @@ -22,6 +22,7 @@ type RRCache struct { Domain string // constant Question dns.Type // constant + RCode int // constant Answer []dns.RR // constant Ns []dns.RR // constant @@ -84,8 +85,8 @@ func (rrCache *RRCache) Clean(minExpires uint32) { // shorten caching switch { - case rrCache.IsNXDomain(): - // NXDomain + case rrCache.RCode != dns.RcodeSuccess: + // Any sort of error. lowestTTL = 10 case netenv.IsConnectivityDomain(rrCache.Domain): // Responses from these domains might change very quickly depending on the environment. @@ -127,6 +128,7 @@ func (rrCache *RRCache) ToNameRecord() *NameRecord { new := &NameRecord{ Domain: rrCache.Domain, Question: rrCache.Question.String(), + RCode: rrCache.RCode, TTL: rrCache.TTL, Server: rrCache.Server, ServerScope: rrCache.ServerScope, @@ -147,8 +149,27 @@ func (rrCache *RRCache) ToNameRecord() *NameRecord { return new } +// rcodeIsCacheable returns whether a record with the given RCode should be cached. +func rcodeIsCacheable(rCode int) bool { + switch rCode { + case dns.RcodeSuccess, dns.RcodeNameError, dns.RcodeRefused: + return true + default: + return false + } +} + +// Cacheable returns whether the record should be cached. +func (rrCache *RRCache) Cacheable() bool { + return rcodeIsCacheable(rrCache.RCode) +} + // Save saves the RRCache to the database as a NameRecord. func (rrCache *RRCache) Save() error { + if !rrCache.Cacheable() { + return nil + } + return rrCache.ToNameRecord().Save() } @@ -164,6 +185,7 @@ func GetRRCache(domain string, question dns.Type) (*RRCache, error) { return nil, err } + rrCache.RCode = nameRecord.RCode rrCache.TTL = nameRecord.TTL for _, entry := range nameRecord.Answer { rrCache.Answer = parseRR(rrCache.Answer, entry) @@ -227,16 +249,12 @@ func (rrCache *RRCache) Flags() string { return "" } -// IsNXDomain returnes whether the result is nxdomain. -func (rrCache *RRCache) IsNXDomain() bool { - return len(rrCache.Answer) == 0 -} - // ShallowCopy returns a shallow copy of the cache. slices are not copied, but referenced. func (rrCache *RRCache) ShallowCopy() *RRCache { return &RRCache{ Domain: rrCache.Domain, Question: rrCache.Question, + RCode: rrCache.RCode, Answer: rrCache.Answer, Ns: rrCache.Ns, Extra: rrCache.Extra, @@ -259,14 +277,11 @@ func (rrCache *RRCache) ShallowCopy() *RRCache { func (rrCache *RRCache) ReplyWithDNS(ctx context.Context, request *dns.Msg) *dns.Msg { // reply to query reply := new(dns.Msg) - reply.SetRcode(request, dns.RcodeSuccess) + reply.SetRcode(request, rrCache.RCode) reply.Ns = rrCache.Ns reply.Extra = rrCache.Extra - if rrCache.IsNXDomain() { - // Set NXDomain return code if not the reply has no answers. - reply.Rcode = dns.RcodeNameError - } else { + if len(rrCache.Answer) > 0 { // Copy answers, as we randomize their order a little. reply.Answer = make([]dns.RR, len(rrCache.Answer)) copy(reply.Answer, rrCache.Answer)