Skip to content

Commit

Permalink
all: upd proxy ratelimit whitelist
Browse files Browse the repository at this point in the history
  • Loading branch information
schzhn committed Nov 29, 2023
1 parent 849abaf commit e753bb5
Show file tree
Hide file tree
Showing 13 changed files with 73 additions and 102 deletions.
3 changes: 2 additions & 1 deletion go.mod
Expand Up @@ -3,7 +3,8 @@ module github.com/AdguardTeam/AdGuardHome
go 1.20

require (
github.com/AdguardTeam/dnsproxy v0.59.1
// TODO(a.garipov): Upgrade to v0.60.
github.com/AdguardTeam/dnsproxy v0.59.2-0.20231129094552-f661fdcf9edc
github.com/AdguardTeam/golibs v0.17.2
github.com/AdguardTeam/urlfilter v0.17.3
github.com/NYTimes/gziphandler v1.1.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
@@ -1,5 +1,5 @@
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.20231129094552-f661fdcf9edc h1:y2Q9zxOXMvDhAetcwrsvxRp8O3Sz5CSlIvPm4SfiTAk=
github.com/AdguardTeam/dnsproxy v0.59.2-0.20231129094552-f661fdcf9edc/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/urlfilter v0.17.3 h1:fg/ObbnO0Cv6aw0tW6N/ETDMhhNvmcUUOZ7HlmKC3rw=
Expand Down
38 changes: 19 additions & 19 deletions internal/dnsforward/config.go
Expand Up @@ -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"`
Expand Down Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion internal/dnsforward/dnsforward.go
Expand Up @@ -298,7 +298,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)
Expand Down
5 changes: 1 addition & 4 deletions internal/dnsforward/dnsforward_test.go
Expand Up @@ -57,10 +57,7 @@ const (
// testClientAddr 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 testClientAddr = netip.MustParseAddrPort("1.2.3.4:12345")

func startDeferStop(t *testing.T, s *Server) {
t.Helper()
Expand Down
7 changes: 2 additions & 5 deletions internal/dnsforward/filter.go
Expand Up @@ -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"
Expand All @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/dnsforward/filter_test.go
Expand Up @@ -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: netip.MustParseAddrPort("127.0.0.1:1"),
}

t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -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: netip.MustParseAddrPort("127.0.0.1:1"),
}

dctx := &dnsContext{
Expand Down
13 changes: 8 additions & 5 deletions internal/dnsforward/http.go
Expand Up @@ -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"`
Expand Down Expand Up @@ -129,7 +129,10 @@ 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 := slices.Clone(s.conf.RatelimitWhitelist)
if ratelimitWhitelist == nil {
ratelimitWhitelist = []netip.Addr{}
}

customIP := s.conf.EDNSClientSubnet.CustomIP
enableEDNSClientSubnet := s.conf.EDNSClientSubnet.Enabled
Expand Down Expand Up @@ -411,9 +414,9 @@ func (req *jsonDNSConfig) checkRatelimitWhitelist() (err error) {
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)
for i, ip := range *req.RatelimitWhitelist {
if !ip.IsValid() {
return fmt.Errorf("ratelimit whitelist: at index %d: invalid ip", i)
}
}

Expand Down
5 changes: 2 additions & 3 deletions internal/dnsforward/http_test.go
Expand Up @@ -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: "",
Expand Down
20 changes: 7 additions & 13 deletions internal/dnsforward/process.go
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
50 changes: 23 additions & 27 deletions internal/dnsforward/process_internal_test.go
Expand Up @@ -104,7 +104,7 @@ func TestServer_ProcessInitial(t *testing.T) {

gotRC := s.processInitial(dctx)
assert.Equal(t, tc.wantRC, gotRC)
assert.Equal(t, netutil.NetAddrToAddrPort(testClientAddr).Addr(), gotAddr)
assert.Equal(t, testClientAddr.Addr(), gotAddr)

if tc.wantRCode > 0 {
gotResp := dctx.proxyCtx.Res
Expand Down Expand Up @@ -397,33 +397,31 @@ func TestServer_ProcessDetermineLocal(t *testing.T) {
}

testCases := []struct {
want assert.BoolAssertionFunc
name string
cliIP net.IP
want assert.BoolAssertionFunc
name string
cliAddr netip.AddrPort
}{{
want: assert.True,
name: "local",
cliIP: net.IP{192, 168, 0, 1},
want: assert.True,
name: "local",
cliAddr: netip.MustParseAddrPort("192.168.0.1:1"),
}, {
want: assert.False,
name: "external",
cliIP: net.IP{250, 249, 0, 1},
want: assert.False,
name: "external",
cliAddr: netip.MustParseAddrPort("250.249.0.1:1"),
}, {
want: assert.False,
name: "invalid",
cliIP: net.IP{1, 2, 3, 4, 5},
want: assert.False,
name: "invalid",
cliAddr: netip.AddrPort{},
}, {
want: assert.False,
name: "nil",
cliIP: nil,
want: assert.False,
name: "nil",
cliAddr: netip.AddrPort{},
}}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
proxyCtx := &proxy.DNSContext{
Addr: &net.TCPAddr{
IP: tc.cliIP,
},
Addr: tc.cliAddr,
}
dctx := &dnsContext{
proxyCtx: proxyCtx,
Expand Down Expand Up @@ -711,31 +709,31 @@ func TestServer_ProcessRestrictLocal(t *testing.T) {
name string
want string
question net.IP
cliIP net.IP
cliAddr netip.AddrPort
wantLen int
}{{
name: "from_local_to_external",
want: "host1.example.net.",
question: net.IP{254, 253, 252, 251},
cliIP: net.IP{192, 168, 10, 10},
cliAddr: netip.MustParseAddrPort("192.168.10.10:1"),
wantLen: 1,
}, {
name: "from_external_for_local",
want: "",
question: net.IP{192, 168, 1, 1},
cliIP: net.IP{254, 253, 252, 251},
cliAddr: netip.MustParseAddrPort("254.253.252.251:1"),
wantLen: 0,
}, {
name: "from_local_for_local",
want: "some.local-client.",
question: net.IP{192, 168, 1, 1},
cliIP: net.IP{192, 168, 1, 2},
cliAddr: netip.MustParseAddrPort("192.168.1.2:1"),
wantLen: 1,
}, {
name: "from_external_for_external",
want: "host1.example.net.",
question: net.IP{254, 253, 252, 251},
cliIP: net.IP{254, 253, 252, 255},
cliAddr: netip.MustParseAddrPort("254.253.252.255:1"),
wantLen: 1,
}}

Expand All @@ -747,9 +745,7 @@ func TestServer_ProcessRestrictLocal(t *testing.T) {
pctx := &proxy.DNSContext{
Proto: proxy.ProtoTCP,
Req: req,
Addr: &net.TCPAddr{
IP: tc.cliIP,
},
Addr: tc.cliAddr,
}

t.Run(tc.name, func(t *testing.T) {
Expand Down
7 changes: 2 additions & 5 deletions internal/dnsforward/stats.go
Expand Up @@ -10,9 +10,7 @@ import (
"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"
"golang.org/x/exp/slices"
)

// Write Stats data and logs
Expand All @@ -25,13 +23,12 @@ func (s *Server) processQueryLogsAndStats(dctx *dnsContext) (rc resultCode) {
host := aghnet.NormalizeDomain(q.Name)
processingTime := time.Since(dctx.startTime)

ip, _ := netutil.IPAndPortFromAddr(pctx.Addr)
ip = slices.Clone(ip)
ip := pctx.Addr.Addr().AsSlice()
s.anonymizer.Load()(ip)

log.Debug("dnsforward: client ip for stats and querylog: %s", ip)

ipStr := ip.String()
ipStr := pctx.Addr.Addr().String()
ids := []string{ipStr, dctx.clientID}
qt, cl := q.Qtype, q.Qclass

Expand Down

0 comments on commit e753bb5

Please sign in to comment.