s3api/policy_engine: use forwarded client IP for aws:SourceIp (#8304)

* s3api: honor forwarded source IP for policy conditions

Prefer X-Forwarded-For/X-Real-Ip before RemoteAddr when populating aws:SourceIp in policy condition evaluation. Also avoid noisy parsing behavior for unix socket markers and add coverage for precedence/fallback paths.\n\nFixes #8301.

* s3api: simplify remote addr parsing

* s3api: guard aws:SourceIp against DNS hosts

* s3api: simplify remote addr fallback

* s3api: simplify remote addr parsing

* Update weed/s3api/policy_engine/engine.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix TestExtractConditionValuesFromRequestSourceIPPrecedence using trusted private IP

* Refactor extractSourceIP to use R-to-L XFF parsing and net.IP.IsPrivate

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

weed/s3api/policy_engine/engine.go

@@ -381,15 +381,7 @@ func ExtractConditionValuesFromRequest(r *http.Request) map[string][]string {
 	values := make(map[string][]string)
 
 	// AWS condition keys
-	// Extract IP address without port for proper IP matching
-	host, _, err := net.SplitHostPort(r.RemoteAddr)
-	if err != nil {
-		// Log a warning if splitting fails
-		glog.Warningf("Failed to parse IP address from RemoteAddr %q: %v", r.RemoteAddr, err)
-		// If splitting fails, use the original RemoteAddr (might be just IP without port)
-		host = r.RemoteAddr
-	}
-	values["aws:SourceIp"] = []string{host}
+	values["aws:SourceIp"] = []string{extractSourceIP(r)}
 	values["aws:SecureTransport"] = []string{fmt.Sprintf("%t", r.TLS != nil)}
 	// Use AWS standard condition key for current time
 	values["aws:CurrentTime"] = []string{time.Now().Format(time.RFC3339)}
@@ -445,6 +437,101 @@ func ExtractConditionValuesFromRequest(r *http.Request) map[string][]string {
 	return values
 }
 
+// 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()
+}
+
+func extractSourceIP(r *http.Request) string {
+	if r == nil {
+		return ""
+	}
+
+	remoteAddr := strings.TrimSpace(r.RemoteAddr)
+	if remoteAddr == "" {
+		return ""
+	}
+
+	// Fall back to unix socket markers or other non-IP placeholders.
+	if remoteAddr == "@" {
+		return remoteAddr
+	}
+
+	host := remoteAddr
+	if h, _, err := net.SplitHostPort(remoteAddr); err == nil {
+		host = h
+	}
+
+	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()
+}
+
 // BuildResourceArn builds an ARN for the given bucket and object
 func BuildResourceArn(bucketName, objectName string) string {
 	if objectName == "" {

weed/s3api/policy_engine/engine_test.go

@@ -458,6 +458,89 @@ func TestExtractConditionValuesFromRequest(t *testing.T) {
 	}
 }
 
+func TestExtractConditionValuesFromRequestSourceIPPrecedence(t *testing.T) {
+	tests := []struct {
+		name       string
+		header     map[string][]string
+		remoteAddr string
+		expectedIP string
+	}{
+		{
+			name: "uses right-most public X-Forwarded-For entry",
+			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",
+		},
+		{
+			name: "falls back to X-Real-Ip when X-Forwarded-For has no valid ip",
+			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",
+		},
+		{
+			name:       "uses RemoteAddr ip when no forwarding headers",
+			header:     map[string][]string{},
+			remoteAddr: "192.168.1.100:12345",
+			expectedIP: "192.168.1.100",
+		},
+		{
+			name:       "keeps unix socket marker when RemoteAddr is not an ip",
+			header:     map[string][]string{},
+			remoteAddr: "@",
+			expectedIP: "@",
+		},
+		{
+			name: "uses IPv6 X-Forwarded-For entry",
+			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",
+		},
+		{
+			name: "ignores spoofed IP when real client is public",
+			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",
+		},
+		{
+			name:       "handles bracketed IPv6 remote address",
+			header:     map[string][]string{},
+			remoteAddr: "[2001:db8::1]:12345",
+			expectedIP: "2001:db8::1",
+		},
+		{
+			name:       "avoids returning DNS host names",
+			header:     map[string][]string{},
+			remoteAddr: "example.com:9000",
+			expectedIP: "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := &http.Request{
+				Method:     "GET",
+				URL:        &url.URL{Path: "/"},
+				Header:     tt.header,
+				RemoteAddr: tt.remoteAddr,
+			}
+
+			values := ExtractConditionValuesFromRequest(req)
+			if len(values["aws:SourceIp"]) != 1 || values["aws:SourceIp"][0] != tt.expectedIP {
+				t.Errorf("Expected SourceIp %q, got %v", tt.expectedIP, values["aws:SourceIp"])
+			}
+		})
+	}
+}
+
 func TestPolicyEvaluationWithConditions(t *testing.T) {
 	engine := NewPolicyEngine()