diff --git a/weed/command/scaffold/security.toml b/weed/command/scaffold/security.toml index e5019b36c2..efa49cce02 100644 --- a/weed/command/scaffold/security.toml +++ b/weed/command/scaffold/security.toml @@ -232,11 +232,14 @@ kek = "" # Cannot be used while /etc/s3/sse_kek exists on the filer — delete it first. key = "" -# Trusted reverse proxies allowed to set X-Forwarded-For / X-Real-Ip for -# aws:SourceIp condition evaluation in S3 bucket and IAM policies. When the -# direct TCP peer is in this list, forwarded headers are honored; otherwise -# the direct peer address is used (matching AWS S3 semantics). Leave empty to -# always use the direct peer, which is the safe default. +# Trusted reverse proxies allowed to set forwarding headers on behalf of the +# real client: X-Forwarded-For / X-Real-Ip for aws:SourceIp condition +# evaluation in S3 bucket and IAM policies, and X-Forwarded-Prefix for SigV4 +# signature verification of prefix-stripped paths. When the direct TCP peer is +# in this list, these headers are honored; otherwise they are ignored. A +# trusted proxy must overwrite or strip client-supplied values of these +# headers, since honoring them means trusting them. Leave empty to always use +# the direct peer, which is the safe default. # Can also be set via env var: WEED_S3_TRUSTED_PROXIES_WHITE_LIST [s3.trusted_proxies] white_list = "" # comma separated; bare IPs or CIDRs, e.g. "10.10.10.0/24" diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index e46cb679a3..0dc5731da7 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -364,7 +364,15 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh pathForSignature = r.URL.Path } } - forwardedPrefix := r.Header.Get("X-Forwarded-Prefix") + // The signature is checked against prefix+path but the request is routed + // and authorized on path alone, so only a trusted proxy may set the prefix. + iam.m.RLock() + trustedPeer := iam.trustedProxies.IsTrustedPeer(r) + iam.m.RUnlock() + forwardedPrefix := "" + if trustedPeer { + forwardedPrefix = r.Header.Get("X-Forwarded-Prefix") + } var matchedHost string for i, hostCandidate := range extractHostHeaderCandidates(r, iam.externalHost) { if i > 0 && !replaceSignedHostHeader(extractedSignedHeaders, hostCandidate) { diff --git a/weed/s3api/auto_signature_v4_test.go b/weed/s3api/auto_signature_v4_test.go index 849befe4c9..e7747515d6 100644 --- a/weed/s3api/auto_signature_v4_test.go +++ b/weed/s3api/auto_signature_v4_test.go @@ -18,6 +18,7 @@ import ( "github.com/gorilla/mux" "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" @@ -302,6 +303,14 @@ func newTestIAM() *IdentityAccessManagement { return iam } +// newTestIAMWithTrustedProxy returns a test IAM that trusts the proxy peer +// (10.0.0.1) used by the forwarded-header tests. +func newTestIAMWithTrustedProxy() *IdentityAccessManagement { + iam := newTestIAM() + iam.SetTrustedProxies(policy_engine.NewTrustedProxies([]string{"10.0.0.1"})) + return iam +} + // Test X-Forwarded-Prefix support for reverse proxy scenarios func TestSignatureV4WithForwardedPrefix(t *testing.T) { tests := []struct { @@ -323,7 +332,7 @@ func TestSignatureV4WithForwardedPrefix(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - iam := newTestIAM() + iam := newTestIAMWithTrustedProxy() // Create a request with X-Forwarded-Prefix header r, err := newTestRequest("GET", "https://example.com/test-bucket/test-object", 0, nil) @@ -340,6 +349,7 @@ func TestSignatureV4WithForwardedPrefix(t *testing.T) { r.Header.Set("X-Forwarded-Prefix", tt.forwardedPrefix) r.Header.Set("Host", "example.com") r.Header.Set("X-Forwarded-Host", "example.com") + r.RemoteAddr = "10.0.0.1:1234" // Sign the request with the expected normalized path signV4WithPath(r, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", tt.expectedPath) @@ -391,7 +401,7 @@ func TestSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - iam := newTestIAM() + iam := newTestIAMWithTrustedProxy() // Create a request with the URL path that has a trailing slash r, err := newTestRequest("GET", "https://example.com"+tt.urlPath, 0, nil) @@ -405,6 +415,7 @@ func TestSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { r.Header.Set("X-Forwarded-Prefix", tt.forwardedPrefix) r.Header.Set("Host", "example.com") r.Header.Set("X-Forwarded-Host", "example.com") + r.RemoteAddr = "10.0.0.1:1234" // Sign the request with the full path including the trailing slash // This simulates what S3 SDK does for listObjects operations @@ -826,7 +837,7 @@ func TestPresignedSignatureV4WithForwardedPrefix(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - iam := newTestIAM() + iam := newTestIAMWithTrustedProxy() // Create a presigned request that simulates reverse proxy scenario: // 1. Client generates presigned URL with prefixed path @@ -859,6 +870,7 @@ func TestPresignedSignatureV4WithForwardedPrefix(t *testing.T) { r.Header.Set("X-Forwarded-Prefix", tt.forwardedPrefix) r.Header.Set("Host", "example.com") r.Header.Set("X-Forwarded-Host", "example.com") + r.RemoteAddr = "10.0.0.1:1234" // Test presigned signature verification _, _, errCode := iam.doesPresignedSignatureMatch(r) @@ -900,7 +912,7 @@ func TestPresignedSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - iam := newTestIAM() + iam := newTestIAMWithTrustedProxy() // Create a presigned request that simulates reverse proxy scenario with trailing slashes: // 1. Client generates presigned URL with prefixed path including trailing slash @@ -927,6 +939,7 @@ func TestPresignedSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { r.Header.Set("X-Forwarded-Prefix", tt.forwardedPrefix) r.Header.Set("Host", "example.com") r.Header.Set("X-Forwarded-Host", "example.com") + r.RemoteAddr = "10.0.0.1:1234" // Test presigned signature verification - this should succeed with trailing slashes _, _, errCode := iam.doesPresignedSignatureMatch(r) @@ -938,6 +951,53 @@ func TestPresignedSignatureV4WithForwardedPrefixTrailingSlash(t *testing.T) { } } +// A presigned URL holder may replay the signed query against a different +// request path and claim the skipped part via X-Forwarded-Prefix. The header +// is only meaningful when the direct peer is a configured trusted proxy. +func TestPresignedSignatureV4ForwardedPrefixPeerTrust(t *testing.T) { + const proxyIP = "10.0.0.1" + + newRetargetedRequest := func(t *testing.T) *http.Request { + // Sign over the full external path, as a client of the reverse proxy would. + r, err := newTestRequest("GET", "https://example.com/shared/tenanta/report.txt", 0, nil) + if err != nil { + t.Fatalf("Failed to create test request: %v", err) + } + if err = preSignV4WithPath(newTestIAM(), r, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", 3600, "/shared/tenanta/report.txt"); err != nil { + t.Fatalf("Failed to presign request: %v", err) + } + + // Replay the same query string at a suffix path, supplying the + // skipped segment as the forwarded prefix. + r.URL.Path = "/tenanta/report.txt" + r = mux.SetURLVars(r, map[string]string{ + "bucket": "tenanta", + "object": "report.txt", + }) + r.Header.Set("X-Forwarded-Prefix", "/shared") + r.Header.Set("Host", "example.com") + r.RemoteAddr = proxyIP + ":54321" + return r + } + + t.Run("untrusted peer", func(t *testing.T) { + iam := newTestIAM() + _, _, errCode := iam.doesPresignedSignatureMatch(newRetargetedRequest(t)) + if errCode != s3err.ErrSignatureDoesNotMatch { + t.Errorf("Expected ErrSignatureDoesNotMatch for untrusted peer, got: %v (code: %d)", errCode, int(errCode)) + } + }) + + t.Run("trusted proxy", func(t *testing.T) { + iam := newTestIAM() + iam.SetTrustedProxies(policy_engine.NewTrustedProxies([]string{proxyIP})) + _, _, errCode := iam.doesPresignedSignatureMatch(newRetargetedRequest(t)) + if errCode != s3err.ErrNone { + t.Errorf("Expected successful presigned signature validation from trusted proxy, got: %v (code: %d)", errCode, int(errCode)) + } + }) +} + // preSignV4WithPath adds presigned URL parameters to the request with a custom path func preSignV4WithPath(iam *IdentityAccessManagement, req *http.Request, accessKey, secretKey string, expires int64, urlPath string) error { // Create credential scope diff --git a/weed/s3api/policy_engine/trusted_proxies.go b/weed/s3api/policy_engine/trusted_proxies.go index 9aa35f4519..e2495f2deb 100644 --- a/weed/s3api/policy_engine/trusted_proxies.go +++ b/weed/s3api/policy_engine/trusted_proxies.go @@ -42,6 +42,20 @@ func NewTrustedProxies(whiteList []string) *TrustedProxies { return tp } +// IsTrustedPeer reports whether the direct TCP peer of the request is in the +// allowlist. Use it to decide whether forwarding headers a proxy may have +// added (X-Forwarded-*, X-Real-*) can be honored for this request. +func (tp *TrustedProxies) IsTrustedPeer(r *http.Request) bool { + if r == nil { + return false + } + host := strings.TrimSpace(r.RemoteAddr) + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + return tp.IsTrusted(host) +} + func (tp *TrustedProxies) IsTrusted(ipStr string) bool { if tp == nil { return false diff --git a/weed/s3api/policy_engine/trusted_proxies_test.go b/weed/s3api/policy_engine/trusted_proxies_test.go index 7f0cfefb88..0af0f6861d 100644 --- a/weed/s3api/policy_engine/trusted_proxies_test.go +++ b/weed/s3api/policy_engine/trusted_proxies_test.go @@ -49,6 +49,29 @@ func TestTrustedProxies_InvalidBareIPSkipped(t *testing.T) { } } +func TestTrustedProxies_IsTrustedPeer(t *testing.T) { + tp := NewTrustedProxies([]string{"10.0.0.0/24"}) + if !tp.IsTrustedPeer(newReq("10.0.0.5:1234", "", "")) { + t.Error("peer in allowlist should be trusted") + } + if !tp.IsTrustedPeer(newReq("10.0.0.5:1234", "8.8.8.8", "")) { + t.Error("forwarded headers must not affect peer trust") + } + if tp.IsTrustedPeer(newReq("8.8.8.8:1234", "", "")) { + t.Error("peer outside allowlist should not be trusted") + } + if tp.IsTrustedPeer(newReq("not-an-addr", "", "")) { + t.Error("malformed RemoteAddr should not be trusted") + } + if tp.IsTrustedPeer(nil) { + t.Error("nil request should not be trusted") + } + var nilTP *TrustedProxies + if nilTP.IsTrustedPeer(newReq("10.0.0.5:1234", "", "")) { + t.Error("nil TrustedProxies should not trust any peer") + } +} + func TestTrustedProxies_NilNotTrusted(t *testing.T) { var tp *TrustedProxies if tp.IsTrusted("127.0.0.1") {