Github-Mirror/seaweedfs
1
0
Fork 0
mirror of https://github.com/seaweedfs/seaweedfs.git synced 2026-09-15 19:10:48 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(s3api): evaluate aws:SourceIp from the direct TCP peer, not forwarded headers (#11231)

Browse Source 
* fix(s3api): use direct peer IP for aws:SourceIp in bucket policy engine

extractSourceIP in the bucket-policy engine trusted X-Forwarded-For and
X-Real-Ip whenever the TCP peer looked private (loopback/RFC1918/link-local),
with no configurable trusted-proxy allowlist. In containerized deployments
the gateway peer is almost always private, so any caller reaching it directly
or from a co-located workload could spoof aws:SourceIp and bypass
IpAddress/NotIpAddress bucket-policy restrictions.

Always return the direct peer address (r.RemoteAddr), matching AWS S3
semantics. Remove the now-unused isPrivateIP helper and header-trust branch.

Update TestExtractConditionValuesFromRequestSourceIPPrecedence to assert the
peer IP is used regardless of forwarding headers, and add regression tests
TestExtractSourceIP_IgnoresForwardedHeaders and
TestExtractSourceIP_EnforcesIPRestrictionPolicy.

* fix(s3api): use direct peer IP for aws:SourceIp in IAM role/session policies

The IAM middleware's extractSourceIP trusted X-Forwarded-For and X-Real-IP
whenever the TCP peer looked private (loopback/RFC1918/link-local), with no
configurable trusted-proxy allowlist. In containerized deployments the gateway
peer is almost always private, so any caller reaching it directly or from a
co-located workload could spoof aws:SourceIp and bypass IpAddress/NotIpAddress
conditions on role and session policies (IsPrincipalActionExplicitlyDenied).

Always return the direct peer address (r.RemoteAddr), matching AWS S3
semantics. Remove the now-unused isPrivateIP helper, privateNetworks table,
and its init().

Update TestRequestContextExtraction and TestIPBasedPolicyEnforcement to assert
the peer IP is enforced regardless of forwarding headers, and add regression
test TestUserInlinePolicySourceIpCondition_IgnoresForwardedHeaders.
This commit is contained in:
Chris Lu
2026-09-08 15:09:31 -07:00
committed by GitHub
parent 557fffa350
commit b88156fe6b
5 changed files with 99 additions and 153 deletions
Download Patch File Download Diff File Expand all files Collapse all files
weed/s3api/policy_engine/engine.go
+5 -68
View File
@@ -519,23 +519,11 @@ func injectSSEForMultipart(conditions map[string][]string, inheritedSSE string)
return modified
}
// extractSourceIP returns the best-effort client IP address for condition evaluation.
// Preference order: X-Forwarded-For (first valid IP), X-Real-Ip, then RemoteAddr.
// IMPORTANT: X-Forwarded-For and X-Real-Ip are trusted without validation.
// When the service is exposed directly, clients can spoof aws:SourceIp unless a
// reverse proxy overwrites these headers.
// isPrivateIP returns true if the given IP is considered a "trusted proxy"
// address, such as loopback, link-local, or RFC1918 private ranges.
// isPrivateIP returns true if the given IP is considered a "trusted proxy"
// address, such as loopback, link-local, or private ranges.
func isPrivateIP(ip net.IP) bool {
if ip == nil {
return false
}
return ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsPrivate()
}
// extractSourceIP returns the direct TCP peer address for aws:SourceIp
// condition evaluation. Forwarding headers (X-Forwarded-For, X-Real-Ip) are
// intentionally ignored: without a configurable trusted-proxy allowlist they
// are client-controlled and spoofable, which would let a caller behind a
// private-looking peer bypass any aws:SourceIp restriction.
func extractSourceIP(r *http.Request) string {
if r == nil {
return ""
@@ -546,7 +534,6 @@ func extractSourceIP(r *http.Request) string {
return ""
}
// Fall back to unix socket markers or other non-IP placeholders.
if remoteAddr == "@" {
return remoteAddr
}
@@ -558,59 +545,9 @@ func extractSourceIP(r *http.Request) string {
remoteIP := net.ParseIP(host)
if remoteIP == nil {
// Do not return DNS names or unparseable values.
return ""
}
// Only trust forwarding headers when the connection appears to come from
// a trusted proxy (e.g., private/loopback address).
if isPrivateIP(remoteIP) {
if xff := r.Header.Get("X-Forwarded-For"); xff != "" {
// Iterate right-to-left to find the first non-trusted (public) IP
entries := strings.Split(xff, ",")
for i := len(entries) - 1; i >= 0; i-- {
candidate := strings.TrimSpace(entries[i])
if candidate == "" {
continue
}
ip := net.ParseIP(candidate)
if ip == nil {
continue
}
// If the IP is trusted/private, we treat it as another proxy in the chain and continue
if isPrivateIP(ip) {
continue
}
// Found a public/non-trusted IP, return it as the client IP
return ip.String()
}
// If we exhausted the list (all were private/trusted) or found no valid IPs,
// fallback related logic could go here.
// For now, if all are private, we continue to check X-Real-Ip or return RemoteIP?
// The prompt implies we should prefer the extracted IP.
// If all in XFF are private, likely the original client IS private (internal network).
// The best guess for "original client" in a fully trusted chain is the left-most valid IP.
for _, candidate := range entries {
candidate = strings.TrimSpace(candidate)
if ip := net.ParseIP(candidate); ip != nil {
return ip.String()
}
}
}
if xRealIP := strings.TrimSpace(r.Header.Get("X-Real-Ip")); xRealIP != "" {
if ip := net.ParseIP(xRealIP); ip != nil {
return ip.String()
}
}
}
// Default to the actual peer IP when no trusted proxy is detected or the
// forwarding headers are absent/invalid.
return remoteIP.String()
}
weed/s3api/policy_engine/engine_test.go
+57 -8
View File
@@ -2,6 +2,7 @@ package policy_engine
import (
"net/http"
"net/http/httptest"
"net/url"
"testing"
@@ -425,21 +426,21 @@ func TestExtractConditionValuesFromRequestSourceIPPrecedence(t *testing.T) {
expectedIP string
}{
{
name: "uses right-most public X-Forwarded-For entry",
name: "ignores X-Forwarded-For and uses RemoteAddr",
header: map[string][]string{
"X-Forwarded-For": {"bad-ip, 203.0.113.10, 198.51.100.5"},
},
remoteAddr: "192.168.1.100:12345",
expectedIP: "198.51.100.5",
expectedIP: "192.168.1.100",
},
{
name: "falls back to X-Real-Ip when X-Forwarded-For has no valid ip",
name: "ignores X-Real-Ip and uses RemoteAddr",
header: map[string][]string{
"X-Forwarded-For": {"bad-ip"},
"X-Real-Ip": {"198.51.100.7"},
},
remoteAddr: "192.168.1.100:12345",
expectedIP: "198.51.100.7",
expectedIP: "192.168.1.100",
},
{
name: "uses RemoteAddr ip when no forwarding headers",
@@ -454,20 +455,20 @@ func TestExtractConditionValuesFromRequestSourceIPPrecedence(t *testing.T) {
expectedIP: "@",
},
{
name: "uses IPv6 X-Forwarded-For entry",
name: "ignores IPv6 X-Forwarded-For entry and uses RemoteAddr",
header: map[string][]string{
"X-Forwarded-For": {"2001:db8::8, 198.51.100.7"},
},
remoteAddr: "192.168.1.100:12345",
expectedIP: "198.51.100.7",
expectedIP: "192.168.1.100",
},
{
name: "ignores spoofed IP when real client is public",
name: "ignores spoofed X-Forwarded-For behind private peer",
header: map[string][]string{
"X-Forwarded-For": {"8.8.8.8, 203.0.113.10, 10.0.0.1"},
},
remoteAddr: "192.168.1.100:12345",
expectedIP: "203.0.113.10",
expectedIP: "192.168.1.100",
},
{
name: "handles bracketed IPv6 remote address",
@@ -500,6 +501,54 @@ func TestExtractConditionValuesFromRequestSourceIPPrecedence(t *testing.T) {
}
}
func TestExtractSourceIP_IgnoresForwardedHeaders(t *testing.T) {
req := &http.Request{
Method: "GET",
URL: &url.URL{Path: "/"},
Header: map[string][]string{
"X-Forwarded-For": {"203.0.113.99"},
"X-Real-Ip": {"198.51.100.1"},
},
RemoteAddr: "10.0.0.5:54321",
}
values := ExtractConditionValuesFromRequest(req)
if got := values["aws:SourceIp"]; len(got) != 1 || got[0] != "10.0.0.5" {
t.Errorf("Expected SourceIp to be the direct peer 10.0.0.5, got %v", got)
}
}
func TestExtractSourceIP_EnforcesIPRestrictionPolicy(t *testing.T) {
engine := NewPolicyEngine()
policyJSON := `{
"Version": "2012-10-17",
"Statement": [
{"Effect": "Allow", "Principal": "*", "Action": ["s3:GetObject"],
"Resource": "arn:aws:s3:::secret-bucket/*"},
{"Effect": "Deny", "Principal": "*", "Action": ["s3:GetObject"],
"Resource": "arn:aws:s3:::secret-bucket/*",
"Condition": {"NotIpAddress": {"aws:SourceIp": ["10.0.0.0/24"]}}}
]
}`
if err := engine.SetBucketPolicy("secret-bucket", policyJSON); err != nil {
t.Fatalf("Failed to set bucket policy: %v", err)
}
r := httptest.NewRequest(http.MethodGet, "/secret-bucket/secret-key", nil)
r.RemoteAddr = "10.0.50.5:54321"
r.Header.Set("X-Forwarded-For", "10.0.0.5")
result := engine.EvaluatePolicy("secret-bucket", &PolicyEvaluationArgs{
Action: "s3:GetObject",
Resource: "arn:aws:s3:::secret-bucket/secret-key",
Principal: "*",
Conditions: ExtractConditionValuesFromRequest(r),
})
if result != PolicyResultDeny {
t.Errorf("Expected Deny for peer outside 10.0.0.0/24 despite spoofed X-Forwarded-For, got %v", result)
}
}
func TestPolicyEvaluationWithConditions(t *testing.T) {
engine := NewPolicyEngine()
weed/s3api/s3_iam_middleware.go
+5 -70
View File
@@ -17,26 +17,6 @@ import (
"github.com/seaweedfs/seaweedfs/weed/security"
)
// privateNetworks contains pre-parsed private IP ranges for efficient lookups
var privateNetworks []*net.IPNet
func init() {
// Private IPv4 ranges (RFC1918) and IPv6 Unique Local Addresses (ULA)
privateRanges := []string{
"10.0.0.0/8", // IPv4 private
"172.16.0.0/12", // IPv4 private
"192.168.0.0/16", // IPv4 private
"fc00::/7", // IPv6 Unique Local Addresses (ULA)
}
for _, cidr := range privateRanges {
_, network, err := net.ParseCIDR(cidr)
if err == nil {
privateNetworks = append(privateNetworks, network)
}
}
}
// IAMIntegration defines the interface for IAM integration
type IAMIntegration interface {
AuthenticateJWT(ctx context.Context, r *http.Request) (*IAMIdentity, s3err.ErrorCode)
@@ -423,64 +403,19 @@ func extractRequestContext(r *http.Request) map[string]interface{} {
return context
}
// extractSourceIP extracts the real source IP from the request
// SECURITY: Prioritizes RemoteAddr over client-controlled headers to prevent spoofing
// Only trusts X-Forwarded-For/X-Real-IP if RemoteAddr appears to be from a trusted proxy
// extractSourceIP returns the direct TCP peer address for aws:SourceIp
// condition evaluation. Forwarding headers (X-Forwarded-For, X-Real-IP) are
// intentionally ignored: without a configurable trusted-proxy allowlist they
// are client-controlled and spoofable, which would let a caller behind a
// private-looking peer bypass any aws:SourceIp restriction.
func extractSourceIP(r *http.Request) string {
// Always start with RemoteAddr as the most trustworthy source
remoteIP := r.RemoteAddr
if ip, _, err := net.SplitHostPort(remoteIP); err == nil {
remoteIP = ip
}
// NOTE: The current heuristic of using isPrivateIP assumes reverse proxies are on a
// private/local network. This may be insufficient for some cloud, CDN, or multi-tier
// proxy deployments where proxies terminate connections from public IPs. In such
// environments, deployment-specific controls (e.g., network ACLs or proxy configs)
// should be used to ensure only trusted components can set forwarding headers.
// Future enhancements may introduce an explicit, configurable trusted proxy CIDR list.
isTrustedProxy := isPrivateIP(remoteIP)
if isTrustedProxy {
// Check X-Real-IP header first (single IP, more reliable than X-Forwarded-For)
if realIP := r.Header.Get("X-Real-IP"); realIP != "" {
return strings.TrimSpace(realIP)
}
// Check X-Forwarded-For header (can contain multiple IPs, take the first one)
if forwardedFor := r.Header.Get("X-Forwarded-For"); forwardedFor != "" {
if ips := strings.Split(forwardedFor, ","); len(ips) > 0 {
return strings.TrimSpace(ips[0])
}
}
}
// Fall back to RemoteAddr (most secure)
return remoteIP
}
// isPrivateIP checks if an IP is in a private range (localhost or RFC1918)
func isPrivateIP(ipStr string) bool {
ip := net.ParseIP(ipStr)
if ip == nil {
return false
}
// Check for localhost and link-local addresses (IPv4/IPv6)
if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() {
return true
}
// Check against pre-parsed private CIDR ranges
for _, network := range privateNetworks {
if network.Contains(ip) {
return true
}
}
return false
}
// ParseUnverifiedJWTToken parses a JWT token and returns its claims WITHOUT cryptographic verification
//
// SECURITY WARNING: This function does NOT validate the token signature!
weed/s3api/s3_jwt_auth_test.go
+3 -7
View File
@@ -171,11 +171,10 @@ func TestRequestContextExtraction(t *testing.T) {
req := httptest.NewRequest("GET", "/test-bucket/test-file.txt", http.NoBody)
req.Header.Set("X-Forwarded-For", "192.168.1.100")
req.Header.Set("User-Agent", "aws-sdk-go/1.0")
// Set RemoteAddr to private IP to simulate trusted proxy
req.RemoteAddr = "127.0.0.1:12345"
return req
},
expectedIP: "192.168.1.100",
expectedIP: "127.0.0.1",
expectedUA: "aws-sdk-go/1.0",
},
{
@@ -184,11 +183,10 @@ func TestRequestContextExtraction(t *testing.T) {
req := httptest.NewRequest("GET", "/test-bucket/test-file.txt", http.NoBody)
req.Header.Set("X-Real-IP", "10.0.0.1")
req.Header.Set("User-Agent", "boto3/1.0")
// Set RemoteAddr to private IP to simulate trusted proxy
req.RemoteAddr = "127.0.0.1:12345"
return req
},
expectedIP: "10.0.0.1",
expectedIP: "127.0.0.1",
expectedUA: "boto3/1.0",
},
}
@@ -258,9 +256,7 @@ func TestIPBasedPolicyEnforcement(t *testing.T) {
// Create request with specific IP
req := httptest.NewRequest("GET", "/restricted-bucket/file.txt", http.NoBody)
req.Header.Set("Authorization", "Bearer "+response.Credentials.SessionToken)
req.Header.Set("X-Forwarded-For", tt.sourceIP)
// Set RemoteAddr to private IP to simulate trusted proxy
req.RemoteAddr = "127.0.0.1:12345"
req.RemoteAddr = tt.sourceIP + ":12345"
// Create IAM identity for testing
identity := &IAMIdentity{
weed/s3api/s3api_inline_policy_condition_test.go
+29
View File
@@ -123,6 +123,35 @@ func TestUserInlinePolicySourceIpCondition_Allows(t *testing.T) {
"PutObject from 127.0.0.1 must be allowed: the user inline policy's aws:SourceIp condition (127.0.0.0/8) matches")
}
// TestUserInlinePolicySourceIpCondition_IgnoresForwardedHeaders proves that a
// spoofed X-Forwarded-For cannot satisfy an aws:SourceIp condition: the policy
// requires 198.51.100.0/24, the peer is 127.0.0.1, and the header claims
// 198.51.100.5. The condition must be evaluated against the peer, not the
// header, so the request is denied.
func TestUserInlinePolicySourceIpCondition_IgnoresForwardedHeaders(t *testing.T) {
api := NewEmbeddedIamApiForTest()
api.mockConfig = &iam_pb.S3ApiConfiguration{}
seedInlineCondUser(t, api)
_, iamErr := api.PutUserPolicy(api.mockConfig, url.Values{
"UserName": {"alice"},
"PolicyName": {"OnlyFromTestNet"},
"PolicyDocument": {inlineCondPolicyDoc("198.51.100.0/24")},
})
require.Nil(t, iamErr, "PutUserPolicy must succeed")
require.NoError(t, api.PutS3ApiConfiguration(api.mockConfig))
require.NoError(t, api.iam.LoadS3ApiConfigurationFromCredentialManager())
ident := api.iam.lookupByIdentityName("alice")
require.NotNil(t, ident)
req := inlineCondRequest(t, http.MethodPut)
req.Header.Set("X-Forwarded-For", "198.51.100.5")
got := api.iam.VerifyActionPermission(req, ident, s3_constants.ACTION_WRITE, inlineCondTestBucket, "obj")
assert.Equal(t, s3err.ErrAccessDenied, got,
"PutObject from 127.0.0.1 must be denied despite X-Forwarded-For claiming 198.51.100.5: aws:SourceIp is the peer, not the header")
}
// TestGroupInlinePolicy_PutAndEnforce verifies that PutGroupPolicy is supported
// (no longer returns NotImplemented) and that an aws:SourceIp condition on the
// resulting inline policy is honored for group members.