Skip to content

Commit

Permalink
Pull request: safesearch cname
Browse files Browse the repository at this point in the history 
Updates AdguardTeam#6352.

Squashed commit of the following:

commit 79d24e0
Merge: 04c2759 c908eec
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Dec 12 12:26:57 2023 +0200

    Merge remote-tracking branch 'origin/master' into 6352-safesearch-cname

    # Conflicts:
    #	CHANGELOG.md

commit 04c2759
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Dec 12 11:14:13 2023 +0200

    all: fix changelog

commit 78d726e
Merge: 2d2c174 79d7a1e
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Tue Dec 12 11:12:58 2023 +0200

    Merge remote-tracking branch 'origin/master' into 6352-safesearch-cname

commit 2d2c174
Merge: 2b1c1ea 7b5cce5
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Mon Dec 11 11:08:08 2023 +0200

    Merge remote-tracking branch 'origin/master' into 6352-safesearch-cname

    # Conflicts:
    #	CHANGELOG.md

commit 2b1c1ea
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Dec 8 15:24:02 2023 +0200

    all: changelog

commit 38afdba
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Dec 8 15:21:23 2023 +0200

    safesearch: imp docs

commit e941f5e
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Dec 8 14:43:39 2023 +0200

    dnsforward: imp code

commit 8dedb4a
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Dec 8 14:26:51 2023 +0200

    dnsforward: imp tests

commit 8f23ade
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Dec 8 13:33:50 2023 +0200

    all: safesearch cnames

commit 061a6de
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Dec 8 13:09:32 2023 +0200

    all: changelog

commit 6f7ff7f
Author: Dimitry Kolyshev <dkolyshev@adguard.com>
Date:   Fri Dec 8 13:07:36 2023 +0200

    all: safesearch cnames
  • Loading branch information
@Mizzick
Mizzick committed Dec 12, 2023
1 parent c908eec commit dae304f
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 53 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -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.
-->
Expand Down
58 changes: 39 additions & 19 deletions internal/dnsforward/dnsforward_test.go
View file
Expand Up @@ -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 {
Expand All @@ -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)
})
}
}
Expand Down
33 changes: 1 addition & 32 deletions internal/dnsforward/filter.go
View file
Expand Up @@ -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
Expand All @@ -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(
Expand Down
36 changes: 35 additions & 1 deletion internal/dnsforward/msg.go
View file
Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion internal/filtering/safesearch/safesearch.go
View file
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit dae304f

Please sign in to comment.