From 3e128d744e7fc7466638aa3367e54ab8a9c180a0 Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Sat, 29 Mar 2025 21:48:56 +0100 Subject: [PATCH] for the web interfaces, ensure the effective configured http paths end in a slash to prevent 404's and/or errors accessing the web interfaces The default paths for the web interfaces, such as /admin/, /account/, /webmail/ and /webapi/ end with a slash. They should end with a slash because we use the path when restricting cookies to just that web interface. You could configure paths not ending with a slash, but due to using http.StripPrefix, and our handler, some of those requests may not work properly. We now warn if configured paths don't end with a trailing slash when parsing the config file. We normally error out when such things happen, but users probably have paths without trailing slashes configured, and we don't want to break them on a future upgrade. We now use an effective path that includes the trailing slash. We would always redirect requests to the configured paths but without trailing slash to the path with trailing slash, and that stays. For issue #325 by odama626. --- config/config.go | 2 +- config/doc.go | 24 ++++++++++++++++-------- http/web.go | 18 +++++++++--------- mox-/config.go | 25 ++++++++++++++++++------- 4 files changed, 44 insertions(+), 25 deletions(-) diff --git a/config/config.go b/config/config.go index b0d267e..c07fb0b 100644 --- a/config/config.go +++ b/config/config.go @@ -222,7 +222,7 @@ type Listener struct { type WebService struct { Enabled bool Port int `sconf:"optional" sconf-doc:"Default 80 for HTTP and 443 for HTTPS. See Hostname at Listener for hostname matching behaviour."` - Path string `sconf:"optional" sconf-doc:"Path to serve requests on."` + Path string `sconf:"optional" sconf-doc:"Path to serve requests on. Should end with a slash, related to cookie paths."` Forwarded bool `sconf:"optional" sconf-doc:"If set, X-Forwarded-* headers are used for the remote IP address for rate limiting and for the \"secure\" status of cookies."` } diff --git a/config/doc.go b/config/doc.go index 44a5588..37c8e5a 100644 --- a/config/doc.go +++ b/config/doc.go @@ -333,7 +333,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate @@ -349,7 +350,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate @@ -368,7 +370,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate @@ -384,7 +387,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate @@ -399,7 +403,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate @@ -415,7 +420,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate @@ -430,7 +436,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate @@ -446,7 +453,8 @@ See https://pkg.go.dev/github.com/mjl-/sconf for details. # matching behaviour. (optional) Port: 0 - # Path to serve requests on. (optional) + # Path to serve requests on. Should end with a slash, related to cookie paths. + # (optional) Path: # If set, X-Forwarded-* headers are used for the remote IP address for rate diff --git a/http/web.go b/http/web.go index 05b7d3b..02f37a0 100644 --- a/http/web.go +++ b/http/web.go @@ -536,7 +536,7 @@ func redirectToTrailingSlash(srv *serve, hostMatch func(dns.IPDomain) bool, name handler := mox.SafeHeaders(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, path, http.StatusSeeOther) })) - srv.ServiceHandle(name, hostMatch, path[:len(path)-1], handler) + srv.ServiceHandle(name, hostMatch, strings.TrimRight(path, "/"), handler) } } @@ -656,7 +656,7 @@ func portServes(name string, l config.Listener) map[int]*serve { path = l.AccountHTTP.Path } srv := ensureServe(false, port, "account-http at "+path, true) - handler := mox.SafeHeaders(http.StripPrefix(path[:len(path)-1], http.HandlerFunc(webaccount.Handler(path, l.AccountHTTP.Forwarded)))) + handler := mox.SafeHeaders(http.StripPrefix(strings.TrimRight(path, "/"), http.HandlerFunc(webaccount.Handler(path, l.AccountHTTP.Forwarded)))) srv.ServiceHandle("account", accountHostMatch, path, handler) redirectToTrailingSlash(srv, accountHostMatch, "account", path) ensureACMEHTTP01(srv) @@ -668,7 +668,7 @@ func portServes(name string, l config.Listener) map[int]*serve { path = l.AccountHTTPS.Path } srv := ensureServe(true, port, "account-https at "+path, true) - handler := mox.SafeHeaders(http.StripPrefix(path[:len(path)-1], http.HandlerFunc(webaccount.Handler(path, l.AccountHTTPS.Forwarded)))) + handler := mox.SafeHeaders(http.StripPrefix(strings.TrimRight(path, "/"), http.HandlerFunc(webaccount.Handler(path, l.AccountHTTPS.Forwarded)))) srv.ServiceHandle("account", accountHostMatch, path, handler) redirectToTrailingSlash(srv, accountHostMatch, "account", path) } @@ -680,7 +680,7 @@ func portServes(name string, l config.Listener) map[int]*serve { path = l.AdminHTTP.Path } srv := ensureServe(false, port, "admin-http at "+path, true) - handler := mox.SafeHeaders(http.StripPrefix(path[:len(path)-1], http.HandlerFunc(webadmin.Handler(path, l.AdminHTTP.Forwarded)))) + handler := mox.SafeHeaders(http.StripPrefix(strings.TrimRight(path, "/"), http.HandlerFunc(webadmin.Handler(path, l.AdminHTTP.Forwarded)))) srv.ServiceHandle("admin", listenerHostMatch, path, handler) redirectToTrailingSlash(srv, listenerHostMatch, "admin", path) ensureACMEHTTP01(srv) @@ -692,7 +692,7 @@ func portServes(name string, l config.Listener) map[int]*serve { path = l.AdminHTTPS.Path } srv := ensureServe(true, port, "admin-https at "+path, true) - handler := mox.SafeHeaders(http.StripPrefix(path[:len(path)-1], http.HandlerFunc(webadmin.Handler(path, l.AdminHTTPS.Forwarded)))) + handler := mox.SafeHeaders(http.StripPrefix(strings.TrimRight(path, "/"), http.HandlerFunc(webadmin.Handler(path, l.AdminHTTPS.Forwarded)))) srv.ServiceHandle("admin", listenerHostMatch, path, handler) redirectToTrailingSlash(srv, listenerHostMatch, "admin", path) } @@ -709,7 +709,7 @@ func portServes(name string, l config.Listener) map[int]*serve { path = l.WebAPIHTTP.Path } srv := ensureServe(false, port, "webapi-http at "+path, true) - handler := mox.SafeHeaders(http.StripPrefix(path[:len(path)-1], webapisrv.NewServer(maxMsgSize, path, l.WebAPIHTTP.Forwarded))) + handler := mox.SafeHeaders(http.StripPrefix(strings.TrimRight(path, "/"), webapisrv.NewServer(maxMsgSize, path, l.WebAPIHTTP.Forwarded))) srv.ServiceHandle("webapi", accountHostMatch, path, handler) redirectToTrailingSlash(srv, accountHostMatch, "webapi", path) ensureACMEHTTP01(srv) @@ -721,7 +721,7 @@ func portServes(name string, l config.Listener) map[int]*serve { path = l.WebAPIHTTPS.Path } srv := ensureServe(true, port, "webapi-https at "+path, true) - handler := mox.SafeHeaders(http.StripPrefix(path[:len(path)-1], webapisrv.NewServer(maxMsgSize, path, l.WebAPIHTTPS.Forwarded))) + handler := mox.SafeHeaders(http.StripPrefix(strings.TrimRight(path, "/"), webapisrv.NewServer(maxMsgSize, path, l.WebAPIHTTPS.Forwarded))) srv.ServiceHandle("webapi", accountHostMatch, path, handler) redirectToTrailingSlash(srv, accountHostMatch, "webapi", path) } @@ -740,7 +740,7 @@ func portServes(name string, l config.Listener) map[int]*serve { accountPath = l.AccountHTTP.Path } } - handler := http.StripPrefix(path[:len(path)-1], http.HandlerFunc(webmail.Handler(maxMsgSize, path, l.WebmailHTTP.Forwarded, accountPath))) + handler := http.StripPrefix(strings.TrimRight(path, "/"), http.HandlerFunc(webmail.Handler(maxMsgSize, path, l.WebmailHTTP.Forwarded, accountPath))) srv.ServiceHandle("webmail", accountHostMatch, path, handler) redirectToTrailingSlash(srv, accountHostMatch, "webmail", path) ensureACMEHTTP01(srv) @@ -759,7 +759,7 @@ func portServes(name string, l config.Listener) map[int]*serve { accountPath = l.AccountHTTPS.Path } } - handler := http.StripPrefix(path[:len(path)-1], http.HandlerFunc(webmail.Handler(maxMsgSize, path, l.WebmailHTTPS.Forwarded, accountPath))) + handler := http.StripPrefix(strings.TrimRight(path, "/"), http.HandlerFunc(webmail.Handler(maxMsgSize, path, l.WebmailHTTPS.Forwarded, accountPath))) srv.ServiceHandle("webmail", accountHostMatch, path, handler) redirectToTrailingSlash(srv, accountHostMatch, "webmail", path) } diff --git a/mox-/config.go b/mox-/config.go index 6b26abe..0672f4b 100644 --- a/mox-/config.go +++ b/mox-/config.go @@ -908,15 +908,26 @@ func PrepareStaticConfig(ctx context.Context, log mlog.Log, configFile string, c addListenerErrorf("NAT ip that is the unspecified or loopback address %s", ipstr) } } - checkPath := func(kind string, enabled bool, path string) { - if enabled && path != "" && !strings.HasPrefix(path, "/") { - addListenerErrorf("%s with path %q that must start with a slash", kind, path) + cleanPath := func(kind string, enabled bool, path string) string { + if !enabled { + return path } + if path != "" && !strings.HasPrefix(path, "/") { + addListenerErrorf("%s with path %q that must start with a slash", kind, path) + } else if path != "" && !strings.HasSuffix(path, "/") { + log.Warn("http service path should end with a slash, using effective path with slash", slog.String("kind", kind), slog.String("path", path), slog.String("effectivepath", path+"/")) + path += "/" + } + return path } - checkPath("AccountHTTP", l.AccountHTTP.Enabled, l.AccountHTTP.Path) - checkPath("AccountHTTPS", l.AccountHTTPS.Enabled, l.AccountHTTPS.Path) - checkPath("AdminHTTP", l.AdminHTTP.Enabled, l.AdminHTTP.Path) - checkPath("AdminHTTPS", l.AdminHTTPS.Enabled, l.AdminHTTPS.Path) + l.AccountHTTP.Path = cleanPath("AccountHTTP", l.AccountHTTP.Enabled, l.AccountHTTP.Path) + l.AccountHTTPS.Path = cleanPath("AccountHTTPS", l.AccountHTTPS.Enabled, l.AccountHTTPS.Path) + l.AdminHTTP.Path = cleanPath("AdminHTTP", l.AdminHTTP.Enabled, l.AdminHTTP.Path) + l.AdminHTTPS.Path = cleanPath("AdminHTTPS", l.AdminHTTPS.Enabled, l.AdminHTTPS.Path) + l.WebmailHTTP.Path = cleanPath("WebmailHTTP", l.WebmailHTTP.Enabled, l.WebmailHTTP.Path) + l.WebmailHTTPS.Path = cleanPath("WebmailHTTPS", l.WebmailHTTPS.Enabled, l.WebmailHTTPS.Path) + l.WebAPIHTTP.Path = cleanPath("WebAPIHTTP", l.WebAPIHTTP.Enabled, l.WebAPIHTTP.Path) + l.WebAPIHTTPS.Path = cleanPath("WebAPIHTTPS", l.WebAPIHTTPS.Enabled, l.WebAPIHTTPS.Path) c.Listeners[name] = l } if haveUnspecifiedSMTPListener {