Pull request: home: provide correct server addrs in mobileconfig

Updates #3607.
Updates #3568.

Squashed commit of the following:

commit a02f9788f88b3a9339a0900baa02881a77f1fb9b
Author: Ainar Garipov <A.Garipov@AdGuard.COM>
Date:   Fri Sep 17 18:18:31 2021 +0300

    home: provide correct server addrs in mobileconfig

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
 
 
 

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

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=

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,
+		PortHTTPS:       defaultPortHTTPS,
-		PortDNSOverTLS:  853, // needs to be passed through to dnsproxy
+		PortDNSOverTLS:  defaultPortTLS, // needs to be passed through to dnsproxy
-		PortDNSOverQUIC: 784,
+		PortDNSOverQUIC: defaultPortQUIC,
 	},
 	logSettings: logSettings{
 		LogCompress:   false,

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)
 		}
 

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.WebPort = defaultPortHTTP
-	data.DNSPort = 53
+	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.WebPort = defaultPortHTTP
-	data.DNSPort = 53
+	data.DNSPort = defaultPortDNS
 
 	ifaces, err := aghnet.GetValidNetInterfacesForWeb()
 	if err != nil {

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)
 			}
 

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
+	// ServerAddresses is a list IP addresses of the server.
-	// resolve the hostname in ServerURL or ServerName.
+	ServerAddresses []net.IP `plist:",omitempty"`
-	ServerAddresses []string `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
+}

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.
+// setupDNSIPs is a helper that sets up the server IP address configuration for
-var testBootstrapDNS = []string{
+// tests and also tears it down in a cleanup function.
-	"94.140.14.14",
+func setupDNSIPs(t testing.TB) {
-	"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) {
 	t.Helper()
 
 	prevConfig := config
+	prevTLS := Context.tls
 	t.Cleanup(func() {
 		config = prevConfig
+		Context.tls = prevTLS
 	})
+
 	config = &configuration{
 		DNS: dnsConfig{
-			FilteringConfig: dnsforward.FilteringConfig{
+			BindHosts: []net.IP{netutil.IPv4Zero()},
-				BootstrapDNS: testBootstrapDNS,
+			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)
 	})