Pull request 2087: AG-27616-upd-proxy-ratelimit-whitelist

Squashed commit of the following:

commit 099a2eb
Author: Stanislav Chzhen <[email protected]>
Date:   Tue Dec 5 17:25:49 2023 +0300

    all: upd proxy

commit db07130
Merge: 9e6e8e7 75cb9d4
Author: Stanislav Chzhen <[email protected]>
Date:   Tue Dec 5 14:44:44 2023 +0300

    Merge branch 'master' into AG-27616-upd-proxy-ratelimit-whitelist

commit 9e6e8e7
Author: Stanislav Chzhen <[email protected]>
Date:   Wed Nov 29 19:46:17 2023 +0300

    all: imp tests

commit e753bb5
Author: Stanislav Chzhen <[email protected]>
Date:   Wed Nov 29 13:35:21 2023 +0300

    all: upd proxy ratelimit whitelist

go.mod

@@ -3,8 +3,8 @@ module github.com/AdguardTeam/AdGuardHome
 go 1.20
 
 require (
-	github.com/AdguardTeam/dnsproxy v0.59.2-0.20231129093901-b1d6bad15937
-	github.com/AdguardTeam/golibs v0.17.2
+	github.com/AdguardTeam/dnsproxy v0.60.0
+	github.com/AdguardTeam/golibs v0.18.0
 	github.com/AdguardTeam/urlfilter v0.17.3
 	github.com/NYTimes/gziphandler v1.1.1
 	github.com/ameshkov/dnscrypt/v2 v2.2.7
@@ -64,5 +64,3 @@ require (
 	golang.org/x/text v0.14.0 // indirect
 	golang.org/x/tools v0.15.0 // indirect
 )
-
-require github.com/jessevdk/go-flags v1.5.0 // indirect

go.sum

@@ -1,9 +1,7 @@
-github.com/AdguardTeam/dnsproxy v0.59.1 h1:G/6T32EuPF0rhRkACkLFwD0pajI9351a1LACpuA2UcE=
-github.com/AdguardTeam/dnsproxy v0.59.1/go.mod h1:ZvkbM71HwpilgkCnTubDiR4Ba6x5Qvnhy2iasMWaTDM=
-github.com/AdguardTeam/dnsproxy v0.59.2-0.20231129093901-b1d6bad15937 h1:ZGOsX3UYLtrPJbBoKL9Zv6PLnO1duE8RgOsoxFmXGjQ=
-github.com/AdguardTeam/dnsproxy v0.59.2-0.20231129093901-b1d6bad15937/go.mod h1:ZvkbM71HwpilgkCnTubDiR4Ba6x5Qvnhy2iasMWaTDM=
-github.com/AdguardTeam/golibs v0.17.2 h1:vg6wHMjUKscnyPGRvxS5kAt7Uw4YxcJiITZliZ476W8=
-github.com/AdguardTeam/golibs v0.17.2/go.mod h1:DKhCIXHcUYtBhU8ibTLKh1paUL96n5zhQBlx763sj+U=
+github.com/AdguardTeam/dnsproxy v0.60.0 h1:0gLYoFyWRhQ1MP6g6AqqZXL5/h2QM4FE1aybUZ5HGuE=
+github.com/AdguardTeam/dnsproxy v0.60.0/go.mod h1:B7FvvTFQZBfey1cJXQo732EyCLX6xj4JqrciCawATzg=
+github.com/AdguardTeam/golibs v0.18.0 h1:ckS2YK7t2Ub6UkXl0fnreVaM15Zb07Hh1gmFqttjpWg=
+github.com/AdguardTeam/golibs v0.18.0/go.mod h1:DKhCIXHcUYtBhU8ibTLKh1paUL96n5zhQBlx763sj+U=
 github.com/AdguardTeam/urlfilter v0.17.3 h1:fg/ObbnO0Cv6aw0tW6N/ETDMhhNvmcUUOZ7HlmKC3rw=
 github.com/AdguardTeam/urlfilter v0.17.3/go.mod h1:Jru7jFfeH2CoDf150uDs+rRYcZBzHHBz05r9REyDKyE=
 github.com/NYTimes/gziphandler v1.1.1 h1:ZUDjpQae29j0ryrS0u/B8HZfJBtBQHjqw2rQ2cqUQ3I=
@@ -54,8 +52,6 @@ github.com/hugelgupf/socketpair v0.0.0-20190730060125-05d35a94e714 h1:/jC7qQFrv8
 github.com/insomniacslk/dhcp v0.0.0-20231016090811-6a2c8fbdcc1c h1:PgxFEySCI41sH0mB7/2XswdXbUykQsRUGod8Rn+NubM=
 github.com/insomniacslk/dhcp v0.0.0-20231016090811-6a2c8fbdcc1c/go.mod h1:3A9PQ1cunSDF/1rbTq99Ts4pVnycWg+vlPkfeD2NLFI=
 github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI=
-github.com/jessevdk/go-flags v1.5.0 h1:1jKYvbxEjfUl0fmqTCOfonvskHHXMjBySTLW4y9LFvc=
-github.com/jessevdk/go-flags v1.5.0/go.mod h1:Fw0T6WPc1dYxT4mKEZRfG5kJhaTDP9pj1c2EWnYs/m4=
 github.com/josharian/native v1.0.0/go.mod h1:7X/raswPFr05uY3HiLlYeyQntB6OO7E/d2Cu7qoaN2w=
 github.com/josharian/native v1.0.1-0.20221213033349-c1e37c09b531/go.mod h1:7X/raswPFr05uY3HiLlYeyQntB6OO7E/d2Cu7qoaN2w=
 github.com/josharian/native v1.1.1-0.20230202152459-5c7d0dd6ab86 h1:elKwZS1OcdQ0WwEDBeqxKwb7WB62QX8bvZ/FJnVXIfk=
@@ -104,7 +100,6 @@ github.com/shirou/gopsutil/v3 v3.23.7 h1:C+fHO8hfIppoJ1WdsVm1RoI0RwXoNdfTK7yWXV0
 github.com/shoenig/go-m1cpu v0.1.6 h1:nxdKQNcEB6vzgA2E2bvzKIYRuNj7XNJ4S/aRSwKzFtM=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/objx v0.5.0 h1:1zr/of2m5FGMsad5YfcqgdqdWrIhu+EBEJRhR1U7z/c=
-github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk=
@@ -150,7 +145,6 @@ golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7w
 golang.org/x/sys v0.0.0-20201015000850-e3ed0017c211/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210315160823-c6e025ad8005/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
-golang.org/x/sys v0.0.0-20210320140829-1e4c9ba3b0c4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220209214540-3681064d5158/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220622161953-175b2fd9d664/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=

internal/dnsforward/config.go

@@ -67,7 +67,7 @@ type Config struct {
 	RatelimitSubnetLenIPv6 int `yaml:"ratelimit_subnet_len_ipv6"`
 
 	// RatelimitWhitelist is the list of whitelisted client IP addresses.
-	RatelimitWhitelist []string `yaml:"ratelimit_whitelist"`
+	RatelimitWhitelist []netip.Addr `yaml:"ratelimit_whitelist"`
 
 	// RefuseAny, if true, refuse ANY requests.
 	RefuseAny bool `yaml:"refuse_any"`
@@ -298,24 +298,24 @@ type ServerConfig struct {
 func (s *Server) newProxyConfig() (conf *proxy.Config, err error) {
 	srvConf := s.conf
 	conf = &proxy.Config{
-		HTTP3:                   srvConf.ServeHTTP3,
-		Ratelimit:               int(srvConf.Ratelimit),
-		RatelimitSubnetMaskIPv4: net.CIDRMask(srvConf.RatelimitSubnetLenIPv4, netutil.IPv4BitLen),
-		RatelimitSubnetMaskIPv6: net.CIDRMask(srvConf.RatelimitSubnetLenIPv6, netutil.IPv6BitLen),
-		RatelimitWhitelist:      srvConf.RatelimitWhitelist,
-		RefuseAny:               srvConf.RefuseAny,
-		TrustedProxies:          srvConf.TrustedProxies,
-		CacheMinTTL:             srvConf.CacheMinTTL,
-		CacheMaxTTL:             srvConf.CacheMaxTTL,
-		CacheOptimistic:         srvConf.CacheOptimistic,
-		UpstreamConfig:          srvConf.UpstreamConfig,
-		BeforeRequestHandler:    s.beforeRequestHandler,
-		RequestHandler:          s.handleDNSRequest,
-		HTTPSServerName:         aghhttp.UserAgent(),
-		EnableEDNSClientSubnet:  srvConf.EDNSClientSubnet.Enabled,
-		MaxGoroutines:           int(srvConf.MaxGoroutines),
-		UseDNS64:                srvConf.UseDNS64,
-		DNS64Prefs:              srvConf.DNS64Prefixes,
+		HTTP3:                  srvConf.ServeHTTP3,
+		Ratelimit:              int(srvConf.Ratelimit),
+		RatelimitSubnetLenIPv4: srvConf.RatelimitSubnetLenIPv4,
+		RatelimitSubnetLenIPv6: srvConf.RatelimitSubnetLenIPv6,
+		RatelimitWhitelist:     srvConf.RatelimitWhitelist,
+		RefuseAny:              srvConf.RefuseAny,
+		TrustedProxies:         srvConf.TrustedProxies,
+		CacheMinTTL:            srvConf.CacheMinTTL,
+		CacheMaxTTL:            srvConf.CacheMaxTTL,
+		CacheOptimistic:        srvConf.CacheOptimistic,
+		UpstreamConfig:         srvConf.UpstreamConfig,
+		BeforeRequestHandler:   s.beforeRequestHandler,
+		RequestHandler:         s.handleDNSRequest,
+		HTTPSServerName:        aghhttp.UserAgent(),
+		EnableEDNSClientSubnet: srvConf.EDNSClientSubnet.Enabled,
+		MaxGoroutines:          int(srvConf.MaxGoroutines),
+		UseDNS64:               srvConf.UseDNS64,
+		DNS64Prefs:             srvConf.DNS64Prefixes,
 	}
 
 	if srvConf.EDNSClientSubnet.UseCustom {

internal/dnsforward/dnsforward.go

@@ -299,7 +299,7 @@ func (s *Server) WriteDiskConfig(c *Config) {
 
 	sc := s.conf.Config
 	*c = sc
-	c.RatelimitWhitelist = stringutil.CloneSlice(sc.RatelimitWhitelist)
+	c.RatelimitWhitelist = slices.Clone(sc.RatelimitWhitelist)
 	c.BootstrapDNS = stringutil.CloneSlice(sc.BootstrapDNS)
 	c.FallbackDNS = stringutil.CloneSlice(sc.FallbackDNS)
 	c.AllowedClients = stringutil.CloneSlice(sc.AllowedClients)

internal/dnsforward/dnsforward_test.go

@@ -54,13 +54,10 @@ const (
 	testMessagesCount = 10
 )
 
-// testClientAddr is the common net.Addr for tests.
+// testClientAddrPort is the common net.Addr for tests.
 //
 // TODO(a.garipov): Use more.
-var testClientAddr net.Addr = &net.TCPAddr{
-	IP:   net.IP{1, 2, 3, 4},
-	Port: 12345,
-}
+var testClientAddrPort = netip.MustParseAddrPort("1.2.3.4:12345")
 
 func startDeferStop(t *testing.T, s *Server) {
 	t.Helper()

internal/dnsforward/filter.go

@@ -10,7 +10,6 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/golibs/log"
-	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/urlfilter/rules"
 	"github.com/miekg/dns"
 	"golang.org/x/exp/slices"
@@ -28,8 +27,7 @@ func (s *Server) beforeRequestHandler(
 		return false, fmt.Errorf("getting clientid: %w", err)
 	}
 
-	addrPort := netutil.NetAddrToAddrPort(pctx.Addr)
-	blocked, _ := s.IsBlockedClient(addrPort.Addr(), clientID)
+	blocked, _ := s.IsBlockedClient(pctx.Addr.Addr(), clientID)
 	if blocked {
 		return s.preBlockedResponse(pctx)
 	}
@@ -60,8 +58,7 @@ func (s *Server) clientRequestFilteringSettings(dctx *dnsContext) (setts *filter
 	setts = s.dnsFilter.Settings()
 	setts.ProtectionEnabled = dctx.protectionEnabled
 	if s.conf.FilterHandler != nil {
-		addrPort := netutil.NetAddrToAddrPort(dctx.proxyCtx.Addr)
-		s.conf.FilterHandler(addrPort.Addr(), dctx.clientID, setts)
+		s.conf.FilterHandler(dctx.proxyCtx.Addr.Addr(), dctx.clientID, setts)
 	}
 
 	return setts

internal/dnsforward/filter_test.go

@@ -187,7 +187,7 @@ func TestHandleDNSRequest_handleDNSRequest(t *testing.T) {
 		dctx := &proxy.DNSContext{
 			Proto: proxy.ProtoUDP,
 			Req:   tc.req,
-			Addr:  &net.UDPAddr{IP: net.IP{127, 0, 0, 1}, Port: 1},
+			Addr:  testClientAddrPort,
 		}
 
 		t.Run(tc.name, func(t *testing.T) {
@@ -326,7 +326,7 @@ func TestHandleDNSRequest_filterDNSResponse(t *testing.T) {
 				Proto: proxy.ProtoUDP,
 				Req:   tc.req,
 				Res:   resp,
-				Addr:  &net.UDPAddr{IP: net.IP{127, 0, 0, 1}, Port: 1},
+				Addr:  testClientAddrPort,
 			}
 
 			dctx := &dnsContext{

internal/dnsforward/http.go

@@ -52,7 +52,7 @@ type jsonDNSConfig struct {
 	RatelimitSubnetLenIPv6 *int `json:"ratelimit_subnet_len_ipv6"`
 
 	// RatelimitWhitelist is a list of IP addresses excluded from rate limiting.
-	RatelimitWhitelist *[]string `json:"ratelimit_whitelist"`
+	RatelimitWhitelist *[]netip.Addr `json:"ratelimit_whitelist"`
 
 	// BlockingMode defines the way blocked responses are constructed.
 	BlockingMode *filtering.BlockingMode `json:"blocking_mode"`
@@ -129,7 +129,7 @@ func (s *Server) getDNSConfig() (c *jsonDNSConfig) {
 	ratelimit := s.conf.Ratelimit
 	ratelimitSubnetLenIPv4 := s.conf.RatelimitSubnetLenIPv4
 	ratelimitSubnetLenIPv6 := s.conf.RatelimitSubnetLenIPv6
-	ratelimitWhitelist := stringutil.CloneSliceOrEmpty(s.conf.RatelimitWhitelist)
+	ratelimitWhitelist := append([]netip.Addr{}, s.conf.RatelimitWhitelist...)
 
 	customIP := s.conf.EDNSClientSubnet.CustomIP
 	enableEDNSClientSubnet := s.conf.EDNSClientSubnet.Enabled
@@ -291,12 +291,6 @@ func (req *jsonDNSConfig) validate(privateNets netutil.SubnetSet) (err error) {
 		return err
 	}
 
-	err = req.checkRatelimitWhitelist()
-	if err != nil {
-		// Don't wrap the error since it's informative enough as is.
-		return err
-	}
-
 	err = req.checkBlockingMode()
 	if err != nil {
 		// Don't wrap the error since it's informative enough as is.
@@ -405,21 +399,6 @@ func checkInclusion(ptr *int, minN, maxN int) (err error) {
 	return nil
 }
 
-// checkRatelimitWhitelist returns an error if any of IP addresses is invalid.
-func (req *jsonDNSConfig) checkRatelimitWhitelist() (err error) {
-	if req.RatelimitWhitelist == nil {
-		return nil
-	}
-
-	for i, ipStr := range *req.RatelimitWhitelist {
-		if _, err = netip.ParseAddr(ipStr); err != nil {
-			return fmt.Errorf("ratelimit whitelist: at index %d: %w", i, err)
-		}
-	}
-
-	return nil
-}
-
 // handleSetConfig handles requests to the POST /control/dns_config endpoint.
 func (s *Server) handleSetConfig(w http.ResponseWriter, r *http.Request) {
 	req := &jsonDNSConfig{}

internal/dnsforward/http_test.go

@@ -195,9 +195,8 @@ func TestDNSForwardHTTP_handleSetConfig(t *testing.T) {
 		name:    "ratelimit_subnet_len",
 		wantSet: "",
 	}, {
-		name: "ratelimit_whitelist_not_ip",
-		wantSet: `validating dns config: ratelimit whitelist: at index 1: ParseAddr("not.ip"): ` +
-			`unexpected character (at "not.ip")`,
+		name:    "ratelimit_whitelist_not_ip",
+		wantSet: `decoding request: ParseAddr("not.ip"): unexpected character (at "not.ip")`,
 	}, {
 		name:    "edns_cs_enabled",
 		wantSet: "",

internal/dnsforward/process.go

@@ -191,7 +191,7 @@ func (s *Server) processInitial(dctx *dnsContext) (rc resultCode) {
 	defer log.Debug("dnsforward: finished processing initial")
 
 	pctx := dctx.proxyCtx
-	s.processClientIP(pctx.Addr)
+	s.processClientIP(pctx.Addr.Addr())
 
 	q := pctx.Req.Question[0]
 	qt := q.Qtype
@@ -228,9 +228,8 @@ func (s *Server) processInitial(dctx *dnsContext) (rc resultCode) {
 }
 
 // processClientIP sends the client IP address to s.addrProc, if needed.
-func (s *Server) processClientIP(addr net.Addr) {
-	clientIP := netutil.NetAddrToAddrPort(addr).Addr()
-	if clientIP == (netip.Addr{}) {
+func (s *Server) processClientIP(addr netip.Addr) {
+	if !addr.IsValid() {
 		log.Info("dnsforward: warning: bad client addr %q", addr)
 
 		return
@@ -241,7 +240,7 @@ func (s *Server) processClientIP(addr net.Addr) {
 	s.serverLock.RLock()
 	defer s.serverLock.RUnlock()
 
-	s.addrProc.Process(clientIP)
+	s.addrProc.Process(addr)
 }
 
 // processDDRQuery responds to Discovery of Designated Resolvers (DDR) SVCB
@@ -351,12 +350,7 @@ func (s *Server) processDetermineLocal(dctx *dnsContext) (rc resultCode) {
 
 	rc = resultCodeSuccess
 
-	var ip net.IP
-	if ip, _ = netutil.IPAndPortFromAddr(dctx.proxyCtx.Addr); ip == nil {
-		return rc
-	}
-
-	dctx.isLocalClient = s.privateNets.Contains(ip)
+	dctx.isLocalClient = s.privateNets.Contains(dctx.proxyCtx.Addr.Addr().AsSlice())
 
 	return rc
 }
@@ -831,12 +825,12 @@ func (s *Server) dhcpHostFromRequest(q *dns.Question) (reqHost string) {
 
 // setCustomUpstream sets custom upstream settings in pctx, if necessary.
 func (s *Server) setCustomUpstream(pctx *proxy.DNSContext, clientID string) {
-	if pctx.Addr == nil || s.conf.ClientsContainer == nil {
+	if !pctx.Addr.IsValid() || s.conf.ClientsContainer == nil {
 		return
 	}
 
 	// Use the ClientID first, since it has a higher priority.
-	id := stringutil.Coalesce(clientID, ipStringFromAddr(pctx.Addr))
+	id := stringutil.Coalesce(clientID, pctx.Addr.Addr().String())
 	upsConf, err := s.conf.ClientsContainer.UpstreamConfigByID(id, s.bootstrap)
 	if err != nil {
 		log.Error("dnsforward: getting custom upstreams for client %s: %s", id, err)