From 1e55db408f78eccb36f2bd4afa719c1c9d88c470 Mon Sep 17 00:00:00 2001 From: Eugene Burkov <e.burkov@adguard.com> Date: Tue, 1 Jun 2021 14:28:34 +0300 Subject: [PATCH] Pull request: 3136 show default resolvers Merge in DNS/adguard-home from 3136-show-default to master Closes #3136. Squashed commit of the following: commit add09a772dddcccd404056e7649c2d3350a79fec Author: Eugene Burkov <e.burkov@adguard.com> Date: Tue Jun 1 11:11:24 2021 +0300 openapi: fix typo commit 69e402c49318d53a0d03b81207644d9fb04a139a Merge: 76e8492c e17e1f20 Author: Eugene Burkov <e.burkov@adguard.com> Date: Tue Jun 1 11:09:51 2021 +0300 Merge branch 'master' into 3136-show-default commit 76e8492c8da679e204ceb7a77f1c6f73a2488040 Author: Ildar Kamalov <ik@adguard.com> Date: Tue Jun 1 10:55:09 2021 +0300 client: check upstreams commit 05fe6ea196a1ed9386eec25dbfbe22779fe7bcfd Author: Vlad <v.abdulmyanov@adguard.com> Date: Mon May 31 19:23:35 2021 +0300 add defaul resolvers line commit 8d63c7df9947c9588478d1173834b42569fd8951 Author: Eugene Burkov <e.burkov@adguard.com> Date: Mon May 31 11:56:08 2021 +0300 all: imp changelog commit e3912e3b20eca9dcf90ddddaa5edb54d1e0cfe6e Author: Eugene Burkov <e.burkov@adguard.com> Date: Fri May 28 20:23:05 2021 +0300 all: add local ptr resolvers addresses output --- CHANGELOG.md | 2 + client/src/__locales/en.json | 2 + .../components/Settings/Dns/Upstream/Form.js | 11 +++++ internal/aghnet/systemresolvers.go | 4 +- internal/dnsforward/dnsforward.go | 48 +++++++++++-------- internal/dnsforward/http.go | 19 ++++++-- internal/dnsforward/http_test.go | 17 +++++++ openapi/CHANGELOG.md | 6 +++ openapi/openapi.yaml | 12 ++++- 9 files changed, 95 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4153a51..651a2516 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ and this project adheres to ### Added +- Output of the default addresses of the upstreams used for resolving PTRs for + private addresses ([#3136]). - Detection and handling of recurrent PTR requests for locally-served addresses ([#3185]). - The ability to completely disable reverse DNS resolving of IPs from diff --git a/client/src/__locales/en.json b/client/src/__locales/en.json index d65585e0..dd39cbfd 100644 --- a/client/src/__locales/en.json +++ b/client/src/__locales/en.json @@ -10,6 +10,8 @@ "bootstrap_dns_desc": "Bootstrap DNS servers are used to resolve IP addresses of the DoH/DoT resolvers you specify as upstreams.", "local_ptr_title": "Private reverse DNS servers", "local_ptr_desc": "The DNS servers that AdGuard Home uses for local PTR queries. These servers are used to resolve the hostnames of clients with private IP addresses, for example \"192.168.12.34\", using rDNS. If not set, AdGuard Home uses the default DNS resolvers of your OS.", + "local_ptr_default_resolver": "By default, AdGuard Home will use the following DNS resolvers: {{ip}}", + "local_ptr_no_default_resolver": "AdGuard Home could not determine suitable private rDNS resolvers for this system.", "local_ptr_placeholder": "Enter one server address per line", "resolve_clients_title": "Enable reverse resolving of clients' IP addresses", "resolve_clients_desc": "If enabled, AdGuard Home will attempt to reversely resolve clients' IP addresses into their hostnames by sending PTR queries to corresponding resolvers (private DNS servers for local clients, upstream servers for clients with public IP addresses).", diff --git a/client/src/components/Settings/Dns/Upstream/Form.js b/client/src/components/Settings/Dns/Upstream/Form.js index 99364549..0e688393 100644 --- a/client/src/components/Settings/Dns/Upstream/Form.js +++ b/client/src/components/Settings/Dns/Upstream/Form.js @@ -143,6 +143,9 @@ const Form = ({ const upstream_dns = useSelector((store) => store.form[FORM_NAME.UPSTREAM].values.upstream_dns); const processingTestUpstream = useSelector((state) => state.settings.processingTestUpstream); const processingSetConfig = useSelector((state) => state.dnsConfig.processingSetConfig); + const defaultLocalPtrUpstreams = useSelector( + (state) => state.dnsConfig.default_local_ptr_upstreams, + ); const handleUpstreamTest = () => dispatch(testUpstreamWithFormValues()); @@ -212,6 +215,14 @@ const Form = ({ <div className="form__desc form__desc--top"> <Trans>local_ptr_desc</Trans> </div> + <div className="form__desc form__desc--top"> + {/** TODO: Add internazionalization for "" */} + {defaultLocalPtrUpstreams?.length > 0 ? ( + <Trans values={{ ip: defaultLocalPtrUpstreams.map((s) => `"${s}"`).join(', ') }}>local_ptr_default_resolver</Trans> + ) : ( + <Trans>local_ptr_no_default_resolver</Trans> + )} + </div> <Field id="local_ptr_upstreams" name="local_ptr_upstreams" diff --git a/internal/aghnet/systemresolvers.go b/internal/aghnet/systemresolvers.go index 0543c615..777127a3 100644 --- a/internal/aghnet/systemresolvers.go +++ b/internal/aghnet/systemresolvers.go @@ -16,8 +16,8 @@ type HostGenFunc func() (host string) // SystemResolvers helps to work with local resolvers' addresses provided by OS. type SystemResolvers interface { - // Get returns the slice of local resolvers' addresses. - // It should be safe for concurrent use. + // Get returns the slice of local resolvers' addresses. It should be + // safe for concurrent use. Get() (rs []string) // refresh refreshes the local resolvers' addresses cache. It should be // safe for concurrent use. diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index da9497aa..d1d30d42 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -76,6 +76,7 @@ type Server struct { ipset ipsetCtx subnetDetector *aghnet.SubnetDetector localResolvers *proxy.Proxy + sysResolvers aghnet.SystemResolvers recDetector *recursionDetector tableHostToIP hostToIPTable @@ -154,6 +155,13 @@ func NewServer(p DNSCreateParams) (s *Server, err error) { recDetector: newRecursionDetector(recursionTTL, cachedRecurrentReqNum), } + // TODO(e.burkov): Enable the refresher after the actual implementation + // passes the public testing. + s.sysResolvers, err = aghnet.NewSystemResolvers(0, nil) + if err != nil { + return nil, fmt.Errorf("initializing system resolvers: %w", err) + } + if p.DHCPServer != nil { s.dhcpServer = p.DHCPServer s.dhcpServer.SetOnLeaseChanged(s.onDHCPLeaseChanged) @@ -375,28 +383,11 @@ func (s *Server) collectDNSIPAddrs() (addrs []string, err error) { return addrs[:i], nil } -// setupResolvers initializes the resolvers for local addresses. For internal -// use only. -func (s *Server) setupResolvers(localAddrs []string) (err error) { - bootstraps := s.conf.BootstrapDNS - if len(localAddrs) == 0 { - var sysRes aghnet.SystemResolvers - // TODO(e.burkov): Enable the refresher after the actual - // implementation passes the public testing. - sysRes, err = aghnet.NewSystemResolvers(0, nil) - if err != nil { - return err - } - - localAddrs = sysRes.Get() - bootstraps = nil - } - log.Debug("upstreams to resolve PTR for local addresses: %v", localAddrs) - +func (s *Server) filterOurDNSAddrs(addrs []string) (filtered []string, err error) { var ourAddrs []string ourAddrs, err = s.collectDNSIPAddrs() if err != nil { - return err + return nil, err } ourAddrsSet := aghstrings.NewSet(ourAddrs...) @@ -405,7 +396,24 @@ func (s *Server) setupResolvers(localAddrs []string) (err error) { // really applicable here since in case of listening on all network // interfaces we should check the whole interface's network to cut off // all the loopback addresses as well. - localAddrs = aghstrings.FilterOut(localAddrs, ourAddrsSet.Has) + return aghstrings.FilterOut(addrs, ourAddrsSet.Has), nil +} + +// setupResolvers initializes the resolvers for local addresses. For internal +// use only. +func (s *Server) setupResolvers(localAddrs []string) (err error) { + bootstraps := s.conf.BootstrapDNS + if len(localAddrs) == 0 { + localAddrs = s.sysResolvers.Get() + bootstraps = nil + } + + localAddrs, err = s.filterOurDNSAddrs(localAddrs) + if err != nil { + return err + } + + log.Debug("upstreams to resolve PTR for local addresses: %v", localAddrs) var upsConfig proxy.UpstreamConfig upsConfig, err = proxy.ParseUpstreamsConfig(localAddrs, upstream.Options{ diff --git a/internal/dnsforward/http.go b/internal/dnsforward/http.go index 8ec9313d..28c0dd47 100644 --- a/internal/dnsforward/http.go +++ b/internal/dnsforward/http.go @@ -96,12 +96,25 @@ func (s *Server) getDNSConfig() dnsConfig { } func (s *Server) handleGetConfig(w http.ResponseWriter, r *http.Request) { - resp := s.getDNSConfig() + defLocalPTRUps, err := s.filterOurDNSAddrs(s.sysResolvers.Get()) + if err != nil { + log.Debug("getting dns configuration: %s", err) + } + + resp := struct { + dnsConfig + // DefautLocalPTRUpstreams is used to pass the addresses from + // systemResolvers to the front-end. It's not a pointer to the slice + // since there is no need to omit it while decoding from JSON. + DefautLocalPTRUpstreams []string `json:"default_local_ptr_upstreams,omitempty"` + }{ + dnsConfig: s.getDNSConfig(), + DefautLocalPTRUpstreams: defLocalPTRUps, + } w.Header().Set("Content-Type", "application/json") - enc := json.NewEncoder(w) - if err := enc.Encode(resp); err != nil { + if err = json.NewEncoder(w).Encode(resp); err != nil { httpError(r, w, http.StatusInternalServerError, "json.Encoder: %s", err) return } diff --git a/internal/dnsforward/http_test.go b/internal/dnsforward/http_test.go index 07354f62..fc34a3b9 100644 --- a/internal/dnsforward/http_test.go +++ b/internal/dnsforward/http_test.go @@ -12,11 +12,25 @@ import ( "strings" "testing" + "github.com/AdguardTeam/AdGuardHome/internal/aghnet" "github.com/AdguardTeam/AdGuardHome/internal/filtering" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +// fakeSystemResolvers is a mock aghnet.SystemResolvers implementation for tests. +type fakeSystemResolvers struct { + // SystemResolvers is embedded here simply to make *fakeSystemResolvers + // an aghnet.SystemResolvers without actually implementing all methods. + aghnet.SystemResolvers +} + +// Get implements the aghnet.SystemResolvers interface for *fakeSystemResolvers. +// It always returns nil. +func (fsr *fakeSystemResolvers) Get() (rs []string) { + return nil +} + func loadTestData(t *testing.T, casesFileName string, cases interface{}) { t.Helper() @@ -52,6 +66,8 @@ func TestDNSForwardHTTTP_handleGetConfig(t *testing.T) { ConfigModified: func() {}, } s := createTestServer(t, filterConf, forwardConf, nil) + s.sysResolvers = &fakeSystemResolvers{} + require.Nil(t, s.Start()) t.Cleanup(func() { require.Nil(t, s.Stop()) @@ -125,6 +141,7 @@ func TestDNSForwardHTTTP_handleSetConfig(t *testing.T) { ConfigModified: func() {}, } s := createTestServer(t, filterConf, forwardConf, nil) + s.sysResolvers = &fakeSystemResolvers{} defaultConf := s.conf diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md index edccb1ee..e00bd39e 100644 --- a/openapi/CHANGELOG.md +++ b/openapi/CHANGELOG.md @@ -4,6 +4,12 @@ ## v0.107: API changes +### The new field `"default_local_ptr_upstreams"` in `GET /control/dns_info` + +* The new optional field `"default_local_ptr_upstreams"` is the list of IP + addresses AdGuard Home would use by default to resolve PTR request for + addresses from locally-served networks. + ### The field `"use_private_ptr_resolvers"` in DNS configuration * The new optional field `"use_private_ptr_resolvers"` of `"DNSConfig"` diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 917f1c95..90e25992 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -69,7 +69,17 @@ 'content': 'application/json': 'schema': - '$ref': '#/components/schemas/DNSConfig' + 'allOf': + - '$ref': '#/components/schemas/DNSConfig' + - 'type': 'object' + 'properties': + 'default_local_ptr_upstreams': + 'type': 'array' + 'items': + 'type': 'string' + 'example': + - '192.168.168.192' + - '10.0.0.10' '/dns_config': 'post': 'tags':