From 1714a986e3305450f80acbee61596a1475f1c3b4 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Fri, 17 Sep 2021 18:31:07 +0300 Subject: [PATCH] Pull request: home: provide correct server addrs in mobileconfig Updates #3607. Updates #3568. Squashed commit of the following: commit a02f9788f88b3a9339a0900baa02881a77f1fb9b Author: Ainar Garipov Date: Fri Sep 17 18:18:31 2021 +0300 home: provide correct server addrs in mobileconfig --- CHANGELOG.md | 4 ++- go.mod | 2 +- go.sum | 3 +- internal/home/config.go | 8 ++--- internal/home/control.go | 6 ++-- internal/home/controlinstall.go | 8 ++--- internal/home/dns.go | 11 ++++++- internal/home/mobileconfig.go | 48 ++++++++++++++++++++++-------- internal/home/mobileconfig_test.go | 37 +++++++++++------------ 9 files changed, 79 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07827d96..5630211a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,8 @@ and this project adheres to ### Added -- Bootstrap DNS server IPs to the `mobileconfig` API responses ([#3568]). +- DNS server IP addresses to the `mobileconfig` API responses ([#3568], + [#3607]). - Setting the timeout for IP address pinging in the "Fastest IP address" mode through the new `fastest_timeout` field in the configuration file ([#1992]). - Static IP address detection on FreeBSD ([#3289]). @@ -199,6 +200,7 @@ In this release, the schema version has changed from 10 to 12. [#3567]: https://github.com/AdguardTeam/AdGuardHome/issues/3567 [#3568]: https://github.com/AdguardTeam/AdGuardHome/issues/3568 [#3579]: https://github.com/AdguardTeam/AdGuardHome/issues/3579 +[#3607]: https://github.com/AdguardTeam/AdGuardHome/issues/3607 diff --git a/go.mod b/go.mod index 01ac5bc9..a4f7b25d 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.16 require ( github.com/AdguardTeam/dnsproxy v0.39.5 - github.com/AdguardTeam/golibs v0.9.2 + github.com/AdguardTeam/golibs v0.9.3 github.com/AdguardTeam/urlfilter v0.14.6 github.com/NYTimes/gziphandler v1.1.1 github.com/ameshkov/dnscrypt/v2 v2.2.2 diff --git a/go.sum b/go.sum index 3d318c6f..1b4d185a 100644 --- a/go.sum +++ b/go.sum @@ -13,8 +13,9 @@ github.com/AdguardTeam/dnsproxy v0.39.5 h1:SQorhRLR1241t6hy9CiAGZUjRULhsDJlPJTl0 github.com/AdguardTeam/dnsproxy v0.39.5/go.mod h1:eDpJKAdkHORRwAedjuERv+7SWlcz4cn+5uwrbUAWHRY= github.com/AdguardTeam/golibs v0.4.0/go.mod h1:skKsDKIBB7kkFflLJBpfGX+G8QFTx0WKUzB6TIgtUj4= github.com/AdguardTeam/golibs v0.4.2/go.mod h1:skKsDKIBB7kkFflLJBpfGX+G8QFTx0WKUzB6TIgtUj4= -github.com/AdguardTeam/golibs v0.9.2 h1:H3BDFkaosxvb+UgFlNVyN66GZ+JglcZULnJ7z7PukyQ= github.com/AdguardTeam/golibs v0.9.2/go.mod h1:fCAMwPBJ8S7YMYbTWvYS+eeTLblP5E04IDtNAo7y7IY= +github.com/AdguardTeam/golibs v0.9.3 h1:noeKHJEzrSwxzX0Zi3USM3cXf1qQV99SO772jet/uEY= +github.com/AdguardTeam/golibs v0.9.3/go.mod h1:fCAMwPBJ8S7YMYbTWvYS+eeTLblP5E04IDtNAo7y7IY= github.com/AdguardTeam/gomitmproxy v0.2.0/go.mod h1:Qdv0Mktnzer5zpdpi5rAwixNJzW2FN91LjKJCkVbYGU= github.com/AdguardTeam/urlfilter v0.14.6 h1:emqoKZElooHACYehRBYENeKVN1a/rspxiqTIMYLuoIo= github.com/AdguardTeam/urlfilter v0.14.6/go.mod h1:klx4JbOfc4EaNb5lWLqOwfg+pVcyRukmoJRvO55lL5U= diff --git a/internal/home/config.go b/internal/home/config.go index 8f47e790..f7cbd0be 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -173,7 +173,7 @@ var config = &configuration{ AuthBlockMin: 15, DNS: dnsConfig{ BindHosts: []net.IP{{0, 0, 0, 0}}, - Port: 53, + Port: defaultPortDNS, StatsInterval: 1, FilteringConfig: dnsforward.FilteringConfig{ ProtectionEnabled: true, // whether or not use any of filtering features @@ -202,9 +202,9 @@ var config = &configuration{ UsePrivateRDNS: true, }, TLS: tlsConfigSettings{ - PortHTTPS: 443, - PortDNSOverTLS: 853, // needs to be passed through to dnsproxy - PortDNSOverQUIC: 784, + PortHTTPS: defaultPortHTTPS, + PortDNSOverTLS: defaultPortTLS, // needs to be passed through to dnsproxy + PortDNSOverQUIC: defaultPortQUIC, }, logSettings: logSettings{ LogCompress: false, diff --git a/internal/home/control.go b/internal/home/control.go index a0c44ad8..d9d809ba 100644 --- a/internal/home/control.go +++ b/internal/home/control.go @@ -39,7 +39,7 @@ func httpError(w http.ResponseWriter, code int, format string, args ...interface func appendDNSAddrs(dst []string, addrs ...net.IP) (res []string) { for _, addr := range addrs { var hostport string - if config.DNS.Port != 53 { + if config.DNS.Port != defaultPortDNS { hostport = netutil.JoinHostPort(addr.String(), config.DNS.Port) } else { hostport = addr.String() @@ -285,8 +285,6 @@ func preInstallHandler(handler http.Handler) http.Handler { return &preInstallHandlerStruct{handler} } -const defaultHTTPSPort = 443 - // handleHTTPSRedirect redirects the request to HTTPS, if needed. If ok is // true, the middleware must continue handling the request. func handleHTTPSRedirect(w http.ResponseWriter, r *http.Request) (ok bool) { @@ -304,7 +302,7 @@ func handleHTTPSRedirect(w http.ResponseWriter, r *http.Request) (ok bool) { if r.TLS == nil && web.forceHTTPS { hostPort := host - if port := web.conf.PortHTTPS; port != defaultHTTPSPort { + if port := web.conf.PortHTTPS; port != defaultPortHTTPS { hostPort = netutil.JoinHostPort(host, port) } diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go index 1f54a645..ff532ff6 100644 --- a/internal/home/controlinstall.go +++ b/internal/home/controlinstall.go @@ -29,8 +29,8 @@ type getAddrsResponse struct { // handleInstallGetAddresses is the handler for /install/get_addresses endpoint. func (web *Web) handleInstallGetAddresses(w http.ResponseWriter, r *http.Request) { data := getAddrsResponse{} - data.WebPort = 80 - data.DNSPort = 53 + data.WebPort = defaultPortHTTP + data.DNSPort = defaultPortDNS ifaces, err := aghnet.GetValidNetInterfacesForWeb() if err != nil { @@ -553,8 +553,8 @@ type getAddrsResponseBeta struct { // functionality will appear in default handleInstallGetAddresses. func (web *Web) handleInstallGetAddressesBeta(w http.ResponseWriter, r *http.Request) { data := getAddrsResponseBeta{} - data.WebPort = 80 - data.DNSPort = 53 + data.WebPort = defaultPortHTTP + data.DNSPort = defaultPortDNS ifaces, err := aghnet.GetValidNetInterfacesForWeb() if err != nil { diff --git a/internal/home/dns.go b/internal/home/dns.go index e3986acd..4b120581 100644 --- a/internal/home/dns.go +++ b/internal/home/dns.go @@ -19,6 +19,15 @@ import ( yaml "gopkg.in/yaml.v2" ) +// Default ports. +const ( + defaultPortDNS = 53 + defaultPortHTTP = 80 + defaultPortHTTPS = 443 + defaultPortQUIC = 784 + defaultPortTLS = 853 +) + // Called by other modules when configuration is changed func onConfigModified() { _ = config.write() @@ -253,7 +262,7 @@ func getDNSEncryption() (de dnsEncryption) { hostname := tlsConf.ServerName if tlsConf.PortHTTPS != 0 { addr := hostname - if tlsConf.PortHTTPS != 443 { + if tlsConf.PortHTTPS != defaultPortHTTPS { addr = netutil.JoinHostPort(addr, tlsConf.PortHTTPS) } diff --git a/internal/home/mobileconfig.go b/internal/home/mobileconfig.go index 3cef41c1..560407b0 100644 --- a/internal/home/mobileconfig.go +++ b/internal/home/mobileconfig.go @@ -3,6 +3,7 @@ package home import ( "encoding/json" "fmt" + "net" "net/http" "net/url" "path" @@ -10,7 +11,6 @@ import ( "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" - "github.com/AdguardTeam/golibs/stringutil" uuid "github.com/satori/go.uuid" "howett.net/plist" ) @@ -31,9 +31,8 @@ type dnsSettings struct { // DNSProtocol is not "TLS". ServerName string `plist:",omitempty"` - // ServerAddresses is a list of plain DNS server IP addresses used to - // resolve the hostname in ServerURL or ServerName. - ServerAddresses []string `plist:",omitempty"` + // ServerAddresses is a list IP addresses of the server. + ServerAddresses []net.IP `plist:",omitempty"` } // payloadContent is a Device Management Profile payload. @@ -158,10 +157,19 @@ func handleMobileConfig(w http.ResponseWriter, r *http.Request, dnsp string) { } } + dnsIPs, err := collectDNSIPs() + if err != nil { + // Don't add a lot of formatting, since the error is already + // wrapped by collectDNSIPs. + respondJSONError(w, http.StatusInternalServerError, err.Error()) + + return + } + d := &dnsSettings{ DNSProtocol: dnsp, ServerName: host, - ServerAddresses: cloneBootstrap(), + ServerAddresses: dnsIPs, } mobileconfig, err := encodeMobileConfig(d, clientID) @@ -188,14 +196,6 @@ func handleMobileConfig(w http.ResponseWriter, r *http.Request, dnsp string) { _, _ = w.Write(mobileconfig) } -// cloneBootstrap returns a clone of the current bootstrap DNS servers. -func cloneBootstrap() (bootstrap []string) { - config.RLock() - defer config.RUnlock() - - return stringutil.CloneSlice(config.DNS.BootstrapDNS) -} - func handleMobileConfigDoH(w http.ResponseWriter, r *http.Request) { handleMobileConfig(w, r, dnsProtoHTTPS) } @@ -203,3 +203,25 @@ func handleMobileConfigDoH(w http.ResponseWriter, r *http.Request) { func handleMobileConfigDoT(w http.ResponseWriter, r *http.Request) { handleMobileConfig(w, r, dnsProtoTLS) } + +// collectDNSIPs returns a slice of IP addresses the server is listening +// on, including the addresses on all interfaces in cases of unspecified IPs but +// excluding loopback addresses. +func collectDNSIPs() (ips []net.IP, err error) { + // TODO(a.garipov): This really shouldn't be a function that parses + // a list of strings. Instead, we need a function that returns this + // data as []net.IP or []*netutil.IPPort. Maybe someday. + addrs, err := collectDNSAddresses() + if err != nil { + return nil, err + } + + for _, addr := range addrs { + ip := net.ParseIP(addr) + if ip != nil && !ip.IsLoopback() { + ips = append(ips, ip) + } + } + + return ips, nil +} diff --git a/internal/home/mobileconfig_test.go b/internal/home/mobileconfig_test.go index 9a896d1f..87b69f10 100644 --- a/internal/home/mobileconfig_test.go +++ b/internal/home/mobileconfig_test.go @@ -3,42 +3,41 @@ package home import ( "bytes" "encoding/json" + "net" "net/http" "net/http/httptest" "testing" - "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" + "github.com/AdguardTeam/golibs/netutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "howett.net/plist" ) -// testBootstrapDNS are the bootstrap plain DNS server addresses for tests. -var testBootstrapDNS = []string{ - "94.140.14.14", - "94.140.15.15", -} - -// setupBootstraps is a helper that sets up the bootstrap plain DNS server -// configuration for tests and also tears it down in a cleanup function. -func setupBootstraps(t testing.TB) { +// setupDNSIPs is a helper that sets up the server IP address configuration for +// tests and also tears it down in a cleanup function. +func setupDNSIPs(t testing.TB) { t.Helper() prevConfig := config + prevTLS := Context.tls t.Cleanup(func() { config = prevConfig + Context.tls = prevTLS }) + config = &configuration{ DNS: dnsConfig{ - FilteringConfig: dnsforward.FilteringConfig{ - BootstrapDNS: testBootstrapDNS, - }, + BindHosts: []net.IP{netutil.IPv4Zero()}, + Port: defaultPortDNS, }, } + + Context.tls = &TLSMod{} } func TestHandleMobileConfigDoH(t *testing.T) { - setupBootstraps(t) + setupDNSIPs(t) t.Run("success", func(t *testing.T) { r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/doh.mobileconfig?host=example.org", nil) @@ -59,7 +58,7 @@ func TestHandleMobileConfigDoH(t *testing.T) { s := mc.PayloadContent[0].DNSSettings require.NotNil(t, s) - assert.Equal(t, testBootstrapDNS, s.ServerAddresses) + assert.NotEmpty(t, s.ServerAddresses) assert.Empty(t, s.ServerName) assert.Equal(t, "https://example.org/dns-query", s.ServerURL) }) @@ -105,14 +104,14 @@ func TestHandleMobileConfigDoH(t *testing.T) { s := mc.PayloadContent[0].DNSSettings require.NotNil(t, s) - assert.Equal(t, testBootstrapDNS, s.ServerAddresses) + assert.NotEmpty(t, s.ServerAddresses) assert.Empty(t, s.ServerName) assert.Equal(t, "https://example.org/dns-query/cli42", s.ServerURL) }) } func TestHandleMobileConfigDoT(t *testing.T) { - setupBootstraps(t) + setupDNSIPs(t) t.Run("success", func(t *testing.T) { r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig?host=example.org", nil) @@ -133,7 +132,7 @@ func TestHandleMobileConfigDoT(t *testing.T) { s := mc.PayloadContent[0].DNSSettings require.NotNil(t, s) - assert.Equal(t, testBootstrapDNS, s.ServerAddresses) + assert.NotEmpty(t, s.ServerAddresses) assert.Equal(t, "example.org", s.ServerName) assert.Empty(t, s.ServerURL) }) @@ -180,7 +179,7 @@ func TestHandleMobileConfigDoT(t *testing.T) { s := mc.PayloadContent[0].DNSSettings require.NotNil(t, s) - assert.Equal(t, testBootstrapDNS, s.ServerAddresses) + assert.NotEmpty(t, s.ServerAddresses) assert.Equal(t, "cli42.example.org", s.ServerName) assert.Empty(t, s.ServerURL) })