Merge pull request #532 from inercia/weave-225-fix-2

Fix in WeaveDNS: do not modify cache when getting a reply

nameserver/cache.go

@@ -93,31 +93,33 @@ func (e *cacheEntry) getReply(request *dns.Msg, maxLen int, now time.Time) (*dns
 	}
 
 	if e.ReplyLen >= maxLen {
-		Debug.Printf("[cache] returning truncated reponse: %d > %d", e.ReplyLen, maxLen)
+		Debug.Printf("[cache msgid %d] returning truncated reponse: %d > %d", request.MsgHdr.Id, e.ReplyLen, maxLen)
 		return makeTruncatedReply(request), nil
 	}
 
 	// create a copy of the reply, with values for this particular query
-	reply := e.reply
+	reply := e.reply.Copy()
 	reply.SetReply(request)
-	reply.Rcode = e.reply.Rcode
-	reply.Authoritative = true
 
 	// adjust the TTLs
 	passedSecs := uint32(now.Sub(e.putTime).Seconds())
 	for _, rr := range reply.Answer {
-		ttl := rr.Header().Ttl
+		hdr := rr.Header()
+		ttl := hdr.Ttl
 		if passedSecs < ttl {
-			rr.Header().Ttl = ttl - passedSecs
+			hdr.Ttl = ttl - passedSecs
 		} else {
-			rr.Header().Ttl = 0
+			return nil, nil // it is expired: do not spend more time and return nil...
 		}
 	}
 
+	reply.Rcode = e.reply.Rcode
+	reply.Authoritative = true
+
 	// shuffle the values, etc...
 	reply.Answer = shuffleAnswers(reply.Answer)
 
-	return &reply, nil
+	return reply, nil
 }
 
 func (e cacheEntry) hasExpired(now time.Time) bool {
@@ -129,6 +131,9 @@ func (e cacheEntry) hasExpired(now time.Time) bool {
 func (e *cacheEntry) setReply(reply *dns.Msg, ttl int, flags uint8, now time.Time) bool {
 	var prevValidUntil time.Time
 	if e.Status == stResolved {
+		if reply != nil {
+			Debug.Printf("[cache msgid %d] replacing response in cache", reply.MsgHdr.Id)
+		}
 		prevValidUntil = e.validUntil
 	}
 

nameserver/server.go

@@ -18,7 +18,6 @@ const (
 	DefaultCacheLen      = 8192               // default cache capacity
 	DefaultResolvWorkers = 8                  // default number of resolution workers
 	DefaultTimeout       = 5                  // default timeout for DNS resolutions
-	DefaultResolvTries   = 3                  // max # of times a worker tries to resolve a query
 )
 
 type DNSServerConfig struct {
@@ -223,14 +222,6 @@ func (s *DNSServer) queryHandler(proto dnsProtocol) dns.HandlerFunc {
 		lookups := []Lookup{s.Zone, s.mdnsCli}
 		Debug.Printf("Query: %+v", q)
 
-		if q.Qtype != dns.TypeA {
-			Debug.Printf("[dns msgid %d] Unsuported query type %s", r.MsgHdr.Id, dns.TypeToString[q.Qtype])
-			m := makeDNSNotImplResponse(r)
-			s.cache.Put(r, m, negLocalTTL, 0, now)
-			w.WriteMsg(m)
-			return
-		}
-
 		reply, err := s.cache.Get(r, maxLen, time.Now())
 		if err != nil {
 			if err == errNoLocalReplies {
@@ -248,9 +239,20 @@ func (s *DNSServer) queryHandler(proto dnsProtocol) dns.HandlerFunc {
 			w.WriteMsg(reply)
 			return
 		}
+
+		// catch unsupported queries
+		if q.Qtype != dns.TypeA {
+			Debug.Printf("[dns msgid %d] Unsuported query type %s", r.MsgHdr.Id, dns.TypeToString[q.Qtype])
+			m := makeDNSNotImplResponse(r)
+			s.cache.Put(r, m, negLocalTTL, 0, now)
+			w.WriteMsg(m)
+			return
+		}
+
 		for _, lookup := range lookups {
 			if ip, err := lookup.LookupName(q.Name); err == nil {
 				m := makeAddressReply(r, &q, []net.IP{ip})
+				m.Authoritative = true
 
 				Debug.Printf("[dns msgid %d] Caching response for %s-query for \"%s\": %s [code:%s]",
 					m.MsgHdr.Id, dns.TypeToString[q.Qtype], q.Name, ip, dns.RcodeToString[m.Rcode])
@@ -261,7 +263,7 @@ func (s *DNSServer) queryHandler(proto dnsProtocol) dns.HandlerFunc {
 			now = time.Now()
 		}
 
-		Info.Printf("[dns msgid %d] No results for type %s query %s",
+		Info.Printf("[dns msgid %d] No results for type %s query %s [caching no-local]",
 			r.MsgHdr.Id, dns.TypeToString[q.Qtype], q.Name)
 		s.cache.Put(r, nil, negLocalTTL, CacheNoLocalReplies, now)
 		w.WriteMsg(makeDNSFailResponse(r))
@@ -275,17 +277,8 @@ func (s *DNSServer) rdnsHandler(proto dnsProtocol) dns.HandlerFunc {
 		q := r.Question[0]
 		lookups := []Lookup{s.Zone, s.mdnsCli}
 		maxLen := getMaxReplyLen(r, proto)
-		Debug.Printf("Reverse query: %+v", q)
-
-		if q.Qtype != dns.TypePTR {
-			Warning.Printf("[dns msgid %d] Unexpected reverse query type %s: %+v",
-				r.MsgHdr.Id, dns.TypeToString[q.Qtype], q)
-			m := makeDNSNotImplResponse(r)
-			s.cache.Put(r, m, negLocalTTL, 0, now)
-			w.WriteMsg(m)
-			return
-		}
 
+ 		Debug.Printf("Reverse query: %+v", q)
 		reply, err := s.cache.Get(r, maxLen, time.Now())
 		if err != nil {
 			if err == errNoLocalReplies {
@@ -303,9 +296,22 @@ func (s *DNSServer) rdnsHandler(proto dnsProtocol) dns.HandlerFunc {
 			w.WriteMsg(reply)
 			return
 		}
+
+		// catch unsupported queries
+		if q.Qtype != dns.TypePTR {
+			Warning.Printf("[dns msgid %d] Unexpected reverse query type %s: %+v",
+			r.MsgHdr.Id, dns.TypeToString[q.Qtype], q)
+			m := makeDNSNotImplResponse(r)
+			s.cache.Put(r, m, negLocalTTL, 0, now)
+			w.WriteMsg(m)
+			return
+		}
+
 		for _, lookup := range lookups {
 			if name, err := lookup.LookupInaddr(q.Name); err == nil {
 				m := makePTRReply(r, &q, []string{name})
+				m.Authoritative = true
+
 				Debug.Printf("[dns msgid %d] Caching response for %s-query for \"%s\": %s [code:%s]",
 					m.MsgHdr.Id, dns.TypeToString[q.Qtype], q.Name, name, dns.RcodeToString[m.Rcode])
 				s.cache.Put(r, m, nullTTL, 0, now)
@@ -353,8 +359,7 @@ func (s *DNSServer) notUsHandler(proto dnsProtocol) dns.HandlerFunc {
 			w.WriteMsg(reply)
 			return
 		}
-		Warning.Printf("[dns msgid %d] Failed lookup for external name %s",
-			r.MsgHdr.Id, q.Name)
+		Warning.Printf("[dns msgid %d] Failed lookup for external name %s", r.MsgHdr.Id, q.Name)
 		w.WriteMsg(makeDNSFailResponse(r))
 	}
 }

weavedns/main.go

@@ -53,6 +53,7 @@ func main() {
 	}
 
 	InitDefaultLogging(debug)
+	Info.Printf("WeaveDNS version %s\n", version)	// first thing in log: the version
 
 	var zone = new(weavedns.ZoneDb)