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 <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 17:22:16 2021 +0300

    Merge branch 'master' into 3890-fix-stats

commit 834493a22ae79199efcc44e0715e2ac6f6272963
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 17:09:30 2021 +0300

    querylog: load once

commit b8000e7ba7a998fcd4553230ec5e5f9c90106e31
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 16:54:41 2021 +0300

    querylog: fix docs

commit 7db99ccfa19b58100950c11d67b23bca7af3e5cb
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 16:51:31 2021 +0300

    querylog: imp docs

commit 2a84650bd7ac5195730a7ab47b9562a83f721499
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 15:48:09 2021 +0300

    querylog: imp anonyization

commit 0f63feb1ff5f006fc528c3b681ef3b9d2199581e
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 14:44:37 2021 +0300

    all: imp code & docs

commit c4ccdcbb7248897edd178fd5cb77127e39ada73d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 14:24:30 2021 +0300

    all: log changes

commit 60bb777a5aff36bba129a078fa11ae566298178a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Dec 6 14:08:41 2021 +0300

    all: use atomic value

commit c45886bd20eee2212b42686ff369830d8c08fe36
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Tue Nov 30 18:50:02 2021 +0300

    all: anonymize separately

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
 
 
 

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)
+}

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
+		// TODO(e.burkov): Inspect special cases when user want to apply some
-		// some rules handled by other processors to the hosts with TLD.
+		// rules handled by other processors to the hosts with TLD.
 		d.Res = s.genNXDomain(req)
 
 		return resultCodeFinish

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

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.
-	if shouldLog && s.queryLog != nil {
 	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 {
 		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 {
+	} else if clientIP != nil {
-		e.Client = ip.String()
+		e.Client = clientIP.String()
 	}
 
 	e.Time = uint32(elapsed / 1000)

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"
@@ -165,6 +166,7 @@ func TestProcessQueryLogsAndStats(t *testing.T) {
 		srv := &Server{
 			queryLog:   ql,
 			stats:      st,
+			anonymizer: aghnet.NewIPMut(nil),
 		}
 		t.Run(tc.name, func(t *testing.T) {
 			req := &dns.Msg{

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
+	if !restartHTTP {
-	// goroutine and with its own context, because it waits until all
+		return
-	// 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)
 	}
+
+	// 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,

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,14 +37,18 @@ 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 {
+func initDNSServer() (err error) {
-	var 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,
 	}
@@ -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)
 	}
 

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, web.httpsServer.server)
-	shutdownSrv(ctx, cancel, web.httpServer)
+	shutdownSrv(ctx, web.httpServer)
-	shutdownSrv(ctx, cancel, web.httpServerBeta)
+	shutdownSrv(ctx, web.httpServerBeta)
 
 	log.Info("stopped http server")
 }

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)
+		})
+	}
+}

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{}
+	d := &qlogConfig{}
-	req, err := jsonutil.DecodeObject(&d, r.Body)
+	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

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)
+	// The elements order is already reversed to be from newer to older.
-	for i := 0; i < len(entries); i++ {
+	for _, entry := range entries {
-		entry := entries[i]
+		jsonEntry := l.entryToJSON(entry, l.anonymizer.Load())
-		jsonEntry := l.logEntryToJSONEntry(entry)
 		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":       eip,
-		"client_info":  entry.client,
 		"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

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,

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()
@@ -116,6 +120,7 @@ func newQueryLog(conf Config) (l *queryLog) {
 		findClient: findClient,
 
 		logFile:    filepath.Join(conf.BaseDir, queryLogFileName),
+		anonymizer: conf.Anonymizer,
 	}
 
 	l.conf = &Config{}

internal/stats/stats.go

@@ -19,7 +19,6 @@ 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
 
 	// Called when the configuration is changed by HTTP request
 	ConfigModified func()

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 {
+func (s *statsCtx) swapUnit(new *unit) (u *unit) {
-	s.unitLock.Lock()
+	s.mu.Lock()
-	u := s.unit
+	defer s.mu.Unlock()
-	s.unit = new
+
-	s.unitLock.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.ongoing()
-		ptr := s.unit
-		s.unitLock.Unlock()
-
 		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()
+	s.mu.Lock()
-	defer s.unitLock.Unlock()
+	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()
+	cur := s.ongoing()
-	curUnit := serialize(s.unit)
+	curID := cur.id
-	curID := s.unit.id
-	s.unitLock.Unlock()
 
 	// 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)