mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-20 13:04:25 +02:00
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>
This commit is contained in:
1 parent
8b09c96cec
commit
7e348951c2
2 files changed
+87
-7
No files matched your search
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user