From 424f20da985b043ce4a6c5b5830d3e414e832196 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 13 Sep 2021 16:00:36 +0300 Subject: [PATCH] Pull request: home: add bootstrap to mobileconfig, imp code Updates #3568. Squashed commit of the following: commit ec342e6223e2b2efe9a8bf833d5406a44c6417e4 Author: Ainar Garipov Date: Mon Sep 13 15:16:07 2021 +0300 home: imp tests commit 67cd771e631938d3e8a5340315314210de796174 Author: Ainar Garipov Date: Mon Sep 13 14:34:03 2021 +0300 home: add bootstrap to mobileconfig, imp code --- CHANGELOG.md | 2 + internal/aghnet/etchostscontainer_test.go | 4 +- internal/aghtest/upstream.go | 2 +- internal/aghtime/duration.go | 4 +- internal/aghtime/duration_test.go | 4 +- internal/dnsforward/dns.go | 2 +- internal/dnsforward/dnsforward.go | 2 +- internal/dnsforward/dnsforward_test.go | 4 +- internal/home/config.go | 6 +- internal/home/controlinstall.go | 78 +++++++++--------- internal/home/home.go | 2 +- internal/home/mobileconfig.go | 99 +++++++++++++++-------- internal/home/mobileconfig_test.go | 73 ++++++++++++++--- 13 files changed, 187 insertions(+), 95 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79fd7709..8afef5a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to ### Added +- Bootstrap DNS server IPs to the `mobileconfig` API responses ([#3568]). - 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]). @@ -189,6 +190,7 @@ In this release, the schema version has changed from 10 to 12. [#3506]: https://github.com/AdguardTeam/AdGuardHome/issues/3506 [#3551]: https://github.com/AdguardTeam/AdGuardHome/issues/3551 [#3564]: https://github.com/AdguardTeam/AdGuardHome/issues/3564 +[#3568]: https://github.com/AdguardTeam/AdGuardHome/issues/3568 diff --git a/internal/aghnet/etchostscontainer_test.go b/internal/aghnet/etchostscontainer_test.go index b9e7d8da..b83f2dd2 100644 --- a/internal/aghnet/etchostscontainer_test.go +++ b/internal/aghnet/etchostscontainer_test.go @@ -117,8 +117,8 @@ func TestEtcHostsContainerFSNotify(t *testing.T) { assertWriting(t, f, "127.0.0.2 newhost\n") require.NoError(t, f.Sync()) - // Wait until fsnotify has triggerred and processed the - // file-modification event. + // Wait until fsnotify has triggered and processed the file-modification + // event. time.Sleep(50 * time.Millisecond) t.Run("notified", func(t *testing.T) { diff --git a/internal/aghtest/upstream.go b/internal/aghtest/upstream.go index 6ba3cc5e..aa364310 100644 --- a/internal/aghtest/upstream.go +++ b/internal/aghtest/upstream.go @@ -167,7 +167,7 @@ func (u *TestBlockUpstream) RequestsCount() int { // TestErrUpstream implements upstream.Upstream interface for replacing real // upstream in tests. type TestErrUpstream struct { - // The error returned by Exchange may be unwraped to the Err. + // The error returned by Exchange may be unwrapped to the Err. Err error } diff --git a/internal/aghtime/duration.go b/internal/aghtime/duration.go index bf073656..af6c3d8a 100644 --- a/internal/aghtime/duration.go +++ b/internal/aghtime/duration.go @@ -38,7 +38,7 @@ func (d Duration) String() (str string) { rounded == 0, rounded*time.Second != d.Duration, rounded%60 != 0: - // Return the uncutted value if it's either equal to zero or has + // Return the uncut value if it's either equal to zero or has // fractions of a second or even whole seconds in it. return str @@ -60,7 +60,7 @@ func (d Duration) MarshalText() (text []byte, err error) { // // TODO(e.burkov): Make it able to parse larger units like days. func (d *Duration) UnmarshalText(b []byte) (err error) { - defer func() { err = errors.Annotate(err, "unmarshalling duration: %w") }() + defer func() { err = errors.Annotate(err, "unmarshaling duration: %w") }() d.Duration, err = time.ParseDuration(string(b)) diff --git a/internal/aghtime/duration_test.go b/internal/aghtime/duration_test.go index ae1b26f3..76518107 100644 --- a/internal/aghtime/duration_test.go +++ b/internal/aghtime/duration_test.go @@ -60,7 +60,7 @@ func TestDuration_String(t *testing.T) { } // durationEncodingTester is a helper struct to simplify testing different -// Duration marshalling and unmarshalling cases. +// Duration marshalling and unmarshaling cases. type durationEncodingTester struct { PtrMap map[string]*Duration `json:"ptr_map" yaml:"ptr_map"` PtrSlice []*Duration `json:"ptr_slice" yaml:"ptr_slice"` @@ -104,7 +104,7 @@ const ( // Duration. const defaultTestDur = time.Millisecond -// checkFields verifies m's fields. It expects the m to be unmarshalled from +// checkFields verifies m's fields. It expects the m to be unmarshaled from // one of the constant strings above. func (m *durationEncodingTester) checkFields(t *testing.T, d Duration) { t.Run("pointers_map", func(t *testing.T) { diff --git a/internal/dnsforward/dns.go b/internal/dnsforward/dns.go index 57fc7e0d..fc2f245a 100644 --- a/internal/dnsforward/dns.go +++ b/internal/dnsforward/dns.go @@ -340,7 +340,7 @@ func (s *Server) processRestrictLocal(ctx *dnsContext) (rc resultCode) { // Restrict an access to local addresses for external clients. We also // assume that all the DHCP leases we give are locally-served or at - // least don't need to be unaccessable externally. + // least don't need to be inaccessible externally. if s.subnetDetector.IsLocallyServedNetwork(ip) { if !ctx.isLocalClient { log.Debug("dns: %q requests for internal ip", d.Addr) diff --git a/internal/dnsforward/dnsforward.go b/internal/dnsforward/dnsforward.go index 6f9246bb..80d51548 100644 --- a/internal/dnsforward/dnsforward.go +++ b/internal/dnsforward/dnsforward.go @@ -364,7 +364,7 @@ func (s *Server) startLocked() error { const defaultLocalTimeout = 1 * time.Second // collectDNSIPAddrs returns IP addresses the server is listening on without -// port numbersю 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 diff --git a/internal/dnsforward/dnsforward_test.go b/internal/dnsforward/dnsforward_test.go index c7ceab58..8552ea7c 100644 --- a/internal/dnsforward/dnsforward_test.go +++ b/internal/dnsforward/dnsforward_test.go @@ -275,7 +275,7 @@ func TestServer(t *testing.T) { client := dns.Client{Net: tc.net} reply, _, err := client.Exchange(createGoogleATestMessage(), addr.String()) - require.NoErrorf(t, err, "сouldn't talk to server %s: %s", addr, err) + require.NoErrorf(t, err, "couldn't talk to server %s: %s", addr, err) assertGoogleAResponse(t, reply) }) @@ -330,7 +330,7 @@ func TestServerWithProtectionDisabled(t *testing.T) { client := &dns.Client{} reply, _, err := client.Exchange(req, addr.String()) - require.NoErrorf(t, err, "сouldn't talk to server %s: %s", addr, err) + require.NoErrorf(t, err, "couldn't talk to server %s: %s", addr, err) assertGoogleAResponse(t, reply) } diff --git a/internal/home/config.go b/internal/home/config.go index 710a582c..8f47e790 100644 --- a/internal/home/config.go +++ b/internal/home/config.go @@ -162,8 +162,10 @@ type tlsConfigSettings struct { dnsforward.TLSConfig `yaml:",inline" json:",inline"` } -// initialize to default values, will be changed later when reading config or parsing command line -var config = configuration{ +// config is the global configuration structure. +// +// TODO(a.garipov, e.burkov): This global is afwul and must be removed. +var config = &configuration{ BindPort: 3000, BetaBindPort: 0, BindHost: net.IP{0, 0, 0, 0}, diff --git a/internal/home/controlinstall.go b/internal/home/controlinstall.go index 0d0d0134..bafa0060 100644 --- a/internal/home/controlinstall.go +++ b/internal/home/controlinstall.go @@ -15,8 +15,8 @@ import ( "time" "github.com/AdguardTeam/AdGuardHome/internal/aghnet" + "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/log" - "github.com/AdguardTeam/golibs/netutil" ) // getAddrsResponse is the response for /install/get_addresses endpoint. @@ -286,55 +286,29 @@ func shutdownSrv(ctx context.Context, cancel context.CancelFunc, srv *http.Serve // Apply new configuration, start DNS server, restart Web server func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) { - req := applyConfigReq{} - err := json.NewDecoder(r.Body).Decode(&req) + req, restartHTTP, err := decodeApplyConfigReq(r.Body) if err != nil { - httpError(w, http.StatusBadRequest, "Failed to parse 'configure' JSON: %s", err) + httpError(w, http.StatusBadRequest, "%s", err) + return } - if req.Web.Port == 0 || req.DNS.Port == 0 { - httpError(w, http.StatusBadRequest, "port value can't be 0") - return - } - - restartHTTP := true - if config.BindHost.Equal(req.Web.IP) && config.BindPort == req.Web.Port { - // no need to rebind - restartHTTP = false - } - - // validate that hosts and ports are bindable - if restartHTTP { - err = aghnet.CheckPortAvailable(req.Web.IP, req.Web.Port) - if err != nil { - httpError( - w, - http.StatusBadRequest, - "can not listen on IP:port %s: %s", - netutil.JoinHostPort(req.Web.IP.String(), req.Web.Port), - err, - ) - - return - } - - } - err = aghnet.CheckPacketPortAvailable(req.DNS.IP, req.DNS.Port) if err != nil { httpError(w, http.StatusBadRequest, "%s", err) + return } err = aghnet.CheckPortAvailable(req.DNS.IP, req.DNS.Port) if err != nil { httpError(w, http.StatusBadRequest, "%s", err) + return } - var curConfig configuration - copyInstallSettings(&curConfig, &config) + var curConfig *configuration + copyInstallSettings(curConfig, config) Context.firstRun = false config.BindHost = req.Web.IP @@ -349,8 +323,9 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) { err = StartMods() if err != nil { Context.firstRun = true - copyInstallSettings(&config, &curConfig) + copyInstallSettings(config, curConfig) httpError(w, http.StatusInternalServerError, "%s", err) + return } @@ -361,8 +336,9 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) { err = config.write() if err != nil { Context.firstRun = true - copyInstallSettings(&config, &curConfig) + copyInstallSettings(config, curConfig) httpError(w, http.StatusInternalServerError, "Couldn't write config: %s", err) + return } @@ -387,6 +363,36 @@ func (web *Web) handleInstallConfigure(w http.ResponseWriter, r *http.Request) { } } +// decodeApplyConfigReq decodes the configuration, validates some parameters, +// and returns it along with the boolean indicating whether or not the HTTP +// server must be restarted. +func decodeApplyConfigReq(r io.Reader) (req *applyConfigReq, restartHTTP bool, err error) { + req = &applyConfigReq{} + err = json.NewDecoder(r).Decode(&req) + if err != nil { + return nil, false, fmt.Errorf("parsing request: %w", err) + } + + if req.Web.Port == 0 || req.DNS.Port == 0 { + return nil, false, errors.Error("ports cannot be 0") + } + + restartHTTP = !config.BindHost.Equal(req.Web.IP) || config.BindPort != req.Web.Port + if restartHTTP { + err = aghnet.CheckPortAvailable(req.Web.IP, req.Web.Port) + if err != nil { + return nil, false, fmt.Errorf( + "checking address %s:%d: %w", + req.Web.IP.String(), + req.Web.Port, + err, + ) + } + } + + return req, restartHTTP, err +} + func (web *Web) registerInstallHandlers() { Context.mux.HandleFunc("/control/install/get_addresses", preInstall(ensureGET(web.handleInstallGetAddresses))) Context.mux.HandleFunc("/control/install/check_config", preInstall(ensurePOST(web.handleInstallCheckConfig))) diff --git a/internal/home/home.go b/internal/home/home.go index 0d7a37d9..d3836920 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -349,7 +349,7 @@ func run(args options, clientBuildFS fs.FS) { setupContext(args) - err = configureOS(&config) + err = configureOS(config) fatalOnError(err) // clients package uses filtering package's static data (filtering.BlockedSvcKnown()), diff --git a/internal/home/mobileconfig.go b/internal/home/mobileconfig.go index 6c4681d8..3cef41c1 100644 --- a/internal/home/mobileconfig.go +++ b/internal/home/mobileconfig.go @@ -10,35 +10,60 @@ 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" ) +// dnsSettings is the DNSSetting.DNSSettings mobileconfig profile. +// +// See https://developer.apple.com/documentation/devicemanagement/dnssettings/dnssettings. type dnsSettings struct { + // DNSProtocol is the required protocol to be used. The valid values + // are "HTTPS" and "TLS". DNSProtocol string - ServerURL string `plist:",omitempty"` - ServerName string `plist:",omitempty"` - clientID string + + // ServerURL is the URI template of the DoH server. It must be empty if + // DNSProtocol is not "HTTPS". + ServerURL string `plist:",omitempty"` + + // ServerName is the hostname of the DoT server. It must be empty if + // 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"` } +// payloadContent is a Device Management Profile payload. +// +// See https://developer.apple.com/documentation/devicemanagement/configuring_multiple_devices_using_profiles#3234127. type payloadContent struct { - Name string - PayloadDescription string - PayloadDisplayName string - PayloadIdentifier string + DNSSettings *dnsSettings + PayloadType string + PayloadIdentifier string PayloadUUID string - DNSSettings dnsSettings + PayloadDisplayName string + PayloadDescription string PayloadVersion int } +// dnsSettingsPayloadType is the payload type for a DNSSettings profile. +const dnsSettingsPayloadType = "com.apple.dnsSettings.managed" + +// mobileConfig contains the TopLevel properties for configuring Device +// Management Profiles. +// +// See https://developer.apple.com/documentation/devicemanagement/toplevel. type mobileConfig struct { PayloadDescription string PayloadDisplayName string PayloadIdentifier string PayloadType string PayloadUUID string - PayloadContent []payloadContent + PayloadContent []*payloadContent PayloadVersion int PayloadRemovalDisallowed bool } @@ -52,7 +77,7 @@ const ( dnsProtoTLS = "TLS" ) -func getMobileConfig(d dnsSettings) ([]byte, error) { +func encodeMobileConfig(d *dnsSettings, clientID string) ([]byte, error) { var dspName string switch proto := d.DNSProtocol; proto { case dnsProtoHTTPS: @@ -60,41 +85,41 @@ func getMobileConfig(d dnsSettings) ([]byte, error) { u := &url.URL{ Scheme: schemeHTTPS, Host: d.ServerName, - Path: path.Join("/dns-query", d.clientID), + Path: path.Join("/dns-query", clientID), } d.ServerURL = u.String() + // Empty the ServerName field since it is only must be presented // in DNS-over-TLS configuration. - // - // See https://developer.apple.com/documentation/devicemanagement/dnssettings/dnssettings. d.ServerName = "" case dnsProtoTLS: dspName = fmt.Sprintf("%s DoT", d.ServerName) - if d.clientID != "" { - d.ServerName = d.clientID + "." + d.ServerName + if clientID != "" { + d.ServerName = clientID + "." + d.ServerName } default: return nil, fmt.Errorf("bad dns protocol %q", proto) } - data := mobileConfig{ - PayloadContent: []payloadContent{{ - Name: dspName, - PayloadDescription: "Configures device to use AdGuard Home", - PayloadDisplayName: dspName, - PayloadIdentifier: fmt.Sprintf("com.apple.dnsSettings.managed.%s", genUUIDv4()), - PayloadType: "com.apple.dnsSettings.managed", + payloadID := fmt.Sprintf("%s.%s", dnsSettingsPayloadType, genUUIDv4()) + data := &mobileConfig{ + PayloadDescription: "Adds AdGuard Home to macOS Big Sur " + + "and iOS 14 or newer systems", + PayloadDisplayName: dspName, + PayloadIdentifier: genUUIDv4(), + PayloadType: "Configuration", + PayloadUUID: genUUIDv4(), + PayloadContent: []*payloadContent{{ + PayloadType: dnsSettingsPayloadType, + PayloadIdentifier: payloadID, PayloadUUID: genUUIDv4(), + PayloadDisplayName: dspName, + PayloadDescription: "Configures device to use AdGuard Home", PayloadVersion: 1, DNSSettings: d, }}, - PayloadDescription: "Adds AdGuard Home to Big Sur and iOS 14 or newer systems", - PayloadDisplayName: dspName, - PayloadIdentifier: genUUIDv4(), - PayloadRemovalDisallowed: false, - PayloadType: "Configuration", - PayloadUUID: genUUIDv4(), PayloadVersion: 1, + PayloadRemovalDisallowed: false, } return plist.MarshalIndent(data, plist.XMLFormat, "\t") @@ -133,13 +158,13 @@ func handleMobileConfig(w http.ResponseWriter, r *http.Request, dnsp string) { } } - d := dnsSettings{ - DNSProtocol: dnsp, - ServerName: host, - clientID: clientID, + d := &dnsSettings{ + DNSProtocol: dnsp, + ServerName: host, + ServerAddresses: cloneBootstrap(), } - mobileconfig, err := getMobileConfig(d) + mobileconfig, err := encodeMobileConfig(d, clientID) if err != nil { respondJSONError(w, http.StatusInternalServerError, err.Error()) @@ -163,6 +188,14 @@ 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) } diff --git a/internal/home/mobileconfig_test.go b/internal/home/mobileconfig_test.go index 3700bf52..9a896d1f 100644 --- a/internal/home/mobileconfig_test.go +++ b/internal/home/mobileconfig_test.go @@ -7,12 +7,39 @@ import ( "net/http/httptest" "testing" + "github.com/AdguardTeam/AdGuardHome/internal/dnsforward" "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) { + t.Helper() + + prevConfig := config + t.Cleanup(func() { + config = prevConfig + }) + config = &configuration{ + DNS: dnsConfig{ + FilteringConfig: dnsforward.FilteringConfig{ + BootstrapDNS: testBootstrapDNS, + }, + }, + } +} + func TestHandleMobileConfigDoH(t *testing.T) { + setupBootstraps(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) require.NoError(t, err) @@ -25,11 +52,16 @@ func TestHandleMobileConfigDoH(t *testing.T) { var mc mobileConfig _, err = plist.Unmarshal(w.Body.Bytes(), &mc) require.NoError(t, err) - require.Len(t, mc.PayloadContent, 1) - assert.Equal(t, "example.org DoH", mc.PayloadContent[0].Name) + assert.Equal(t, "example.org DoH", mc.PayloadContent[0].PayloadDisplayName) - assert.Equal(t, "https://example.org/dns-query", mc.PayloadContent[0].DNSSettings.ServerURL) + + s := mc.PayloadContent[0].DNSSettings + require.NotNil(t, s) + + assert.Equal(t, testBootstrapDNS, s.ServerAddresses) + assert.Empty(t, s.ServerName) + assert.Equal(t, "https://example.org/dns-query", s.ServerURL) }) t.Run("error_no_host", func(t *testing.T) { @@ -66,15 +98,22 @@ func TestHandleMobileConfigDoH(t *testing.T) { var mc mobileConfig _, err = plist.Unmarshal(w.Body.Bytes(), &mc) require.NoError(t, err) - require.Len(t, mc.PayloadContent, 1) - assert.Equal(t, "example.org DoH", mc.PayloadContent[0].Name) + assert.Equal(t, "example.org DoH", mc.PayloadContent[0].PayloadDisplayName) - assert.Equal(t, "https://example.org/dns-query/cli42", mc.PayloadContent[0].DNSSettings.ServerURL) + + s := mc.PayloadContent[0].DNSSettings + require.NotNil(t, s) + + assert.Equal(t, testBootstrapDNS, 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) + t.Run("success", func(t *testing.T) { r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig?host=example.org", nil) require.NoError(t, err) @@ -87,11 +126,16 @@ func TestHandleMobileConfigDoT(t *testing.T) { var mc mobileConfig _, err = plist.Unmarshal(w.Body.Bytes(), &mc) require.NoError(t, err) - require.Len(t, mc.PayloadContent, 1) - assert.Equal(t, "example.org DoT", mc.PayloadContent[0].Name) + assert.Equal(t, "example.org DoT", mc.PayloadContent[0].PayloadDisplayName) - assert.Equal(t, "example.org", mc.PayloadContent[0].DNSSettings.ServerName) + + s := mc.PayloadContent[0].DNSSettings + require.NotNil(t, s) + + assert.Equal(t, testBootstrapDNS, s.ServerAddresses) + assert.Equal(t, "example.org", s.ServerName) + assert.Empty(t, s.ServerURL) }) t.Run("error_no_host", func(t *testing.T) { @@ -129,10 +173,15 @@ func TestHandleMobileConfigDoT(t *testing.T) { var mc mobileConfig _, err = plist.Unmarshal(w.Body.Bytes(), &mc) require.NoError(t, err) - require.Len(t, mc.PayloadContent, 1) - assert.Equal(t, "example.org DoT", mc.PayloadContent[0].Name) + assert.Equal(t, "example.org DoT", mc.PayloadContent[0].PayloadDisplayName) - assert.Equal(t, "cli42.example.org", mc.PayloadContent[0].DNSSettings.ServerName) + + s := mc.PayloadContent[0].DNSSettings + require.NotNil(t, s) + + assert.Equal(t, testBootstrapDNS, s.ServerAddresses) + assert.Equal(t, "cli42.example.org", s.ServerName) + assert.Empty(t, s.ServerURL) }) }