Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge branch 'master' into AG-27492-client-runtime
Browse files Browse the repository at this point in the history
schzhn committed Dec 13, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
2 parents 7411b40 + dae304f commit e4c2abd
Showing 5 changed files with 86 additions and 53 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -28,6 +28,13 @@ NOTE: Add new changes BELOW THIS COMMENT.
- Ability to disable plain-DNS serving via UI if an encrypted protocol is
already used ([#1660]).

### Fixed

- Omitted CNAME records in safe search results, which can cause YouTube to not
work on iOS ([#6352]).

[#6352]: https://github.com/AdguardTeam/AdGuardHome/issues/6352

<!--
NOTE: Add new changes ABOVE THIS COMMENT.
-->
58 changes: 39 additions & 19 deletions internal/dnsforward/dnsforward_test.go
Original file line number Diff line number Diff line change
@@ -547,32 +547,41 @@ func TestSafeSearch(t *testing.T) {
googleIP, _ := aghtest.HostToIPs("forcesafesearch.google.com")

testCases := []struct {
host string
want netip.Addr
host string
want netip.Addr
wantCNAME string
}{{
host: "yandex.com.",
want: yandexIP,
host: "yandex.com.",
want: yandexIP,
wantCNAME: "",
}, {
host: "yandex.by.",
want: yandexIP,
host: "yandex.by.",
want: yandexIP,
wantCNAME: "",
}, {
host: "yandex.kz.",
want: yandexIP,
host: "yandex.kz.",
want: yandexIP,
wantCNAME: "",
}, {
host: "yandex.ru.",
want: yandexIP,
host: "yandex.ru.",
want: yandexIP,
wantCNAME: "",
}, {
host: "www.google.com.",
want: googleIP,
host: "www.google.com.",
want: googleIP,
wantCNAME: "forcesafesearch.google.com.",
}, {
host: "www.google.com.af.",
want: googleIP,
host: "www.google.com.af.",
want: googleIP,
wantCNAME: "forcesafesearch.google.com.",
}, {
host: "www.google.be.",
want: googleIP,
host: "www.google.be.",
want: googleIP,
wantCNAME: "forcesafesearch.google.com.",
}, {
host: "www.google.by.",
want: googleIP,
host: "www.google.by.",
want: googleIP,
wantCNAME: "forcesafesearch.google.com.",
}}

for _, tc := range testCases {
@@ -582,7 +591,18 @@ func TestSafeSearch(t *testing.T) {
var reply *dns.Msg
reply, _, err = client.Exchange(req, addr)
require.NoErrorf(t, err, "couldn't talk to server %s: %s", addr, err)
assertResponse(t, reply, tc.want)

if tc.wantCNAME != "" {
require.Len(t, reply.Answer, 2)

cname := testutil.RequireTypeAssert[*dns.CNAME](t, reply.Answer[0])
assert.Equal(t, tc.wantCNAME, cname.Target)
} else {
require.Len(t, reply.Answer, 1)
}

a := testutil.RequireTypeAssert[*dns.A](t, reply.Answer[len(reply.Answer)-1])
assert.Equal(t, net.IP(tc.want.AsSlice()), a.A)
})
}
}
33 changes: 1 addition & 32 deletions internal/dnsforward/filter.go
Original file line number Diff line number Diff line change
@@ -95,7 +95,7 @@ func (s *Server) filterDNSRequest(dctx *dnsContext) (res *filtering.Result, err
dctx.origQuestion = q
req.Question[0].Name = dns.Fqdn(res.CanonName)
case res.Reason == filtering.Rewritten:
pctx.Res = s.filterRewritten(req, host, res, q.Qtype)
pctx.Res = s.getCNAMEWithIPs(req, res.IPList, res.CanonName)
case res.Reason.In(filtering.RewrittenRule, filtering.RewrittenAutoHosts):
if err = s.filterDNSRewrite(req, res, pctx); err != nil {
return nil, err
@@ -105,37 +105,6 @@ func (s *Server) filterDNSRequest(dctx *dnsContext) (res *filtering.Result, err
return res, err
}

// filterRewritten handles DNS rewrite filters. It returns a DNS response with
// the data from the filtering result. All parameters must not be nil.
func (s *Server) filterRewritten(
req *dns.Msg,
host string,
res *filtering.Result,
qt uint16,
) (resp *dns.Msg) {
resp = s.makeResponse(req)
name := host
if len(res.CanonName) != 0 {
resp.Answer = append(resp.Answer, s.genAnswerCNAME(req, res.CanonName))
name = res.CanonName
}

for _, ip := range res.IPList {
switch qt {
case dns.TypeA:
a := s.genAnswerA(req, ip)
a.Hdr.Name = dns.Fqdn(name)
resp.Answer = append(resp.Answer, a)
case dns.TypeAAAA:
a := s.genAnswerAAAA(req, ip)
a.Hdr.Name = dns.Fqdn(name)
resp.Answer = append(resp.Answer, a)
}
}

return resp
}

// checkHostRules checks the host against filters. It is safe for concurrent
// use.
func (s *Server) checkHostRules(
36 changes: 35 additions & 1 deletion internal/dnsforward/msg.go
Original file line number Diff line number Diff line change
@@ -66,12 +66,46 @@ func (s *Server) genDNSFilterMessage(
// If Safe Search generated the necessary IP addresses, use them.
// Otherwise, if there were no errors, there are no addresses for the
// requested IP version, so produce a NODATA response.
return s.genResponseWithIPs(req, ipsFromRules(res.Rules))
return s.getCNAMEWithIPs(req, ipsFromRules(res.Rules), res.CanonName)
default:
return s.genForBlockingMode(req, ipsFromRules(res.Rules))
}
}

// getCNAMEWithIPs generates a filtered response to req for with CNAME record
// and provided ips.
func (s *Server) getCNAMEWithIPs(req *dns.Msg, ips []netip.Addr, cname string) (resp *dns.Msg) {
resp = s.makeResponse(req)

originalName := req.Question[0].Name

var ans []dns.RR
if cname != "" {
ans = append(ans, s.genAnswerCNAME(req, cname))

// The given IPs actually are resolved for this cname.
req.Question[0].Name = dns.Fqdn(cname)
defer func() { req.Question[0].Name = originalName }()
}

switch req.Question[0].Qtype {
case dns.TypeA:
ans = append(ans, s.genAnswersWithIPv4s(req, ips)...)
case dns.TypeAAAA:
for _, ip := range ips {
if ip.Is6() {
ans = append(ans, s.genAnswerAAAA(req, ip))
}
}
default:
// Go on and return an empty response.
}

resp.Answer = ans

return resp
}

// genForBlockingMode generates a filtered response to req based on the server's
// blocking mode.
func (s *Server) genForBlockingMode(req *dns.Msg, ips []netip.Addr) (resp *dns.Msg) {
5 changes: 4 additions & 1 deletion internal/filtering/safesearch/safesearch.go
Original file line number Diff line number Diff line change
@@ -226,7 +226,8 @@ func (ss *Default) searchHost(host string, qtype rules.RRType) (res *rules.DNSRe
// empty result is converted into a NODATA response.
//
// TODO(a.garipov): Use the main rewrite result mechanism used in
// [dnsforward.Server.filterDNSRequest].
// [dnsforward.Server.filterDNSRequest]. Now we resolve IPs for CNAME to save
// them in the safe search cache.
func (ss *Default) newResult(
rewrite *rules.DNSRewrite,
qtype rules.RRType,
@@ -255,6 +256,8 @@ func (ss *Default) newResult(
return res, nil
}

res.CanonName = host

ss.log(log.DEBUG, "resolving %q", host)

ips, err := ss.resolver.LookupIP(context.Background(), qtypeToProto(qtype), host)

0 comments on commit e4c2abd

Please sign in to comment.