plugin/rewrite: streamline the ResponseRule handling. (#4473)

* plugin/rewrite: streamline the ResponseRule handling.

The functionality of a response rule is now completely encapsulated behind
a `ResponseRule` interface. This significantly simplifies the complete
processing flow, it enables more flexible response handling and it is possible
to eliminate lots of state flags, ifs and switches.

Based on the new flexibility the pull request also enables to support a
response name rewrite for all name rewrite types.
To be compatible, an explicit `answer auto` option is added to support
a best effort response rewrite (name and value).
Additionally now all name rewrite rules support additional name and value
reponse rewrite options.

Using this feature it is also possible now to rewrite a complete sub domain
hierarchy to a single domain name combined with a correct rewrite (#2389).
Signed-off-by: Uwe Krueger <uwe.krueger@sap.com>

* revert policy
Signed-off-by: Uwe Krueger <uwe.krueger@sap.com>
Co-authored-by: Miek Gieben <miek@miek.nl>

plugin/rewrite/README.md

@@ -13,23 +13,36 @@ Rewrites are invisible to the client. There are simple rewrites (fast) and compl
 
 A simplified/easy-to-digest syntax for *rewrite* is...
 ~~~
-rewrite [continue|stop] FIELD [FROM TO|FROM TTL]
+rewrite [continue|stop] FIELD [TYPE] [(FROM TO)|TTL] [OPTIONS]
 ~~~
 
 * **FIELD** indicates what part of the request/response is being re-written.
 
    * `type` - the type field of the request will be rewritten. FROM/TO must be a DNS record type (`A`, `MX`, etc.);
 e.g., to rewrite ANY queries to HINFO, use `rewrite type ANY HINFO`.
-   * `class` - the class of the message will be rewritten. FROM/TO must be a DNS class type (`IN`, `CH`, or `HS`); e.g., to rewrite CH queries to IN use `rewrite class CH IN`.
    * `name` - the query name in the _request_ is rewritten; by default this is a full match of the
      name, e.g., `rewrite name example.net example.org`. Other match types are supported, see the **Name Field Rewrites** section below.
-   * `answer name` - the query name in the _response_ is rewritten.  This option has special restrictions and requirements, in particular it must always combined with a `name` rewrite.  See below in the **Response Rewrites** section.
-   *  `edns0` - an EDNS0 option can be appended to the request as described below in the **EDNS0 Options** section.
+   * `class` - the class of the message will be rewritten. FROM/TO must be a DNS class type (`IN`, `CH`, or `HS`); e.g., to rewrite CH queries to IN use `rewrite class CH IN`.
+   * `edns0` - an EDNS0 option can be appended to the request as described below in the **EDNS0 Options** section.
    * `ttl` - the TTL value in the _response_ is rewritten.
 
+* **TYPE** this optional element can be specified for a `name` or `ttl` field.
+  If not given type `exact` will be assumed. If options should be specified the
+  type must be given.
 * **FROM** is the name (exact, suffix, prefix, substring, or regex) or type to match
 * **TO** is the destination name or type to rewrite to
-* **TTL** is the number of seconds to set the TTL value to
+* **TTL** is the number of seconds to set the TTL value to (only for field `ttl`)
+
+* **OPTIONS**
+
+  for field `name` further options are possible controlling the response rewrites.
+  All name matching types support the following options
+
+     * `answer auto` - the names in the _response_ is rewritten in a best effort manner.
+     * `answer name FROM TO` - the query name in the _response_ is rewritten matching the from regex pattern.
+     * `answer value FROM TO` - the names in the _response_ is rewritten matching the from regex pattern.
+
+  See below in the **Response Rewrites** section for further details.
 
 If you specify multiple rules and an incoming query matches multiple rules, the rewrite
 will behave as follows:
@@ -49,7 +62,7 @@ client.
 The syntax for name rewriting is as follows:
 
 ```
-rewrite [continue|stop] name [exact|prefix|suffix|substring|regex] STRING STRING
+rewrite [continue|stop] name [exact|prefix|suffix|substring|regex] STRING STRING [OPTIONS]
 ```
 
 The match type, e.g., `exact`, `substring`, etc., triggers rewrite:
@@ -60,7 +73,8 @@ The match type, e.g., `exact`, `substring`, etc., triggers rewrite:
 * **suffix**: when the name ends with the matching string
 * **regex**: when the name in the question section of a request matches a regular expression
 
-If the match type is omitted, the `exact` match type is assumed.
+If the match type is omitted, the `exact` match type is assumed. If OPTIONS are
+given, the type must be specified.
 
 The following instruction allows rewriting names in the query that
 contain the substring `service.us-west-1.example.org`:
@@ -95,7 +109,8 @@ rewrite name suffix .schmoogle.com. .google.com.
 
 ### Response Rewrites
 
-When rewriting incoming DNS requests' names, CoreDNS re-writes the `QUESTION SECTION`
+When rewriting incoming DNS requests' names (field `name`), CoreDNS re-writes
+the `QUESTION SECTION`
 section of the requests. It may be necessary to rewrite the `ANSWER SECTION` of the
 requests, because some DNS resolvers treat mismatches between the `QUESTION SECTION`
 and `ANSWER SECTION` as a man-in-the-middle attack (MITM).
@@ -127,6 +142,35 @@ ftp.service.us-west-1.consul. 0    IN A    10.30.30.30
 
 The above is a mismatch between the question asked and the answer provided.
 
+There are three possibilities to specify an answer rewrite:
+- A rewrite can request a best effort answer rewrite by adding the option `answer auto`.
+- A rewrite may specify a dedicated regex based response name rewrite with the
+  `answer name FROM TO` option.
+- A regex based rewrite of record values like `CNAME`, `SRV`, etc, can be requested by
+  an `answer value FROM TO` option.
+
+Hereby FROM/TO follow the rules for the `regex` name rewrite syntax.
+
+#### Auto Response Name Rewrite
+
+The following configuration snippet allows for rewriting of the
+`ANSWER SECTION` according to the rewrite of the `QUESTION SECTION`:
+
+```
+    rewrite stop {
+        name suffix .coredns.rocks .service.consul answer auto
+    }
+```
+
+Any occurrence of the rewritten question in the answer is mapped
+back to the original value before the rewrite.
+
+Please note that answers for rewrites of type `exact` are always rewritten.
+For a `suffix` name rule `auto` leads to a reverse suffix response rewrite,
+exchanging FROM and TO from the rewrite request.
+
+#### Explicit Response Name Rewrite
+
 The following configuration snippet allows for rewriting of the
 `ANSWER SECTION`, provided that the `QUESTION SECTION` was rewritten:
 
@@ -151,9 +195,11 @@ ftp-us-west-1.coredns.rocks. 0    IN A    10.20.20.20
 ftp-us-west-1.coredns.rocks. 0    IN A    10.30.30.30
 ```
 
+#### Rewriting other Response Values
+
 It is also possible to rewrite other values returned in the DNS response records
 (e.g. the server names returned in `SRV` and `MX` records). This can be enabled by adding
-the `answer value` to a name regex rule as specified below. `answer value` takes a
+the `answer value FROM TO` option to a name rule as specified below. `answer value` takes a
 regular expression and a rewrite name as parameters and works in the same way as the
 `answer name` rule.
 
@@ -173,20 +219,29 @@ rewrite [continue|stop] {
 ```
 
 Note that the above syntax is strict.  For response rewrites, only `name`
-rules are allowed to match the question section, and only by match type
-`regex`. The answer rewrite must be after the name, as in the
-syntax example.
+rules are allowed to match the question section. The answer rewrite must be
+after the name, as in the syntax example.
+
+#### Multiple Response Rewrites
+
+`name` and `value` rewrites can be chained by appending multiple answer rewrite
+options. For all occurrences but the first one the keyword `answer` might be
+omitted.
 
-An alternate syntax for rewriting a DNS request and response is as
-follows:
+```options
+answer (auto | (name|value FROM TO)) { [answer] (auto | (name|value FROM TO)) }
+```
 
+For example:
 ```
-rewrite [continue|stop] name regex STRING STRING answer name STRING STRING [answer value STRING STRING]
+rewrite [continue|stop] name regex FROM TO answer name FROM TO [answer] value FROM TO
 ```
 
 When using `exact` name rewrite rules, the answer gets rewritten automatically,
-and there is no need to define `answer name`. The rule below
-rewrites the name in a request from `RED` to `BLUE`, and subsequently
+and there is no need to define `answer name auto`. But it is still possible to define
+additional `answer value` and `answer value` options.
+
+The rule below rewrites the name in a request from `RED` to `BLUE`, and subsequently
 rewrites the name in a corresponding response from `BLUE` to `RED`. The
 client in the request would see only `RED` and no `BLUE`.
 
@@ -215,6 +270,7 @@ setting the TTL value really low.
 
 The syntax for the TTL rewrite rule is as follows. The meaning of
 `exact|prefix|suffix|substring|regex` is the same as with the name rewrite rules.
+An omitted type is defaulted to `exact`.
 
 ```
 rewrite [continue|stop] ttl [exact|prefix|suffix|substring|regex] STRING SECONDS
@@ -290,12 +346,3 @@ rewrite edns0 subnet set 24 56
 
 * If the query's source IP address is an IPv4 address, the first 24 bits in the IP will be the network subnet.
 * If the query's source IP address is an IPv6 address, the first 56 bits in the IP will be the network subnet.
-
-## Full Syntax
-
-The full plugin usage syntax is harder to digest...
-~~~
-rewrite [continue|stop] {type|class|edns0|name [exact|prefix|suffix|substring|regex [FROM TO answer name]]} FROM TO
-~~~
-
-The syntax above doesn't cover the multi-line block option for specifying a name request+response rewrite rule described in the **Response Rewrite** section.

plugin/rewrite/class.go

@@ -30,18 +30,15 @@ func newClassRule(nextAction string, args ...string) (Rule, error) {
 }
 
 // Rewrite rewrites the current request.
-func (rule *classRule) Rewrite(ctx context.Context, state request.Request) Result {
+func (rule *classRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
 	if rule.fromClass > 0 && rule.toClass > 0 {
 		if state.Req.Question[0].Qclass == rule.fromClass {
 			state.Req.Question[0].Qclass = rule.toClass
-			return RewriteDone
+			return nil, RewriteDone
 		}
 	}
-	return RewriteIgnored
+	return nil, RewriteIgnored
 }
 
 // Mode returns the processing mode.
 func (rule *classRule) Mode() string { return rule.NextAction }
-
-// GetResponseRules return rules to rewrite the response with. Currently not implemented.
-func (rule *classRule) GetResponseRules() []ResponseRule { return []ResponseRule{} }

plugin/rewrite/edns0.go

@@ -49,14 +49,14 @@ func setupEdns0Opt(r *dns.Msg) *dns.OPT {
 }
 
 // Rewrite will alter the request EDNS0 NSID option
-func (rule *edns0NsidRule) Rewrite(ctx context.Context, state request.Request) Result {
+func (rule *edns0NsidRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
 	o := setupEdns0Opt(state.Req)
 
 	for _, s := range o.Option {
 		if e, ok := s.(*dns.EDNS0_NSID); ok {
 			if rule.action == Replace || rule.action == Set {
 				e.Nsid = "" // make sure it is empty for request
-				return RewriteDone
+				return nil, RewriteDone
 			}
 		}
 	}
@@ -64,20 +64,17 @@ func (rule *edns0NsidRule) Rewrite(ctx context.Context, state request.Request) R
 	// add option if not found
 	if rule.action == Append || rule.action == Set {
 		o.Option = append(o.Option, &dns.EDNS0_NSID{Code: dns.EDNS0NSID, Nsid: ""})
-		return RewriteDone
+		return nil, RewriteDone
 	}
 
-	return RewriteIgnored
+	return nil, RewriteIgnored
 }
 
 // Mode returns the processing mode.
 func (rule *edns0NsidRule) Mode() string { return rule.mode }
 
-// GetResponseRules returns rules to rewrite the response with. Currently not implemented.
-func (rule *edns0NsidRule) GetResponseRules() []ResponseRule { return []ResponseRule{} }
-
 // Rewrite will alter the request EDNS0 local options.
-func (rule *edns0LocalRule) Rewrite(ctx context.Context, state request.Request) Result {
+func (rule *edns0LocalRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
 	o := setupEdns0Opt(state.Req)
 
 	for _, s := range o.Option {
@@ -85,7 +82,7 @@ func (rule *edns0LocalRule) Rewrite(ctx context.Context, state request.Request)
 			if rule.code == e.Code {
 				if rule.action == Replace || rule.action == Set {
 					e.Data = rule.data
-					return RewriteDone
+					return nil, RewriteDone
 				}
 			}
 		}
@@ -94,18 +91,15 @@ func (rule *edns0LocalRule) Rewrite(ctx context.Context, state request.Request)
 	// add option if not found
 	if rule.action == Append || rule.action == Set {
 		o.Option = append(o.Option, &dns.EDNS0_LOCAL{Code: rule.code, Data: rule.data})
-		return RewriteDone
+		return nil, RewriteDone
 	}
 
-	return RewriteIgnored
+	return nil, RewriteIgnored
 }
 
 // Mode returns the processing mode.
 func (rule *edns0LocalRule) Mode() string { return rule.mode }
 
-// GetResponseRules returns a rule to rewrite the response with. Currently not implemented.
-func (rule *edns0LocalRule) GetResponseRules() []ResponseRule { return []ResponseRule{} }
-
 // newEdns0Rule creates an EDNS0 rule of the appropriate type based on the args
 func newEdns0Rule(mode string, args ...string) (Rule, error) {
 	if len(args) < 2 {
@@ -222,10 +216,10 @@ func (rule *edns0VariableRule) ruleData(ctx context.Context, state request.Reque
 }
 
 // Rewrite will alter the request EDNS0 local options with specified variables.
-func (rule *edns0VariableRule) Rewrite(ctx context.Context, state request.Request) Result {
+func (rule *edns0VariableRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
 	data, err := rule.ruleData(ctx, state)
 	if err != nil || data == nil {
-		return RewriteIgnored
+		return nil, RewriteIgnored
 	}
 
 	o := setupEdns0Opt(state.Req)
@@ -234,9 +228,9 @@ func (rule *edns0VariableRule) Rewrite(ctx context.Context, state request.Reques
 			if rule.code == e.Code {
 				if rule.action == Replace || rule.action == Set {
 					e.Data = data
-					return RewriteDone
+					return nil, RewriteDone
 				}
-				return RewriteIgnored
+				return nil, RewriteIgnored
 			}
 		}
 	}
@@ -244,18 +238,15 @@ func (rule *edns0VariableRule) Rewrite(ctx context.Context, state request.Reques
 	// add option if not found
 	if rule.action == Append || rule.action == Set {
 		o.Option = append(o.Option, &dns.EDNS0_LOCAL{Code: rule.code, Data: data})
-		return RewriteDone
+		return nil, RewriteDone
 	}
 
-	return RewriteIgnored
+	return nil, RewriteIgnored
 }
 
 // Mode returns the processing mode.
 func (rule *edns0VariableRule) Mode() string { return rule.mode }
 
-// GetResponseRules returns rules to rewrite the response with. Currently not implemented.
-func (rule *edns0VariableRule) GetResponseRules() []ResponseRule { return []ResponseRule{} }
-
 func isValidVariable(variable string) bool {
 	switch variable {
 	case
@@ -333,17 +324,17 @@ func (rule *edns0SubnetRule) fillEcsData(state request.Request, ecs *dns.EDNS0_S
 }
 
 // Rewrite will alter the request EDNS0 subnet option.
-func (rule *edns0SubnetRule) Rewrite(ctx context.Context, state request.Request) Result {
+func (rule *edns0SubnetRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
 	o := setupEdns0Opt(state.Req)
 
 	for _, s := range o.Option {
 		if e, ok := s.(*dns.EDNS0_SUBNET); ok {
 			if rule.action == Replace || rule.action == Set {
 				if rule.fillEcsData(state, e) == nil {
-					return RewriteDone
+					return nil, RewriteDone
 				}
 			}
-			return RewriteIgnored
+			return nil, RewriteIgnored
 		}
 	}
 
@@ -352,19 +343,16 @@ func (rule *edns0SubnetRule) Rewrite(ctx context.Context, state request.Request)
 		opt := &dns.EDNS0_SUBNET{Code: dns.EDNS0SUBNET}
 		if rule.fillEcsData(state, opt) == nil {
 			o.Option = append(o.Option, opt)
-			return RewriteDone
+			return nil, RewriteDone
 		}
 	}
 
-	return RewriteIgnored
+	return nil, RewriteIgnored
 }
 
 // Mode returns the processing mode
 func (rule *edns0SubnetRule) Mode() string { return rule.mode }
 
-// GetResponseRules return rules to rewrite the response with. Currently not implemented.
-func (rule *edns0SubnetRule) GetResponseRules() []ResponseRule { return []ResponseRule{} }
-
 // These are all defined actions.
 const (
 	Replace = "replace"

plugin/rewrite/reverter.go

 package rewrite
 
 import (
-	"regexp"
-	"strconv"
-	"strings"
-
 	"github.com/miekg/dns"
 )
 
+// RevertPolicy controls the overall reverting process
+type RevertPolicy interface {
+	DoRevert() bool
+	DoQuestionRestore() bool
+}
+
+type revertPolicy struct {
+	noRevert  bool
+	noRestore bool
+}
+
+func (p revertPolicy) DoRevert() bool {
+	return !p.noRevert
+}
+
+func (p revertPolicy) DoQuestionRestore() bool {
+	return !p.noRestore
+}
+
+// NoRevertPolicy disables all response rewrite rules
+func NoRevertPolicy() RevertPolicy {
+	return revertPolicy{true, false}
+}
+
+// NoRestorePolicy disables the question restoration during the response rewrite
+func NoRestorePolicy() RevertPolicy {
+	return revertPolicy{false, true}
+}
+
+// NewRevertPolicy creates a new reverter policy by dynamically specifying all
+// options.
+func NewRevertPolicy(noRevert, noRestore bool) RevertPolicy {
+	return revertPolicy{noRestore: noRestore, noRevert: noRevert}
+}
+
 // ResponseRule contains a rule to rewrite a response with.
-type ResponseRule struct {
-	Active      bool
-	Type        string
-	Pattern     *regexp.Regexp
-	Replacement string
-	TTL         uint32
+type ResponseRule interface {
+	RewriteResponse(rr dns.RR)
 }
 
+// ResponseRules describes an ordered list of response rules to apply
+// after a name rewrite
+type ResponseRules = []ResponseRule
+
 // ResponseReverter reverses the operations done on the question section of a packet.
 // This is need because the client will otherwise disregards the response, i.e.
 // dig will complain with ';; Question section mismatch: got example.org/HINFO/IN'
 type ResponseReverter struct {
 	dns.ResponseWriter
 	originalQuestion dns.Question
-	ResponseRewrite  bool
-	ResponseRules    []ResponseRule
+	ResponseRules    ResponseRules
+	revertPolicy     RevertPolicy
 }
 
 // NewResponseReverter returns a pointer to a new ResponseReverter.
-func NewResponseReverter(w dns.ResponseWriter, r *dns.Msg) *ResponseReverter {
+func NewResponseReverter(w dns.ResponseWriter, r *dns.Msg, policy RevertPolicy) *ResponseReverter {
 	return &ResponseReverter{
 		ResponseWriter:   w,
 		originalQuestion: r.Question[0],
+		revertPolicy:     policy,
 	}
 }
 
@@ -40,61 +72,33 @@ func (r *ResponseReverter) WriteMsg(res1 *dns.Msg) error {
 	// Deep copy 'res' as to not (e.g). rewrite a message that's also stored in the cache.
 	res := res1.Copy()
 
-	res.Question[0] = r.originalQuestion
-	if r.ResponseRewrite {
+	if r.revertPolicy.DoQuestionRestore() {
+		res.Question[0] = r.originalQuestion
+	}
+	if len(r.ResponseRules) > 0 {
 		for _, rr := range res.Ns {
-			rewriteResourceRecord(res, rr, r)
+			r.rewriteResourceRecord(res, rr)
 		}
-
 		for _, rr := range res.Answer {
-			rewriteResourceRecord(res, rr, r)
+			r.rewriteResourceRecord(res, rr)
 		}
-
 		for _, rr := range res.Extra {
-			rewriteResourceRecord(res, rr, r)
+			r.rewriteResourceRecord(res, rr)
 		}
-
 	}
 	return r.ResponseWriter.WriteMsg(res)
 }
 
-func rewriteResourceRecord(res *dns.Msg, rr dns.RR, r *ResponseReverter) {
-	var (
-		isNameRewritten  bool
-		isTTLRewritten   bool
-		isValueRewritten bool
-		name             = rr.Header().Name
-		ttl              = rr.Header().Ttl
-		value            string
-	)
-
+func (r *ResponseReverter) rewriteResourceRecord(res *dns.Msg, rr dns.RR) {
 	for _, rule := range r.ResponseRules {
-		if rule.Type == "" {
-			rule.Type = "name"
-		}
-		switch rule.Type {
-		case "name":
-			rewriteString(rule, &name, &isNameRewritten)
-		case "value":
-			value = getRecordValueForRewrite(rr)
-			if value != "" {
-				rewriteString(rule, &value, &isValueRewritten)
-			}
-		case "ttl":
-			ttl = rule.TTL
-			isTTLRewritten = true
-		}
+		rule.RewriteResponse(rr)
 	}
+}
 
-	if isNameRewritten {
-		rr.Header().Name = name
-	}
-	if isTTLRewritten {
-		rr.Header().Ttl = ttl
-	}
-	if isValueRewritten {
-		setRewrittenRecordValue(rr, value)
-	}
+// Write is a wrapper that records the size of the message that gets written.
+func (r *ResponseReverter) Write(buf []byte) (int, error) {
+	n, err := r.ResponseWriter.Write(buf)
+	return n, err
 }
 
 func getRecordValueForRewrite(rr dns.RR) (name string) {
@@ -136,24 +140,3 @@ func setRewrittenRecordValue(rr dns.RR, value string) {
 		rr.(*dns.SOA).Ns = value
 	}
 }
-
-func rewriteString(rule ResponseRule, str *string, isStringRewritten *bool) {
-	regexGroups := rule.Pattern.FindStringSubmatch(*str)
-	if len(regexGroups) == 0 {
-		return
-	}
-	s := rule.Replacement
-	for groupIndex, groupValue := range regexGroups {
-		groupIndexStr := "{" + strconv.Itoa(groupIndex) + "}"
-		s = strings.Replace(s, groupIndexStr, groupValue, -1)
-	}
-
-	*isStringRewritten = true
-	*str = s
-}
-
-// Write is a wrapper that records the size of the message that gets written.
-func (r *ResponseReverter) Write(buf []byte) (int, error) {
-	n, err := r.ResponseWriter.Write(buf)
-	return n, err
-}

plugin/rewrite/rewrite.go

@@ -31,40 +31,34 @@ const (
 
 // Rewrite is a plugin to rewrite requests internally before being handled.
 type Rewrite struct {
-	Next     plugin.Handler
-	Rules    []Rule
-	noRevert bool
+	Next  plugin.Handler
+	Rules []Rule
+	RevertPolicy
 }
 
 // ServeDNS implements the plugin.Handler interface.
 func (rw Rewrite) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
-	wr := NewResponseReverter(w, r)
+	if rw.RevertPolicy == nil {
+		rw.RevertPolicy = NewRevertPolicy(false, false)
+	}
+	wr := NewResponseReverter(w, r, rw.RevertPolicy)
 	state := request.Request{W: w, Req: r}
 
 	for _, rule := range rw.Rules {
-		switch result := rule.Rewrite(ctx, state); result {
-		case RewriteDone:
+		respRules, result := rule.Rewrite(ctx, state)
+		if result == RewriteDone {
 			if _, ok := dns.IsDomainName(state.Req.Question[0].Name); !ok {
 				err := fmt.Errorf("invalid name after rewrite: %s", state.Req.Question[0].Name)
 				state.Req.Question[0] = wr.originalQuestion
 				return dns.RcodeServerFailure, err
 			}
-			for _, respRule := range rule.GetResponseRules() {
-				if respRule.Active {
-					wr.ResponseRewrite = true
-					wr.ResponseRules = append(wr.ResponseRules, respRule)
-				}
-			}
+			wr.ResponseRules = append(wr.ResponseRules, respRules...)
 			if rule.Mode() == Stop {
-				if rw.noRevert {
-					return plugin.NextOrFailure(rw.Name(), rw.Next, ctx, w, r)
-				}
-				return plugin.NextOrFailure(rw.Name(), rw.Next, ctx, wr, r)
+				break
 			}
-		case RewriteIgnored:
 		}
 	}
-	if rw.noRevert || len(wr.ResponseRules) == 0 {
+	if !rw.RevertPolicy.DoRevert() || len(wr.ResponseRules) == 0 {
 		return plugin.NextOrFailure(rw.Name(), rw.Next, ctx, w, r)
 	}
 	return plugin.NextOrFailure(rw.Name(), rw.Next, ctx, wr, r)
@@ -76,11 +70,9 @@ func (rw Rewrite) Name() string { return "rewrite" }
 // Rule describes a rewrite rule.
 type Rule interface {
 	// Rewrite rewrites the current request.
-	Rewrite(ctx context.Context, state request.Request) Result
+	Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result)
 	// Mode returns the processing mode stop or continue.
 	Mode() string
-	// GetResponseRules returns rules to rewrite response with, if any.
-	GetResponseRules() []ResponseRule
 }
 
 func newRule(args ...string) (Rule, error) {

plugin/rewrite/rewrite_test.go

@@ -3,6 +3,7 @@ package rewrite
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"reflect"
 	"testing"
 
@@ -16,6 +17,11 @@ import (
 )
 
 func msgPrinter(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+	if len(r.Answer) == 0 {
+		r.Answer = []dns.RR{
+			test.A(fmt.Sprintf("%s  5   IN  A  10.0.0.1", r.Question[0].Name)),
+		}
+	}
 	w.WriteMsg(r)
 	return 0, nil
 }
@@ -44,7 +50,7 @@ func TestNewRule(t *testing.T) {
 		{[]string{"name", "regex", "(cdns)\\.(core)\\.(rocks)", "{2}.{1}.{3}", "answer", "ttl", "(core)\\.(cdns)\\.(rocks)", "{2}.{1}.{3}"}, true, nil},
 		{[]string{"name", "regex", "(ddns)\\.(core)\\.(rocks)", "{2}.{1}.{3}", "answer", "name", "\xecore\\.(ddns)\\.(rocks)", "{2}.{1}.{3}"}, true, nil},
 		{[]string{"name", "regex", "\xedns\\.(core)\\.(rocks)", "{2}.{1}.{3}", "answer", "name", "(core)\\.(edns)\\.(rocks)", "{2}.{1}.{3}"}, true, nil},
-		{[]string{"name", "substring", "fcore.dns.rocks", "dns.fcore.rocks", "answer", "name", "(fcore)\\.(dns)\\.(rocks)", "{2}.{1}.{3}"}, true, nil},
+		{[]string{"name", "substring", "fcore.dns.rocks", "dns.fcore.rocks", "answer", "name", "(fcore)\\.(dns)\\.(rocks)", "{2}.{1}.{3}"}, false, reflect.TypeOf(&substringNameRule{})},
 		{[]string{"name", "substring", "a.com", "b.com", "c.com"}, true, nil},
 		{[]string{"type"}, true, nil},
 		{[]string{"type", "a"}, true, nil},
@@ -185,9 +191,9 @@ func TestRewrite(t *testing.T) {
 	rules = append(rules, r)
 
 	rw := Rewrite{
-		Next:     plugin.HandlerFunc(msgPrinter),
-		Rules:    rules,
-		noRevert: true,
+		Next:         plugin.HandlerFunc(msgPrinter),
+		Rules:        rules,
+		RevertPolicy: NoRevertPolicy(),
 	}
 
 	tests := []struct {
@@ -248,8 +254,8 @@ func TestRewrite(t *testing.T) {
 
 func TestRewriteEDNS0Local(t *testing.T) {
 	rw := Rewrite{
-		Next:     plugin.HandlerFunc(msgPrinter),
-		noRevert: true,
+		Next:         plugin.HandlerFunc(msgPrinter),
+		RevertPolicy: NoRevertPolicy(),
 	}
 
 	tests := []struct {
@@ -336,9 +342,9 @@ func TestEdns0LocalMultiRule(t *testing.T) {
 	rules = append(rules, r)
 
 	rw := Rewrite{
-		Next:     plugin.HandlerFunc(msgPrinter),
-		Rules:    rules,
-		noRevert: true,
+		Next:         plugin.HandlerFunc(msgPrinter),
+		Rules:        rules,
+		RevertPolicy: NoRevertPolicy(),
 	}
 
 	tests := []struct {
@@ -447,8 +453,8 @@ func (tp testProvider) Metadata(ctx context.Context, state request.Request) cont
 
 func TestRewriteEDNS0LocalVariable(t *testing.T) {
 	rw := Rewrite{
-		Next:     plugin.HandlerFunc(msgPrinter),
-		noRevert: true,
+		Next:         plugin.HandlerFunc(msgPrinter),
+		RevertPolicy: NoRevertPolicy(),
 	}
 
 	expectedMetadata := []metadata.Provider{
@@ -569,8 +575,8 @@ func TestRewriteEDNS0LocalVariable(t *testing.T) {
 
 func TestRewriteEDNS0Subnet(t *testing.T) {
 	rw := Rewrite{
-		Next:     plugin.HandlerFunc(msgPrinter),
-		noRevert: true,
+		Next:         plugin.HandlerFunc(msgPrinter),
+		RevertPolicy: NoRevertPolicy(),
 	}
 
 	tests := []struct {

plugin/rewrite/setup_test.go

@@ -44,12 +44,12 @@ func TestParse(t *testing.T) {
 		`rewrite stop {
     name regex foo bar
     answer name bar foo
-    name baz qux
+    name baz
 }`)
 	_, err = rewriteParse(c)
 	if err == nil {
 		t.Errorf("Expected error but got success for invalid response rewrite")
-	} else if !strings.Contains(err.Error(), "must consist only of") {
+	} else if !strings.Contains(err.Error(), "2 arguments required") {
 		t.Errorf("Got wrong error for invalid response rewrite: %v", err.Error())
 	}
 

plugin/rewrite/ttl.go

@@ -9,81 +9,91 @@ import (
 
 	"github.com/coredns/coredns/plugin"
 	"github.com/coredns/coredns/request"
-	//"github.com/miekg/dns"
+
+	"github.com/miekg/dns"
 )
 
+type ttlResponseRule struct {
+	TTL uint32
+}
+
+func (r *ttlResponseRule) RewriteResponse(rr dns.RR) {
+	rr.Header().Ttl = r.TTL
+}
+
+type ttlRuleBase struct {
+	nextAction string
+	response   ttlResponseRule
+}
+
+func newTTLRuleBase(nextAction string, ttl uint32) ttlRuleBase {
+	return ttlRuleBase{
+		nextAction: nextAction,
+		response:   ttlResponseRule{TTL: ttl},
+	}
+}
+
+func (rule *ttlRuleBase) responseRule(match bool) (ResponseRules, Result) {
+	if match {
+		return ResponseRules{&rule.response}, RewriteDone
+	}
+	return nil, RewriteIgnored
+}
+
+// Mode returns the processing nextAction
+func (rule *ttlRuleBase) Mode() string { return rule.nextAction }
+
 type exactTTLRule struct {
-	NextAction    string
-	From          string
-	ResponseRules []ResponseRule
+	ttlRuleBase
+	From string
 }
 
 type prefixTTLRule struct {
-	NextAction    string
-	Prefix        string
-	ResponseRules []ResponseRule
+	ttlRuleBase
+	Prefix string
 }
 
 type suffixTTLRule struct {
-	NextAction    string
-	Suffix        string
-	ResponseRules []ResponseRule
+	ttlRuleBase
+	Suffix string
 }
 
 type substringTTLRule struct {
-	NextAction    string
-	Substring     string
-	ResponseRules []ResponseRule
+	ttlRuleBase
+	Substring string
 }
 
 type regexTTLRule struct {
-	NextAction    string
-	Pattern       *regexp.Regexp
-	ResponseRules []ResponseRule
+	ttlRuleBase
+	Pattern *regexp.Regexp
 }
 
 // Rewrite rewrites the current request based upon exact match of the name
 // in the question section of the request.
-func (rule *exactTTLRule) Rewrite(ctx context.Context, state request.Request) Result {
-	if rule.From == state.Name() {
-		return RewriteDone
-	}
-	return RewriteIgnored
+func (rule *exactTTLRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
+	return rule.responseRule(rule.From == state.Name())
 }
 
 // Rewrite rewrites the current request when the name begins with the matching string.
-func (rule *prefixTTLRule) Rewrite(ctx context.Context, state request.Request) Result {
-	if strings.HasPrefix(state.Name(), rule.Prefix) {
-		return RewriteDone
-	}
-	return RewriteIgnored
+func (rule *prefixTTLRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
+	return rule.responseRule(strings.HasPrefix(state.Name(), rule.Prefix))
 }
 
 // Rewrite rewrites the current request when the name ends with the matching string.
-func (rule *suffixTTLRule) Rewrite(ctx context.Context, state request.Request) Result {
-	if strings.HasSuffix(state.Name(), rule.Suffix) {
-		return RewriteDone
-	}
-	return RewriteIgnored
+func (rule *suffixTTLRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
+	return rule.responseRule(strings.HasSuffix(state.Name(), rule.Suffix))
 }
 
 // Rewrite rewrites the current request based upon partial match of the
 // name in the question section of the request.
-func (rule *substringTTLRule) Rewrite(ctx context.Context, state request.Request) Result {
-	if strings.Contains(state.Name(), rule.Substring) {
-		return RewriteDone
-	}
-	return RewriteIgnored
+func (rule *substringTTLRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
+	return rule.responseRule(strings.Contains(state.Name(), rule.Substring))
 }
 
 // Rewrite rewrites the current request when the name in the question
 // section of the request matches a regular expression.
-func (rule *regexTTLRule) Rewrite(ctx context.Context, state request.Request) Result {
-	regexGroups := rule.Pattern.FindStringSubmatch(state.Name())
-	if len(regexGroups) == 0 {
-		return RewriteIgnored
-	}
-	return RewriteDone
+func (rule *regexTTLRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
+	return rule.responseRule(len(rule.Pattern.FindStringSubmatch(state.Name())) != 0)
 }
 
 // newTTLRule creates a name matching rule based on exact, partial, or regex match
@@ -106,43 +116,23 @@ func newTTLRule(nextAction string, args ...string) (Rule, error) {
 		switch strings.ToLower(args[0]) {
 		case ExactMatch:
 			return &exactTTLRule{
-				nextAction,
+				newTTLRuleBase(nextAction, ttl),
 				plugin.Name(args[1]).Normalize(),
-				[]ResponseRule{{
-					Active: true,
-					Type:   "ttl",
-					TTL:    ttl,
-				}},
 			}, nil
 		case PrefixMatch:
 			return &prefixTTLRule{
-				nextAction,
+				newTTLRuleBase(nextAction, ttl),
 				plugin.Name(args[1]).Normalize(),
-				[]ResponseRule{{
-					Active: true,
-					Type:   "ttl",
-					TTL:    ttl,
-				}},
 			}, nil
 		case SuffixMatch:
 			return &suffixTTLRule{
-				nextAction,
+				newTTLRuleBase(nextAction, ttl),
 				plugin.Name(args[1]).Normalize(),
-				[]ResponseRule{{
-					Active: true,
-					Type:   "ttl",
-					TTL:    ttl,
-				}},
 			}, nil
 		case SubstringMatch:
 			return &substringTTLRule{
-				nextAction,
+				newTTLRuleBase(nextAction, ttl),
 				plugin.Name(args[1]).Normalize(),
-				[]ResponseRule{{
-					Active: true,
-					Type:   "ttl",
-					TTL:    ttl,
-				}},
 			}, nil
 		case RegexMatch:
 			regexPattern, err := regexp.Compile(args[1])
@@ -150,13 +140,8 @@ func newTTLRule(nextAction string, args ...string) (Rule, error) {
 				return nil, fmt.Errorf("invalid regex pattern in a ttl rule: %s", args[1])
 			}
 			return &regexTTLRule{
-				nextAction,
+				newTTLRuleBase(nextAction, ttl),
 				regexPattern,
-				[]ResponseRule{{
-					Active: true,
-					Type:   "ttl",
-					TTL:    ttl,
-				}},
 			}, nil
 		default:
 			return nil, fmt.Errorf("ttl rule supports only exact, prefix, suffix, substring, and regex name matching")
@@ -166,48 +151,11 @@ func newTTLRule(nextAction string, args ...string) (Rule, error) {
 		return nil, fmt.Errorf("many few arguments for a ttl rule")
 	}
 	return &exactTTLRule{
-		nextAction,
+		newTTLRuleBase(nextAction, ttl),
 		plugin.Name(args[0]).Normalize(),
-		[]ResponseRule{{
-			Active: true,
-			Type:   "ttl",
-			TTL:    ttl,
-		}},
 	}, nil
 }
 
-// Mode returns the processing nextAction
-func (rule *exactTTLRule) Mode() string     { return rule.NextAction }
-func (rule *prefixTTLRule) Mode() string    { return rule.NextAction }
-func (rule *suffixTTLRule) Mode() string    { return rule.NextAction }
-func (rule *substringTTLRule) Mode() string { return rule.NextAction }
-func (rule *regexTTLRule) Mode() string     { return rule.NextAction }
-
-// GetResponseRules returns rules to rewrite the response with. Currently not implemented.
-func (rule *exactTTLRule) GetResponseRules() []ResponseRule {
-	return rule.ResponseRules
-}
-
-// GetResponseRules returns rules to rewrite the response with. Currently not implemented.
-func (rule *prefixTTLRule) GetResponseRules() []ResponseRule {
-	return rule.ResponseRules
-}
-
-// GetResponseRules returns rules to rewrite the response with. Currently not implemented.
-func (rule *suffixTTLRule) GetResponseRules() []ResponseRule {
-	return rule.ResponseRules
-}
-
-// GetResponseRules returns rules to rewrite the response with. Currently not implemented.
-func (rule *substringTTLRule) GetResponseRules() []ResponseRule {
-	return rule.ResponseRules
-}
-
-// GetResponseRules returns rules to rewrite the response with.
-func (rule *regexTTLRule) GetResponseRules() []ResponseRule {
-	return rule.ResponseRules
-}
-
 // validTTL returns true if v is valid TTL value.
 func isValidTTL(v string) (uint32, bool) {
 	i, err := strconv.Atoi(v)

plugin/rewrite/ttl_test.go

@@ -121,9 +121,8 @@ func doTTLTests(rules []Rule, t *testing.T) {
 		m.Question[0].Qclass = dns.ClassINET
 		m.Answer = tc.answer
 		rw := Rewrite{
-			Next:     plugin.HandlerFunc(msgPrinter),
-			Rules:    rules,
-			noRevert: false,
+			Next:  plugin.HandlerFunc(msgPrinter),
+			Rules: rules,
 		}
 		rec := dnstest.NewRecorder(&test.ResponseWriter{})
 		rw.ServeDNS(ctx, rec, m)

plugin/rewrite/type.go

@@ -31,18 +31,15 @@ func newTypeRule(nextAction string, args ...string) (Rule, error) {
 }
 
 // Rewrite rewrites the current request.
-func (rule *typeRule) Rewrite(ctx context.Context, state request.Request) Result {
+func (rule *typeRule) Rewrite(ctx context.Context, state request.Request) (ResponseRules, Result) {
 	if rule.fromType > 0 && rule.toType > 0 {
 		if state.QType() == rule.fromType {
 			state.Req.Question[0].Qtype = rule.toType
-			return RewriteDone
+			return nil, RewriteDone
 		}
 	}
-	return RewriteIgnored
+	return nil, RewriteIgnored
 }
 
 // Mode returns the processing mode.
 func (rule *typeRule) Mode() string { return rule.nextAction }
-
-// GetResponseRules return rules to rewrite the response with. Currently not implemented.
-func (rule *typeRule) GetResponseRules() []ResponseRule { return []ResponseRule{} }