Remove trusted proxies (#2229)

This commit is contained in:
kermieisinthehouse
2022-02-02 15:16:22 -08:00
committed by GitHub
parent a3c20ce8da
commit def9ad88b0
12 changed files with 14 additions and 120 deletions

View File

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

View File

@@ -76,7 +76,7 @@ input ConfigGeneralInput {
"""Maximum session cookie age""" """Maximum session cookie age"""
maxSessionAge: Int maxSessionAge: Int
"""Comma separated list of proxies to allow traffic from""" """Comma separated list of proxies to allow traffic from"""
trustedProxies: [String!] trustedProxies: [String!] @deprecated(reason: "no longer supported")
"""Name of the log file""" """Name of the log file"""
logFile: String logFile: String
"""Whether to also output to stderr""" """Whether to also output to stderr"""
@@ -157,7 +157,7 @@ type ConfigGeneralResult {
"""Maximum session cookie age""" """Maximum session cookie age"""
maxSessionAge: Int! maxSessionAge: Int!
"""Comma separated list of proxies to allow traffic from""" """Comma separated list of proxies to allow traffic from"""
trustedProxies: [String!]! trustedProxies: [String!] @deprecated(reason: "no longer supported")
"""Name of the log file""" """Name of the log file"""
logFile: String logFile: String
"""Whether to also output to stderr""" """Whether to also output to stderr"""

View File

@@ -57,15 +57,10 @@ func authenticateHandler() func(http.Handler) http.Handler {
if err := session.CheckAllowPublicWithoutAuth(c, r); err != nil { if err := session.CheckAllowPublicWithoutAuth(c, r); err != nil {
var externalAccess session.ExternalAccessError var externalAccess session.ExternalAccessError
var untrustedProxy session.UntrustedProxyError
switch { switch {
case errors.As(err, &externalAccess): case errors.As(err, &externalAccess):
securityActivateTripwireAccessedFromInternetWithoutAuth(c, externalAccess, w) securityActivateTripwireAccessedFromInternetWithoutAuth(c, externalAccess, w)
return return
case errors.As(err, &untrustedProxy):
logger.Warnf("Rejected request from untrusted proxy: %v", net.IP(untrustedProxy))
w.WriteHeader(http.StatusForbidden)
return
default: default:
logger.Errorf("Error checking external access security: %v", err) logger.Errorf("Error checking external access security: %v", err)
w.WriteHeader(http.StatusInternalServerError) w.WriteHeader(http.StatusInternalServerError)

View File

@@ -197,10 +197,6 @@ func (r *mutationResolver) ConfigureGeneral(ctx context.Context, input models.Co
c.Set(config.MaxSessionAge, *input.MaxSessionAge) c.Set(config.MaxSessionAge, *input.MaxSessionAge)
} }
if input.TrustedProxies != nil {
c.Set(config.TrustedProxies, input.TrustedProxies)
}
if input.LogFile != nil { if input.LogFile != nil {
c.Set(config.LogFile, input.LogFile) c.Set(config.LogFile, input.LogFile)
} }

View File

@@ -86,7 +86,6 @@ func makeConfigGeneralResult() *models.ConfigGeneralResult {
Username: config.GetUsername(), Username: config.GetUsername(),
Password: config.GetPasswordHash(), Password: config.GetPasswordHash(),
MaxSessionAge: config.GetMaxSessionAge(), MaxSessionAge: config.GetMaxSessionAge(),
TrustedProxies: config.GetTrustedProxies(),
LogFile: &logFile, LogFile: &logFile,
LogOut: config.GetLogOut(), LogOut: config.GetLogOut(),
LogLevel: config.GetLogLevel(), LogLevel: config.GetLogLevel(),

View File

@@ -147,7 +147,6 @@ const (
FunscriptOffset = "funscript_offset" FunscriptOffset = "funscript_offset"
// Security // Security
TrustedProxies = "trusted_proxies"
dangerousAllowPublicWithoutAuth = "dangerous_allow_public_without_auth" dangerousAllowPublicWithoutAuth = "dangerous_allow_public_without_auth"
dangerousAllowPublicWithoutAuthDefault = "false" dangerousAllowPublicWithoutAuthDefault = "false"
SecurityTripwireAccessedFromPublicInternet = "security_tripwire_accessed_from_public_internet" SecurityTripwireAccessedFromPublicInternet = "security_tripwire_accessed_from_public_internet"
@@ -1014,12 +1013,6 @@ func (i *Instance) GetDefaultGenerateSettings() *models.GenerateMetadataOptions
return nil return nil
} }
// 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 {
return i.getStringSlice(TrustedProxies)
}
// GetDangerousAllowPublicWithoutAuth determines if the security feature is enabled. // GetDangerousAllowPublicWithoutAuth determines if the security feature is enabled.
// See https://github.com/stashapp/stash/wiki/Authentication-Required-When-Accessing-Stash-From-the-Internet // See https://github.com/stashapp/stash/wiki/Authentication-Required-When-Accessing-Stash-From-the-Internet
func (i *Instance) GetDangerousAllowPublicWithoutAuth() bool { func (i *Instance) GetDangerousAllowPublicWithoutAuth() bool {

View File

@@ -99,7 +99,6 @@ func TestConcurrentConfigAccess(t *testing.T) {
i.Set(DefaultIdentifySettings, i.GetDefaultIdentifySettings()) i.Set(DefaultIdentifySettings, i.GetDefaultIdentifySettings())
i.Set(DeleteGeneratedDefault, i.GetDeleteGeneratedDefault()) i.Set(DeleteGeneratedDefault, i.GetDeleteGeneratedDefault())
i.Set(DeleteFileDefault, i.GetDeleteFileDefault()) i.Set(DeleteFileDefault, i.GetDeleteFileDefault())
i.Set(TrustedProxies, i.GetTrustedProxies())
i.Set(dangerousAllowPublicWithoutAuth, i.GetDangerousAllowPublicWithoutAuth()) i.Set(dangerousAllowPublicWithoutAuth, i.GetDangerousAllowPublicWithoutAuth())
i.Set(SecurityTripwireAccessedFromPublicInternet, i.GetSecurityTripwireAccessedFromPublicInternet()) i.Set(SecurityTripwireAccessedFromPublicInternet, i.GetSecurityTripwireAccessedFromPublicInternet())
i.Set(DisableDropdownCreatePerformer, i.GetDisableDropdownCreate().Performer) i.Set(DisableDropdownCreatePerformer, i.GetDisableDropdownCreate().Performer)

View File

@@ -16,12 +16,6 @@ func (e ExternalAccessError) Error() string {
return fmt.Sprintf("stash accessed from external IP %s", net.IP(e).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 { func CheckAllowPublicWithoutAuth(c *config.Instance, r *http.Request) error {
if !c.HasCredentials() && !c.GetDangerousAllowPublicWithoutAuth() && !c.IsNewSystem() { if !c.HasCredentials() && !c.GetDangerousAllowPublicWithoutAuth() && !c.IsNewSystem() {
requestIPString, _, err := net.SplitHostPort(r.RemoteAddr) requestIPString, _, err := net.SplitHostPort(r.RemoteAddr)
@@ -42,43 +36,21 @@ func CheckAllowPublicWithoutAuth(c *config.Instance, r *http.Request) error {
if r.Header.Get("X-FORWARDED-FOR") != "" { if r.Header.Get("X-FORWARDED-FOR") != "" {
// Request was proxied // Request was proxied
trustedProxies := c.GetTrustedProxies()
proxyChain := strings.Split(r.Header.Get("X-FORWARDED-FOR"), ", ") proxyChain := strings.Split(r.Header.Get("X-FORWARDED-FOR"), ", ")
if len(trustedProxies) == 0 { // validate proxies against local network only
// validate proxies against local network only if !isLocalIP(requestIP) {
if !isLocalIP(requestIP) { return ExternalAccessError(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 { } else {
// validate proxies against trusted proxies list // Safe to validate X-Forwarded-For
if isIPTrustedProxy(requestIP, trustedProxies) { for i := range proxyChain {
// Safe to validate X-Forwarded-For ip := net.ParseIP(proxyChain[i])
// validate backwards, as only the last one is not attacker-controlled if !isLocalIP(ip) {
for i := len(proxyChain) - 1; i >= 0; i-- { return ExternalAccessError(ip)
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 if !isLocalIP(requestIP) { // request was not proxied } else if !isLocalIP(requestIP) { // request was not proxied
return ExternalAccessError(requestIP) return ExternalAccessError(requestIP)
} }
@@ -104,18 +76,6 @@ func isLocalIP(requestIP net.IP) bool {
return requestIP.IsPrivate() || requestIP.IsLoopback() || requestIP.IsLinkLocalUnicast() || cgNatAddrSpace.Contains(requestIP) return requestIP.IsPrivate() || requestIP.IsLoopback() || requestIP.IsLinkLocalUnicast() || cgNatAddrSpace.Contains(requestIP)
} }
func isIPTrustedProxy(ip net.IP, trustedProxies []string) bool {
if len(trustedProxies) == 0 {
return isLocalIP(ip)
}
for _, v := range trustedProxies {
if ip.Equal(net.ParseIP(v)) {
return true
}
}
return false
}
func LogExternalAccessError(err ExternalAccessError) { func LogExternalAccessError(err ExternalAccessError) {
logger.Errorf("Stash has been accessed from the internet (public IP %s), without authentication. \n"+ 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"+ "This is extremely dangerous! The whole world can see your stash page and browse your files! \n"+

View File

@@ -66,7 +66,7 @@ func TestCheckAllowPublicWithoutAuth(t *testing.T) {
} }
{ {
// X-FORWARDED-FOR without trusted proxy // X-FORWARDED-FOR
testCases := []struct { testCases := []struct {
proxyChain string proxyChain string
err error err error
@@ -91,39 +91,6 @@ func TestCheckAllowPublicWithoutAuth(t *testing.T) {
} }
} }
{
// X-FORWARDED-FOR with trusted proxy
var trustedProxies = []string{"8.8.8.8", "4.4.4.4"}
c.Set(config.TrustedProxies, trustedProxies)
testCases := []struct {
address string
proxyChain string
err error
}{
{"192.168.1.1:8080", "192.168.1.1, 192.168.1.2, 100.64.0.1, 127.0.0.1", &UntrustedProxyError{}},
{"8.8.8.8:8080", "192.168.1.2, 127.0.0.1", &UntrustedProxyError{}},
{"8.8.8.8:8080", "193.168.1.1, 4.4.4.4", &ExternalAccessError{}},
{"8.8.8.8:8080", "4.4.4.4", &ExternalAccessError{}},
{"8.8.8.8:8080", "192.168.1.1, 4.4.4.4a", &UntrustedProxyError{}},
{"8.8.8.8:8080", "192.168.1.1a, 4.4.4.4", &ExternalAccessError{}},
{"8.8.8.8:8080", "192.168.1.1, 4.4.4.4", nil},
{"8.8.8.8:8080", "192.168.1.1", nil},
}
header := make(http.Header)
for i, tc := range testCases {
header.Set("X-FORWARDED-FOR", tc.proxyChain)
r := &http.Request{
RemoteAddr: tc.address,
Header: header,
}
doTest(i, r, tc.err)
}
}
{ {
// test invalid request IPs // test invalid request IPs
invalidIPs := []string{"192.168.1.a:9999", "192.168.1.1"} invalidIPs := []string{"192.168.1.a:9999", "192.168.1.1"}
@@ -134,11 +101,6 @@ func TestCheckAllowPublicWithoutAuth(t *testing.T) {
} }
err := CheckAllowPublicWithoutAuth(c, r) err := CheckAllowPublicWithoutAuth(c, r)
if errors.As(err, &UntrustedProxyError{}) || errors.As(err, &ExternalAccessError{}) {
t.Errorf("[%s]: unexpected error: %v", remoteAddr, err)
continue
}
if err == nil { if err == nil {
t.Errorf("[%s]: expected error", remoteAddr) t.Errorf("[%s]: expected error", remoteAddr)
continue continue

View File

@@ -8,6 +8,7 @@
* Show counts on list tabs in Performer, Studio and Tag pages. ([#2169](https://github.com/stashapp/stash/pull/2169)) * Show counts on list tabs in Performer, Studio and Tag pages. ([#2169](https://github.com/stashapp/stash/pull/2169))
### 🐛 Bug fixes ### 🐛 Bug fixes
* Removed trusted proxies setting. ([#2229](https://github.com/stashapp/stash/pull/2229))
* Allow Stash to be iframed. ([#2217](https://github.com/stashapp/stash/pull/2217)) * Allow Stash to be iframed. ([#2217](https://github.com/stashapp/stash/pull/2217))
* Resolve CDP hostname if necessary. ([#2174](https://github.com/stashapp/stash/pull/2174)) * Resolve CDP hostname if necessary. ([#2174](https://github.com/stashapp/stash/pull/2174))
* Generate sprites for short video files. ([#2167](https://github.com/stashapp/stash/pull/2167)) * Generate sprites for short video files. ([#2167](https://github.com/stashapp/stash/pull/2167))

View File

@@ -1,5 +1,5 @@
import React from "react"; import React from "react";
import { ModalSetting, NumberSetting, StringListSetting } from "./Inputs"; import { ModalSetting, NumberSetting } from "./Inputs";
import { SettingSection } from "./SettingSection"; import { SettingSection } from "./SettingSection";
import * as GQL from "src/core/generated-graphql"; import * as GQL from "src/core/generated-graphql";
import { Button, Form } from "react-bootstrap"; import { Button, Form } from "react-bootstrap";
@@ -162,14 +162,6 @@ export const SettingsSecurityPanel: React.FC = () => {
value={general.maxSessionAge ?? undefined} value={general.maxSessionAge ?? undefined}
onChange={(v) => saveGeneral({ maxSessionAge: v })} onChange={(v) => saveGeneral({ maxSessionAge: v })}
/> />
<StringListSetting
id="trusted-proxies"
headingID="config.general.auth.trusted_proxies"
subHeadingID="config.general.auth.trusted_proxies_desc"
value={general.trustedProxies ?? undefined}
onChange={(v) => saveGeneral({ trustedProxies: v })}
/>
</SettingSection> </SettingSection>
</> </>
); );

View File

@@ -221,8 +221,6 @@
"password": "Password", "password": "Password",
"password_desc": "Password to access Stash. Leave blank to disable user authentication", "password_desc": "Password to access Stash. Leave blank to disable user authentication",
"stash-box_integration": "Stash-box integration", "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": "Username",
"username_desc": "Username to access Stash. Leave blank to disable user authentication" "username_desc": "Username to access Stash. Leave blank to disable user authentication"
}, },