From f1da6cb1b25bd1094cc498f54c6c3385a3b25e4e Mon Sep 17 00:00:00 2001 From: kermieisinthehouse Date: Mon, 4 Oct 2021 07:16:01 +0000 Subject: [PATCH] Disallow access in publicly exposed services (#1761) * Add security against publicly exposed services * Add trusted proxies setting, validate proxy chain against internet access * Validate chain on local proxies too * Move authentication handler to separate file * Add startup check and log if tripwire is active Co-authored-by: WithoutPants <53250216+WithoutPants@users.noreply.github.com> --- graphql/documents/data/config.graphql | 1 + graphql/schema/types/config.graphql | 4 + pkg/api/authentication.go | 138 ++++++++++++++++++ pkg/api/resolver_mutation_configure.go | 4 + pkg/api/resolver_query_configuration.go | 1 + pkg/api/server.go | 63 +------- pkg/manager/config/config.go | 43 ++++++ pkg/manager/manager.go | 8 + pkg/session/authentication.go | 117 +++++++++++++++ .../components/Changelog/versions/v0100.md | 3 + .../Settings/SettingsConfigurationPanel.tsx | 22 +++ ui/v2.5/src/locales/en-GB.json | 2 + 12 files changed, 344 insertions(+), 62 deletions(-) create mode 100644 pkg/api/authentication.go create mode 100644 pkg/session/authentication.go diff --git a/graphql/documents/data/config.graphql b/graphql/documents/data/config.graphql index 80f8f4caa..bb2e41d8a 100644 --- a/graphql/documents/data/config.graphql +++ b/graphql/documents/data/config.graphql @@ -24,6 +24,7 @@ fragment ConfigGeneralData on ConfigGeneralResult { username password maxSessionAge + trustedProxies logFile logOut logLevel diff --git a/graphql/schema/types/config.graphql b/graphql/schema/types/config.graphql index 6c5f10f59..475d4b272 100644 --- a/graphql/schema/types/config.graphql +++ b/graphql/schema/types/config.graphql @@ -73,6 +73,8 @@ input ConfigGeneralInput { password: String """Maximum session cookie age""" maxSessionAge: Int + """Comma separated list of proxies to allow traffic from""" + trustedProxies: [String!] """Name of the log file""" logFile: String """Whether to also output to stderr""" @@ -152,6 +154,8 @@ type ConfigGeneralResult { password: String! """Maximum session cookie age""" maxSessionAge: Int! + """Comma separated list of proxies to allow traffic from""" + trustedProxies: [String!]! """Name of the log file""" logFile: String """Whether to also output to stderr""" diff --git a/pkg/api/authentication.go b/pkg/api/authentication.go new file mode 100644 index 000000000..82ed921bb --- /dev/null +++ b/pkg/api/authentication.go @@ -0,0 +1,138 @@ +package api + +import ( + "net" + "net/http" + "net/url" + "strings" + + "github.com/stashapp/stash/pkg/logger" + "github.com/stashapp/stash/pkg/manager" + "github.com/stashapp/stash/pkg/manager/config" + "github.com/stashapp/stash/pkg/session" +) + +const loginEndPoint = "/login" + +const ( + tripwireActivatedErrMsg = "Stash is exposed to the public internet without authentication, and is not serving any more content to protect your privacy. " + + "More information and fixes are available at https://github.com/stashapp/stash/wiki/Authentication-Required-When-Accessing-Stash-From-the-Internet" + + externalAccessErrMsg = "You have attempted to access Stash over the internet, and authentication is not enabled. " + + "This is extremely dangerous! The whole world can see your your stash page and browse your files! " + + "Stash is not answering any other requests to protect your privacy. " + + "Please read the log entry or visit https://github.com/stashapp/stash/wiki/Authentication-Required-When-Accessing-Stash-From-the-Internet" +) + +func allowUnauthenticated(r *http.Request) bool { + return strings.HasPrefix(r.URL.Path, loginEndPoint) || r.URL.Path == "/css" +} + +func authenticateHandler() func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + c := config.GetInstance() + + if !checkSecurityTripwireActivated(c, w) { + return + } + + userID, err := manager.GetInstance().SessionStore.Authenticate(w, r) + if err != nil { + if err != session.ErrUnauthorized { + w.WriteHeader(http.StatusInternalServerError) + _, err = w.Write([]byte(err.Error())) + if err != nil { + logger.Error(err) + } + return + } + + // unauthorized error + w.Header().Add("WWW-Authenticate", `FormBased`) + w.WriteHeader(http.StatusUnauthorized) + return + } + + if err := session.CheckAllowPublicWithoutAuth(c, r); err != nil { + switch err := err.(type) { + case session.ExternalAccessError: + securityActivateTripwireAccessedFromInternetWithoutAuth(c, err, w) + return + case session.UntrustedProxyError: + logger.Warnf("Rejected request from untrusted proxy: %s", net.IP(err).String()) + w.WriteHeader(http.StatusForbidden) + return + default: + logger.Errorf("Error checking external access security: %s", err.Error()) + w.WriteHeader(http.StatusInternalServerError) + return + } + } + + ctx := r.Context() + + if c.HasCredentials() { + // authentication is required + if userID == "" && !allowUnauthenticated(r) { + // authentication was not received, redirect + // if graphql was requested, we just return a forbidden error + if r.URL.Path == "/graphql" { + w.Header().Add("WWW-Authenticate", `FormBased`) + w.WriteHeader(http.StatusUnauthorized) + return + } + + // otherwise redirect to the login page + u := url.URL{ + Path: "/login", + } + q := u.Query() + q.Set(returnURLParam, r.URL.Path) + u.RawQuery = q.Encode() + http.Redirect(w, r, u.String(), http.StatusFound) + return + } + } + + ctx = session.SetCurrentUserID(ctx, userID) + + r = r.WithContext(ctx) + + next.ServeHTTP(w, r) + }) + } +} + +func checkSecurityTripwireActivated(c *config.Instance, w http.ResponseWriter) bool { + if accessErr := session.CheckExternalAccessTripwire(c); accessErr != nil { + w.WriteHeader(http.StatusForbidden) + _, err := w.Write([]byte(tripwireActivatedErrMsg)) + if err != nil { + logger.Error(err) + } + return false + } + + return true +} + +func securityActivateTripwireAccessedFromInternetWithoutAuth(c *config.Instance, accessErr session.ExternalAccessError, w http.ResponseWriter) { + session.LogExternalAccessError(accessErr) + + err := c.ActivatePublicAccessTripwire(net.IP(accessErr).String()) + if err != nil { + logger.Error(err) + } + + w.WriteHeader(http.StatusForbidden) + _, err = w.Write([]byte(externalAccessErrMsg)) + if err != nil { + logger.Error(err) + } + + err = manager.GetInstance().Shutdown() + if err != nil { + logger.Error(err) + } +} diff --git a/pkg/api/resolver_mutation_configure.go b/pkg/api/resolver_mutation_configure.go index 659787916..6672a1993 100644 --- a/pkg/api/resolver_mutation_configure.go +++ b/pkg/api/resolver_mutation_configure.go @@ -146,6 +146,10 @@ func (r *mutationResolver) ConfigureGeneral(ctx context.Context, input models.Co c.Set(config.MaxSessionAge, *input.MaxSessionAge) } + if input.TrustedProxies != nil { + c.Set(config.TrustedProxies, input.TrustedProxies) + } + if input.LogFile != nil { c.Set(config.LogFile, input.LogFile) } diff --git a/pkg/api/resolver_query_configuration.go b/pkg/api/resolver_query_configuration.go index 21eb9adc1..283148113 100644 --- a/pkg/api/resolver_query_configuration.go +++ b/pkg/api/resolver_query_configuration.go @@ -79,6 +79,7 @@ func makeConfigGeneralResult() *models.ConfigGeneralResult { Username: config.GetUsername(), Password: config.GetPasswordHash(), MaxSessionAge: config.GetMaxSessionAge(), + TrustedProxies: config.GetTrustedProxies(), LogFile: &logFile, LogOut: config.GetLogOut(), LogLevel: config.GetLogLevel(), diff --git a/pkg/api/server.go b/pkg/api/server.go index c925a5c6e..bd654aa59 100644 --- a/pkg/api/server.go +++ b/pkg/api/server.go @@ -8,7 +8,6 @@ import ( "fmt" "io/fs" "net/http" - "net/url" "os" "path" "runtime/debug" @@ -29,7 +28,6 @@ import ( "github.com/stashapp/stash/pkg/manager" "github.com/stashapp/stash/pkg/manager/config" "github.com/stashapp/stash/pkg/models" - "github.com/stashapp/stash/pkg/session" "github.com/stashapp/stash/pkg/utils" ) @@ -37,65 +35,6 @@ var version string var buildstamp string var githash string -func allowUnauthenticated(r *http.Request) bool { - return strings.HasPrefix(r.URL.Path, "/login") || r.URL.Path == "/css" -} - -func authenticateHandler() func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - userID, err := manager.GetInstance().SessionStore.Authenticate(w, r) - if err != nil { - if err != session.ErrUnauthorized { - w.WriteHeader(http.StatusInternalServerError) - _, err = w.Write([]byte(err.Error())) - if err != nil { - logger.Error(err) - } - return - } - - // unauthorized error - w.Header().Add("WWW-Authenticate", `FormBased`) - w.WriteHeader(http.StatusUnauthorized) - return - } - - c := config.GetInstance() - ctx := r.Context() - - // handle redirect if no user and user is required - if userID == "" && c.HasCredentials() && !allowUnauthenticated(r) { - // if we don't have a userID, then redirect - // if graphql was requested, we just return a forbidden error - if r.URL.Path == "/graphql" { - w.Header().Add("WWW-Authenticate", `FormBased`) - w.WriteHeader(http.StatusUnauthorized) - return - } - - // otherwise redirect to the login page - u := url.URL{ - Path: "/login", - } - q := u.Query() - q.Set(returnURLParam, r.URL.Path) - u.RawQuery = q.Encode() - http.Redirect(w, r, u.String(), http.StatusFound) - return - } - - ctx = session.SetCurrentUserID(ctx, userID) - - r = r.WithContext(ctx) - - next.ServeHTTP(w, r) - }) - } -} - -const loginEndPoint = "/login" - func Start(uiBox embed.FS, loginUIBox embed.FS) { initialiseImages() @@ -274,7 +213,7 @@ func Start(uiBox embed.FS, loginUIBox embed.FS) { } uiRoot, err := fs.Sub(uiBox, uiRootDir) if err != nil { - panic(error.Error(err)) + panic(err) } http.FileServer(http.FS(uiRoot)).ServeHTTP(w, r) } diff --git a/pkg/manager/config/config.go b/pkg/manager/config/config.go index 8f658fdd3..92235394b 100644 --- a/pkg/manager/config/config.go +++ b/pkg/manager/config/config.go @@ -138,6 +138,13 @@ const SlideshowDelay = "slideshow_delay" const HandyKey = "handy_key" const FunscriptOffset = "funscript_offset" +// Security +const TrustedProxies = "trusted_proxies" +const dangerousAllowPublicWithoutAuth = "dangerous_allow_public_without_auth" +const dangerousAllowPublicWithoutAuthDefault = "false" +const SecurityTripwireAccessedFromPublicInternet = "security_tripwire_accessed_from_public_internet" +const securityTripwireAccessedFromPublicInternetDefault = "" + // DLNA options const DLNAServerName = "dlna.server_name" const DLNADefaultEnabled = "dlna.default_enabled" @@ -838,6 +845,31 @@ func (i *Instance) GetFunscriptOffset() int { return viper.GetInt(FunscriptOffset) } +// GetTrustedProxies returns a comma separated list of ip addresses that should allow proxying. +// When empty, allow from any private network +func (i *Instance) GetTrustedProxies() []string { + i.RLock() + defer i.RUnlock() + return viper.GetStringSlice(TrustedProxies) +} + +// GetDangerousAllowPublicWithoutAuth determines if the security feature is enabled. +// See https://github.com/stashapp/stash/wiki/Authentication-Required-When-Accessing-Stash-From-the-Internet +func (i *Instance) GetDangerousAllowPublicWithoutAuth() bool { + i.RLock() + defer i.RUnlock() + return viper.GetBool(dangerousAllowPublicWithoutAuth) +} + +// GetSecurityTripwireAccessedFromPublicInternet returns a public IP address if stash +// has been accessed from the public internet, with no auth enabled, and +// DangerousAllowPublicWithoutAuth disabled. Returns an empty string otherwise. +func (i *Instance) GetSecurityTripwireAccessedFromPublicInternet() string { + i.RLock() + defer i.RUnlock() + return viper.GetString(SecurityTripwireAccessedFromPublicInternet) +} + // GetDLNAServerName returns the visible name of the DLNA server. If empty, // "stash" will be used. func (i *Instance) GetDLNAServerName() string { @@ -930,6 +962,14 @@ func (i *Instance) GetMaxUploadSize() int64 { return ret << 20 } +// ActivatePublicAccessTripwire sets the security_tripwire_accessed_from_public_internet +// config field to the provided IP address to indicate that stash has been accessed +// from this public IP without authentication. +func (i *Instance) ActivatePublicAccessTripwire(requestIP string) error { + i.Set(SecurityTripwireAccessedFromPublicInternet, requestIP) + return i.Write() +} + func (i *Instance) Validate() error { i.RLock() defer i.RUnlock() @@ -982,6 +1022,9 @@ func (i *Instance) setDefaultValues(write bool) error { viper.SetDefault(Database, defaultDatabaseFilePath) + viper.SetDefault(dangerousAllowPublicWithoutAuth, dangerousAllowPublicWithoutAuthDefault) + viper.SetDefault(SecurityTripwireAccessedFromPublicInternet, securityTripwireAccessedFromPublicInternetDefault) + // Set generated to the metadata path for backwards compat viper.SetDefault(Generated, viper.GetString(Metadata)) diff --git a/pkg/manager/manager.go b/pkg/manager/manager.go index 64160ed94..03f0fe260 100644 --- a/pkg/manager/manager.go +++ b/pkg/manager/manager.go @@ -97,6 +97,8 @@ func Initialize() *singleton { panic(err) } } + + initSecurity(cfg) } else { cfgFile := cfg.GetConfigFile() if cfgFile != "" { @@ -125,6 +127,12 @@ func Initialize() *singleton { return instance } +func initSecurity(cfg *config.Instance) { + if err := session.CheckExternalAccessTripwire(cfg); err != nil { + session.LogExternalAccessError(*err) + } +} + func initProfiling(cpuProfilePath string) { if cpuProfilePath == "" { return diff --git a/pkg/session/authentication.go b/pkg/session/authentication.go new file mode 100644 index 000000000..90b2e54dd --- /dev/null +++ b/pkg/session/authentication.go @@ -0,0 +1,117 @@ +package session + +import ( + "fmt" + "net" + "net/http" + "strings" + + "github.com/stashapp/stash/pkg/logger" + "github.com/stashapp/stash/pkg/manager/config" +) + +type ExternalAccessError net.IP + +func (e ExternalAccessError) Error() string { + return fmt.Sprintf("stash accessed from external IP %s", net.IP(e).String()) +} + +type UntrustedProxyError net.IP + +func (e UntrustedProxyError) Error() string { + return fmt.Sprintf("untrusted proxy %s", net.IP(e).String()) +} + +func CheckAllowPublicWithoutAuth(c *config.Instance, r *http.Request) error { + if !c.HasCredentials() && !c.GetDangerousAllowPublicWithoutAuth() && !c.IsNewSystem() { + requestIPString, _, err := net.SplitHostPort(r.RemoteAddr) + if err != nil { + return fmt.Errorf("error parsing remote host (%s): %w", r.RemoteAddr, err) + } + + requestIP := net.ParseIP(requestIPString) + + if r.Header.Get("X-FORWARDED-FOR") != "" { + // Request was proxied + trustedProxies := c.GetTrustedProxies() + proxyChain := strings.Split(r.Header.Get("X-FORWARDED-FOR"), ", ") + + if trustedProxies == nil { + // validate proxies against local network only + if !isLocalIP(requestIP) { + return ExternalAccessError(requestIP) + } else { + // Safe to validate X-Forwarded-For + for i := range proxyChain { + ip := net.ParseIP(proxyChain[i]) + if !isLocalIP(ip) { + return ExternalAccessError(ip) + } + } + } + } else { + // validate proxies against trusted proxies list + if isIPTrustedProxy(requestIP, trustedProxies) { + // Safe to validate X-Forwarded-For + // validate backwards, as only the last one is not attacker-controlled + for i := len(proxyChain) - 1; i >= 0; i-- { + ip := net.ParseIP(proxyChain[i]) + if i == 0 { + // last entry is originating device, check if from the public internet + if !isLocalIP(ip) { + return ExternalAccessError(ip) + } + } else if !isIPTrustedProxy(ip, trustedProxies) { + return UntrustedProxyError(ip) + } + } + } else { + // Proxy not on safe proxy list + return UntrustedProxyError(requestIP) + } + } + } else { + // request was not proxied + if !isLocalIP(requestIP) { + return ExternalAccessError(requestIP) + } + } + } + + return nil +} + +func CheckExternalAccessTripwire(c *config.Instance) *ExternalAccessError { + if !c.HasCredentials() && !c.GetDangerousAllowPublicWithoutAuth() { + if remoteIP := c.GetSecurityTripwireAccessedFromPublicInternet(); remoteIP != "" { + err := ExternalAccessError(net.ParseIP(remoteIP)) + return &err + } + } + + return nil +} + +func isLocalIP(requestIP net.IP) bool { + _, cgNatAddrSpace, _ := net.ParseCIDR("100.64.0.0/10") + return requestIP.IsPrivate() || requestIP.IsLoopback() || cgNatAddrSpace.Contains(requestIP) +} + +func isIPTrustedProxy(ip net.IP, trustedProxies []string) bool { + for _, v := range trustedProxies { + if ip.Equal(net.ParseIP(v)) { + return true + } + } + return false +} + +func LogExternalAccessError(err ExternalAccessError) { + logger.Errorf("Stash has been accessed from the internet (public IP %s), without authentication. \n"+ + "This is extremely dangerous! The whole world can see your stash page and browse your files! \n"+ + "You probably forwarded a port from your router. At the very least, add a password to stash in the settings. \n"+ + "Stash will not serve requests until you edit config.yml, remove the security_tripwire_accessed_from_public_internet key and restart stash. \n"+ + "This behaviour can be overridden (but not recommended) by setting dangerous_allow_public_without_auth to true in config.yml. \n"+ + "More information is available at https://github.com/stashapp/stash/wiki/Authentication-Required-When-Accessing-Stash-From-the-Internet \n"+ + "Stash is not answering any other requests to protect your privacy.", net.IP(err).String()) +} diff --git a/ui/v2.5/src/components/Changelog/versions/v0100.md b/ui/v2.5/src/components/Changelog/versions/v0100.md index 684ad9832..c63eeb85b 100644 --- a/ui/v2.5/src/components/Changelog/versions/v0100.md +++ b/ui/v2.5/src/components/Changelog/versions/v0100.md @@ -1,6 +1,9 @@ #### 💥 Note: Please check your logs after migrating to this release. A log warning will be generated on startup if duplicate image checksums exist in your system. Search for the images using the logged checksums, and remove the unwanted ones. +#### 💥 Note: The system will now stop serving requests if authentication is not configured and it detects a connection from public internet. See [this link](https://github.com/stashapp/stash/wiki/Authentication-Required-When-Accessing-Stash-From-the-Internet) for details. + ### ✨ New Features +* Disallow access from public internet addresses when authentication is not configured. ([#1761](https://github.com/stashapp/stash/pull/1761)) * Revamped image lightbox to support zoom, pan and various display modes. ([#1708](https://github.com/stashapp/stash/pull/1708)) * Support subpaths when serving stash via reverse proxy. ([#1719](https://github.com/stashapp/stash/pull/1719)) * Added options to generate webp and static preview files for markers. ([#1604](https://github.com/stashapp/stash/pull/1604)) diff --git a/ui/v2.5/src/components/Settings/SettingsConfigurationPanel.tsx b/ui/v2.5/src/components/Settings/SettingsConfigurationPanel.tsx index be5e0a8a9..1c5b459d4 100644 --- a/ui/v2.5/src/components/Settings/SettingsConfigurationPanel.tsx +++ b/ui/v2.5/src/components/Settings/SettingsConfigurationPanel.tsx @@ -13,6 +13,7 @@ import StashBoxConfiguration, { IStashBoxInstance, } from "./StashBoxConfiguration"; import StashConfiguration from "./StashConfiguration"; +import { StringListInput } from "../Shared/StringListInput"; interface IExclusionPatternsProps { excludes: string[]; @@ -113,6 +114,9 @@ export const SettingsConfigurationPanel: React.FC = () => { const [username, setUsername] = useState(undefined); const [password, setPassword] = useState(undefined); const [maxSessionAge, setMaxSessionAge] = useState(0); + const [trustedProxies, setTrustedProxies] = useState( + undefined + ); const [logFile, setLogFile] = useState(); const [logOut, setLogOut] = useState(true); const [logLevel, setLogLevel] = useState("Info"); @@ -166,6 +170,7 @@ export const SettingsConfigurationPanel: React.FC = () => { username, password, maxSessionAge, + trustedProxies, logFile, logOut, logLevel, @@ -214,6 +219,7 @@ export const SettingsConfigurationPanel: React.FC = () => { setUsername(conf.general.username); setPassword(conf.general.password); setMaxSessionAge(conf.general.maxSessionAge); + setTrustedProxies(conf.general.trustedProxies ?? undefined); setLogFile(conf.general.logFile ?? undefined); setLogOut(conf.general.logOut); setLogLevel(conf.general.logLevel); @@ -1010,6 +1016,22 @@ export const SettingsConfigurationPanel: React.FC = () => { + +
+ {intl.formatMessage({ id: "config.general.auth.trusted_proxies" })} +
+ setTrustedProxies(value)} + defaultNewValue="" + /> + + {intl.formatMessage({ + id: "config.general.auth.trusted_proxies_desc", + })} + + +

{intl.formatMessage({ id: "config.general.logging" })}

diff --git a/ui/v2.5/src/locales/en-GB.json b/ui/v2.5/src/locales/en-GB.json index 9919b1630..5edfa4ac2 100644 --- a/ui/v2.5/src/locales/en-GB.json +++ b/ui/v2.5/src/locales/en-GB.json @@ -193,6 +193,8 @@ "password": "Password", "password_desc": "Password to access Stash. Leave blank to disable user authentication", "stash-box_integration": "Stash-box integration", + "trusted_proxies": "Trusted proxies", + "trusted_proxies_desc": "List of proxies that are allowed to proxy traffic into stash. Leave empty to allow from private network.", "username": "Username", "username_desc": "Username to access Stash. Leave blank to disable user authentication" },