Pull request 1881: 5913-fix-safesearch-ipv6-better
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
parent
2902f030be
commit
ca313521dc
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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].
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue