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

@@ -24,6 +24,7 @@ fragment ConfigGeneralData on ConfigGeneralResult {
   username
   password
   maxSessionAge
+  trustedProxies
   logFile
   logOut
   logLevel

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"""

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)
+	}
+}

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)
 	}

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(),

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)
 		}

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))
 

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

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())
+}

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))

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<string | undefined>(undefined);
   const [password, setPassword] = useState<string | undefined>(undefined);
   const [maxSessionAge, setMaxSessionAge] = useState<number>(0);
+  const [trustedProxies, setTrustedProxies] = useState<string[] | undefined>(
+    undefined
+  );
   const [logFile, setLogFile] = useState<string | undefined>();
   const [logOut, setLogOut] = useState<boolean>(true);
   const [logLevel, setLogLevel] = useState<string>("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 = () => {
         </Form.Group>
       </Form.Group>
 
+      <Form.Group id="trusted-proxies">
+        <h6>
+          {intl.formatMessage({ id: "config.general.auth.trusted_proxies" })}
+        </h6>
+        <StringListInput
+          value={trustedProxies ?? []}
+          setValue={(value) => setTrustedProxies(value)}
+          defaultNewValue=""
+        />
+        <Form.Text className="text-muted">
+          {intl.formatMessage({
+            id: "config.general.auth.trusted_proxies_desc",
+          })}
+        </Form.Text>
+      </Form.Group>
+
       <hr />
 
       <h4>{intl.formatMessage({ id: "config.general.logging" })}</h4>

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"
       },