+
+
+
{
upstream_mode,
resolve_clients,
local_ptr_upstreams,
+ use_private_ptr_resolvers,
} = useSelector((state) => state.dnsConfig, shallowEqual);
const upstream_dns_file = useSelector((state) => state.dnsConfig.upstream_dns_file);
@@ -25,6 +26,7 @@ const Upstream = () => {
upstream_mode,
resolve_clients,
local_ptr_upstreams,
+ use_private_ptr_resolvers,
} = values;
const dnsConfig = {
@@ -32,6 +34,7 @@ const Upstream = () => {
upstream_mode,
resolve_clients,
local_ptr_upstreams,
+ use_private_ptr_resolvers,
...(upstream_dns_file ? null : { upstream_dns }),
};
@@ -53,6 +56,7 @@ const Upstream = () => {
upstream_mode,
resolve_clients,
local_ptr_upstreams,
+ use_private_ptr_resolvers,
}}
onSubmit={handleSubmit}
/>
diff --git a/internal/dnsforward/access.go b/internal/dnsforward/access.go
index f27eb667..62c1e35b 100644
--- a/internal/dnsforward/access.go
+++ b/internal/dnsforward/access.go
@@ -142,14 +142,19 @@ type accessListJSON struct {
BlockedHosts []string `json:"blocked_hosts"`
}
-func (s *Server) handleAccessList(w http.ResponseWriter, r *http.Request) {
- s.RLock()
- j := accessListJSON{
- AllowedClients: s.conf.AllowedClients,
- DisallowedClients: s.conf.DisallowedClients,
- BlockedHosts: s.conf.BlockedHosts,
+func (s *Server) accessListJSON() (j accessListJSON) {
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
+ return accessListJSON{
+ AllowedClients: aghstrings.CloneSlice(s.conf.AllowedClients),
+ DisallowedClients: aghstrings.CloneSlice(s.conf.DisallowedClients),
+ BlockedHosts: aghstrings.CloneSlice(s.conf.BlockedHosts),
}
- s.RUnlock()
+}
+
+func (s *Server) handleAccessList(w http.ResponseWriter, r *http.Request) {
+ j := s.accessListJSON()
w.Header().Set("Content-Type", "application/json")
err := json.NewEncoder(w).Encode(j)
@@ -200,14 +205,16 @@ func (s *Server) handleAccessSet(w http.ResponseWriter, r *http.Request) {
return
}
- s.Lock()
+ defer log.Debug("Access: updated lists: %d, %d, %d",
+ len(j.AllowedClients), len(j.DisallowedClients), len(j.BlockedHosts))
+
+ defer s.conf.ConfigModified()
+
+ s.serverLock.Lock()
+ defer s.serverLock.Unlock()
+
s.conf.AllowedClients = j.AllowedClients
s.conf.DisallowedClients = j.DisallowedClients
s.conf.BlockedHosts = j.BlockedHosts
s.access = a
- s.Unlock()
- s.conf.ConfigModified()
-
- log.Debug("Access: updated lists: %d, %d, %d",
- len(j.AllowedClients), len(j.DisallowedClients), len(j.BlockedHosts))
}
diff --git a/internal/dnsforward/config.go b/internal/dnsforward/config.go
index 03710238..0fea1bd9 100644
--- a/internal/dnsforward/config.go
+++ b/internal/dnsforward/config.go
@@ -153,6 +153,10 @@ type ServerConfig struct {
// ResolveClients signals if the RDNS should resolve clients' addresses.
ResolveClients bool
+ // UsePrivateRDNS defines if the PTR requests for unknown addresses from
+ // locally-served networks should be resolved via private PTR resolvers.
+ UsePrivateRDNS bool
+
// LocalPTRResolvers is a slice of addresses to be used as upstreams for
// resolving PTR queries for local addresses.
LocalPTRResolvers []string
diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go
index 5fbd4070..364d447a 100644
--- a/internal/dnsforward/dns.go
+++ b/internal/dnsforward/dns.go
@@ -414,20 +414,25 @@ func (s *Server) processLocalPTR(ctx *dnsContext) (rc resultCode) {
return resultCodeSuccess
}
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
if !s.subnetDetector.IsLocallyServedNetwork(ip) {
return resultCodeSuccess
}
- err := s.localResolvers.Resolve(d)
- if err != nil {
- ctx.err = err
+ if s.conf.UsePrivateRDNS {
+ if err := s.localResolvers.Resolve(d); err != nil {
+ ctx.err = err
- return resultCodeError
+ return resultCodeError
+ }
}
if d.Res == nil {
d.Res = s.genNXDomain(d.Req)
+ // Do not even put into query log.
return resultCodeFinish
}
@@ -443,24 +448,20 @@ func processFilteringBeforeRequest(ctx *dnsContext) (rc resultCode) {
return resultCodeSuccess // response is already set - nothing to do
}
- s.RLock()
- // Synchronize access to s.dnsFilter so it won't be suddenly uninitialized while in use.
- // This could happen after proxy server has been stopped, but its workers are not yet exited.
- //
- // A better approach is for proxy.Stop() to wait until all its workers exit,
- // but this would require the Upstream interface to have Close() function
- // (to prevent from hanging while waiting for unresponsive DNS server to respond).
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
+ ctx.protectionEnabled = s.conf.ProtectionEnabled && s.dnsFilter != nil
+ if !ctx.protectionEnabled {
+ return resultCodeSuccess
+ }
+
+ if ctx.setts == nil {
+ ctx.setts = s.getClientRequestFilteringSettings(ctx)
+ }
var err error
- ctx.protectionEnabled = s.conf.ProtectionEnabled && s.dnsFilter != nil
- if ctx.protectionEnabled {
- if ctx.setts == nil {
- ctx.setts = s.getClientRequestFilteringSettings(ctx)
- }
- ctx.result, err = s.filterDNSRequest(ctx)
- }
- s.RUnlock()
-
+ ctx.result, err = s.filterDNSRequest(ctx)
if err != nil {
ctx.err = err
diff --git a/internal/dnsforward/dns_test.go b/internal/dnsforward/dns_test.go
index 7e016909..06a3c0e1 100644
--- a/internal/dnsforward/dns_test.go
+++ b/internal/dnsforward/dns_test.go
@@ -260,7 +260,7 @@ func TestServer_ProcessInternalHosts(t *testing.T) {
}
}
-func TestLocalRestriction(t *testing.T) {
+func TestServer_ProcessRestrictLocal(t *testing.T) {
ups := &aghtest.TestUpstream{
Reverse: map[string][]string{
"251.252.253.254.in-addr.arpa.": {"host1.example.net."},
@@ -318,14 +318,64 @@ func TestLocalRestriction(t *testing.T) {
IP: tc.cliIP,
},
}
+
t.Run(tc.name, func(t *testing.T) {
err = s.handleDNSRequest(nil, pctx)
require.NoError(t, err)
require.NotNil(t, pctx.Res)
require.Len(t, pctx.Res.Answer, tc.wantLen)
+
if tc.wantLen > 0 {
assert.Equal(t, tc.want, pctx.Res.Answer[0].Header().Name)
}
})
}
}
+
+func TestServer_ProcessLocalPTR_usingResolvers(t *testing.T) {
+ const locDomain = "some.local."
+ const reqAddr = "1.1.168.192.in-addr.arpa."
+
+ s := createTestServer(t, &filtering.Config{}, ServerConfig{
+ UDPListenAddrs: []*net.UDPAddr{{}},
+ TCPListenAddrs: []*net.TCPAddr{{}},
+ }, &aghtest.TestUpstream{
+ Reverse: map[string][]string{
+ reqAddr: {locDomain},
+ },
+ })
+
+ var proxyCtx *proxy.DNSContext
+ var dnsCtx *dnsContext
+ setup := func(use bool) {
+ proxyCtx = &proxy.DNSContext{
+ Addr: &net.TCPAddr{
+ IP: net.IP{127, 0, 0, 1},
+ },
+ Req: createTestMessageWithType(reqAddr, dns.TypePTR),
+ }
+ dnsCtx = &dnsContext{
+ proxyCtx: proxyCtx,
+ unreversedReqIP: net.IP{192, 168, 1, 1},
+ }
+ s.conf.UsePrivateRDNS = use
+ }
+
+ t.Run("enabled", func(t *testing.T) {
+ setup(true)
+
+ rc := s.processLocalPTR(dnsCtx)
+ require.Equal(t, resultCodeSuccess, rc)
+ require.NotEmpty(t, proxyCtx.Res.Answer)
+
+ assert.Equal(t, locDomain, proxyCtx.Res.Answer[0].Header().Name)
+ })
+
+ t.Run("disabled", func(t *testing.T) {
+ setup(false)
+
+ rc := s.processLocalPTR(dnsCtx)
+ require.Equal(t, resultCodeFinish, rc)
+ require.Empty(t, proxyCtx.Res.Answer)
+ })
+}
diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go
index cf31500c..15ddbc57 100644
--- a/internal/dnsforward/dnsforward.go
+++ b/internal/dnsforward/dnsforward.go
@@ -89,8 +89,9 @@ type Server struct {
isRunning bool
- sync.RWMutex
conf ServerConfig
+ // serverLock protects Server.
+ serverLock sync.RWMutex
}
// defaultLocalDomainSuffix is the default suffix used to detect internal hosts
@@ -167,25 +168,31 @@ func NewCustomServer(internalProxy *proxy.Proxy) *Server {
return s
}
-// Close - close object
+// Close gracefully closes the server. It is safe for concurrent use.
+//
+// TODO(e.burkov): A better approach would be making Stop method waiting for all
+// its workers finished. But it would require the upstream.Upstream to have the
+// Close method to prevent from hanging while waiting for unresponsive server to
+// respond.
func (s *Server) Close() {
- s.Lock()
+ s.serverLock.Lock()
+ defer s.serverLock.Unlock()
+
s.dnsFilter = nil
s.stats = nil
s.queryLog = nil
s.dnsProxy = nil
- err := s.ipset.Close()
- if err != nil {
+ if err := s.ipset.Close(); err != nil {
log.Error("closing ipset: %s", err)
}
-
- s.Unlock()
}
// WriteDiskConfig - write configuration
func (s *Server) WriteDiskConfig(c *FilteringConfig) {
- s.RLock()
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
sc := s.conf.FilteringConfig
*c = sc
c.RatelimitWhitelist = aghstrings.CloneSlice(sc.RatelimitWhitelist)
@@ -194,15 +201,16 @@ func (s *Server) WriteDiskConfig(c *FilteringConfig) {
c.DisallowedClients = aghstrings.CloneSlice(sc.DisallowedClients)
c.BlockedHosts = aghstrings.CloneSlice(sc.BlockedHosts)
c.UpstreamDNS = aghstrings.CloneSlice(sc.UpstreamDNS)
- s.RUnlock()
}
// RDNSSettings returns the copy of actual RDNS configuration.
-func (s *Server) RDNSSettings() (localPTRResolvers []string, resolveClients bool) {
- s.RLock()
- defer s.RUnlock()
+func (s *Server) RDNSSettings() (localPTRResolvers []string, resolveClients, resolvePTR bool) {
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
- return aghstrings.CloneSlice(s.conf.LocalPTRResolvers), s.conf.ResolveClients
+ return aghstrings.CloneSlice(s.conf.LocalPTRResolvers),
+ s.conf.ResolveClients,
+ s.conf.UsePrivateRDNS
}
// Resolve - get IP addresses by host name from an upstream server.
@@ -210,8 +218,9 @@ func (s *Server) RDNSSettings() (localPTRResolvers []string, resolveClients bool
// Query log and Stats are not updated.
// This method may be called before Start().
func (s *Server) Resolve(host string) ([]net.IPAddr, error) {
- s.RLock()
- defer s.RUnlock()
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
return s.internalProxy.LookupIPAddr(host)
}
@@ -220,6 +229,9 @@ type RDNSExchanger interface {
// Exchange tries to resolve the ip in a suitable way, e.g. either as
// local or as external.
Exchange(ip net.IP) (host string, err error)
+ // ResolvesPrivatePTR returns true if the RDNSExchanger is able to
+ // resolve PTR requests for locally-served addresses.
+ ResolvesPrivatePTR() (ok bool)
}
const (
@@ -234,13 +246,20 @@ const (
// Exchange implements the RDNSExchanger interface for *Server.
func (s *Server) Exchange(ip net.IP) (host string, err error) {
- s.RLock()
- defer s.RUnlock()
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
if !s.conf.ResolveClients {
return "", nil
}
+ var resolver *proxy.Proxy = s.localResolvers
+ if !s.subnetDetector.IsLocallyServedNetwork(ip) {
+ resolver = s.internalProxy
+ } else if !s.conf.UsePrivateRDNS {
+ return "", nil
+ }
+
arpa := dns.Fqdn(aghnet.ReverseAddr(ip))
req := &dns.Msg{
MsgHdr: dns.MsgHdr{
@@ -259,13 +278,8 @@ func (s *Server) Exchange(ip net.IP) (host string, err error) {
Req: req,
StartTime: time.Now(),
}
-
var resp *dns.Msg
- if s.subnetDetector.IsLocallyServedNetwork(ip) {
- err = s.localResolvers.Resolve(ctx)
- } else {
- err = s.internalProxy.Resolve(ctx)
- }
+ err = resolver.Resolve(ctx)
if err != nil {
return "", err
}
@@ -284,10 +298,19 @@ func (s *Server) Exchange(ip net.IP) (host string, err error) {
return strings.TrimSuffix(ptr.Ptr, "."), nil
}
+// ResolvesPrivatePTR implements the RDNSExchanger interface for *Server.
+func (s *Server) ResolvesPrivatePTR() (ok bool) {
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
+ return s.conf.UsePrivateRDNS
+}
+
// Start starts the DNS server.
func (s *Server) Start() error {
- s.Lock()
- defer s.Unlock()
+ s.serverLock.Lock()
+ defer s.serverLock.Unlock()
+
return s.startLocked()
}
@@ -306,7 +329,7 @@ func (s *Server) startLocked() error {
const defaultLocalTimeout = 1 * time.Second
// collectDNSIPAddrs returns IP addresses the server is listening on without
-// port numbers as a map. For internal use only.
+// port numbersю For internal use only.
func (s *Server) collectDNSIPAddrs() (addrs []string, err error) {
addrs = make([]string, len(s.conf.TCPListenAddrs)+len(s.conf.UDPListenAddrs))
var i int
@@ -472,8 +495,9 @@ func (s *Server) Prepare(config *ServerConfig) error {
// Stop stops the DNS server.
func (s *Server) Stop() error {
- s.Lock()
- defer s.Unlock()
+ s.serverLock.Lock()
+ defer s.serverLock.Unlock()
+
return s.stopLocked()
}
@@ -490,17 +514,18 @@ func (s *Server) stopLocked() error {
return nil
}
-// IsRunning returns true if the DNS server is running
+// IsRunning returns true if the DNS server is running.
func (s *Server) IsRunning() bool {
- s.RLock()
- defer s.RUnlock()
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
return s.isRunning
}
-// Reconfigure applies the new configuration to the DNS server
+// Reconfigure applies the new configuration to the DNS server.
func (s *Server) Reconfigure(config *ServerConfig) error {
- s.Lock()
- defer s.Unlock()
+ s.serverLock.Lock()
+ defer s.serverLock.Unlock()
log.Print("Start reconfiguring the server")
err := s.stopLocked()
@@ -525,12 +550,18 @@ func (s *Server) Reconfigure(config *ServerConfig) error {
return nil
}
-// ServeHTTP is a HTTP handler method we use to provide DNS-over-HTTPS
+// ServeHTTP is a HTTP handler method we use to provide DNS-over-HTTPS.
func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
- s.RLock()
- p := s.dnsProxy
- s.RUnlock()
- if p != nil { // an attempt to protect against race in case we're here after Close() was called
+ var p *proxy.Proxy
+
+ func() {
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
+ p = s.dnsProxy
+ }()
+
+ if p != nil {
p.ServeHTTP(w, r)
}
}
diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go
index 8cf48ff4..6ced4805 100644
--- a/internal/dnsforward/dnsforward_test.go
+++ b/internal/dnsforward/dnsforward_test.go
@@ -86,11 +86,12 @@ func createTestServer(
err = s.Prepare(nil)
require.NoError(t, err)
- s.Lock()
- defer s.Unlock()
+ s.serverLock.Lock()
+ defer s.serverLock.Unlock()
if localUps != nil {
s.localResolvers.Config.UpstreamConfig.Upstreams = []upstream.Upstream{localUps}
+ s.conf.UsePrivateRDNS = true
}
return s
@@ -1207,17 +1208,18 @@ func TestServer_Exchange(t *testing.T) {
Block: true,
}
- dns := NewCustomServer(&proxy.Proxy{
+ srv := NewCustomServer(&proxy.Proxy{
Config: proxy.Config{
UpstreamConfig: &proxy.UpstreamConfig{
Upstreams: []upstream.Upstream{extUpstream},
},
},
})
- dns.conf.ResolveClients = true
+ srv.conf.ResolveClients = true
+ srv.conf.UsePrivateRDNS = true
var err error
- dns.subnetDetector, err = aghnet.NewSubnetDetector()
+ srv.subnetDetector, err = aghnet.NewSubnetDetector()
require.NoError(t, err)
localIP := net.IP{192, 168, 1, 1}
@@ -1265,12 +1267,12 @@ func TestServer_Exchange(t *testing.T) {
Upstreams: []upstream.Upstream{tc.locUpstream},
},
}
- dns.localResolvers = &proxy.Proxy{
+ srv.localResolvers = &proxy.Proxy{
Config: pcfg,
}
t.Run(tc.name, func(t *testing.T) {
- host, eerr := dns.Exchange(tc.req)
+ host, eerr := srv.Exchange(tc.req)
require.ErrorIs(t, eerr, tc.wantErr)
assert.Equal(t, tc.want, host)
@@ -1278,12 +1280,11 @@ func TestServer_Exchange(t *testing.T) {
}
t.Run("resolving_disabled", func(t *testing.T) {
- dns.conf.ResolveClients = false
- for _, tc := range testCases {
- host, eerr := dns.Exchange(tc.req)
+ srv.conf.UsePrivateRDNS = false
- require.NoError(t, eerr)
- assert.Empty(t, host)
- }
+ host, eerr := srv.Exchange(localIP)
+
+ require.NoError(t, eerr)
+ assert.Empty(t, host)
})
}
diff --git a/internal/dnsforward/filter.go b/internal/dnsforward/filter.go
index 1bacc98c..3dd3530d 100644
--- a/internal/dnsforward/filter.go
+++ b/internal/dnsforward/filter.go
@@ -117,8 +117,31 @@ func (s *Server) filterDNSRequest(ctx *dnsContext) (*filtering.Result, error) {
return &res, err
}
-// If response contains CNAME, A or AAAA records, we apply filtering to each canonical host name or IP address.
-// If this is a match, we set a new response in d.Res and return.
+// checkHostRules checks the host against filters. It is safe for concurrent
+// use.
+func (s *Server) checkHostRules(host string, qtype uint16, setts *filtering.Settings) (
+ r *filtering.Result,
+ err error,
+) {
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
+
+ if s.dnsFilter == nil {
+ return nil, nil
+ }
+
+ var res filtering.Result
+ res, err = s.dnsFilter.CheckHostRules(host, qtype, setts)
+ if err != nil {
+ return nil, err
+ }
+
+ return &res, err
+}
+
+// If response contains CNAME, A or AAAA records, we apply filtering to each
+// canonical host name or IP address. If this is a match, we set a new response
+// in d.Res and return.
func (s *Server) filterDNSResponse(ctx *dnsContext) (*filtering.Result, error) {
d := ctx.proxyCtx
for _, a := range d.Res.Answer {
@@ -141,22 +164,16 @@ func (s *Server) filterDNSResponse(ctx *dnsContext) (*filtering.Result, error) {
continue
}
- s.RLock()
- // Synchronize access to s.dnsFilter so it won't be suddenly uninitialized while in use.
- // This could happen after proxy server has been stopped, but its workers are not yet exited.
- if !s.conf.ProtectionEnabled || s.dnsFilter == nil {
- s.RUnlock()
- continue
- }
- res, err := s.dnsFilter.CheckHostRules(host, d.Req.Question[0].Qtype, ctx.setts)
- s.RUnlock()
-
+ res, err := s.checkHostRules(host, d.Req.Question[0].Qtype, ctx.setts)
if err != nil {
return nil, err
+ } else if res == nil {
+ continue
} else if res.IsFiltered {
- d.Res = s.genDNSFilterMessage(d, &res)
+ d.Res = s.genDNSFilterMessage(d, res)
log.Debug("DNSFwd: Matched %s by response: %s", d.Req.Question[0].Name, host)
- return &res, nil
+
+ return res, nil
}
}
diff --git a/internal/dnsforward/http.go b/internal/dnsforward/http.go
index 35c1d9d4..8ec9313d 100644
--- a/internal/dnsforward/http.go
+++ b/internal/dnsforward/http.go
@@ -41,12 +41,13 @@ type dnsConfig struct {
CacheMinTTL *uint32 `json:"cache_ttl_min"`
CacheMaxTTL *uint32 `json:"cache_ttl_max"`
ResolveClients *bool `json:"resolve_clients"`
+ UsePrivateRDNS *bool `json:"use_private_ptr_resolvers"`
LocalPTRUpstreams *[]string `json:"local_ptr_upstreams"`
}
func (s *Server) getDNSConfig() dnsConfig {
- s.RLock()
- defer s.RUnlock()
+ s.serverLock.RLock()
+ defer s.serverLock.RUnlock()
upstreams := aghstrings.CloneSliceOrEmpty(s.conf.UpstreamDNS)
upstreamFile := s.conf.UpstreamDNSFileName
@@ -63,6 +64,7 @@ func (s *Server) getDNSConfig() dnsConfig {
cacheMinTTL := s.conf.CacheMinTTL
cacheMaxTTL := s.conf.CacheMaxTTL
resolveClients := s.conf.ResolveClients
+ usePrivateRDNS := s.conf.UsePrivateRDNS
localPTRUpstreams := aghstrings.CloneSliceOrEmpty(s.conf.LocalPTRResolvers)
var upstreamMode string
if s.conf.FastestAddr {
@@ -88,6 +90,7 @@ func (s *Server) getDNSConfig() dnsConfig {
CacheMaxTTL: &cacheMaxTTL,
UpstreamMode: &upstreamMode,
ResolveClients: &resolveClients,
+ UsePrivateRDNS: &usePrivateRDNS,
LocalPTRUpstreams: &localPTRUpstreams,
}
}
@@ -280,8 +283,8 @@ func (s *Server) setConfigRestartable(dc dnsConfig) (restart bool) {
}
func (s *Server) setConfig(dc dnsConfig) (restart bool) {
- s.Lock()
- defer s.Unlock()
+ s.serverLock.Lock()
+ defer s.serverLock.Unlock()
if dc.ProtectionEnabled != nil {
s.conf.ProtectionEnabled = *dc.ProtectionEnabled
@@ -312,6 +315,10 @@ func (s *Server) setConfig(dc dnsConfig) (restart bool) {
s.conf.ResolveClients = *dc.ResolveClients
}
+ if dc.UsePrivateRDNS != nil {
+ s.conf.UsePrivateRDNS = *dc.UsePrivateRDNS
+ }
+
return s.setConfigRestartable(dc)
}
diff --git a/internal/dnsforward/stats.go b/internal/dnsforward/stats.go
index f03f6f64..d0eb39ca 100644
--- a/internal/dnsforward/stats.go
+++ b/internal/dnsforward/stats.go
@@ -25,7 +25,9 @@ func processQueryLogsAndStats(ctx *dnsContext) (rc resultCode) {
shouldLog = false
}
- s.RLock()
+ 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 {
@@ -61,7 +63,6 @@ func processQueryLogsAndStats(ctx *dnsContext) (rc resultCode) {
}
s.updateStats(ctx, elapsed, *ctx.result)
- s.RUnlock()
return resultCodeSuccess
}
diff --git a/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleGetConfig.json b/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleGetConfig.json
index 562f0fcc..4d810c94 100644
--- a/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleGetConfig.json
+++ b/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleGetConfig.json
@@ -24,6 +24,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
},
"fastest_addr": {
@@ -51,6 +52,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
},
"parallel": {
@@ -78,6 +80,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
}
\ No newline at end of file
diff --git a/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleSetConfig.json b/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleSetConfig.json
index e7f98c6e..2d2e34b5 100644
--- a/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleSetConfig.json
+++ b/internal/dnsforward/testdata/TestDNSForwardHTTTP_handleSetConfig.json
@@ -31,6 +31,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -62,6 +63,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -94,6 +96,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -126,6 +129,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -158,6 +162,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -190,6 +195,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -222,6 +228,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -254,6 +261,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -286,6 +294,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -318,6 +327,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -352,6 +362,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -386,6 +397,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -419,6 +431,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -451,6 +464,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
},
@@ -485,6 +499,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": [
"123.123.123.123"
]
@@ -519,6 +534,7 @@
"cache_ttl_min": 0,
"cache_ttl_max": 0,
"resolve_clients": false,
+ "use_private_ptr_resolvers": false,
"local_ptr_upstreams": []
}
}
diff --git a/internal/home/clients.go b/internal/home/clients.go
index 00caa34e..5ccb73ff 100644
--- a/internal/home/clients.go
+++ b/internal/home/clients.go
@@ -53,6 +53,8 @@ type Client struct {
type clientSource uint
// Client sources. The order determines the priority.
+//
+// TODO(e.burkov): Is ARP a higher priority source than DHCP?
const (
ClientSourceWHOIS clientSource = iota
ClientSourceRDNS
diff --git a/internal/home/config.go b/internal/home/config.go
index 7071e364..53b0e303 100644
--- a/internal/home/config.go
+++ b/internal/home/config.go
@@ -108,6 +108,10 @@ type dnsConfig struct {
// ResolveClients enables and disables resolving clients with RDNS.
ResolveClients bool `yaml:"resolve_clients"`
+ // UsePrivateRDNS defines if the PTR requests for unknown addresses from
+ // locally-served networks should be resolved via private PTR resolvers.
+ UsePrivateRDNS bool `yaml:"use_private_ptr_resolvers"`
+
// LocalPTRResolvers is the slice of addresses to be used as upstreams
// for PTR queries for locally-served networks.
LocalPTRResolvers []string `yaml:"local_ptr_upstreams"`
@@ -166,6 +170,7 @@ var config = configuration{
FiltersUpdateIntervalHours: 24,
LocalDomainName: "lan",
ResolveClients: true,
+ UsePrivateRDNS: true,
},
TLS: tlsConfigSettings{
PortHTTPS: 443,
@@ -314,9 +319,11 @@ func (c *configuration) write() error {
if s := Context.dnsServer; s != nil {
c := dnsforward.FilteringConfig{}
s.WriteDiskConfig(&c)
- config.DNS.FilteringConfig = c
-
- config.DNS.LocalPTRResolvers, config.DNS.ResolveClients = s.RDNSSettings()
+ dns := &config.DNS
+ dns.FilteringConfig = c
+ dns.LocalPTRResolvers,
+ dns.ResolveClients,
+ dns.UsePrivateRDNS = s.RDNSSettings()
}
if Context.dhcpServer != nil {
diff --git a/internal/home/dns.go b/internal/home/dns.go
index 809bb011..82ed4fb2 100644
--- a/internal/home/dns.go
+++ b/internal/home/dns.go
@@ -94,7 +94,7 @@ func initDNSServer() error {
return fmt.Errorf("dnsServer.Prepare: %w", err)
}
- Context.rdns = NewRDNS(Context.dnsServer, &Context.clients)
+ Context.rdns = NewRDNS(Context.dnsServer, &Context.clients, config.DNS.UsePrivateRDNS)
Context.whois = initWhois(&Context.clients)
Context.filters.Init()
@@ -200,6 +200,7 @@ func generateServerConfig() (newConf dnsforward.ServerConfig, err error) {
newConf.GetCustomUpstreamByClient = Context.clients.FindUpstreams
newConf.ResolveClients = dnsConf.ResolveClients
+ newConf.UsePrivateRDNS = dnsConf.UsePrivateRDNS
newConf.LocalPTRResolvers = dnsConf.LocalPTRResolvers
return newConf, nil
diff --git a/internal/home/rdns.go b/internal/home/rdns.go
index 81d7ca5a..d19af7c8 100644
--- a/internal/home/rdns.go
+++ b/internal/home/rdns.go
@@ -3,6 +3,7 @@ package home
import (
"encoding/binary"
"net"
+ "sync/atomic"
"time"
"github.com/AdguardTeam/AdGuardHome/internal/dnsforward"
@@ -15,6 +16,10 @@ type RDNS struct {
exchanger dnsforward.RDNSExchanger
clients *clientsContainer
+ // usePrivate is used to store the state of current private RDNS
+ // resolving settings and to react to it's changes.
+ usePrivate uint32
+
// ipCh used to pass client's IP to rDNS workerLoop.
ipCh chan net.IP
@@ -37,6 +42,7 @@ const (
func NewRDNS(
exchanger dnsforward.RDNSExchanger,
clients *clientsContainer,
+ usePrivate bool,
) (rDNS *RDNS) {
rDNS = &RDNS{
exchanger: exchanger,
@@ -47,19 +53,39 @@ func NewRDNS(
}),
ipCh: make(chan net.IP, defaultRDNSIPChSize),
}
+ if usePrivate {
+ rDNS.usePrivate = 1
+ }
go rDNS.workerLoop()
return rDNS
}
-// Begin adds the ip to the resolving queue if it is not cached or already
-// resolved.
-func (r *RDNS) Begin(ip net.IP) {
+// ensurePrivateCache ensures that the state of the RDNS cache is consistent
+// with the current private client RDNS resolving settings.
+//
+// TODO(e.burkov): Clearing cache each time this value changed is not a perfect
+// approach since only unresolved locally-served addresses should be removed.
+// Implement when improving the cache.
+func (r *RDNS) ensurePrivateCache() {
+ var usePrivate uint32
+ if r.exchanger.ResolvesPrivatePTR() {
+ usePrivate = 1
+ }
+
+ if atomic.CompareAndSwapUint32(&r.usePrivate, 1-usePrivate, usePrivate) {
+ r.ipCache.Clear()
+ }
+}
+
+// isCached returns true if ip is already cached and not expired yet. It also
+// caches it otherwise.
+func (r *RDNS) isCached(ip net.IP) (ok bool) {
now := uint64(time.Now().Unix())
if expire := r.ipCache.Get(ip); len(expire) != 0 {
if binary.BigEndian.Uint64(expire) > now {
- return
+ return true
}
}
@@ -68,6 +94,18 @@ func (r *RDNS) Begin(ip net.IP) {
binary.BigEndian.PutUint64(ttl, now+defaultRDNSCacheTTL)
r.ipCache.Set(ip, ttl)
+ return false
+}
+
+// Begin adds the ip to the resolving queue if it is not cached or already
+// resolved.
+func (r *RDNS) Begin(ip net.IP) {
+ r.ensurePrivateCache()
+
+ if r.isCached(ip) {
+ return
+ }
+
id := ip.String()
if r.clients.Exists(id, ClientSourceRDNS) {
return
diff --git a/internal/home/rdns_test.go b/internal/home/rdns_test.go
index 329e0bf4..3655b6b4 100644
--- a/internal/home/rdns_test.go
+++ b/internal/home/rdns_test.go
@@ -16,6 +16,7 @@ import (
"github.com/AdguardTeam/golibs/log"
"github.com/miekg/dns"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func TestRDNS_Begin(t *testing.T) {
@@ -78,7 +79,8 @@ func TestRDNS_Begin(t *testing.T) {
binary.BigEndian.PutUint64(ttl, uint64(time.Now().Add(100*time.Hour).Unix()))
rdns := &RDNS{
- ipCache: ipCache,
+ ipCache: ipCache,
+ exchanger: &rDNSExchanger{},
clients: &clientsContainer{
list: map[string]*Client{},
idIndex: tc.cliIDIndex,
@@ -105,7 +107,8 @@ func TestRDNS_Begin(t *testing.T) {
// rDNSExchanger is a mock dnsforward.RDNSExchanger implementation for tests.
type rDNSExchanger struct {
- aghtest.Exchanger
+ ex aghtest.Exchanger
+ usePrivate bool
}
// Exchange implements dnsforward.RDNSExchanger interface for *RDNSExchanger.
@@ -117,7 +120,7 @@ func (e *rDNSExchanger) Exchange(ip net.IP) (host string, err error) {
}},
}
- resp, err := e.Exchanger.Exchange(req)
+ resp, err := e.ex.Exchange(req)
if err != nil {
return "", err
}
@@ -129,6 +132,35 @@ func (e *rDNSExchanger) Exchange(ip net.IP) (host string, err error) {
return resp.Answer[0].Header().Name, nil
}
+// Exchange implements dnsforward.RDNSExchanger interface for *RDNSExchanger.
+func (e *rDNSExchanger) ResolvesPrivatePTR() (ok bool) {
+ return e.usePrivate
+}
+
+func TestRDNS_ensurePrivateCache(t *testing.T) {
+ data := []byte{1, 2, 3, 4}
+
+ ipCache := cache.New(cache.Config{
+ EnableLRU: true,
+ MaxCount: defaultRDNSCacheSize,
+ })
+
+ ex := &rDNSExchanger{}
+
+ rdns := &RDNS{
+ ipCache: ipCache,
+ exchanger: ex,
+ }
+
+ rdns.ipCache.Set(data, data)
+ require.NotZero(t, rdns.ipCache.Stats().Count)
+
+ ex.usePrivate = !ex.usePrivate
+
+ rdns.ensurePrivateCache()
+ require.Zero(t, rdns.ipCache.Stats().Count)
+}
+
func TestRDNS_WorkerLoop(t *testing.T) {
aghtest.ReplaceLogLevel(t, log.DEBUG)
w := &bytes.Buffer{}
@@ -178,7 +210,7 @@ func TestRDNS_WorkerLoop(t *testing.T) {
ch := make(chan net.IP)
rdns := &RDNS{
exchanger: &rDNSExchanger{
- Exchanger: aghtest.Exchanger{
+ ex: aghtest.Exchanger{
Ups: tc.ups,
},
},
diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md
index 0fd19bb8..edccb1ee 100644
--- a/openapi/CHANGELOG.md
+++ b/openapi/CHANGELOG.md
@@ -2,11 +2,18 @@
+## v0.107: API changes
+
+### The field `"use_private_ptr_resolvers"` in DNS configuration
+
+* The new optional field `"use_private_ptr_resolvers"` of `"DNSConfig"`
+ specifies if the DNS server should use `"local_ptr_upstreams"` at all.
+
## v0.106: API changes
### The field `"supported_tags"` in `GET /control/clients`
-* Prefiously undocumented field `"supported_tags"` in the response is now
+* Previously undocumented field `"supported_tags"` in the response is now
documented.
### The field `"whois_info"` in `GET /control/clients`
diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml
index 187d764e..917f1c95 100644
--- a/openapi/openapi.yaml
+++ b/openapi/openapi.yaml
@@ -1300,6 +1300,8 @@
- ''
- 'parallel'
- 'fastest_addr'
+ 'use_private_ptr_resolvers':
+ 'type': 'boolean'
'resolve_clients':
'type': 'boolean'
'local_ptr_upstreams':