proxy: cache servfails

proxy/cache.go

@@ -303,7 +303,6 @@ func isCacheable(m *dns.Msg) bool {
 		return false
 	}
 
-	qName := m.Question[0].Name
 	switch rcode := m.Rcode; rcode {
 	case dns.RcodeSuccess:
 		if qType := m.Question[0].Qtype; qType != dns.TypeA && qType != dns.TypeAAAA {
@@ -313,10 +312,12 @@ func isCacheable(m *dns.Msg) bool {
 		return hasIPAns(m) || isCacheableNegative(m)
 	case dns.RcodeNameError:
 		return isCacheableNegative(m)
+	case dns.RcodeServerFailure:
+		return true
 	default:
 		log.Tracef(
 			"%s: refusing to cache message with response code %s",
-			qName,
+			m.Question[0].Name,
 			dns.RcodeToString[rcode],
 		)
 
@@ -357,22 +358,36 @@ func isCacheableNegative(m *dns.Msg) (ok bool) {
 	return ok
 }
 
-// lowestTTL returns the lowest TTL in m's RRs or 0 if the information is
-// absent.
+// ServFailMaxCacheTTL is the maximum time-to-live value for caching
+// SERVFAIL responses in seconds.  It's consistent with the upper constraint
+// of 5 minutes given by RFC 2308.
+//
+// See https://datatracker.ietf.org/doc/html/rfc2308#section-7.1.
+const ServFailMaxCacheTTL = 30
+
+// lowestTTL returns the lowest TTL in m's resource records or 0 if it's absent.
 func lowestTTL(m *dns.Msg) (ttl uint32) {
+	// Use the maximum value as a guard value.  If the inner loop is entered,
+	// it's going to be rewritten with an actual TTL value that is lower than
+	// MaxUint32.  If the inner loop isn't entered, catch that and return zero.
 	ttl = math.MaxUint32
-
 	for _, rrset := range [...][]dns.RR{m.Answer, m.Ns, m.Extra} {
-		for _, r := range rrset {
-			ttl = minTTL(r.Header(), ttl)
+		for _, rr := range rrset {
+			ttl = minTTL(rr.Header(), ttl)
+			if ttl == 0 {
+				return 0
+			}
 		}
 	}
 
-	if ttl == math.MaxUint32 {
+	switch {
+	case m.Rcode == dns.RcodeServerFailure && ttl > ServFailMaxCacheTTL:
+		return ServFailMaxCacheTTL
+	case ttl == math.MaxUint32:
 		return 0
+	default:
+		return ttl
 	}
-
-	return ttl
 }
 
 // minTTL returns the minimum of h's ttl and the passed ttl.