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>

4 files changed, 42 insertions, 11 deletions

diff --git a/request/edns0.go b/request/edns0.go
new file mode 100644
index 000000000..89eb6b468
--- /dev/null
+++ b/request/edns0.go
@@ -0,0 +1,31 @@
+package request
+
+import (
+	"github.com/coredns/coredns/plugin/pkg/edns"
+
+	"github.com/miekg/dns"
+)
+
+func supportedOptions(o []dns.EDNS0) []dns.EDNS0 {
+	var supported = make([]dns.EDNS0, 0, 3)
+	// For as long as possible try avoid looking up in the map, because that need an Rlock.
+	for _, opt := range o {
+		switch code := opt.Option(); code {
+		case dns.EDNS0NSID:
+			fallthrough
+		case dns.EDNS0EXPIRE:
+			fallthrough
+		case dns.EDNS0COOKIE:
+			fallthrough
+		case dns.EDNS0TCPKEEPALIVE:
+			fallthrough
+		case dns.EDNS0PADDING:
+			supported = append(supported, opt)
+		default:
+			if edns.SupportedOption(code) {
+				supported = append(supported, opt)
+			}
+		}
+	}
+	return supported
+}
diff --git a/request/request.go b/request/request.go
index 105cd8528..52bf8629b 100644
--- a/request/request.go
+++ b/request/request.go
@@ -194,7 +194,7 @@ func (r *Request) Size() int {
 // SizeAndDo adds an OPT record that the reflects the intent from request.
 // The returned bool indicated if an record was found and normalised.
 func (r *Request) SizeAndDo(m *dns.Msg) bool {
-	o := r.Req.IsEdns0() // TODO(miek): speed this up
+	o := r.Req.IsEdns0()
 	if o == nil {
 		return false
 	}
@@ -208,6 +208,10 @@ func (r *Request) SizeAndDo(m *dns.Msg) bool {
 		mo.SetUDPSize(o.UDPSize())
 		mo.Hdr.Ttl &= 0xff00 // clear flags
 
+		if len(o.Option) > 0 {
+			o.Option = supportedOptions(o.Option)
+		}
+
 		if odo {
 			mo.SetDo()
 		}
@@ -219,6 +223,10 @@ func (r *Request) SizeAndDo(m *dns.Msg) bool {
 	o.SetVersion(0)
 	o.Hdr.Ttl &= 0xff00 // clear flags
 
+	if len(o.Option) > 0 {
+		o.Option = supportedOptions(o.Option)
+	}
+
 	if odo {
 		o.SetDo()
 	}
@@ -305,7 +313,6 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
 	}
 
 	if rl <= size {
-		r.SizeAndDo(reply)
 		return reply
 	}
 
@@ -341,7 +348,6 @@ func (r *Request) Scrub(reply *dns.Msg) *dns.Msg {
 		// this extra m-1 step does make it fit in the client's buffer however.
 	}
 
-	r.SizeAndDo(reply)
 	reply.Truncated = true
 	return reply
 }
diff --git a/request/request_test.go b/request/request_test.go
index 5e814f76e..4411c6a82 100644
--- a/request/request_test.go
+++ b/request/request_test.go
@@ -123,10 +123,6 @@ func TestRequestScrubExtraEdns0(t *testing.T) {
 	if reply.Truncated {
 		t.Errorf("Want scrub to not set truncated bit")
 	}
-	opt := reply.Extra[len(reply.Extra)-1]
-	if opt.Header().Rrtype != dns.TypeOPT {
-		t.Errorf("Last RR must be OPT record")
-	}
 }
 
 func TestRequestScrubExtraRegression(t *testing.T) {
@@ -153,10 +149,6 @@ func TestRequestScrubExtraRegression(t *testing.T) {
 	if reply.Truncated {
 		t.Errorf("Want scrub to not set truncated bit")
 	}
-	opt := reply.Extra[len(reply.Extra)-1]
-	if opt.Header().Rrtype != dns.TypeOPT {
-		t.Errorf("Last RR must be OPT record")
-	}
 }
 
 func TestTruncation(t *testing.T) {
diff --git a/request/writer.go b/request/writer.go
index ffbbe93e3..67be53ebb 100644
--- a/request/writer.go
+++ b/request/writer.go
@@ -15,6 +15,8 @@ func NewScrubWriter(req *dns.Msg, w dns.ResponseWriter) *ScrubWriter { return &S
 // scrub on the message m and will then write it to the client.
 func (s *ScrubWriter) WriteMsg(m *dns.Msg) error {
 	state := Request{Req: s.req, W: s.ResponseWriter}
+
 	n := state.Scrub(m)
+	state.SizeAndDo(n)
 	return s.ResponseWriter.WriteMsg(n)
 }