Failback immediately if reset-after is set to 0 and new flag to fail on empty responses (#448)

* Failback immediately

* Add `empty-error` for detecting unusual empty responses

* Treat all empty responses as errors

* Multiple CNAMES + No records

* folbricht's suggestions

* Clarify `0` seconds

* Suggestion for single-request failover

* update

* Check for EDE when deciding on 'empty-error'

* fix failback

---------

Co-authored-by: Anuskuss <[email protected]>

Author: folbricht

cmd/routedns/config.go

@@ -109,8 +109,9 @@ type group struct {
 	EDNS0Data  []byte                  `toml:"edns0-data"`  // EDNS0 modifier option data
 
 	// Failover/Failback options
-	ResetAfter    int  `toml:"reset-after"`    // Time in seconds after which to reset resolvers in fail-back and random groups, default 60.
+	ResetAfter    *int `toml:"reset-after"`    // Time in seconds after which to reset resolvers in fail-back and random groups, or 0 to reset immediately, default 60.
 	ServfailError bool `toml:"servfail-error"` // If true, SERVFAIL responses are considered errors and cause failover etc.
+	EmptyError    bool `toml:"empty-error"`    // If true, empty responses are considered errors and cause failover etc.
 
 	// Cache options
 	Backend                  *cacheBackend

cmd/routedns/main.go

@@ -370,20 +370,31 @@ func instantiateGroup(id string, g group, resolvers map[string]rdns.Resolver) er
 	case "fail-rotate":
 		opt := rdns.FailRotateOptions{
 			ServfailError: g.ServfailError,
+			EmptyError:    g.EmptyError,
 		}
 		resolvers[id] = rdns.NewFailRotate(id, opt, gr...)
 	case "fail-back":
+		var resetAfterDefault int = 60
+		if g.ResetAfter == nil {
+			g.ResetAfter = &resetAfterDefault
+		}
 		opt := rdns.FailBackOptions{
-			ResetAfter:    time.Duration(time.Duration(g.ResetAfter) * time.Second),
+			ResetAfter:    time.Duration(time.Duration(*g.ResetAfter) * time.Second),
 			ServfailError: g.ServfailError,
+			EmptyError:    g.EmptyError,
 		}
 		resolvers[id] = rdns.NewFailBack(id, opt, gr...)
 	case "fastest":
 		resolvers[id] = rdns.NewFastest(id, gr...)
 	case "random":
+		var resetAfterDefault int = 60
+		if g.ResetAfter == nil {
+			g.ResetAfter = &resetAfterDefault
+		}
 		opt := rdns.RandomOptions{
-			ResetAfter:    time.Duration(time.Duration(g.ResetAfter) * time.Second),
+			ResetAfter:    time.Duration(time.Duration(*g.ResetAfter) * time.Second),
 			ServfailError: g.ServfailError,
+			EmptyError:    g.EmptyError,
 		}
 		resolvers[id] = rdns.NewRandom(id, opt, gr...)
 	case "blocklist":

doc/configuration.md

@@ -519,6 +519,7 @@ Options:
 
 - `resolvers` - An array of upstream resolvers or modifiers.
 - `servfail-error` - If `true`, a SERVFAIL response from an upstream resolver is considered a failure triggering a switch to the next resolver. This can happen when DNSSEC validation fails for example. Default `false`.
+- `empty-error` - If `true`, an empty reponse from an upstream resolver is considered a failure triggering a switch to the next resolver. Default `false`.
 
 #### Examples
 
@@ -539,8 +540,9 @@ Fail-Back groups are instantiated with `type = "fail-back"` in the groups sectio
 Options:
 
 - `resolvers` - An array of upstream resolvers or modifiers. The first in the array is the preferred resolver.
-- `reset-after` - Time in seconds before switching from an alternative resolver back to the preferred resolver (first in the list), default 60. Note: This is not a timeout argument. After a failure of the preferred resolver, this defines the amount of time to use alternative/failover resolvers before switching back to the preferred. You can have as many resolvers in the array as the time limit allows.
+- `reset-after` - Non-zero time in seconds before switching from an alternative resolver back to the preferred resolver (first in the list), or a negative number to switch back immediately, default 60. Note: This is not a timeout argument. After a failure of the preferred resolver, this defines the amount of time to use alternative/failover resolvers before switching back to the preferred. You can have as many resolvers in the array as the time limit allows.
 - `servfail-error` - If `true`, a SERVFAIL response from an upstream resolver is considered a failure triggering a failover. This can happen when DNSSEC validation fails for example. Default `false`.
+- `empty-error` - If `true`, an empty reponse from an upstream resolver is considered a failure triggering a switch to the next resolver. Default `false`.
 
 #### Examples
 
@@ -561,8 +563,9 @@ Random groups are instantiated with `type = "random"` in the groups section of t
 Options:
 
 - `resolvers` - An array of upstream resolvers or modifiers.
-- `reset-after` - Time in seconds to disable a failed resolver, default 60.
+- `reset-after` - Non-zero time in seconds to disable a failed resolver, or a negative number to disable only for a single request, default 60.
 - `servfail-error` - If `true`, a SERVFAIL response from an upstream resolver is considered a failure which will take the resolver temporarily out of the group. This can happen when DNSSEC validation fails for example. Default `false`.
+- `empty-error` - If `true`, an empty reponse from an upstream resolver is considered a failure triggering a switch to the next resolver. Default `false`.
 
 #### Examples
 

failback.go

@@ -36,6 +36,10 @@ type FailBackOptions struct {
 	// Determines if a SERVFAIL returned by a resolver should be considered an
 	// error response and trigger a failover.
 	ServfailError bool
+
+	// Determines if an empty reponse returned by a resolver should be considered an
+	// error respone and trigger a failover.
+	EmptyError bool
 }
 
 var _ Resolver = &FailBack{}
@@ -61,9 +65,6 @@ func NewFailRouterMetrics(id string, available int) *FailRouterMetrics {
 
 // NewFailBack returns a new instance of a failover resolver group.
 func NewFailBack(id string, opt FailBackOptions, resolvers ...Resolver) *FailBack {
-	if opt.ResetAfter == 0 {
-		opt.ResetAfter = time.Minute
-	}
 	return &FailBack{
 		id:        id,
 		resolvers: resolvers,
@@ -81,7 +82,7 @@ func (r *FailBack) Resolve(q *dns.Msg, ci ClientInfo) (*dns.Msg, error) {
 		a   *dns.Msg
 	)
 	for i := 0; i < len(r.resolvers); i++ {
-		resolver, active := r.current()
+		resolver, active := r.current(i)
 		log.With("resolver", resolver.String()).Debug("forwarding query to resolver")
 		r.metrics.route.Add(resolver.String(), 1)
 		a, err = resolver.Resolve(q, ci)
@@ -102,7 +103,12 @@ func (r *FailBack) String() string {
 }
 
 // Thread-safe method to return the currently active resolver.
-func (r *FailBack) current() (Resolver, int) {
+func (r *FailBack) current(attempt int) (Resolver, int) {
+	// With ResetAfter set to 0, simply iterate through the list of
+	// resolvers on a per-request basis.
+	if r.opt.ResetAfter == 0 {
+		return r.resolvers[attempt], attempt
+	}
 	r.mu.RLock()
 	defer r.mu.RUnlock()
 	return r.resolvers[r.active], r.active
@@ -113,6 +119,11 @@ func (r *FailBack) current() (Resolver, int) {
 // requests. Another request could have initiated the failover already. So ignore if i is not
 // (no longer) the active store.
 func (r *FailBack) errorFrom(i int) {
+	// If ResetAfter is set to 0, we fail-over to the next resolver, but
+	// only for this single request. It won't affect any other requests.
+	if r.opt.ResetAfter == 0 {
+		return
+	}
 	r.mu.Lock()
 	if i != r.active {
 		r.mu.Unlock()
@@ -158,5 +169,46 @@ func (r *FailBack) startResetTimer() chan struct{} {
 
 // Returns true is the response is considered successful given the options.
 func (r *FailBack) isSuccessResponse(a *dns.Msg) bool {
-	return a == nil || !(r.opt.ServfailError && a.Rcode == dns.RcodeServerFailure)
+	if a == nil {
+		return true
+	}
+	if r.opt.ServfailError && a.Rcode == dns.RcodeServerFailure {
+		return false
+	}
+	if r.opt.EmptyError {
+		// Check if there are only CNAMEs in the answer section
+		var onlyCNAME = true
+		for _, rr := range a.Answer {
+			if rr.Header().Rrtype != dns.TypeCNAME {
+				onlyCNAME = false
+				break
+			}
+		}
+		// The answer may be blank because it was blocked by a filter.
+		// If so (as determined by the presence of an EDE option), we
+		// consider it "successful" as we shouldn't retry or fail-over
+		// in that case.
+		if onlyCNAME && !hasExtendedErrorBlocked(a) {
+			return false
+		}
+	}
+	return true
+}
+
+// Returns true if the message contains an extended error option indicating it
+// was blocked, censored, or filtered.
+func hasExtendedErrorBlocked(msg *dns.Msg) bool {
+	edns0 := msg.IsEdns0()
+	if edns0 == nil {
+		return false
+	}
+	for _, opt := range edns0.Option {
+		if ede, ok := opt.(*dns.EDNS0_EDE); ok {
+			switch ede.InfoCode {
+			case dns.ExtendedErrorCodeBlocked, dns.ExtendedErrorCodeCensored, dns.ExtendedErrorCodeFiltered:
+				return true
+			}
+		}
+	}
+	return false
 }

failback_test.go

@@ -113,7 +113,7 @@ func TestFailBackServfailOKOption(t *testing.T) {
 	goodResolver := new(TestResolver)
 
 	// With ServfailError == false
-	g1 := NewFailBack("test-fb", FailBackOptions{}, failResolver, goodResolver)
+	g1 := NewFailBack("test-fb", FailBackOptions{ResetAfter: time.Second}, failResolver, goodResolver)
 	q := new(dns.Msg)
 	q.SetQuestion("test.com.", dns.TypeA)
 
@@ -123,7 +123,7 @@ func TestFailBackServfailOKOption(t *testing.T) {
 	require.Equal(t, 0, goodResolver.hitCount)
 
 	// With ServfailError == true
-	g2 := NewFailBack("test-fb", FailBackOptions{ServfailError: true}, failResolver, goodResolver)
+	g2 := NewFailBack("test-fb", FailBackOptions{ResetAfter: time.Second, ServfailError: true}, failResolver, goodResolver)
 
 	a, err = g2.Resolve(q, ci)
 	require.NoError(t, err)

failrotate.go

@@ -27,6 +27,10 @@ type FailRotateOptions struct {
 	// Determines if a SERVFAIL returned by a resolver should be considered an
 	// error response and trigger a failover.
 	ServfailError bool
+
+	// Determines if an empty reponse returned by a resolver should be considered an
+	// error respone and trigger a failover.
+	EmptyError bool
 }
 
 var _ Resolver = &FailRotate{}
@@ -94,5 +98,28 @@ func (r *FailRotate) errorFrom(i int) {
 
 // Returns true is the response is considered successful given the options.
 func (r *FailRotate) isSuccessResponse(a *dns.Msg) bool {
-	return a == nil || !(r.opt.ServfailError && a.Rcode == dns.RcodeServerFailure)
+	if a == nil {
+		return true
+	}
+	if r.opt.ServfailError && a.Rcode == dns.RcodeServerFailure {
+		return false
+	}
+	if r.opt.EmptyError {
+		// Check if there are only CNAMEs in the answer section
+		var onlyCNAME = true
+		for _, rr := range a.Answer {
+			if rr.Header().Rrtype != dns.TypeCNAME {
+				onlyCNAME = false
+				break
+			}
+		}
+		// The answer may be blank because it was blocked by a filter.
+		// If so (as determined by the presence of an EDE option), we
+		// consider it "successful" as we shouldn't retry or fail-over
+		// in that case.
+		if onlyCNAME && !hasExtendedErrorBlocked(a) {
+			return false
+		}
+	}
+	return true
 }

random.go

@@ -30,14 +30,14 @@ type RandomOptions struct {
 	// Determines if a SERVFAIL returned by a resolver should be considered an
 	// error response and cause the resolver to be removed from the group temporarily.
 	ServfailError bool
+
+	// Determines if an empty reponse returned by a resolver should be considered an
+	// error respone and trigger a failover.
+	EmptyError bool
 }
 
 // NewRandom returns a new instance of a random resolver group.
 func NewRandom(id string, opt RandomOptions, resolvers ...Resolver) *Random {
-	rand.Seed(time.Now().UnixNano())
-	if opt.ResetAfter == 0 {
-		opt.ResetAfter = time.Minute
-	}
 	return &Random{
 		id:        id,
 		resolvers: resolvers,
@@ -121,5 +121,29 @@ func (r *Random) reactivateLater(resolver Resolver) {
 
 // Returns true is the response is considered successful given the options.
 func (r *Random) isSuccessResponse(a *dns.Msg) bool {
-	return a == nil || !(r.opt.ServfailError && a.Rcode == dns.RcodeServerFailure)
+	if a == nil {
+		return true
+	}
+	if r.opt.ServfailError && a.Rcode == dns.RcodeServerFailure {
+		return false
+	}
+	if r.opt.EmptyError {
+		// Check if there are only CNAMEs in the answer section
+		var onlyCNAME = true
+		for _, rr := range a.Answer {
+			if rr.Header().Rrtype != dns.TypeCNAME {
+				onlyCNAME = false
+				break
+			}
+		}
+		// The answer may be blank because it was blocked by a filter.
+		// If so (as determined by the presence of an EDE option), we
+		// consider it "successful" as we shouldn't retry or fail-over
+		// in that case.
+		if onlyCNAME && !hasExtendedErrorBlocked(a) {
+			return false
+		}
+	}
+	return true
+
 }