From d2cf3233b8fc65538bdc052d600f224a7126f2e0 Mon Sep 17 00:00:00 2001 From: Eugene Burkov Date: Mon, 6 Dec 2021 17:26:43 +0300 Subject: [PATCH] Pull request: 3890 fix anonymization Merge in DNS/adguard-home from 3890-fix-stats to master Updates #3890. Squashed commit of the following: commit a77a6204bc8a58f62a4fac70efdcae4267a64810 Merge: 834493a2 90e65b66 Author: Eugene Burkov Date: Mon Dec 6 17:22:16 2021 +0300 Merge branch 'master' into 3890-fix-stats commit 834493a22ae79199efcc44e0715e2ac6f6272963 Author: Eugene Burkov Date: Mon Dec 6 17:09:30 2021 +0300 querylog: load once commit b8000e7ba7a998fcd4553230ec5e5f9c90106e31 Author: Eugene Burkov Date: Mon Dec 6 16:54:41 2021 +0300 querylog: fix docs commit 7db99ccfa19b58100950c11d67b23bca7af3e5cb Author: Eugene Burkov Date: Mon Dec 6 16:51:31 2021 +0300 querylog: imp docs commit 2a84650bd7ac5195730a7ab47b9562a83f721499 Author: Eugene Burkov Date: Mon Dec 6 15:48:09 2021 +0300 querylog: imp anonyization commit 0f63feb1ff5f006fc528c3b681ef3b9d2199581e Author: Eugene Burkov Date: Mon Dec 6 14:44:37 2021 +0300 all: imp code & docs commit c4ccdcbb7248897edd178fd5cb77127e39ada73d Author: Eugene Burkov Date: Mon Dec 6 14:24:30 2021 +0300 all: log changes commit 60bb777a5aff36bba129a078fa11ae566298178a Author: Eugene Burkov Date: Mon Dec 6 14:08:41 2021 +0300 all: use atomic value commit c45886bd20eee2212b42686ff369830d8c08fe36 Author: Eugene Burkov Date: Tue Nov 30 18:50:02 2021 +0300 all: anonymize separately --- CHANGELOG.md | 3 ++ internal/aghnet/ipmut.go | 43 +++++++++++++++++++ internal/dnsforward/dns.go | 4 +- internal/dnsforward/dnsforward.go | 8 ++++ internal/dnsforward/stats.go | 27 ++++++++---- internal/dnsforward/stats_test.go | 6 ++- internal/home/controlinstall.go | 26 +++++++----- internal/home/dns.go | 25 +++++++---- internal/home/web.go | 10 +++-- internal/querylog/decode_test.go | 56 +++++++++++++++++++++++++ internal/querylog/http.go | 55 ++++++++++++++++++++---- internal/querylog/json.go | 39 +++++++---------- internal/querylog/qlog.go | 5 ++- internal/querylog/querylog.go | 7 +++- internal/stats/stats.go | 7 ++-- internal/stats/unit.go | 69 ++++++++++++++----------------- 16 files changed, 279 insertions(+), 111 deletions(-) create mode 100644 internal/aghnet/ipmut.go diff --git a/CHANGELOG.md b/CHANGELOG.md index efb91061..6fb63881 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,8 @@ In this release, the schema version has changed from 10 to 12. ### Fixed +- Incomplete propagation of the client's IP anonymization setting to the + statistics ([#3890]). - Incorrect `$dnsrewrite` results for entries from the operating system's hosts file ([#3815]). - Matching against rules with `|` at the end of the domain name ([#3371]). @@ -222,6 +224,7 @@ In this release, the schema version has changed from 10 to 12. [#3707]: https://github.com/AdguardTeam/AdGuardHome/issues/3707 [#3744]: https://github.com/AdguardTeam/AdGuardHome/issues/3744 [#3815]: https://github.com/AdguardTeam/AdGuardHome/issues/3815 +[#3890]: https://github.com/AdguardTeam/AdGuardHome/issues/3890 diff --git a/internal/aghnet/ipmut.go b/internal/aghnet/ipmut.go new file mode 100644 index 00000000..f227fc9a --- /dev/null +++ b/internal/aghnet/ipmut.go @@ -0,0 +1,43 @@ +package aghnet + +import ( + "net" + "sync/atomic" +) + +// IPMutFunc is the signature of a function which modifies the IP address +// instance. It should be safe for concurrent use. +type IPMutFunc func(ip net.IP) + +// nopIPMutFunc is the IPMutFunc that does nothing. +func nopIPMutFunc(net.IP) {} + +// IPMut is a type-safe wrapper of atomic.Value to store the IPMutFunc. +type IPMut struct { + f atomic.Value +} + +// NewIPMut returns the new properly initialized *IPMut. The m is guaranteed to +// always store non-nil IPMutFunc which is safe to call. +func NewIPMut(f IPMutFunc) (m *IPMut) { + m = &IPMut{ + f: atomic.Value{}, + } + m.Store(f) + + return m +} + +// Store sets the IPMutFunc to return from Func. It's safe for concurrent use. +// If f is nil, the stored function is the no-op one. +func (m *IPMut) Store(f IPMutFunc) { + if f == nil { + f = nopIPMutFunc + } + m.f.Store(f) +} + +// Load returns the previously stored IPMutFunc. +func (m *IPMut) Load() (f IPMutFunc) { + return m.f.Load().(IPMutFunc) +} diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index d498fcf8..b6bafa97 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -307,8 +307,8 @@ func (s *Server) processInternalHosts(dctx *dnsContext) (rc resultCode) { ip, ok := s.hostToIP(host) if !ok { - // TODO(e.burkov): Inspect special cases when user want to apply - // some rules handled by other processors to the hosts with TLD. + // TODO(e.burkov): Inspect special cases when user want to apply some + // rules handled by other processors to the hosts with TLD. d.Res = s.genNXDomain(req) return resultCodeFinish diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index a62a6614..e4547307 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -79,6 +79,9 @@ type Server struct { sysResolvers aghnet.SystemResolvers recDetector *recursionDetector + // anonymizer masks the client's IP addresses if needed. + anonymizer *aghnet.IPMut + tableHostToIP hostToIPTable tableHostToIPLock sync.Mutex @@ -113,6 +116,7 @@ type DNSCreateParams struct { QueryLog querylog.QueryLog DHCPServer dhcpd.ServerInterface SubnetDetector *aghnet.SubnetDetector + Anonymizer *aghnet.IPMut LocalDomain string } @@ -150,6 +154,9 @@ func NewServer(p DNSCreateParams) (s *Server, err error) { localDomainSuffix = domainNameToSuffix(p.LocalDomain) } + if p.Anonymizer == nil { + p.Anonymizer = aghnet.NewIPMut(nil) + } s = &Server{ dnsFilter: p.DNSFilter, stats: p.Stats, @@ -161,6 +168,7 @@ func NewServer(p DNSCreateParams) (s *Server, err error) { EnableLRU: true, MaxCount: defaultClientIDCacheCount, }), + anonymizer: p.Anonymizer, } // TODO(e.burkov): Enable the refresher after the actual implementation diff --git a/internal/dnsforward/stats.go b/internal/dnsforward/stats.go index b7760c68..abdb519e 100644 --- a/internal/dnsforward/stats.go +++ b/internal/dnsforward/stats.go @@ -1,6 +1,7 @@ package dnsforward import ( + "net" "strings" "time" @@ -8,6 +9,7 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/querylog" "github.com/AdguardTeam/AdGuardHome/internal/stats" "github.com/AdguardTeam/dnsproxy/proxy" + "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/netutil" "github.com/miekg/dns" ) @@ -28,10 +30,16 @@ func (s *Server) processQueryLogsAndStats(ctx *dnsContext) (rc resultCode) { s.serverLock.RLock() defer s.serverLock.RUnlock() - // Synchronize access to s.queryLog and s.stats so they won't be suddenly uninitialized while in use. - // This can happen after proxy server has been stopped, but its workers haven't yet exited. + ip, _ := netutil.IPAndPortFromAddr(pctx.Addr) + ip = netutil.CloneIP(ip) + s.anonymizer.Load()(ip) + + log.Debug("client ip: %s", ip) + + // Synchronize access to s.queryLog and s.stats so they won't be suddenly + // uninitialized while in use. This can happen after proxy server has been + // stopped, but its workers haven't yet exited. if shouldLog && s.queryLog != nil { - ip, _ := netutil.IPAndPortFromAddr(pctx.Addr) p := querylog.AddParams{ Question: msg, Answer: pctx.Res, @@ -63,12 +71,17 @@ func (s *Server) processQueryLogsAndStats(ctx *dnsContext) (rc resultCode) { s.queryLog.Add(p) } - s.updateStats(ctx, elapsed, *ctx.result) + s.updateStats(ctx, elapsed, *ctx.result, ip) return resultCodeSuccess } -func (s *Server) updateStats(ctx *dnsContext, elapsed time.Duration, res filtering.Result) { +func (s *Server) updateStats( + ctx *dnsContext, + elapsed time.Duration, + res filtering.Result, + clientIP net.IP, +) { if s.stats == nil { return } @@ -80,8 +93,8 @@ func (s *Server) updateStats(ctx *dnsContext, elapsed time.Duration, res filteri if clientID := ctx.clientID; clientID != "" { e.Client = clientID - } else if ip, _ := netutil.IPAndPortFromAddr(pctx.Addr); ip != nil { - e.Client = ip.String() + } else if clientIP != nil { + e.Client = clientIP.String() } e.Time = uint32(elapsed / 1000) diff --git a/internal/dnsforward/stats_test.go b/internal/dnsforward/stats_test.go index 22780ef2..aa98a387 100644 --- a/internal/dnsforward/stats_test.go +++ b/internal/dnsforward/stats_test.go @@ -5,6 +5,7 @@ import ( "testing" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/AdGuardHome/internal/querylog" "github.com/AdguardTeam/AdGuardHome/internal/stats" @@ -163,8 +164,9 @@ func TestProcessQueryLogsAndStats(t *testing.T) { ql := &testQueryLog{} st := &testStats{} srv := &Server{ - queryLog: ql, - stats: st, + queryLog: ql, + stats: st, + anonymizer: aghnet.NewIPMut(nil), } t.Run(tc.name, func(t *testing.T) { req := &dns.Msg{ diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go index ff532ff6..84996017 100644 --- a/internal/home/controlinstall.go +++ b/internal/home/controlinstall.go @@ -270,15 +270,13 @@ func copyInstallSettings(dst, src *configuration) { // shutdownTimeout is the timeout for shutting HTTP server down operation. const shutdownTimeout = 5 * time.Second -func shutdownSrv(ctx context.Context, cancel context.CancelFunc, srv *http.Server) { +func shutdownSrv(ctx context.Context, srv *http.Server) { defer log.OnPanic("") if srv == nil { return } - defer cancel() - err := srv.Shutdown(ctx) if err != nil { log.Error("error while shutting down http server %q: %s", srv.Addr, err) @@ -354,14 +352,22 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) { f.Flush() } - // Method http.(*Server).Shutdown needs to be called in a separate - // goroutine and with its own context, because it waits until all - // requests are handled and will be blocked by it's own caller. - if restartHTTP { - ctx, cancel := context.WithTimeout(context.Background(), shutdownTimeout) - go shutdownSrv(ctx, cancel, web.httpServer) - go shutdownSrv(ctx, cancel, web.httpServerBeta) + if !restartHTTP { + return } + + // Method http.(*Server).Shutdown needs to be called in a separate goroutine + // and with its own context, because it waits until all requests are handled + // and will be blocked by it's own caller. + go func(timeout time.Duration) { + defer log.OnPanic("web") + + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + shutdownSrv(ctx, web.httpServer) + shutdownSrv(ctx, web.httpServerBeta) + }(shutdownTimeout) } // decodeApplyConfigReq decodes the configuration, validates some parameters, diff --git a/internal/home/dns.go b/internal/home/dns.go index 4b120581..dfd133eb 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/AdGuardHome/internal/querylog" @@ -36,16 +37,20 @@ func onConfigModified() { // initDNSServer creates an instance of the dnsforward.Server // Please note that we must do it even if we don't start it // so that we had access to the query log and the stats -func initDNSServer() error { - var err error +func initDNSServer() (err error) { baseDir := Context.getDataDir() + var anonFunc aghnet.IPMutFunc + if config.DNS.AnonymizeClientIP { + anonFunc = querylog.AnonymizeIP + } + anonymizer := aghnet.NewIPMut(anonFunc) + statsConf := stats.Config{ - Filename: filepath.Join(baseDir, "stats.db"), - LimitDays: config.DNS.StatsInterval, - AnonymizeClientIP: config.DNS.AnonymizeClientIP, - ConfigModified: onConfigModified, - HTTPRegister: httpRegister, + Filename: filepath.Join(baseDir, "stats.db"), + LimitDays: config.DNS.StatsInterval, + ConfigModified: onConfigModified, + HTTPRegister: httpRegister, } Context.stats, err = stats.New(statsConf) if err != nil { @@ -62,6 +67,7 @@ func initDNSServer() error { Enabled: config.DNS.QueryLogEnabled, FileEnabled: config.DNS.QueryLogFileEnabled, AnonymizeClientIP: config.DNS.AnonymizeClientIP, + Anonymizer: anonymizer, } Context.queryLog = querylog.New(conf) @@ -76,6 +82,7 @@ func initDNSServer() error { Stats: Context.stats, QueryLog: Context.queryLog, SubnetDetector: Context.subnetDetector, + Anonymizer: anonymizer, LocalDomain: config.DNS.LocalDomainName, } if Context.dhcpServer != nil { @@ -90,7 +97,8 @@ func initDNSServer() error { } Context.clients.dnsServer = Context.dnsServer - dnsConfig, err := generateServerConfig() + var dnsConfig dnsforward.ServerConfig + dnsConfig, err = generateServerConfig() if err != nil { closeDNSServer() @@ -100,6 +108,7 @@ func initDNSServer() error { err = Context.dnsServer.Prepare(&dnsConfig) if err != nil { closeDNSServer() + return fmt.Errorf("dnsServer.Prepare: %w", err) } diff --git a/internal/home/web.go b/internal/home/web.go index 9be037cc..23e629b9 100644 --- a/internal/home/web.go +++ b/internal/home/web.go @@ -151,7 +151,8 @@ func (web *Web) TLSConfigChanged(ctx context.Context, tlsConf tlsConfigSettings) if web.httpsServer.server != nil { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, shutdownTimeout) - shutdownSrv(ctx, cancel, web.httpsServer.server) + shutdownSrv(ctx, web.httpsServer.server) + cancel() } web.httpsServer.enabled = enabled @@ -222,10 +223,11 @@ func (web *Web) Close(ctx context.Context) { var cancel context.CancelFunc ctx, cancel = context.WithTimeout(ctx, shutdownTimeout) + defer cancel() - shutdownSrv(ctx, cancel, web.httpsServer.server) - shutdownSrv(ctx, cancel, web.httpServer) - shutdownSrv(ctx, cancel, web.httpServerBeta) + shutdownSrv(ctx, web.httpsServer.server) + shutdownSrv(ctx, web.httpServer) + shutdownSrv(ctx, web.httpServerBeta) log.Info("stopped http server") } diff --git a/internal/querylog/decode_test.go b/internal/querylog/decode_test.go index 245681dd..0a1b41fe 100644 --- a/internal/querylog/decode_test.go +++ b/internal/querylog/decode_test.go @@ -244,3 +244,59 @@ func TestDecodeLogEntry_backwardCompatability(t *testing.T) { }) } } + +func BenchmarkAnonymizeIP(b *testing.B) { + benchCases := []struct { + name string + ip net.IP + want net.IP + }{{ + name: "v4", + ip: net.IP{1, 2, 3, 4}, + want: net.IP{1, 2, 0, 0}, + }, { + name: "v4_mapped", + ip: net.IP{1, 2, 3, 4}.To16(), + want: net.IP{1, 2, 0, 0}.To16(), + }, { + name: "v6", + ip: net.IP{ + 0xa, 0xb, 0x0, 0x0, + 0x0, 0xb, 0xa, 0x9, + 0x8, 0x7, 0x6, 0x5, + 0x4, 0x3, 0x2, 0x1, + }, + want: net.IP{ + 0xa, 0xb, 0x0, 0x0, + 0x0, 0xb, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, + }, + }, { + name: "invalid", + ip: net.IP{1, 2, 3}, + want: net.IP{1, 2, 3}, + }} + + for _, bc := range benchCases { + b.Run(bc.name, func(b *testing.B) { + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + AnonymizeIP(bc.ip) + } + + assert.Equal(b, bc.want, bc.ip) + }) + + b.Run(bc.name+"_slow", func(b *testing.B) { + b.ReportAllocs() + + for i := 0; i < b.N; i++ { + anonymizeIPSlow(bc.ip) + } + + assert.Equal(b, bc.want, bc.ip) + }) + } +} diff --git a/internal/querylog/http.go b/internal/querylog/http.go index 7d7c0493..73bdbe56 100644 --- a/internal/querylog/http.go +++ b/internal/querylog/http.go @@ -3,6 +3,7 @@ package querylog import ( "encoding/json" "fmt" + "net" "net/http" "net/url" "strconv" @@ -12,6 +13,7 @@ import ( "github.com/AdguardTeam/golibs/jsonutil" "github.com/AdguardTeam/golibs/log" "github.com/AdguardTeam/golibs/stringutil" + "github.com/AdguardTeam/golibs/timeutil" "golang.org/x/net/idna" ) @@ -88,23 +90,59 @@ func (l *queryLog) handleQueryLogInfo(w http.ResponseWriter, r *http.Request) { } } +// anonymizeIPSlow masks ip to anonymize the client if the ip is a valid one. +// It only exists in purposes of benchmark demonstration. +func anonymizeIPSlow(ip net.IP) { + if ip4 := ip.To4(); ip4 != nil { + copy(ip4[net.IPv4len-2:], []byte{0, 0}) + } else if len(ip) == net.IPv6len { + copy(ip[net.IPv6len-10:], []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0}) + } +} + +// AnonymizeIP masks ip to anonymize the client if the ip is a valid one. +func AnonymizeIP(ip net.IP) { + // We use an assignment operator here since it compiles into more efficient + // code than copy(). See BenchmarkAnonymizeIP. + if ip4 := ip.To4(); ip4 != nil { + ip4[net.IPv4len-2], ip4[net.IPv4len-1] = 0, 0 + } else if len(ip) == net.IPv6len { + ip[net.IPv6len-10], + ip[net.IPv6len-9], + ip[net.IPv6len-8], + ip[net.IPv6len-7], + ip[net.IPv6len-6], + ip[net.IPv6len-5], + ip[net.IPv6len-4], + ip[net.IPv6len-3], + ip[net.IPv6len-2], + ip[net.IPv6len-1] = + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + } +} + // Set configuration func (l *queryLog) handleQueryLogConfig(w http.ResponseWriter, r *http.Request) { - d := qlogConfig{} - req, err := jsonutil.DecodeObject(&d, r.Body) + d := &qlogConfig{} + req, err := jsonutil.DecodeObject(d, r.Body) if err != nil { httpError(r, w, http.StatusBadRequest, "%s", err) return } - ivl := time.Duration(24*d.Interval) * time.Hour + ivl := time.Duration(float64(timeutil.Day) * d.Interval) if req.Exists("interval") && !checkInterval(ivl) { httpError(r, w, http.StatusBadRequest, "Unsupported interval") return } + defer l.conf.ConfigModified() + l.lock.Lock() - // copy data, modify it, then activate. Other threads (readers) don't need to use this lock. + defer l.lock.Unlock() + + // Copy data, modify it, then activate. Other threads (readers) don't need + // to use this lock. conf := *l.conf if req.Exists("enabled") { conf.Enabled = d.Enabled @@ -113,12 +151,13 @@ func (l *queryLog) handleQueryLogConfig(w http.ResponseWriter, r *http.Request) conf.RotationIvl = ivl } if req.Exists("anonymize_client_ip") { - conf.AnonymizeClientIP = d.AnonymizeClientIP + if conf.AnonymizeClientIP = d.AnonymizeClientIP; conf.AnonymizeClientIP { + l.anonymizer.Store(AnonymizeIP) + } else { + l.anonymizer.Store(nil) + } } l.conf = &conf - l.lock.Unlock() - - l.conf.ConfigModified() } // "value" -> value, return TRUE diff --git a/internal/querylog/json.go b/internal/querylog/json.go index 2dfaf68b..23953f80 100644 --- a/internal/querylog/json.go +++ b/internal/querylog/json.go @@ -2,46 +2,30 @@ package querylog import ( "fmt" - "net" "strconv" "strings" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/golibs/log" + "github.com/AdguardTeam/golibs/netutil" "github.com/miekg/dns" "golang.org/x/net/idna" ) // TODO(a.garipov): Use a proper structured approach here. -// Get Client IP address -func (l *queryLog) getClientIP(ip net.IP) (clientIP net.IP) { - if l.conf.AnonymizeClientIP && ip != nil { - const AnonymizeClientIPv4Mask = 16 - const AnonymizeClientIPv6Mask = 112 - - if ip.To4() != nil { - return ip.Mask(net.CIDRMask(AnonymizeClientIPv4Mask, 32)) - } - - return ip.Mask(net.CIDRMask(AnonymizeClientIPv6Mask, 128)) - } - - return ip -} - // jobject is a JSON object alias. type jobject = map[string]interface{} // entriesToJSON converts query log entries to JSON. func (l *queryLog) entriesToJSON(entries []*logEntry, oldest time.Time) (res jobject) { - data := []jobject{} + data := make([]jobject, 0, len(entries)) - // the elements order is already reversed (from newer to older) - for i := 0; i < len(entries); i++ { - entry := entries[i] - jsonEntry := l.logEntryToJSONEntry(entry) + // The elements order is already reversed to be from newer to older. + for _, entry := range entries { + jsonEntry := l.entryToJSON(entry, l.anonymizer.Load()) data = append(data, jsonEntry) } @@ -56,7 +40,7 @@ func (l *queryLog) entriesToJSON(entries []*logEntry, oldest time.Time) (res job return res } -func (l *queryLog) logEntryToJSONEntry(entry *logEntry) (jsonEntry jobject) { +func (l *queryLog) entryToJSON(entry *logEntry, anonFunc aghnet.IPMutFunc) (jsonEntry jobject) { var msg *dns.Msg if len(entry.Answer) > 0 { @@ -81,16 +65,21 @@ func (l *queryLog) logEntryToJSONEntry(entry *logEntry) (jsonEntry jobject) { log.Debug("translating %q into unicode: %s", hostname, err) } + eip := netutil.CloneIP(entry.IP) + anonFunc(eip) + jsonEntry = jobject{ "reason": entry.Result.Reason.String(), "elapsedMs": strconv.FormatFloat(entry.Elapsed.Seconds()*1000, 'f', -1, 64), "time": entry.Time.Format(time.RFC3339Nano), - "client": l.getClientIP(entry.IP), - "client_info": entry.client, + "client": eip, "client_proto": entry.ClientProto, "upstream": entry.Upstream, "question": question, } + if eip.Equal(entry.IP) { + jsonEntry["client_info"] = entry.client + } if entry.ClientID != "" { jsonEntry["client_id"] = entry.ClientID diff --git a/internal/querylog/qlog.go b/internal/querylog/qlog.go index c3cdedfa..48f0d351 100644 --- a/internal/querylog/qlog.go +++ b/internal/querylog/qlog.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" @@ -36,6 +37,8 @@ type queryLog struct { fileFlushLock sync.Mutex // synchronize a file-flushing goroutine and main thread flushPending bool // don't start another goroutine while the previous one is still running fileWriteLock sync.Mutex + + anonymizer *aghnet.IPMut } // ClientProto values are names of the client protocols. @@ -162,7 +165,7 @@ func (l *queryLog) Add(params AddParams) { now := time.Now() entry := logEntry{ - IP: l.getClientIP(params.ClientIP), + IP: params.ClientIP, Time: now, Result: *params.Result, diff --git a/internal/querylog/querylog.go b/internal/querylog/querylog.go index 0ab20a27..58b5a8e0 100644 --- a/internal/querylog/querylog.go +++ b/internal/querylog/querylog.go @@ -6,6 +6,7 @@ import ( "path/filepath" "time" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" @@ -67,6 +68,9 @@ type Config struct { // AnonymizeClientIP tells if the query log should anonymize clients' IP // addresses. AnonymizeClientIP bool + + // Anonymizer proccesses the IP addresses to anonymize those if needed. + Anonymizer *aghnet.IPMut } // AddParams - parameters for Add() @@ -115,7 +119,8 @@ func newQueryLog(conf Config) (l *queryLog) { l = &queryLog{ findClient: findClient, - logFile: filepath.Join(conf.BaseDir, queryLogFileName), + logFile: filepath.Join(conf.BaseDir, queryLogFileName), + anonymizer: conf.Anonymizer, } l.conf = &Config{} diff --git a/internal/stats/stats.go b/internal/stats/stats.go index 7ed1d320..2944a163 100644 --- a/internal/stats/stats.go +++ b/internal/stats/stats.go @@ -16,10 +16,9 @@ type DiskConfig struct { // Config - module configuration type Config struct { - Filename string // database file name - LimitDays uint32 // time limit (in days) - UnitID unitIDCallback // user function to get the current unit ID. If nil, the current time hour is used. - AnonymizeClientIP bool // anonymize clients' IP addresses + Filename string // database file name + LimitDays uint32 // time limit (in days) + UnitID unitIDCallback // user function to get the current unit ID. If nil, the current time hour is used. // Called when the configuration is changed by HTTP request ConfigModified func() diff --git a/internal/stats/unit.go b/internal/stats/unit.go index 71752137..43119907 100644 --- a/internal/stats/unit.go +++ b/internal/stats/unit.go @@ -26,11 +26,13 @@ const ( // statsCtx - global context type statsCtx struct { + // mu protects unit. + mu *sync.Mutex + // current is the actual statistics collection result. + current *unit + db *bolt.DB conf *Config - - unit *unit // the current unit - unitLock sync.Mutex // protect 'unit' } // data for 1 time unit @@ -66,7 +68,9 @@ type unitDB struct { } func createObject(conf Config) (s *statsCtx, err error) { - s = &statsCtx{} + s = &statsCtx{ + mu: &sync.Mutex{}, + } if !checkInterval(conf.LimitDays) { conf.LimitDays = 1 } @@ -112,7 +116,7 @@ func createObject(conf Config) (s *statsCtx, err error) { if udb != nil { deserialize(&u, udb) } - s.unit = &u + s.current = &u log.Debug("stats: initialized") @@ -178,11 +182,13 @@ func (s *statsCtx) dbOpen() bool { // Atomically swap the currently active unit with a new value // Return old value -func (s *statsCtx) swapUnit(new *unit) *unit { - s.unitLock.Lock() - u := s.unit - s.unit = new - s.unitLock.Unlock() +func (s *statsCtx) swapUnit(new *unit) (u *unit) { + s.mu.Lock() + defer s.mu.Unlock() + + u = s.current + s.current = new + return u } @@ -250,6 +256,13 @@ func unitNameToID(name []byte) (id uint32, ok bool) { return uint32(binary.BigEndian.Uint64(name)), true } +func (s *statsCtx) ongoing() (u *unit) { + s.mu.Lock() + defer s.mu.Unlock() + + return s.current +} + // Flush the current unit to DB and delete an old unit when a new hour is started // If a unit must be flushed: // . lock DB @@ -260,10 +273,7 @@ func unitNameToID(name []byte) (id uint32, ok bool) { // . unlock DB func (s *statsCtx) periodicFlush() { for { - s.unitLock.Lock() - ptr := s.unit - s.unitLock.Unlock() - + ptr := s.ongoing() if ptr == nil { break } @@ -491,22 +501,6 @@ func (s *statsCtx) clear() { log.Debug("stats: cleared") } -// Get Client IP address -func (s *statsCtx) getClientIP(ip net.IP) (clientIP net.IP) { - if s.conf.AnonymizeClientIP && ip != nil { - const AnonymizeClientIP4Mask = 16 - const AnonymizeClientIP6Mask = 112 - - if ip.To4() != nil { - return ip.Mask(net.CIDRMask(AnonymizeClientIP4Mask, 32)) - } - - return ip.Mask(net.CIDRMask(AnonymizeClientIP6Mask, 128)) - } - - return ip -} - func (s *statsCtx) Update(e Entry) { if s.conf.limit == 0 { return @@ -521,14 +515,13 @@ func (s *statsCtx) Update(e Entry) { clientID := e.Client if ip := net.ParseIP(clientID); ip != nil { - ip = s.getClientIP(ip) clientID = ip.String() } - s.unitLock.Lock() - defer s.unitLock.Unlock() + s.mu.Lock() + defer s.mu.Unlock() - u := s.unit + u := s.current u.nResult[e.Result]++ @@ -549,10 +542,8 @@ func (s *statsCtx) loadUnits(limit uint32) ([]*unitDB, uint32) { return nil, 0 } - s.unitLock.Lock() - curUnit := serialize(s.unit) - curID := s.unit.id - s.unitLock.Unlock() + cur := s.ongoing() + curID := cur.id // Per-hour units. units := []*unitDB{} @@ -568,7 +559,7 @@ func (s *statsCtx) loadUnits(limit uint32) ([]*unitDB, uint32) { _ = tx.Rollback() - units = append(units, curUnit) + units = append(units, serialize(cur)) if len(units) != int(limit) { log.Fatalf("len(units) != limit: %d %d", len(units), limit)