From a50a8abb45e3db7b9085ab2dd9cd3ff0102141af Mon Sep 17 00:00:00 2001
From: Ainar Garipov <a.garipov@adguard.com>
Date: Mon, 13 Feb 2023 19:07:10 +0300
Subject: [PATCH] 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 | 4 +++
internal/dnsforward/dns.go | 54 +++++++++++++++++++++++++++++++-------
2 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6a23d483..c29314a9 100644
--- a/CHANGELOG.md
+++ b/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.
diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go
index c132cd96..8d924f3b 100644
--- a/internal/dnsforward/dns.go
+++ b/internal/dnsforward/dns.go
@@ -651,13 +651,7 @@ func (s *Server) processUpstream(dctx *dnsContext) (rc resultCode) {
s.setCustomUpstream(pctx, dctx.clientID)
- origReqAD := false
- if s.conf.EnableDNSSEC {
- origReqAD = req.AuthenticatedData
- if !req.AuthenticatedData {
- req.AuthenticatedData = true
- }
- }
+ reqWantsDNSSEC := s.setReqAD(req)
// 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