diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index b8d611ce4e..8a0491e7b1 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -1670,6 +1670,7 @@ func (iam *IdentityAccessManagement) isConfiguredAccountID(id string) bool { func (iam *IdentityAccessManagement) Auth(f http.HandlerFunc, action Action) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + scrubInternalHeaders(r) if !iam.isEnabled() { f(w, r) return @@ -1695,6 +1696,7 @@ func (iam *IdentityAccessManagement) Auth(f http.HandlerFunc, action Action) htt // signature verification, which is mandatory for all PostPolicy uploads. func (iam *IdentityAccessManagement) AuthPostPolicy(f http.HandlerFunc, action Action) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + scrubInternalHeaders(r) if !iam.isEnabled() { f(w, r) return @@ -1780,19 +1782,33 @@ func (iam *IdentityAccessManagement) AuthenticateRequest(r *http.Request) (*Iden return ident, err } +// scrubInternalHeaders drops internal headers only the server may set, so +// client-supplied values cannot spoof identity claims or reach the handlers' +// internal consumers (putToFiler persists the object-lock and version-id ones +// into authoritative metadata; the SSE ones carry key material between handler +// stages of a request). The server sets them itself after this point. +func scrubInternalHeaders(r *http.Request) { + r.Header.Del(s3_constants.SeaweedFSPrincipalHeader) + r.Header.Del(s3_constants.SeaweedFSSessionTokenHeader) + r.Header.Del(s3_constants.ExtObjectLockModeKey) + r.Header.Del(s3_constants.ExtRetentionUntilDateKey) + r.Header.Del(s3_constants.ExtLegalHoldKey) + r.Header.Del(s3_constants.ExtVersionIdKey) + r.Header.Del(s3_constants.SeaweedFSSSEKMSBaseIVHeader) + r.Header.Del(s3_constants.SeaweedFSSSES3BaseIVHeader) + r.Header.Del(s3_constants.SeaweedFSSSES3KeyDataHeader) +} + func (iam *IdentityAccessManagement) authenticateRequestInternal(r *http.Request) (*Identity, s3err.ErrorCode, authType) { var identity *Identity var s3Err s3err.ErrorCode var found bool var amzAuthType string - // SECURITY: Prevent clients from spoofing internal IAM headers. - // These headers are only set by the server after successful JWT authentication - // and are trusted by downstream authorization (including S3Tables IAM checks - // reached via AuthSignatureOnly). Clearing them here — the shared entry point - // for every auth path — prevents privilege escalation via header injection. - r.Header.Del(s3_constants.SeaweedFSPrincipalHeader) - r.Header.Del(s3_constants.SeaweedFSSessionTokenHeader) + // Clearing them here — the shared entry point for every auth path — + // prevents privilege escalation via header injection, including through + // the S3Tables checks reached via AuthSignatureOnly. + scrubInternalHeaders(r) // Deferred so signature verification can still read a client-signed value. defer r.Header.Del(s3_constants.AmzAccountId) diff --git a/weed/s3api/s3api_object_lock_actions_authz_test.go b/weed/s3api/s3api_object_lock_actions_authz_test.go index e74229e018..f3d2b8b70b 100644 --- a/weed/s3api/s3api_object_lock_actions_authz_test.go +++ b/weed/s3api/s3api_object_lock_actions_authz_test.go @@ -227,3 +227,67 @@ func TestObjectLockActionsRoundTrip(t *testing.T) { }) } } + +// Internal Seaweed-X-Amz-* headers carry server-validated object lock values +// into entry metadata; client-supplied ones must be dropped on entry or the +// dedicated-permission check on x-amz-object-lock-* is bypassed. +func TestInternalObjectLockHeadersStrippedOnAuth(t *testing.T) { + iam := newTestIAM() + + internal := []string{ + s3_constants.ExtObjectLockModeKey, + s3_constants.ExtRetentionUntilDateKey, + s3_constants.ExtLegalHoldKey, + s3_constants.ExtVersionIdKey, + s3_constants.SeaweedFSSSEKMSBaseIVHeader, + s3_constants.SeaweedFSSSES3BaseIVHeader, + s3_constants.SeaweedFSSSES3KeyDataHeader, + } + req := httptest.NewRequest(http.MethodPut, "http://localhost:8333/worm/doc.txt", nil) + for _, k := range internal { + req.Header.Set(k, "injected") + } + require.NoError(t, signRawHTTPRequest(context.Background(), req, + "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", "us-east-1")) + + _, _, _ = iam.authenticateRequestInternal(req) + + for _, k := range internal { + if got := req.Header.Get(k); got != "" { + t.Errorf("client-supplied %s survived authentication: %q", k, got) + } + } +} + +// With IAM disabled the Auth wrapper calls the handler directly; internal +// headers must still be stripped so they never reach putToFiler. +func TestInternalHeadersStrippedWithoutAuth(t *testing.T) { + iam := &IdentityAccessManagement{} + + internal := []string{ + s3_constants.ExtObjectLockModeKey, + s3_constants.ExtRetentionUntilDateKey, + s3_constants.ExtLegalHoldKey, + s3_constants.ExtVersionIdKey, + s3_constants.SeaweedFSSSEKMSBaseIVHeader, + s3_constants.SeaweedFSSSES3BaseIVHeader, + s3_constants.SeaweedFSSSES3KeyDataHeader, + } + var leaked []string + handler := iam.Auth(func(w http.ResponseWriter, r *http.Request) { + for _, k := range internal { + if v := r.Header.Get(k); v != "" { + leaked = append(leaked, k) + } + } + }, s3_constants.ACTION_WRITE) + + req := httptest.NewRequest(http.MethodPut, "http://localhost:8333/worm/doc.txt", nil) + for _, k := range internal { + req.Header.Set(k, "injected") + } + handler(httptest.NewRecorder(), req) + if len(leaked) > 0 { + t.Errorf("client-supplied internal headers reached handler with auth disabled: %v", leaked) + } +}