From 7e348951c2148731915d45ecbf0925f256cfe23e Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 19 Oct 2026 18:35:20 -0700 Subject: [PATCH] s3: strip internal metadata headers on auth entry (#11850) * s3: strip internal object-lock and version headers on auth entry putToFiler persists Seaweed-X-Amz-Object-Lock-Mode, -Retention-Until-Date, -Legal-Hold and -Version-Id request headers into authoritative entry metadata without a permission check, so an s3:PutObject-only writer could pin COMPLIANCE retention and legal hold on its own uploads, bypassing the dedicated-permission gate on the standard x-amz-object-lock-* headers. The server only sets these headers from validated inputs, so drop them in authenticateRequestInternal alongside the identity headers already scrubbed there. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: strip internal SSE headers on auth entry X-SeaweedFS-SSE-S3-Key-Data, -SSE-S3-Base-IV and -SSE-KMS-Base-IV are read from the request to drive multipart encryption, so a client could inject its own key material and IVs. The server sets them itself between handler stages of a request; drop inbound values in authenticateRequestInternal. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: scrub internal headers before the auth-enabled early return Auth and AuthPostPolicy skip authenticateRequestInternal when IAM is disabled, so client-supplied internal headers still reached putToFiler. Hoist the scrub into scrubInternalHeaders and run it unconditionally in both wrappers as well as the shared auth entry point. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: test internal header scrub through the auth-disabled wrapper Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/s3api/auth_credentials.go | 30 +++++++-- .../s3api_object_lock_actions_authz_test.go | 64 +++++++++++++++++++ 2 files changed, 87 insertions(+), 7 deletions(-) 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) + } +}