From bc01f4f8bcf6acef8dbe40d082a881c82cff42cf Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Fri, 5 Feb 2021 14:11:53 +0300 Subject: [PATCH] Pull request: home: don't allow all origins Merge in DNS/adguard-home from 2484-access-control to master Updates #2484. Squashed commit of the following: commit 4f0c6ddae35b55a9251f4d5e14c4d92fd2107443 Author: Ainar Garipov Date: Fri Feb 5 12:42:22 2021 +0300 home: don't allow all origins --- CHANGELOG.md | 3 ++ internal/home/control.go | 89 ++++++++++++++++++++++++++++---------- internal/home/home_test.go | 2 +- 3 files changed, 71 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 858d11e4..e9da24ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,6 +43,8 @@ and this project adheres to ### Changed +- `Access-Control-Allow-Origin` is now only set to the same origin as the + domain, but with an HTTP scheme as opposed to `*` ([#2484]). - `workDir` now supports symlinks. - Stopped mounting together the directories `/opt/adguardhome/conf` and `/opt/adguardhome/work` in our Docker images ([#2589]). @@ -67,6 +69,7 @@ and this project adheres to [#2358]: https://github.com/AdguardTeam/AdGuardHome/issues/2358 [#2391]: https://github.com/AdguardTeam/AdGuardHome/issues/2391 [#2394]: https://github.com/AdguardTeam/AdGuardHome/issues/2394 +[#2484]: https://github.com/AdguardTeam/AdGuardHome/issues/2484 [#2509]: https://github.com/AdguardTeam/AdGuardHome/issues/2509 [#2552]: https://github.com/AdguardTeam/AdGuardHome/issues/2552 [#2589]: https://github.com/AdguardTeam/AdGuardHome/issues/2589 diff --git a/internal/home/control.go b/internal/home/control.go index 49d6b31c..71bf52e5 100644 --- a/internal/home/control.go +++ b/internal/home/control.go @@ -2,6 +2,7 @@ package home import ( "encoding/json" + "errors" "fmt" "net" "net/http" @@ -202,37 +203,81 @@ func preInstallHandler(handler http.Handler) http.Handler { return &preInstallHandlerStruct{handler} } -// postInstall lets the handler run only if firstRun is false, and redirects to /install.html otherwise -// it also enforces HTTPS if it is enabled and configured +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) { + web := Context.web + if web.httpsServer.server == nil { + return true + } + + host, _, err := net.SplitHostPort(r.Host) + if err != nil { + // Check for the missing port error. If it is that error, just + // use the host as is. + // + // See the source code for net.SplitHostPort. + const missingPort = "missing port in address" + + addrErr := &net.AddrError{} + if !errors.As(err, &addrErr) || addrErr.Err != missingPort { + httpError(w, http.StatusBadRequest, "bad host: %s", err) + + return false + } + + host = r.Host + } + + if r.TLS == nil && web.forceHTTPS { + hostPort := host + if port := web.conf.PortHTTPS; port != defaultHTTPSPort { + portStr := strconv.Itoa(port) + hostPort = net.JoinHostPort(host, portStr) + } + + httpsURL := &url.URL{ + Scheme: "https", + Host: hostPort, + Path: r.URL.Path, + RawQuery: r.URL.RawQuery, + } + http.Redirect(w, r, httpsURL.String(), http.StatusTemporaryRedirect) + + return false + } + + // Allow the frontend from the HTTP origin to send requests to the HTTPS + // server. This can happen when the user has just set up HTTPS with + // redirects. + originURL := &url.URL{ + Scheme: "http", + Host: r.Host, + } + w.Header().Set("Access-Control-Allow-Origin", originURL.String()) + + return true +} + +// postInstall lets the handler to run only if firstRun is false. Otherwise, it +// redirects to /install.html. It also enforces HTTPS if it is enabled and +// configured and sets appropriate access control headers. func postInstall(handler func(http.ResponseWriter, *http.Request)) func(http.ResponseWriter, *http.Request) { return func(w http.ResponseWriter, r *http.Request) { - if Context.firstRun && - !strings.HasPrefix(r.URL.Path, "/install.") && - !strings.HasPrefix(r.URL.Path, "/assets/") { + path := r.URL.Path + if Context.firstRun && !strings.HasPrefix(path, "/install.") && + !strings.HasPrefix(path, "/assets/") { http.Redirect(w, r, "/install.html", http.StatusFound) + return } - // enforce https? - if r.TLS == nil && Context.web.forceHTTPS && Context.web.httpsServer.server != nil { - // yes, and we want host from host:port - host, _, err := net.SplitHostPort(r.Host) - if err != nil { - // no port in host - host = r.Host - } - // construct new URL to redirect to - newURL := url.URL{ - Scheme: "https", - Host: net.JoinHostPort(host, strconv.Itoa(Context.web.conf.PortHTTPS)), - Path: r.URL.Path, - RawQuery: r.URL.RawQuery, - } - http.Redirect(w, r, newURL.String(), http.StatusTemporaryRedirect) + if !handleHTTPSRedirect(w, r) { return } - w.Header().Set("Access-Control-Allow-Origin", "*") handler(w, r) } } diff --git a/internal/home/home_test.go b/internal/home/home_test.go index 5f3f3d81..02613b7c 100644 --- a/internal/home/home_test.go +++ b/internal/home/home_test.go @@ -133,7 +133,7 @@ func TestHome(t *testing.T) { h := http.Client{} for i := 0; i != 50; i++ { resp, err = h.Get("http://127.0.0.1:3000/") - if err == nil && resp.StatusCode != 404 { + if err == nil && resp.StatusCode != http.StatusNotFound { break } time.Sleep(100 * time.Millisecond)