mirrors/AdGuardHome
1
0
Fork 0
mirror of https://github.com/AdguardTeam/AdGuardHome.git synced 2024-11-21 12:35:33 +03:00
Code Issues Projects Releases Packages Wiki Activity

Pull request 1881: 5913-fix-safesearch-ipv6-better

Browse source 
Updates #5913.

Squashed commit of the following:

commit 6bff5ee1b77ae1812e2803361e60ef12148da5c7
Merge: 0a6f49008 2902f030b
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jun 20 14:14:47 2023 +0300

    Merge branch 'master' into 5913-fix-safesearch-ipv6-better

commit 0a6f49008ac1c786ba380cc3c9a4c2c0c1f60815
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Tue Jun 20 14:11:18 2023 +0300

    safesearch: imp tests

commit 3f9056d26816fb753a394a7bcf86f2ae1201d19c
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Mon Jun 19 23:01:03 2023 +0300

    safesearch: fix ipv6 more
This commit is contained in:
Ainar Garipov 2023-06-20 14:48:36 +03:00
parent 2902f030be
commit ca313521dc
4 changed files with 81 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
CHANGELOG.md
View file

@ -14,11 +14,11 @@ and this project adheres to
<!--
## [v0.108.0] - TBA
## [v0.107.32] - 2023-06-28 (APPROX.)
## [v0.107.33] - 2023-06-28 (APPROX.)
See also the [v0.107.32 GitHub milestone][ms-v0.107.32].
See also the [v0.107.33 GitHub milestone][ms-v0.107.33].
[ms-v0.107.32]: https://github.com/AdguardTeam/AdGuardHome/milestone/68?closed=1
[ms-v0.107.33]: https://github.com/AdguardTeam/AdGuardHome/milestone/68?closed=1
NOTE: Add new changes BELOW THIS COMMENT.
-->
@ -84,7 +84,8 @@ In this release, the schema version has changed from 20 to 21.
- Queries with the question-section target `.`, for example `NS .`, are now
counted in the statistics and correctly shown in the query log ([#5910]).
- Safe Search not working with `AAAA` queries for Yandex domains ([#5913]).
- Safe Search not working with `AAAA` queries for domains that don't have `AAAA`
records ([#5913]).
[#951]: https://github.com/AdguardTeam/AdGuardHome/issues/951
[#1577]: https://github.com/AdguardTeam/AdGuardHome/issues/1577

42
internal/filtering/safesearch/safesearch.go
View file

@ -161,12 +161,8 @@ func (ss *Default) resetEngine(
// type check
var _ filtering.SafeSearch = (*Default)(nil)
// CheckHost implements the [filtering.SafeSearch] interface for
// *DefaultSafeSearch.
func (ss *Default) CheckHost(
host string,
qtype rules.RRType,
) (res filtering.Result, err error) {
// CheckHost implements the [filtering.SafeSearch] interface for *Default.
func (ss *Default) CheckHost(host string, qtype rules.RRType) (res filtering.Result, err error) {
start := time.Now()
defer func() {
ss.log(log.DEBUG, "lookup for %q finished in %s", host, time.Since(start))
@ -196,14 +192,10 @@ func (ss *Default) CheckHost(
return filtering.Result{}, err
}
if fltRes != nil {
res = *fltRes
ss.setCacheResult(host, qtype, res)
res = *fltRes
ss.setCacheResult(host, qtype, res)
return res, nil
}
return filtering.Result{}, fmt.Errorf("no ip addresses for %q", host)
return res, nil
}
// searchHost looks up DNS rewrites in the internal DNS filtering engine.
@ -229,7 +221,11 @@ func (ss *Default) searchHost(host string, qtype rules.RRType) (res *rules.DNSRe
}
// newResult creates Result object from rewrite rule. qtype must be either
// [dns.TypeA] or [dns.TypeAAAA].
// [dns.TypeA] or [dns.TypeAAAA]. If err is nil, res is never nil, so that the
// empty result is converted into a NODATA response.
//
// TODO(a.garipov): Use the main rewrite result mechanism used in
// [dnsforward.Server.filterDNSRequest].
func (ss *Default) newResult(
rewrite *rules.DNSRewrite,
qtype rules.RRType,
@ -243,9 +239,10 @@ func (ss *Default) newResult(
}
if rewrite.RRType == qtype {
ip, ok := rewrite.Value.(net.IP)
v := rewrite.Value
ip, ok := v.(net.IP)
if !ok || ip == nil {
return nil, nil
return nil, fmt.Errorf("expected ip rewrite value, got %T(%[1]v)", v)
}
res.Rules[0].IP = ip
@ -255,13 +252,6 @@ func (ss *Default) newResult(
host := rewrite.NewCNAME
if host == "" {
// If there is a rewrite, but it's neither a CNAME one nor one matching
// the IP version, then it's a service that only has one type of IP
// record but not the other. Return the empty result to be converted
// into a NODATA response.
//
// TODO(a.garipov): Use the main rewrite result mechanism used in
// [dnsforward.Server.filterDNSRequest].
return res, nil
}
@ -269,7 +259,7 @@ func (ss *Default) newResult(
ips, err := ss.resolver.LookupIP(context.Background(), qtypeToProto(qtype), host)
if err != nil {
return nil, err
return nil, fmt.Errorf("resolving cname: %w", err)
}
ss.log(log.DEBUG, "resolved %s", ips)
@ -283,11 +273,9 @@ func (ss *Default) newResult(
}
res.Rules[0].IP = ip
return res, nil
}
return nil, nil
return res, nil
}
// qtypeToProto returns "ip4" for [dns.TypeA] and "ip6" for [dns.TypeAAAA].

59
internal/filtering/safesearch/safesearch_test.go
View file

@ -1,6 +1,7 @@
package safesearch_test
import (
"context"
"net"
"testing"
"time"
@ -80,6 +81,14 @@ func TestDefault_CheckHost_yandexAAAA(t *testing.T) {
require.NoError(t, err)
assert.True(t, res.IsFiltered)
// TODO(a.garipov): Currently, the safe-search filter returns a single rule
// with a nil IP address. This isn't really necessary and should be changed
// once the TODO in [safesearch.Default.newResult] is resolved.
require.Len(t, res.Rules, 1)
assert.Nil(t, res.Rules[0].IP)
assert.EqualValues(t, filtering.SafeSearchListID, res.Rules[0].FilterListID)
}
func TestDefault_CheckHost_google(t *testing.T) {
@ -116,6 +125,56 @@ func TestDefault_CheckHost_google(t *testing.T) {
}
}
// testResolver is a [filtering.Resolver] for tests.
//
// TODO(a.garipov): Move to aghtest and use everywhere.
type testResolver struct {
OnLookupIP func(ctx context.Context, network, host string) (ips []net.IP, err error)
}
// type check
var _ filtering.Resolver = (*testResolver)(nil)
// LookupIP implements the [filtering.Resolver] interface for *testResolver.
func (r *testResolver) LookupIP(
ctx context.Context,
network string,
host string,
) (ips []net.IP, err error) {
return r.OnLookupIP(ctx, network, host)
}
func TestDefault_CheckHost_duckduckgoAAAA(t *testing.T) {
conf := testConf
conf.CustomResolver = &testResolver{
OnLookupIP: func(_ context.Context, network, host string) (ips []net.IP, err error) {
assert.Equal(t, "ip6", network)
assert.Equal(t, "safe.duckduckgo.com", host)
return nil, nil
},
}
ss, err := safesearch.NewDefault(conf, "", testCacheSize, testCacheTTL)
require.NoError(t, err)
// The DuckDuckGo safe-search addresses are resolved through CNAMEs, but
// DuckDuckGo doesn't have a safe-search IPv6 address. The result should be
// the same as the one for Yandex IPv6. That is, a NODATA response.
res, err := ss.CheckHost("www.duckduckgo.com", dns.TypeAAAA)
require.NoError(t, err)
assert.True(t, res.IsFiltered)
// TODO(a.garipov): Currently, the safe-search filter returns a single rule
// with a nil IP address. This isn't really necessary and should be changed
// once the TODO in [safesearch.Default.newResult] is resolved.
require.Len(t, res.Rules, 1)
assert.Nil(t, res.Rules[0].IP)
assert.EqualValues(t, filtering.SafeSearchListID, res.Rules[0].FilterListID)
}
func TestDefault_Update(t *testing.T) {
conf := testConf
ss, err := safesearch.NewDefault(conf, "", testCacheSize, testCacheTTL)

3
internal/next/changelog.md
View file

@ -24,7 +24,7 @@ enough.
### Fixed
- Inconsistent application of `--work-dir/-w` ([#2902]).
- Inconsistent application of `--work-dir/-w` ([#2598], [#2902]).
- The order of `-v/--verbose` and `--version` being significant ([#2893]).
### Removed
@ -33,6 +33,7 @@ enough.
- `--host` and `-p/--port` flags. Use `--web-addr=host:port` to set an address
on which to serve the Web UI. `-h` is now an alias for `--help`, see above.
[#2598]: https://github.com/AdguardTeam/AdGuardHome/issues/2598
[#2893]: https://github.com/AdguardTeam/AdGuardHome/issues/2893
[#2902]: https://github.com/AdguardTeam/AdGuardHome/issues/2902
[#5676]: https://github.com/AdguardTeam/AdGuardHome/issues/5676