From 96e83a133f066a5f8b9cb812476aa31e2923ba1e Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Wed, 25 Nov 2020 18:09:41 +0300 Subject: [PATCH] Pull request: home: improve mobileconfig http api Merge in DNS/adguard-home from 2358-mobileconfig to master Updates #2358. Squashed commit of the following: commit ab3c7a75ae21f6978904f2dc237cb84cbedff7ab Merge: fa002e400 b4a35fa88 Author: Ainar Garipov Date: Wed Nov 25 16:11:06 2020 +0300 Merge branch 'master' into 2358-mobileconfig commit fa002e40004656db08d32c926892c6c820fb1338 Author: Ainar Garipov Date: Wed Nov 25 15:19:00 2020 +0300 home: improve mobileconfig http api --- CHANGELOG.md | 3 + internal/dhcpd/dhcphttp.go | 3 + internal/home/control.go | 4 +- internal/home/home.go | 9 ++ internal/home/mobileconfig.go | 54 ++++++----- internal/home/mobileconfig_test.go | 139 +++++++++++++++++++++++++---- openapi/openapi.yaml | 52 +++++++++-- 7 files changed, 217 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 99183e01..f0dfe8c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ and this project adheres to ### Changed +- Make the mobileconfig HTTP API more robust and predictable, add parameters and + improve error response ([#2358]). - Improved HTTP requests handling and timeouts. ([#2343]). - Our snap package now uses the `core20` image as its base [#2306]. - Various internal improvements ([#2271], [#2297]). @@ -31,6 +33,7 @@ and this project adheres to [#2297]: https://github.com/AdguardTeam/AdGuardHome/issues/2297 [#2306]: https://github.com/AdguardTeam/AdGuardHome/issues/2306 [#2343]: https://github.com/AdguardTeam/AdGuardHome/issues/2343 +[#2358]: https://github.com/AdguardTeam/AdGuardHome/issues/2358 ### Fixed diff --git a/internal/dhcpd/dhcphttp.go b/internal/dhcpd/dhcphttp.go index 1cacd83c..c7584f8b 100644 --- a/internal/dhcpd/dhcphttp.go +++ b/internal/dhcpd/dhcphttp.go @@ -509,6 +509,9 @@ func (s *Server) registerHandlers() { } // jsonError is a generic JSON error response. +// +// TODO(a.garipov): Merge together with the implementations in .../home and +// other packages after refactoring the web handler registering. type jsonError struct { // Message is the error message, an opaque string. Message string `json:"message"` diff --git a/internal/home/control.go b/internal/home/control.go index 5c8bc1cb..732fa919 100644 --- a/internal/home/control.go +++ b/internal/home/control.go @@ -112,8 +112,8 @@ func registerControlHandlers() { httpRegister(http.MethodGet, "/control/profile", handleGetProfile) // No auth is necessary for DOH/DOT configurations - Context.mux.HandleFunc("/apple/doh.mobileconfig", postInstall(handleMobileConfigDoh)) - Context.mux.HandleFunc("/apple/dot.mobileconfig", postInstall(handleMobileConfigDot)) + Context.mux.HandleFunc("/apple/doh.mobileconfig", postInstall(handleMobileConfigDOH)) + Context.mux.HandleFunc("/apple/dot.mobileconfig", postInstall(handleMobileConfigDOT)) RegisterAuthHandlers() } diff --git a/internal/home/home.go b/internal/home/home.go index 96bc4d68..82f5ccf4 100644 --- a/internal/home/home.go +++ b/internal/home/home.go @@ -683,3 +683,12 @@ func getHTTPProxy(req *http.Request) (*url.URL, error) { } return url.Parse(config.ProxyURL) } + +// jsonError is a generic JSON error response. +// +// TODO(a.garipov): Merge together with the implementations in .../dhcpd and +// other packages after refactoring the web handler registering. +type jsonError struct { + // Message is the error message, an opaque string. + Message string `json:"message"` +} diff --git a/internal/home/mobileconfig.go b/internal/home/mobileconfig.go index 81fe4b1a..58bdea88 100644 --- a/internal/home/mobileconfig.go +++ b/internal/home/mobileconfig.go @@ -1,10 +1,11 @@ package home import ( + "encoding/json" "fmt" - "net" "net/http" + "github.com/AdguardTeam/golibs/log" uuid "github.com/satori/go.uuid" "howett.net/plist" ) @@ -51,6 +52,7 @@ func getMobileConfig(d DNSSettings) ([]byte, error) { switch d.DNSProtocol { case dnsProtoHTTPS: name = fmt.Sprintf("%s DoH", d.ServerName) + d.ServerURL = fmt.Sprintf("https://%s/dns-query", d.ServerName) case dnsProtoTLS: name = fmt.Sprintf("%s DoT", d.ServerName) default: @@ -80,34 +82,46 @@ func getMobileConfig(d DNSSettings) ([]byte, error) { return plist.MarshalIndent(data, plist.XMLFormat, "\t") } -func handleMobileConfig(w http.ResponseWriter, d DNSSettings) { +func handleMobileConfig(w http.ResponseWriter, r *http.Request, dnsp string) { + host := r.URL.Query().Get("host") + if host == "" { + host = Context.tls.conf.ServerName + } + + if host == "" { + w.WriteHeader(http.StatusInternalServerError) + + const msg = "no host in query parameters and no server_name" + err := json.NewEncoder(w).Encode(&jsonError{ + Message: msg, + }) + if err != nil { + log.Debug("writing 500 json response: %s", err) + } + + return + } + + d := DNSSettings{ + DNSProtocol: dnsp, + ServerName: host, + } + mobileconfig, err := getMobileConfig(d) if err != nil { httpError(w, http.StatusInternalServerError, "plist.MarshalIndent: %s", err) + + return } w.Header().Set("Content-Type", "application/xml") _, _ = w.Write(mobileconfig) } -func handleMobileConfigDoh(w http.ResponseWriter, r *http.Request) { - handleMobileConfig(w, DNSSettings{ - DNSProtocol: dnsProtoHTTPS, - ServerURL: fmt.Sprintf("https://%s/dns-query", r.Host), - }) +func handleMobileConfigDOH(w http.ResponseWriter, r *http.Request) { + handleMobileConfig(w, r, dnsProtoHTTPS) } -func handleMobileConfigDot(w http.ResponseWriter, r *http.Request) { - var err error - - var host string - host, _, err = net.SplitHostPort(r.Host) - if err != nil { - httpError(w, http.StatusBadRequest, "getting host: %s", err) - } - - handleMobileConfig(w, DNSSettings{ - DNSProtocol: dnsProtoTLS, - ServerName: host, - }) +func handleMobileConfigDOT(w http.ResponseWriter, r *http.Request) { + handleMobileConfig(w, r, dnsProtoTLS) } diff --git a/internal/home/mobileconfig_test.go b/internal/home/mobileconfig_test.go index f58f4e99..520d70e1 100644 --- a/internal/home/mobileconfig_test.go +++ b/internal/home/mobileconfig_test.go @@ -9,25 +9,132 @@ import ( "howett.net/plist" ) -func TestHandleMobileConfigDot(t *testing.T) { - var err error +func TestHandleMobileConfigDOH(t *testing.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) + assert.Nil(t, err) - var r *http.Request - r, err = http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig", nil) - assert.Nil(t, err) + w := httptest.NewRecorder() - w := httptest.NewRecorder() + handleMobileConfigDOH(w, r) + assert.Equal(t, http.StatusOK, w.Code) - handleMobileConfigDot(w, r) - assert.Equal(t, http.StatusOK, w.Code) + var mc MobileConfig + _, err = plist.Unmarshal(w.Body.Bytes(), &mc) + assert.Nil(t, err) - var mc MobileConfig - _, err = plist.Unmarshal(w.Body.Bytes(), &mc) - assert.Nil(t, err) + if assert.Equal(t, 1, len(mc.PayloadContent)) { + assert.Equal(t, "example.org DoH", mc.PayloadContent[0].Name) + assert.Equal(t, "example.org DoH", mc.PayloadContent[0].PayloadDisplayName) + assert.Equal(t, "example.org", mc.PayloadContent[0].DNSSettings.ServerName) + assert.Equal(t, "https://example.org/dns-query", mc.PayloadContent[0].DNSSettings.ServerURL) + } + }) - if assert.Equal(t, 1, len(mc.PayloadContent)) { - assert.Equal(t, "example.com DoT", mc.PayloadContent[0].Name) - assert.Equal(t, "example.com DoT", mc.PayloadContent[0].PayloadDisplayName) - assert.Equal(t, "example.com", mc.PayloadContent[0].DNSSettings.ServerName) - } + t.Run("success_no_host", func(t *testing.T) { + oldTLSConf := Context.tls + t.Cleanup(func() { Context.tls = oldTLSConf }) + + Context.tls = &TLSMod{ + conf: tlsConfigSettings{ServerName: "example.org"}, + } + + r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/doh.mobileconfig", nil) + assert.Nil(t, err) + + w := httptest.NewRecorder() + + handleMobileConfigDOH(w, r) + assert.Equal(t, http.StatusOK, w.Code) + + var mc MobileConfig + _, err = plist.Unmarshal(w.Body.Bytes(), &mc) + assert.Nil(t, err) + + if assert.Equal(t, 1, len(mc.PayloadContent)) { + assert.Equal(t, "example.org DoH", mc.PayloadContent[0].Name) + assert.Equal(t, "example.org DoH", mc.PayloadContent[0].PayloadDisplayName) + assert.Equal(t, "example.org", mc.PayloadContent[0].DNSSettings.ServerName) + assert.Equal(t, "https://example.org/dns-query", mc.PayloadContent[0].DNSSettings.ServerURL) + } + }) + + t.Run("error_no_host", func(t *testing.T) { + oldTLSConf := Context.tls + t.Cleanup(func() { Context.tls = oldTLSConf }) + + Context.tls = &TLSMod{conf: tlsConfigSettings{}} + + r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/doh.mobileconfig", nil) + assert.Nil(t, err) + + w := httptest.NewRecorder() + + handleMobileConfigDOH(w, r) + assert.Equal(t, http.StatusInternalServerError, w.Code) + }) +} + +func TestHandleMobileConfigDOT(t *testing.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) + assert.Nil(t, err) + + w := httptest.NewRecorder() + + handleMobileConfigDOT(w, r) + assert.Equal(t, http.StatusOK, w.Code) + + var mc MobileConfig + _, err = plist.Unmarshal(w.Body.Bytes(), &mc) + assert.Nil(t, err) + + if assert.Equal(t, 1, len(mc.PayloadContent)) { + 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) + } + }) + + t.Run("success_no_host", func(t *testing.T) { + oldTLSConf := Context.tls + t.Cleanup(func() { Context.tls = oldTLSConf }) + + Context.tls = &TLSMod{ + conf: tlsConfigSettings{ServerName: "example.org"}, + } + + r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig", nil) + assert.Nil(t, err) + + w := httptest.NewRecorder() + + handleMobileConfigDOT(w, r) + assert.Equal(t, http.StatusOK, w.Code) + + var mc MobileConfig + _, err = plist.Unmarshal(w.Body.Bytes(), &mc) + assert.Nil(t, err) + + if assert.Equal(t, 1, len(mc.PayloadContent)) { + 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) + } + }) + + t.Run("error_no_host", func(t *testing.T) { + oldTLSConf := Context.tls + t.Cleanup(func() { Context.tls = oldTLSConf }) + + Context.tls = &TLSMod{conf: tlsConfigSettings{}} + + r, err := http.NewRequest(http.MethodGet, "https://example.com:12345/apple/dot.mobileconfig", nil) + assert.Nil(t, err) + + w := httptest.NewRecorder() + + handleMobileConfigDOT(w, r) + assert.Equal(t, http.StatusInternalServerError, w.Code) + }) } diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 05552218..d2b36cdf 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -959,27 +959,61 @@ 'application/json': 'schema': '$ref': '#/components/schemas/ProfileInfo' + '/apple/doh.mobileconfig': 'get': - 'tags': - - 'mobileconfig' - - 'global' 'operationId': 'mobileConfigDoH' - 'summary': 'Get DNS over HTTPS .mobileconfig' + 'parameters': + - 'description': > + Host for which the config is generated. If no host is provided, + `tls.server_name` from the configuration file is used. If + `tls.server_name` is not set, the API returns an error with a 500 + status. + 'example': 'example.org' + 'in': 'query' + 'name': 'host' + 'schema': + 'type': 'string' 'responses': '200': - 'description': 'DNS over HTTPS plist file' - - '/apple/dot.mobileconfig': - 'get': + 'description': 'DNS over HTTPS plist file.' + '500': + 'content': + 'application/json': + 'schema': + '$ref': '#/components/schemas/Error' + 'description': 'Server configuration error.' + 'summary': 'Get DNS over HTTPS .mobileconfig.' 'tags': - 'mobileconfig' - 'global' + '/apple/dot.mobileconfig': + 'get': 'operationId': 'mobileConfigDoT' - 'summary': 'Get TLS over TLS .mobileconfig' + 'parameters': + - 'description': > + Host for which the config is generated. If no host is provided, + `tls.server_name` from the configuration file is used. If + `tls.server_name` is not set, the API returns an error with a 500 + status. + 'example': 'example.org' + 'in': 'query' + 'name': 'host' + 'schema': + 'type': 'string' 'responses': '200': 'description': 'DNS over TLS plist file' + '500': + 'content': + 'application/json': + 'schema': + '$ref': '#/components/schemas/Error' + 'description': 'Server configuration error.' + 'summary': 'Get DNS over TLS .mobileconfig.' + 'tags': + - 'mobileconfig' + - 'global' 'components': 'requestBodies':