diff --git a/weed/s3api/s3err/audit_fluent.go b/weed/s3api/s3err/audit_fluent.go index ad101cca2..69a8ad54e 100644 --- a/weed/s3api/s3err/audit_fluent.go +++ b/weed/s3api/s3err/audit_fluent.go @@ -3,8 +3,10 @@ package s3err import ( "encoding/json" "fmt" + "net" "net/http" "os" + "strings" "time" "github.com/fluent/fluent-logger-golang/fluent" @@ -82,6 +84,28 @@ func InitAuditLog(config string) { } } +// getRemoteIP returns the client IP for the audit log, honoring forwarding +// headers set by reverse proxies. Preference order: X-Forwarded-For (first +// non-empty entry), X-Real-IP, then r.RemoteAddr. Headers are trusted as-is; +// operators who expose the S3 endpoint directly to untrusted networks should +// strip these headers at the proxy boundary so clients cannot spoof them. +func getRemoteIP(r *http.Request) string { + if forwardedFor := r.Header.Get("X-Forwarded-For"); forwardedFor != "" { + for _, entry := range strings.Split(forwardedFor, ",") { + if ip := strings.TrimSpace(entry); ip != "" { + return ip + } + } + } + if realIP := strings.TrimSpace(r.Header.Get("X-Real-IP")); realIP != "" { + return realIP + } + if host, _, err := net.SplitHostPort(r.RemoteAddr); err == nil { + return host + } + return r.RemoteAddr +} + func getREST(httpMetod string, resourceType string) string { return fmt.Sprintf("REST.%s.%s", httpMetod, resourceType) } @@ -134,10 +158,7 @@ func GetAccessLog(r *http.Request, HTTPStatusCode int, s3errCode ErrorCode) *Acc if s3errCode != ErrNone { errorCode = GetAPIError(s3errCode).Code } - remoteIP := r.Header.Get("X-Real-IP") - if len(remoteIP) == 0 { - remoteIP = r.RemoteAddr - } + remoteIP := getRemoteIP(r) hostHeader := r.Header.Get("X-Forwarded-Host") if len(hostHeader) == 0 { hostHeader = r.Host diff --git a/weed/s3api/s3err/audit_fluent_test.go b/weed/s3api/s3err/audit_fluent_test.go index bfe2e788f..4d06ecdb4 100644 --- a/weed/s3api/s3err/audit_fluent_test.go +++ b/weed/s3api/s3err/audit_fluent_test.go @@ -17,3 +17,71 @@ func TestGetAccessLogUsesAmzRequestID(t *testing.T) { assert.Equal(t, "req-123", log.RequestID) } + +func TestGetAccessLogRemoteIP(t *testing.T) { + tests := []struct { + name string + remoteAddr string + xRealIP string + xForwardedFor string + expectedRemote string + }{ + { + name: "falls back to RemoteAddr (port stripped) when no headers set", + remoteAddr: "10.89.0.1:35832", + expectedRemote: "10.89.0.1", + }, + { + name: "preserves IPv6 host from RemoteAddr", + remoteAddr: "[2001:db8::1]:35832", + expectedRemote: "2001:db8::1", + }, + { + name: "returns RemoteAddr unchanged when no port present", + remoteAddr: "@", + expectedRemote: "@", + }, + { + name: "uses X-Real-IP when X-Forwarded-For is absent", + remoteAddr: "10.89.0.1:35832", + xRealIP: "203.0.113.7", + expectedRemote: "203.0.113.7", + }, + { + name: "prefers X-Forwarded-For over X-Real-IP", + remoteAddr: "10.89.0.1:35832", + xRealIP: "203.0.113.7", + xForwardedFor: "198.51.100.42", + expectedRemote: "198.51.100.42", + }, + { + name: "uses first hop in X-Forwarded-For chain", + remoteAddr: "10.89.0.1:35832", + xForwardedFor: "198.51.100.42, 10.0.0.5, 10.89.0.1", + expectedRemote: "198.51.100.42", + }, + { + name: "skips empty leading entries in X-Forwarded-For", + remoteAddr: "10.89.0.1:35832", + xForwardedFor: ", 198.51.100.42", + expectedRemote: "198.51.100.42", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/bucket/object", nil) + req.RemoteAddr = tc.remoteAddr + if tc.xRealIP != "" { + req.Header.Set("X-Real-IP", tc.xRealIP) + } + if tc.xForwardedFor != "" { + req.Header.Set("X-Forwarded-For", tc.xForwardedFor) + } + + log := GetAccessLog(req, http.StatusOK, ErrNone) + + assert.Equal(t, tc.expectedRemote, log.RemoteIP) + }) + } +}