From 9067e85b55a1fdf7298b70d30aa65c42585be56b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Mon, 13 Sep 2021 20:09:19 -0700 Subject: [PATCH] fix: TLS issues with console (#1043) This PR fixes two bugs one is - incorrect termination of the HTTP connections when the resource URL path is `/`, since `/` doesn't exist we should never call h.ServeHTTP() instead should be directly served from public assets. - add SSLHostFunc() such that if the Hostname is empty redirection is not empty and this value is handled properly when redirecting from 9090 to 9443. Co-authored-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com> --- go.mod | 2 +- go.sum | 8 ++-- restapi/config.go | 6 ++- restapi/configure_console.go | 83 ++++++++++++++++++++++++------------ 4 files changed, 65 insertions(+), 34 deletions(-) diff --git a/go.mod b/go.mod index d638eadbd..8522f0604 100644 --- a/go.mod +++ b/go.mod @@ -31,7 +31,7 @@ require ( github.com/rs/xid v1.2.1 github.com/secure-io/sio-go v0.3.1 github.com/stretchr/testify v1.7.0 - github.com/unrolled/secure v1.0.7 + github.com/unrolled/secure v1.0.9 golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b golang.org/x/net v0.0.0-20210421230115-4e50805a0758 golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d diff --git a/go.sum b/go.sum index 65c93f22c..79af2c40d 100644 --- a/go.sum +++ b/go.sum @@ -196,8 +196,6 @@ github.com/cockroachdb/cockroach-go/v2 v2.0.3 h1:ZA346ACHIZctef6trOTwBAEvPVm1k0u github.com/cockroachdb/cockroach-go/v2 v2.0.3/go.mod h1:hAuDgiVgDVkfirP9JnhXEfcXEPRKBpYdGz+l7mvYSzw= github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8= github.com/codahale/hdrhistogram v0.0.0-20161010025455-3a0bb77429bd/go.mod h1:sE/e/2PUdi/liOCUjSTXgM1o87ZssimdTWN964YiIeI= -github.com/codegangsta/negroni v1.0.0 h1:+aYywywx4bnKXWvoWtRfJ91vC59NbEhEY03sZjQhbVY= -github.com/codegangsta/negroni v1.0.0/go.mod h1:v0y3T5G7Y1UlFfyxFn/QLRU4a2EuNau2iZY63YTKWo0= github.com/container-storage-interface/spec v1.1.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4= github.com/container-storage-interface/spec v1.3.0/go.mod h1:6URME8mwIBbpVyZV93Ce5St17xBiQJQY67NDsuohiy4= github.com/containerd/containerd v1.3.0/go.mod h1:bC6axHOhabU15QhwfG7w5PipXdVtMXFTttgp+kVtyUA= @@ -1209,10 +1207,12 @@ github.com/ulikunitz/xz v0.5.6/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4A github.com/ulikunitz/xz v0.5.7/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14= github.com/ultraware/funlen v0.0.2/go.mod h1:Dp4UiAus7Wdb9KUZsYWZEWiRzGuM2kXM1lPbfaF6xhA= github.com/ultraware/whitespace v0.0.4/go.mod h1:aVMh/gQve5Maj9hQ/hg+F75lr/X5A89uZnzAmWSineA= -github.com/unrolled/secure v1.0.7 h1:BcQHp3iKZyZCKj5gRqwQG+5urnGBF00wGgoPPwtheVQ= -github.com/unrolled/secure v1.0.7/go.mod h1:uGc1OcRF8gCVBA+ANksKmvM85Hka6SZtQIbrKc3sHS4= +github.com/unrolled/secure v1.0.9 h1:BWRuEb1vDrBFFDdbCnKkof3gZ35I/bnHGyt0LB0TNyQ= +github.com/unrolled/secure v1.0.9/go.mod h1:fO+mEan+FLB0CdEnHf6Q4ZZVNqG+5fuLFnP8p0BXDPI= github.com/urfave/cli v1.20.0/go.mod h1:70zkFmudgCuE/ngEzBv17Jvp/497gISqfk5gWijbERA= github.com/urfave/cli v1.22.1/go.mod h1:Gos4lmkARVdJ6EkW0WaNv/tZAAMe9V7XWyB60NtXRu0= +github.com/urfave/negroni v1.0.0 h1:kIimOitoypq34K7TG7DUaJ9kq/N4Ofuwi1sjz0KipXc= +github.com/urfave/negroni v1.0.0/go.mod h1:Meg73S6kFm/4PpbYdq35yYWoCZ9mS/YSx+lKnmiohz4= github.com/uudashr/gocognit v1.0.1/go.mod h1:j44Ayx2KW4+oB6SWMv8KsmHzZrOInQav7D3cQMJ5JUM= github.com/valyala/bytebufferpool v1.0.0/go.mod h1:6bBcMArwyJ5K/AmCkWv1jt77kVWyCJ6HpOuEn7z0Csc= github.com/valyala/fasthttp v1.2.0/go.mod h1:4vX61m6KN+xDduDNwXrhIAVZaZaZiQ1luJk8LWSxF3s= diff --git a/restapi/config.go b/restapi/config.go index 74898a835..dee713f88 100644 --- a/restapi/config.go +++ b/restapi/config.go @@ -171,7 +171,11 @@ func GetSecureHostsProxyHeaders() []string { // TLSHost is the host name that is used to redirect HTTP requests to HTTPS. Default is "", which indicates to use the same host. func GetSecureTLSHost() string { - return env.Get(ConsoleSecureTLSHost, net.JoinHostPort(Hostname, TLSPort)) + tlsHost := env.Get(ConsoleSecureTLSHost, "") + if tlsHost == "" && Hostname != "" { + return net.JoinHostPort(Hostname, TLSPort) + } + return "" } // STSSeconds is the max-age of the Strict-Transport-Security header. Default is 0, which would NOT include the header. diff --git a/restapi/configure_console.go b/restapi/configure_console.go index 3d5ab971c..ff2db3c5d 100644 --- a/restapi/configure_console.go +++ b/restapi/configure_console.go @@ -24,12 +24,15 @@ import ( "io" "io/fs" "log" + "net" "net/http" + "path/filepath" "regexp" "strings" "time" portal_ui "github.com/minio/console/portal-ui" + "github.com/minio/pkg/mimedb" "github.com/go-openapi/errors" "github.com/go-openapi/swag" @@ -150,6 +153,15 @@ func setupGlobalMiddleware(handler http.Handler) http.Handler { next := AuthenticationMiddleware(handler) // serve static files next = FileServerMiddleware(next) + + sslHostFn := secure.SSLHostFunc(func(host string) string { + h, _, err := net.SplitHostPort(host) + if err != nil { + return host + } + return net.JoinHostPort(h, TLSPort) + }) + // Secure middleware, this middleware wrap all the previous handlers and add // HTTP security headers secureOptions := secure.Options{ @@ -157,12 +169,12 @@ func setupGlobalMiddleware(handler http.Handler) http.Handler { AllowedHostsAreRegex: GetSecureAllowedHostsAreRegex(), HostsProxyHeaders: GetSecureHostsProxyHeaders(), SSLRedirect: GetTLSRedirect() == "on" && len(GlobalPublicCerts) > 0, + SSLHostFunc: &sslHostFn, SSLHost: GetSecureTLSHost(), STSSeconds: GetSecureSTSSeconds(), STSIncludeSubdomains: GetSecureSTSIncludeSubdomains(), STSPreload: GetSecureSTSPreload(), - SSLTemporaryRedirect: GetSecureTLSTemporaryRedirect(), - SSLHostFunc: nil, + SSLTemporaryRedirect: false, ForceSTSHeader: GetSecureForceSTSHeader(), FrameDeny: GetSecureFrameDeny(), ContentTypeNosniff: GetSecureContentTypeNonSniff(), @@ -216,6 +228,8 @@ func FileServerMiddleware(next http.Handler) http.Handler { }) } +var reHrefIndex = regexp.MustCompile(`(?m)((href|src)="(.\/).*?")`) + type notFoundRedirectRespWr struct { http.ResponseWriter // We embed http.ResponseWriter status int @@ -235,38 +249,51 @@ func (w *notFoundRedirectRespWr) Write(p []byte) (int, error) { return len(p), nil // Lie that we successfully wrote it } -var reHrefIndex = regexp.MustCompile(`(?m)((href|src)="(.\/).*?")`) +func handleSPA(w http.ResponseWriter, r *http.Request) { + basePath := "/" + // For SPA mode we will replace relative paths with absolute unless we receive query param cp=y + if v := r.URL.Query().Get("cp"); v == "y" { + basePath = "./" + } + + indexPage, err := portal_ui.GetStaticAssets().Open("build/index.html") + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + indexPageBytes, err := io.ReadAll(indexPage) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + if basePath != "./" { + indexPageStr := string(indexPageBytes) + for _, match := range reHrefIndex.FindAllStringSubmatch(indexPageStr, -1) { + toReplace := strings.Replace(match[1], match[3], basePath, 1) + indexPageStr = strings.Replace(indexPageStr, match[1], toReplace, 1) + } + indexPageBytes = []byte(indexPageStr) + } + + w.Header().Set("Content-Type", "text/html; charset=utf-8") + http.ServeContent(w, r, "index.html", time.Now(), bytes.NewReader(indexPageBytes)) +} // wrapHandlerSinglePageApplication handles a http.FileServer returning a 404 and overrides it with index.html func wrapHandlerSinglePageApplication(h http.Handler) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - // This is used to enforce application/javascript MIME on Windows (https://github.com/golang/go/issues/32350) - if strings.HasSuffix(r.URL.Path, ".js") { - w.Header().Set("Content-Type", "application/javascript") + if r.URL.Path == "/" { + handleSPA(w, r) + return } - nfrw := ¬FoundRedirectRespWr{ResponseWriter: w} - h.ServeHTTP(nfrw, r) - if nfrw.status == 404 || r.URL.String() == "/" { - basePath := "/" - // For SPA mode we will replace relative paths with absolute unless we receive query param cp=y - if val, ok := r.URL.Query()["cp"]; ok && len(val) > 0 && val[0] == "y" { - basePath = "./" - } - - indexPage, _ := portal_ui.GetStaticAssets().Open("build/index.html") - indexPageBytes, _ := io.ReadAll(indexPage) - if basePath != "./" { - indexPageStr := string(indexPageBytes) - for _, match := range reHrefIndex.FindAllStringSubmatch(indexPageStr, -1) { - toReplace := strings.Replace(match[1], match[3], basePath, 1) - indexPageStr = strings.Replace(indexPageStr, match[1], toReplace, 1) - } - indexPageBytes = []byte(indexPageStr) - } - - w.Header().Set("Content-Type", "text/html; charset=utf-8") - http.ServeContent(w, r, "index.html", time.Now(), bytes.NewReader(indexPageBytes)) + w.Header().Set("Content-Type", mimedb.TypeByExtension(filepath.Ext(r.URL.Path))) + nfw := ¬FoundRedirectRespWr{ResponseWriter: w} + h.ServeHTTP(nfw, r) + if nfw.status == http.StatusNotFound { + handleSPA(w, r) } } }