Pull request 1734: 5479-ad-do-fix

Updates #5479.

Squashed commit of the following:

commit 348d0b94412aee510f291228b76c41a1b2f31d3e
Merge: a0cf6f35 ff04b2a7
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Feb 13 18:42:47 2023 +0300

    Merge branch 'master' into 5479-ad-do-fix

commit a0cf6f3565c22b049c1e98d24c19854d911fd6e0
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Feb 13 18:02:54 2023 +0300

    dnsforward: imp names, docs

commit dfc0be504b3844ba65c2f21ff604edfc6d9040cd
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Feb 13 14:34:49 2023 +0300

    dnsforward: fix ad flag for do reqs

CHANGELOG.md

@@ -84,6 +84,8 @@ In this release, the schema version has changed from 14 to 16.
 
 ### Fixed
 
+- Setting the AD (Authenticated Data) flag on responses that have the DO (DNSSEC
+  OK) flag set but not the AD flag ([#5479]).
 - Client names resolved via reverse DNS not being updated ([#4939]).
 - The icon for League Of Legends on the Blocked services page ([#5433]).
 
@@ -91,10 +93,12 @@ In this release, the schema version has changed from 14 to 16.
 
 - Go 1.18 support, as it has reached end of life.
 
+
 [#1717]: https://github.com/AdguardTeam/AdGuardHome/issues/1717
 [#4299]: https://github.com/AdguardTeam/AdGuardHome/issues/4299
 [#4939]: https://github.com/AdguardTeam/AdGuardHome/issues/4939
 [#5433]: https://github.com/AdguardTeam/AdGuardHome/issues/5433
+[#5479]: https://github.com/AdguardTeam/AdGuardHome/issues/5479
 
 <!--
 NOTE: Add new changes ABOVE THIS COMMENT.

internal/dnsforward/dns.go

@@ -651,13 +651,7 @@ func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) {
 
 	s.setCustomUpstream(pctx, dctx.clientID)
 
-	origReqAD := false
+	reqWantsDNSSEC := s.setReqAD(req)
-	if s.conf.EnableDNSSEC {
-		origReqAD = req.AuthenticatedData
-		if !req.AuthenticatedData {
-			req.AuthenticatedData = true
-		}
-	}
 
 	// Process the request further since it wasn't filtered.
 	prx := s.proxy()
@@ -688,12 +682,52 @@ func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) {
 	dctx.responseFromUpstream = true
 	dctx.responseAD = pctx.Res.AuthenticatedData
 
-	if s.conf.EnableDNSSEC && !origReqAD {
+	s.setRespAD(pctx, reqWantsDNSSEC)
+
+	return resultCodeSuccess
+}
+
+// setReqAD changes the request based on the server settings.  wantsDNSSEC is
+// false if the response should be cleared of the AD bit.
+//
+// TODO(a.garipov, e.burkov): This should probably be done in module dnsproxy.
+func (s *Server) setReqAD(req *dns.Msg) (wantsDNSSEC bool) {
+	if !s.conf.EnableDNSSEC {
+		return false
+	}
+
+	origReqAD := req.AuthenticatedData
+	req.AuthenticatedData = true
+
+	// Per [RFC 6840] says, validating resolvers should only set the AD bit when
+	// the response has the AD bit set and the request contained either a set DO
+	// bit or a set AD bit.  So, if neither of these is true, clear the AD bits
+	// in [Server.setRespAD].
+	//
+	// [RFC 6840]: https://datatracker.ietf.org/doc/html/rfc6840#section-5.8
+	return origReqAD || hasDO(req)
+}
+
+// hasDO returns true if msg has EDNS(0) options and the DNSSEC OK flag is set
+// in there.
+//
+// TODO(a.garipov): Move to golibs/dnsmsg when it's there.
+func hasDO(msg *dns.Msg) (do bool) {
+	o := msg.IsEdns0()
+	if o == nil {
+		return false
+	}
+
+	return o.Do()
+}
+
+// setRespAD changes the request and response based on the server settings and
+// the original request data.
+func (s *Server) setRespAD(pctx *proxy.DNSContext, reqWantsDNSSEC bool) {
+	if s.conf.EnableDNSSEC && !reqWantsDNSSEC {
 		pctx.Req.AuthenticatedData = false
 		pctx.Res.AuthenticatedData = false
 	}
-
-	return resultCodeSuccess
 }
 
 // isDHCPClientHostQ returns true if q is from a request for a DHCP client