retain response AD bit if requestor's AD bit was set (#5191)

Signed-off-by: Chris O'Haver <cohaver@infoblox.com>

plugin/cache/cache.go

@@ -109,6 +109,7 @@ type ResponseWriter struct {
 	server string // Server handling the request.
 
 	do         bool // When true the original request had the DO bit set.
+	ad         bool // When true the original request had the AD bit set.
 	prefetch   bool // When true write nothing back to the client.
 	remoteAddr net.Addr
 }
@@ -185,8 +186,10 @@ func (w *ResponseWriter) WriteMsg(res *dns.Msg) error {
 	res.Ns = filterRRSlice(res.Ns, ttl, w.do, false)
 	res.Extra = filterRRSlice(res.Extra, ttl, w.do, false)
 
-	if !w.do {
+	if !w.do && !w.ad {
-		res.AuthenticatedData = false // unset AD bit if client is not OK with DNSSEC
+		// unset AD bit if requester is not OK with DNSSEC
+		// But retain AD bit if requester set the AD bit in the request, per RFC6840 5.7-5.8
+		res.AuthenticatedData = false
 	}
 
 	return w.ResponseWriter.WriteMsg(res)

plugin/cache/cache_test.go

@@ -217,7 +217,7 @@ func TestCache(t *testing.T) {
 		}
 
 		if ok {
-			resp := i.toMsg(m, time.Now().UTC(), state.Do())
+			resp := i.toMsg(m, time.Now().UTC(), state.Do(), m.AuthenticatedData)
 
 			if err := test.Header(tc.Case, resp); err != nil {
 				t.Logf("Cache %v", resp)

plugin/cache/handler.go

@@ -17,6 +17,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	rc := r.Copy() // We potentially modify r, to prevent other plugins from seeing this (r is a pointer), copy r into rc.
 	state := request.Request{W: w, Req: rc}
 	do := state.Do()
+	ad := r.AuthenticatedData
 
 	zone := plugin.Zones(c.Zones).Matches(state.Name())
 	if zone == "" {
@@ -36,7 +37,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 	ttl := 0
 	i := c.getIgnoreTTL(now, state, server)
 	if i == nil {
-		crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do}
+		crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server, do: do, ad: ad}
 		return c.doRefresh(ctx, state, crr)
 	}
 	ttl = i.ttl(now)
@@ -62,7 +63,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
 		cw := newPrefetchResponseWriter(server, state, c)
 		go c.doPrefetch(ctx, state, cw, i, now)
 	}
-	resp := i.toMsg(r, now, do)
+	resp := i.toMsg(r, now, do, ad)
 	w.WriteMsg(resp)
 
 	return dns.RcodeSuccess, nil

plugin/cache/item.go

@@ -64,7 +64,7 @@ func newItem(m *dns.Msg, now time.Time, d time.Duration) *item {
 // So we're forced to always set this to 1; regardless if the answer came from the cache or not.
 // On newer systems(e.g. ubuntu 16.04 with glib version 2.23), this issue is resolved.
 // So we may set this bit back to 0 in the future ?
-func (i *item) toMsg(m *dns.Msg, now time.Time, do bool) *dns.Msg {
+func (i *item) toMsg(m *dns.Msg, now time.Time, do bool, ad bool) *dns.Msg {
 	m1 := new(dns.Msg)
 	m1.SetReply(m)
 
@@ -73,8 +73,10 @@ func (i *item) toMsg(m *dns.Msg, now time.Time, do bool) *dns.Msg {
 	// just set it to true.
 	m1.Authoritative = true
 	m1.AuthenticatedData = i.AuthenticatedData
-	if !do {
+	if !do && !ad {
-		m1.AuthenticatedData = false // when DNSSEC was not wanted, it can't be authenticated data.
+		// When DNSSEC was not wanted, it can't be authenticated data.
+		// However, retain the AD bit if the requester set the AD bit, per RFC6840 5.7-5.8
+		m1.AuthenticatedData = false
 	}
 	m1.RecursionAvailable = i.RecursionAvailable
 	m1.Rcode = i.Rcode