From 14250821ab992ca6a74bf0b537d78b73b46a27b4 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Mon, 24 May 2021 14:48:42 +0300 Subject: [PATCH] Pull request: 2875 fix client filtering settings Merge in DNS/adguard-home from 2875-client-filtering to master Updates #2875. Squashed commit of the following: commit b3b9582b7dde826005ba79d499ed7e82af067e93 Author: Eugene Burkov Date: Mon May 24 14:22:29 2021 +0300 all: use atomic, log changes commit 9304d8b96d0d064d7741c85165ab885f5547fd4c Author: Eugene Burkov Date: Mon May 24 13:43:22 2021 +0300 all: fix client filtering settings --- CHANGELOG.md | 4 +++ README.md | 2 +- internal/dnsforward/dnsforward_test.go | 6 ++++- internal/dnsforward/filter.go | 1 - internal/filtering/filtering.go | 36 +++++++++++++++++++------- internal/home/control.go | 1 - internal/home/controlfiltering.go | 11 +++++--- internal/home/dns.go | 7 +++-- internal/home/filter.go | 9 +++++++ 9 files changed, 57 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58c0b2fc..358bfa94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,10 @@ released by then. - Go 1.16 support. v0.108.0 will require at least Go 1.17 to build. +### Fixed + +- Incorrect client-based filtering applying logic ([#2875]). + ### Removed - Go 1.15 support. diff --git a/README.md b/README.md index fe8b29be..99723b8f 100644 --- a/README.md +++ b/README.md @@ -317,7 +317,7 @@ Here is a link to AdGuard Home project: https://crowdin.com/project/adguard-appl Here's what you can also do to contribute: 1. [Look for issues](https://github.com/AdguardTeam/AdGuardHome/issues?q=is%3Aissue+is%3Aopen+label%3A%22help+wanted%22+) marked as "help wanted". -2. Actualize the list of *Blocked services*. It it can be found in [filtering/blocked.go](https://github.com/AdguardTeam/AdGuardHome/blob/master/internal/filtering/blocked.go). +2. Actualize the list of *Blocked services*. It can be found in [filtering/blocked.go](https://github.com/AdguardTeam/AdGuardHome/blob/master/internal/filtering/blocked.go). 3. Actualize the list of known *trackers*. It it can be found in [client/src/helpers/trackers/adguard.json](https://github.com/AdguardTeam/AdGuardHome/blob/master/client/src/helpers/trackers/adguard.json). 4. Actualize the list of vetted *blocklists*. It it can be found in [client/src/helpers/filters/filters.json](https://github.com/AdguardTeam/AdGuardHome/blob/master/client/src/helpers/filters/filters.json). diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index f79aa2aa..3037188a 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -68,6 +68,7 @@ func createTestServer( }} f := filtering.New(filterConf, filters) + f.SetEnabled(true) snd, err := aghnet.NewSubnetDetector() require.NoError(t, err) @@ -734,10 +735,11 @@ func TestBlockedCustomIP(t *testing.T) { require.NoError(t, err) require.NotNil(t, snd) + f := filtering.New(&filtering.Config{}, filters) var s *Server s, err = NewServer(DNSCreateParams{ DHCPServer: &testDHCP{}, - DNSFilter: filtering.New(&filtering.Config{}, filters), + DNSFilter: f, SubnetDetector: snd, }) require.NoError(t, err) @@ -763,6 +765,7 @@ func TestBlockedCustomIP(t *testing.T) { err = s.Prepare(conf) require.NoError(t, err) + f.SetEnabled(true) startDeferStop(t, s) addr := s.dnsProxy.Addr(proxy.ProtoUDP) @@ -798,6 +801,7 @@ func TestBlockedByHosts(t *testing.T) { ProtectionEnabled: true, }, } + s := createTestServer(t, &filtering.Config{}, forwardConf, nil) startDeferStop(t, s) addr := s.dnsProxy.Addr(proxy.ProtoUDP) diff --git a/internal/dnsforward/filter.go b/internal/dnsforward/filter.go index 6e085dfd..1bacc98c 100644 --- a/internal/dnsforward/filter.go +++ b/internal/dnsforward/filter.go @@ -38,7 +38,6 @@ func (s *Server) beforeRequestHandler(_ *proxy.Proxy, d *proxy.DNSContext) (bool // the client's IP address and ID, if any, from ctx. func (s *Server) getClientRequestFilteringSettings(ctx *dnsContext) *filtering.Settings { setts := s.dnsFilter.GetConfig() - setts.FilteringEnabled = true if s.conf.FilterHandler != nil { s.conf.FilterHandler(IPFromAddr(ctx.proxyCtx.Addr), ctx.clientID, &setts) } diff --git a/internal/filtering/filtering.go b/internal/filtering/filtering.go index a6a128a0..14df5ef5 100644 --- a/internal/filtering/filtering.go +++ b/internal/filtering/filtering.go @@ -11,6 +11,7 @@ import ( "runtime/debug" "strings" "sync" + "sync/atomic" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/aghstrings" @@ -50,6 +51,11 @@ type Resolver interface { // Config allows you to configure DNS filtering with New() or just change variables directly. type Config struct { + // enabled is used to be returned within Settings. + // + // It is of type uint32 to be accessed by atomic. + enabled uint32 + ParentalEnabled bool `yaml:"parental_enabled"` SafeSearchEnabled bool `yaml:"safesearch_enabled"` SafeBrowsingEnabled bool `yaml:"safebrowsing_enabled"` @@ -118,7 +124,8 @@ type DNSFilter struct { parentalUpstream upstream.Upstream safeBrowsingUpstream upstream.Upstream - Config // for direct access by library users, even a = assignment + Config // for direct access by library users, even a = assignment + // confLock protects Config. confLock sync.RWMutex // Channel for passing data to filters-initializer goroutine @@ -223,15 +230,26 @@ func (r Reason) In(reasons ...Reason) bool { return false } +// SetEnabled sets the status of the *DNSFilter. +func (d *DNSFilter) SetEnabled(enabled bool) { + var i int32 + if enabled { + i = 1 + } + atomic.StoreUint32(&d.enabled, uint32(i)) +} + // GetConfig - get configuration -func (d *DNSFilter) GetConfig() Settings { - c := Settings{} - // d.confLock.RLock() - c.SafeSearchEnabled = d.Config.SafeSearchEnabled - c.SafeBrowsingEnabled = d.Config.SafeBrowsingEnabled - c.ParentalEnabled = d.Config.ParentalEnabled - // d.confLock.RUnlock() - return c +func (d *DNSFilter) GetConfig() (s Settings) { + d.confLock.RLock() + defer d.confLock.RUnlock() + + return Settings{ + FilteringEnabled: atomic.LoadUint32(&d.Config.enabled) == 1, + SafeSearchEnabled: d.Config.SafeSearchEnabled, + SafeBrowsingEnabled: d.Config.SafeBrowsingEnabled, + ParentalEnabled: d.Config.ParentalEnabled, + } } // WriteDiskConfig - write configuration diff --git a/internal/home/control.go b/internal/home/control.go index 54ba69fd..a4f4301d 100644 --- a/internal/home/control.go +++ b/internal/home/control.go @@ -134,7 +134,6 @@ func handleStatus(w http.ResponseWriter, _ *http.Request) { } var resp statusResponse - func() { config.RLock() defer config.RUnlock() diff --git a/internal/home/controlfiltering.go b/internal/home/controlfiltering.go index b1f18e61..229c1e4c 100644 --- a/internal/home/controlfiltering.go +++ b/internal/home/controlfiltering.go @@ -351,8 +351,14 @@ func (f *Filtering) handleFilteringConfig(w http.ResponseWriter, r *http.Request return } - config.DNS.FilteringEnabled = req.Enabled - config.DNS.FiltersUpdateIntervalHours = req.Interval + func() { + config.Lock() + defer config.Unlock() + + config.DNS.FilteringEnabled = req.Enabled + config.DNS.FiltersUpdateIntervalHours = req.Interval + }() + onConfigModified() enableFilters(true) } @@ -364,7 +370,6 @@ type checkHostRespRule struct { type checkHostResp struct { Reason string `json:"reason"` - // FilterID is the ID of the rule's filter list. // // Deprecated: Use Rules[*].FilterListID. diff --git a/internal/home/dns.go b/internal/home/dns.go index 467e1f0d..68008fa2 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -307,7 +307,6 @@ func applyAdditionalFiltering(clientAddr net.IP, clientID string, setts *filteri setts.ClientName = c.Name setts.ClientTags = c.Tags - if !c.UseOwnSettings { return } @@ -319,14 +318,14 @@ func applyAdditionalFiltering(clientAddr net.IP, clientID string, setts *filteri } func startDNSServer() error { - config.Lock() - defer config.Unlock() + config.RLock() + defer config.RUnlock() if isRunning() { return fmt.Errorf("unable to start forwarding DNS server: Already running") } - enableFilters(false) + enableFiltersLocked(false) Context.clients.Start() diff --git a/internal/home/filter.go b/internal/home/filter.go index b6650a4b..7b46a83d 100644 --- a/internal/home/filter.go +++ b/internal/home/filter.go @@ -664,6 +664,13 @@ func (filter *filter) Path() string { } func enableFilters(async bool) { + config.RLock() + defer config.RUnlock() + + enableFiltersLocked(async) +} + +func enableFiltersLocked(async bool) { var whiteFilters []filtering.Filter filters := []filtering.Filter{{ Data: []byte(strings.Join(config.UserRules, "\n")), @@ -693,4 +700,6 @@ func enableFilters(async bool) { if err := Context.dnsFilter.SetFilters(filters, whiteFilters, async); err != nil { log.Debug("enabling filters: %s", err) } + + Context.dnsFilter.SetEnabled(config.DNS.FilteringEnabled) }