mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-20 13:04:25 +02:00
s3: verify X-Forwarded-Prefix only from trusted proxies (#11843)
* s3api: check the direct peer against the trusted-proxy allowlist * s3: verify X-Forwarded-Prefix only from trusted proxies * s3: note that trusted proxies must sanitize forwarded headers
This commit is contained in:
1 parent
4e5fed9e9e
commit
0a96972a99
5 files changed
+118
-10
No files matched your search
@@ -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"
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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") {
|
||||
|
||||
Reference in new issue
Block a user