From 668155b3672168cb15a778bc99554340b283fc58 Mon Sep 17 00:00:00 2001
From: Dimitry Kolyshev <dkolyshev@adguard.com>
Date: Thu, 18 May 2023 13:59:37 +0300
Subject: [PATCH] dnsforward: allowed clients private nets

---
 CHANGELOG.md                       |  4 ++--
 internal/dnsforward/access.go      | 18 ++++++++++++++----
 internal/dnsforward/access_test.go | 13 ++++++++-----
 internal/dnsforward/dnsforward.go  |  1 +
 4 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 611cb089..8e45d86d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -30,8 +30,8 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 ### Fixed
 
-- Locally served address are now ignored from allowed/blocked clients access
-  lists.  These addresses are always allowed ([#5799]).
+- Private networks are now ignored from allowed/blocked clients access lists.
+  These addresses are always allowed ([#5799]).
 
 - Unquoted IPv6 bind hosts with trailing colons erroneously considered
   unspecified addresses are now properly validated ([#5752]).
diff --git a/internal/dnsforward/access.go b/internal/dnsforward/access.go
index 08e12c3f..f340ec3e 100644
--- a/internal/dnsforward/access.go
+++ b/internal/dnsforward/access.go
@@ -31,6 +31,9 @@ type accessManager struct {
 
 	blockedHostsEng *urlfilter.DNSEngine
 
+	// privateNets is the set of IP networks those are ignored by the manager.
+	privateNets netutil.SubnetSet
+
 	// TODO(a.garipov): Create a type for a set of IP networks.
 	allowedNets []netip.Prefix
 	blockedNets []netip.Prefix
@@ -65,13 +68,20 @@ func processAccessClients(
 }
 
 // newAccessCtx creates a new accessCtx.
-func newAccessCtx(allowed, blocked, blockedHosts []string) (a *accessManager, err error) {
+func newAccessCtx(
+	allowed []string,
+	blocked []string,
+	blockedHosts []string,
+	privateNets netutil.SubnetSet,
+) (a *accessManager, err error) {
 	a = &accessManager{
 		allowedIPs: map[netip.Addr]unit{},
 		blockedIPs: map[netip.Addr]unit{},
 
 		allowedClientIDs: stringutil.NewSet(),
 		blockedClientIDs: stringutil.NewSet(),
+
+		privateNets: privateNets,
 	}
 
 	err = processAccessClients(allowed, a.allowedIPs, &a.allowedNets, a.allowedClientIDs)
@@ -140,9 +150,9 @@ func (a *accessManager) isBlockedHost(host string, qt rules.RRType) (ok bool) {
 }
 
 // isBlockedIP returns the status of the IP address blocking as well as the
-// rule that blocked it.  Locally served addresses are always allowed.
+// rule that blocked it.  Addresses from private nets are always allowed.
 func (a *accessManager) isBlockedIP(ip netip.Addr) (blocked bool, rule string) {
-	if netutil.IsLocallyServedAddr(ip) {
+	if a.privateNets.Contains(ip.AsSlice()) {
 		return false, ""
 	}
 
@@ -246,7 +256,7 @@ func (s *Server) handleAccessSet(w http.ResponseWriter, r *http.Request) {
 	}
 
 	var a *accessManager
-	a, err = newAccessCtx(list.AllowedClients, list.DisallowedClients, list.BlockedHosts)
+	a, err = newAccessCtx(list.AllowedClients, list.DisallowedClients, list.BlockedHosts, nil)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "creating access ctx: %s", err)
 
diff --git a/internal/dnsforward/access_test.go b/internal/dnsforward/access_test.go
index ff5af594..83db756e 100644
--- a/internal/dnsforward/access_test.go
+++ b/internal/dnsforward/access_test.go
@@ -4,6 +4,7 @@ import (
 	"net/netip"
 	"testing"
 
+	"github.com/AdguardTeam/golibs/netutil"
 	"github.com/AdguardTeam/urlfilter/rules"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
@@ -14,12 +15,12 @@ func TestIsBlockedClientID(t *testing.T) {
 	clientID := "client-1"
 	clients := []string{clientID}
 
-	a, err := newAccessCtx(clients, nil, nil)
+	a, err := newAccessCtx(clients, nil, nil, nil)
 	require.NoError(t, err)
 
 	assert.False(t, a.isBlockedClientID(clientID))
 
-	a, err = newAccessCtx(nil, clients, nil)
+	a, err = newAccessCtx(nil, clients, nil, nil)
 	require.NoError(t, err)
 
 	assert.True(t, a.isBlockedClientID(clientID))
@@ -31,7 +32,7 @@ func TestIsBlockedHost(t *testing.T) {
 		"*.host.com",
 		"||host3.com^",
 		"||*^$dnstype=HTTPS",
-	})
+	}, nil)
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -109,7 +110,8 @@ func TestAccessManager_IsBlockedIP_allow(t *testing.T) {
 		"5.6.7.8/24",
 	}
 
-	allowCtx, err := newAccessCtx(clients, nil, nil)
+	privateNets := netutil.SubnetSetFunc(netutil.IsLocallyServed)
+	allowCtx, err := newAccessCtx(clients, nil, nil, privateNets)
 	require.NoError(t, err)
 
 	testCases := []struct {
@@ -159,7 +161,8 @@ func TestAccessManager_IsBlockedIP_block(t *testing.T) {
 		"5.6.7.8/24",
 	}
 
-	blockCtx, err := newAccessCtx(nil, clients, nil)
+	privateNets := netutil.SubnetSetFunc(netutil.IsLocallyServed)
+	blockCtx, err := newAccessCtx(nil, clients, nil, privateNets)
 	require.NoError(t, err)
 
 	testCases := []struct {
diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go
index 1aeea73d..8579bf63 100644
--- a/internal/dnsforward/dnsforward.go
+++ b/internal/dnsforward/dnsforward.go
@@ -509,6 +509,7 @@ func (s *Server) Prepare(conf *ServerConfig) (err error) {
 		s.conf.AllowedClients,
 		s.conf.DisallowedClients,
 		s.conf.BlockedHosts,
+		s.privateNets,
 	)
 	if err != nil {
 		return fmt.Errorf("preparing access: %w", err)