plugin/cache: fix removing OPT (#4190)

By checking state.Do() were are checking if the request had DO, but
we are _always_ adding Do now - do we need to save the DO from the
ORIGINAL request, which must be done in the ResponseWriter.

Also skip OPT records in filterDNSSEC as we can't set the TTL on those
records, this prevents writing a number to OPT's MBZ.

Note none of the tests have changed and still PASS. This is due to
the fact that CoreDNSServerAndPorts isn't a full server as we start in
main, it lacks the scrubwriter for instance. This is not bad per se, but
should be documented in the test code.
Signed-off-by: Miek Gieben <miek@miek.nl>

plugin/cache/cache.go

@@ -104,6 +104,7 @@ type ResponseWriter struct {
 	state  request.Request
 	server string // Server handling the request.
 
+	do         bool // When true the original request had the DO bit set.
 	prefetch   bool // When true write nothing back to the client.
 	remoteAddr net.Addr
 }
@@ -176,13 +177,12 @@ func (w *ResponseWriter) WriteMsg(res *dns.Msg) error {
 		return nil
 	}
 
-	do := w.state.Do()
 	// Apply capped TTL to this reply to avoid jarring TTL experience 1799 -> 8 (e.g.)
 	// We also may need to filter out DNSSEC records, see toMsg() for similar code.
 	ttl := uint32(duration.Seconds())
-	resc.Answer = filterRRSlice(resc.Answer, ttl, do, false)
-	resc.Ns = filterRRSlice(resc.Ns, ttl, do, false)
-	resc.Extra = filterRRSlice(resc.Extra, ttl, do, false)
+	resc.Answer = filterRRSlice(resc.Answer, ttl, w.do, false)
+	resc.Ns = filterRRSlice(resc.Ns, ttl, w.do, false)
+	resc.Extra = filterRRSlice(resc.Extra, ttl, w.do, false)
 
 	return w.ResponseWriter.WriteMsg(resc)
 }

plugin/cache/dnssec.go

@@ -31,6 +31,9 @@ func filterRRSlice(rrs []dns.RR, ttl uint32, do, dup bool) []dns.RR {
 		if !do && isDNSSEC(r) {
 			continue
 		}
+		if r.Header().Rrtype == dns.TypeOPT {
+			continue
+		}
 		r.Header().Ttl = ttl
 		if dup {
 			rs[j] = dns.Copy(r)

plugin/cache/handler.go

@@ -41,7 +41,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 		if !do {
 			setDo(r)
 		}
-		crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server}
+		crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do}
 		return plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r)
 	}
 	if ttl < 0 {
@@ -53,7 +53,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 			if !do {
 				setDo(r)
 			}
-			crr := &ResponseWriter{Cache: c, state: state, server: server, prefetch: true, remoteAddr: w.LocalAddr()}
+			crr := &ResponseWriter{Cache: c, state: state, server: server, prefetch: true, remoteAddr: w.LocalAddr(), do: do}
 			plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r)
 		}()
 	}