Fix EDNS0 compliance (#2357)

* Fix EDNS0 compliance

Do SizeAndDo in the server (ScrubWriter) and remove all uses of this
from the plugins. Also *always* do it. This is to get into compliance
for https://dnsflagday.net/.

The pkg/edns0 now exports the EDNS0 options we understand; this is
exported to allow plugins add things there. The *rewrite* plugin used
this to add custom EDNS0 option codes that the server needs to
understand.

This also needs a new release of miekg/dns because it triggered a
race-condition that was basicly there forever.

See:
* https://github.com/miekg/dns/issues/857
* https://github.com/miekg/dns/pull/859

Running a test instance and pointing the https://ednscomp.isc.org/ednscomp
to it shows the tests are now fixed:

~~~
EDNS Compliance Tester
Checking: 'miek.nl' as at 2018-12-01T17:53:15Z

miek.nl. @147.75.204.203 (drone.coredns.io.): dns=ok edns=ok edns1=ok edns@512=ok ednsopt=ok edns1opt=ok do=ok ednsflags=ok docookie=ok edns512tcp=ok optlist=ok
miek.nl. @2604:1380:2002:a000::1 (drone.coredns.io.): dns=ok edns=ok edns1=ok edns@512=ok ednsopt=ok edns1opt=ok do=ok ednsflags=ok docookie=ok edns512tcp=ok optlist=ok

All Ok
Codes
ok - test passed.
~~~

Signed-off-by: Miek Gieben <miek@miek.nl>

Signed-off-by: Miek Gieben <miek@miek.nl>

* typos in comments

Signed-off-by: Miek Gieben <miek@miek.nl>

10 files changed, 37 insertions, 8 deletions

diff --git a/plugin/backend_lookup.go b/plugin/backend_lookup.go
index 090f36215..95ec467fd 100644
--- a/plugin/backend_lookup.go
+++ b/plugin/backend_lookup.go
@@ -425,7 +425,6 @@ func BackendError(b ServiceBackend, zone string, rcode int, state request.Reques
 	m.Authoritative, m.RecursionAvailable = true, true
 	m.Ns, _ = SOA(b, zone, state, opt)
 
-	state.SizeAndDo(m)
 	state.W.WriteMsg(m)
 	// Return success as the rcode to signal we have written to the client.
 	return dns.RcodeSuccess, err
diff --git a/plugin/chaos/chaos.go b/plugin/chaos/chaos.go
index 60b002bdb..a66c5f003 100644
--- a/plugin/chaos/chaos.go
+++ b/plugin/chaos/chaos.go
@@ -46,7 +46,6 @@ func (c Chaos) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 		}
 		m.Answer = []dns.RR{&dns.TXT{Hdr: hdr, Txt: []string{trim(hostname)}}}
 	}
-	state.SizeAndDo(m)
 	w.WriteMsg(m)
 	return 0, nil
 }
diff --git a/plugin/dnssec/handler.go b/plugin/dnssec/handler.go
index c8bddc01c..6153bf331 100644
--- a/plugin/dnssec/handler.go
+++ b/plugin/dnssec/handler.go
@@ -33,7 +33,6 @@ func (d Dnssec) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 			if qname == z {
 				resp := d.getDNSKEY(state, z, do, server)
 				resp.Authoritative = true
-				state.SizeAndDo(resp)
 				w.WriteMsg(resp)
 				return dns.RcodeSuccess, nil
 			}
diff --git a/plugin/dnssec/handler_test.go b/plugin/dnssec/handler_test.go
index 35444eecb..8e997bca8 100644
--- a/plugin/dnssec/handler_test.go
+++ b/plugin/dnssec/handler_test.go
@@ -26,7 +26,7 @@ var dnssecTestCases = []test.Case{
 			test.DNSKEY("miek.nl.	3600	IN	DNSKEY	257 3 13 0J8u0XJ9GNGFEBXuAmLu04taHG4"),
 			test.RRSIG("miek.nl.	3600	IN	RRSIG	DNSKEY 13 2 3600 20160503150844 20160425120844 18512 miek.nl. Iw/kNOyM"),
 		},
-		Extra: []dns.RR{test.OPT(4096, true)},
+		/* Extra: []dns.RR{test.OPT(4096, true)}, this has moved to the server and can't be test here */
 	},
 }
 
diff --git a/plugin/erratic/erratic.go b/plugin/erratic/erratic.go
index f60e605d1..3b22e225d 100644
--- a/plugin/erratic/erratic.go
+++ b/plugin/erratic/erratic.go
@@ -97,7 +97,6 @@ func (e *Erratic) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg
 		time.Sleep(e.duration)
 	}
 
-	state.SizeAndDo(m)
 	w.WriteMsg(m)
 
 	return 0, nil
diff --git a/plugin/file/file.go b/plugin/file/file.go
index ee4d64da0..57ce3fd17 100644
--- a/plugin/file/file.go
+++ b/plugin/file/file.go
@@ -51,7 +51,6 @@ func (f File) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (i
 			m := new(dns.Msg)
 			m.SetReply(r)
 			m.Authoritative, m.RecursionAvailable = true, true
-			state.SizeAndDo(m)
 			w.WriteMsg(m)
 
 			log.Infof("Notify from %s for %s: checking transfer", state.IP(), zone)
diff --git a/plugin/log/log.go b/plugin/log/log.go
index 98f00d762..d963a134f 100644
--- a/plugin/log/log.go
+++ b/plugin/log/log.go
@@ -43,7 +43,6 @@ func (l Logger) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 			} else {
 				answer := new(dns.Msg)
 				answer.SetRcode(r, rc)
-				state.SizeAndDo(answer)
 
 				vars.Report(ctx, state, vars.Dropped, rcode.ToString(rc), answer.Len(), time.Now())
 
diff --git a/plugin/pkg/edns/edns.go b/plugin/pkg/edns/edns.go
index 3f0ea5e16..e99fdfb0a 100644
--- a/plugin/pkg/edns/edns.go
+++ b/plugin/pkg/edns/edns.go
@@ -3,10 +3,37 @@ package edns
 
 import (
 	"errors"
+	"sync"
 
 	"github.com/miekg/dns"
 )
 
+var sup = &supported{m: make(map[uint16]struct{})}
+
+type supported struct {
+	m map[uint16]struct{}
+	sync.RWMutex
+}
+
+// SetSupportedOption adds a new supported option the set of EDNS0 options that we support. Plugins typically call
+// this in their setup code to signal support for a new option.
+// By default we support:
+// dns.EDNS0NSID, dns.EDNS0EXPIRE, dns.EDNS0COOKIE, dns.EDNS0TCPKEEPALIVE, dns.EDNS0PADDING. These
+// values are not in this map and checked directly in the server.
+func SetSupportedOption(option uint16) {
+	sup.Lock()
+	sup.m[option] = struct{}{}
+	sup.Unlock()
+}
+
+// SupportedOption returns true if the option code is supported as an extra EDNS0 option.
+func SupportedOption(option uint16) bool {
+	sup.RLock()
+	_, ok := sup.m[option]
+	sup.RUnlock()
+	return ok
+}
+
 // Version checks the EDNS version in the request. If error
 // is nil everything is OK and we can invoke the plugin. If non-nil, the
 // returned Msg is valid to be returned to the client (and should). For some
diff --git a/plugin/rewrite/edns0.go b/plugin/rewrite/edns0.go
index e97f04d95..34aaf3d67 100644
--- a/plugin/rewrite/edns0.go
+++ b/plugin/rewrite/edns0.go
@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/coredns/coredns/plugin/metadata"
+	"github.com/coredns/coredns/plugin/pkg/edns"
 	"github.com/coredns/coredns/request"
 
 	"github.com/miekg/dns"
@@ -159,6 +160,10 @@ func newEdns0LocalRule(mode, action, code, data string) (*edns0LocalRule, error)
 			return nil, err
 		}
 	}
+
+	// Add this code to the ones the server supports.
+	edns.SetSupportedOption(uint16(c))
+
 	return &edns0LocalRule{mode: mode, action: action, code: uint16(c), data: decoded}, nil
 }
 
@@ -172,6 +177,10 @@ func newEdns0VariableRule(mode, action, code, variable string) (*edns0VariableRu
 	if !isValidVariable(variable) {
 		return nil, fmt.Errorf("unsupported variable name %q", variable)
 	}
+
+	// Add this code to the ones the server supports.
+	edns.SetSupportedOption(uint16(c))
+
 	return &edns0VariableRule{mode: mode, action: action, code: uint16(c), variable: variable}, nil
 }
 
diff --git a/plugin/whoami/whoami.go b/plugin/whoami/whoami.go
index b2ba25e5e..155880eae 100644
--- a/plugin/whoami/whoami.go
+++ b/plugin/whoami/whoami.go
@@ -49,7 +49,6 @@ func (wh Whoami) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 
 	a.Extra = []dns.RR{rr, srv}
 
-	state.SizeAndDo(a)
 	w.WriteMsg(a)
 
 	return 0, nil