From a976b21010724e9bebc334ea6c1264a5a0008704 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 27 Sep 2026 20:17:12 +0800 Subject: [PATCH] s3: require dedicated object-lock permissions for x-amz-object-lock-* headers (#11492) * s3: require dedicated object-lock permissions for x-amz-object-lock-* headers PutObject, CreateMultipartUpload, and PostPolicy honor the retention and legal-hold headers after only the route's s3:PutObject check, so a write-only principal could pin a version under COMPLIANCE retention that nobody can remove before its retain-until date. On AWS these headers require s3:PutObjectRetention / s3:PutObjectLegalHold. validateObjectLockHeaders is the shared funnel for all four call sites; it now authorizes the corresponding dedicated action when each header is present. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: record the verified POST-policy signer as the request identity The handler authenticated the form policy signature but stored only the signer's name, so downstream authorization (the object-lock header check) re-authenticated the form-signed request as anonymous and evaluated the wrong principal. 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/s3api_object_handlers_multipart.go | 2 +- .../s3api/s3api_object_handlers_postpolicy.go | 4 +- weed/s3api/s3api_object_handlers_put.go | 40 +++++++++++++-- .../s3api_object_lock_actions_authz_test.go | 49 +++++++++++++++++++ weed/s3api/s3api_object_lock_headers_test.go | 30 ++++++------ 5 files changed, 104 insertions(+), 21 deletions(-) diff --git a/weed/s3api/s3api_object_handlers_multipart.go b/weed/s3api/s3api_object_handlers_multipart.go index 31728847f..65b3cc90f 100644 --- a/weed/s3api/s3api_object_handlers_multipart.go +++ b/weed/s3api/s3api_object_handlers_multipart.go @@ -69,7 +69,7 @@ func (s3a *S3ApiServer) NewMultipartUploadHandler(w http.ResponseWriter, r *http } // Validate object lock headers before processing - if err := s3a.validateObjectLockHeaders(r, versioningEnabled); err != nil { + if err := s3a.validateObjectLockHeaders(r, bucket, object, versioningEnabled); err != nil { glog.V(2).Infof("NewMultipartUploadHandler: object lock header validation failed for bucket %s, object %s: %v", bucket, object, err) s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err)) return diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index d20d9328d..835d5008b 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/weed/s3api/s3api_object_handlers_postpolicy.go @@ -86,7 +86,7 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R return } if identity != nil { - r = r.WithContext(s3_constants.SetIdentityNameInContext(r.Context(), identity.Name)) + r = r.WithContext(recordIdentityInContext(r, identity)) } policyBytes, err := base64.StdEncoding.DecodeString(formValues.Get("Policy")) @@ -160,7 +160,7 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) return } - if err := s3a.validateObjectLockHeaders(r, objectLockEnabled); err != nil { + if err := s3a.validateObjectLockHeaders(r, bucket, object, objectLockEnabled); err != nil { glog.V(2).Infof("PostPolicyBucketHandler: object lock header validation failed for %s/%s: %v", bucket, object, err) s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err)) return diff --git a/weed/s3api/s3api_object_handlers_put.go b/weed/s3api/s3api_object_handlers_put.go index 9883187d8..2a8f9259f 100644 --- a/weed/s3api/s3api_object_handlers_put.go +++ b/weed/s3api/s3api_object_handlers_put.go @@ -45,6 +45,7 @@ var ( ErrObjectLockModeRequiresDate = errors.New("object lock mode requires retention until date") ErrRetentionDateRequiresMode = errors.New("retention until date requires object lock mode") ErrGovernanceBypassVersioningRequired = errors.New("governance bypass header can only be used on buckets with Object Lock enabled") + ErrObjectLockNotAuthorized = errors.New("object lock headers require the corresponding object lock permission") ErrInvalidObjectLockDuration = errors.New("object lock duration must be greater than 0 days") ErrObjectLockDurationExceeded = errors.New("object lock duration exceeds maximum allowed days") ErrObjectLockConfigurationMissingEnabled = errors.New("object lock configuration must specify ObjectLockEnabled") @@ -152,7 +153,7 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) return } - if validationErr := s3a.validateObjectLockHeaders(r, objectLockEnabled); validationErr != nil { + if validationErr := s3a.validateObjectLockHeaders(r, bucket, object, objectLockEnabled); validationErr != nil { glog.V(2).Infof("PutObjectHandler: object lock header validation failed for %s/%s: %v", bucket, object, validationErr) s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(validationErr)) return @@ -281,7 +282,7 @@ func (s3a *S3ApiServer) PutObjectHandler(w http.ResponseWriter, r *http.Request) } // Validate object lock headers before processing - if err := s3a.validateObjectLockHeaders(r, objectLockEnabled); err != nil { + if err := s3a.validateObjectLockHeaders(r, bucket, object, objectLockEnabled); err != nil { glog.V(2).Infof("PutObjectHandler: object lock header validation failed for bucket %s, object %s: %v", bucket, object, err) s3err.WriteErrorResponse(w, r, mapValidationErrorToS3Error(err)) return @@ -2129,9 +2130,29 @@ func (s3a *S3ApiServer) applyBucketDefaultRetention(bucket string, entry *filer_ return nil } +// checkObjectLockHeaderPermission reports whether the caller may set object +// lock through PutObject-style request headers. On AWS these headers require +// the dedicated s3:PutObjectRetention / s3:PutObjectLegalHold permissions; +// s3:PutObject alone must not be enough, or any writer can pin data with +// COMPLIANCE retention. Without auth there is nothing to check against. +func (s3a *S3ApiServer) checkObjectLockHeaderPermission(r *http.Request, bucket, object string, action Action) bool { + if s3a.iam == nil || !s3a.iam.isEnabled() { + return true + } + identity, _ := s3_constants.GetIdentityFromContext(r).(*Identity) + if identity == nil { + var errCode s3err.ErrorCode + identity, errCode, _ = s3a.iam.authenticateRequestInternal(r) + if errCode != s3err.ErrNone { + return false + } + } + return s3a.iam.authorizeObjectKeyAction(r, identity, r.Method, action, bucket, s3_constants.NormalizeObjectKey(object), "") == s3err.ErrNone +} + // validateObjectLockHeaders validates object lock headers in PUT requests // objectLockEnabled should be true only if the bucket has Object Lock configured -func (s3a *S3ApiServer) validateObjectLockHeaders(r *http.Request, objectLockEnabled bool) error { +func (s3a *S3ApiServer) validateObjectLockHeaders(r *http.Request, bucket, object string, objectLockEnabled bool) error { // Extract object lock headers from request mode := r.Header.Get(s3_constants.AmzObjectLockMode) retainUntilDateStr := r.Header.Get(s3_constants.AmzObjectLockRetainUntilDate) @@ -2193,6 +2214,17 @@ func (s3a *S3ApiServer) validateObjectLockHeaders(r *http.Request, objectLockEna return ErrGovernanceBypassVersioningRequired } + if mode != "" || retainUntilDateStr != "" { + if !s3a.checkObjectLockHeaderPermission(r, bucket, object, s3_constants.ACTION_PUT_OBJECT_RETENTION) { + return ErrObjectLockNotAuthorized + } + } + if legalHold != "" { + if !s3a.checkObjectLockHeaderPermission(r, bucket, object, s3_constants.ACTION_PUT_OBJECT_LEGAL_HOLD) { + return ErrObjectLockNotAuthorized + } + } + return nil } @@ -2232,6 +2264,8 @@ func mapValidationErrorToS3Error(err error) s3err.ErrorCode { // For governance bypass on non-versioned bucket, return InvalidRequest // This matches the test expectations return s3err.ErrInvalidRequest + case errors.Is(err, ErrObjectLockNotAuthorized): + return s3err.ErrAccessDenied case errors.Is(err, ErrMalformedXML): // For malformed XML in request body, return MalformedXML // This matches the test expectations for invalid retention mode and legal hold status diff --git a/weed/s3api/s3api_object_lock_actions_authz_test.go b/weed/s3api/s3api_object_lock_actions_authz_test.go index b785e8324..e74229e01 100644 --- a/weed/s3api/s3api_object_lock_actions_authz_test.go +++ b/weed/s3api/s3api_object_lock_actions_authz_test.go @@ -2,9 +2,11 @@ package s3api import ( "context" + "errors" "net/http" "net/http/httptest" "testing" + "time" "github.com/gorilla/mux" iamlib "github.com/seaweedfs/seaweedfs/weed/iam" @@ -88,6 +90,53 @@ func TestGovernanceBypassDoesNotInheritDeleteObjectVersion(t *testing.T) { } } +// The object-lock request headers on PutObject/CreateMultipartUpload must be +// authorized as s3:PutObjectRetention / s3:PutObjectLegalHold; s3:PutObject +// alone used to set retention and legal hold on new versions. +func TestObjectLockHeadersRequireDedicatedActions(t *testing.T) { + iam := &IdentityAccessManagement{isAuthEnabled: true} + require.NoError(t, iam.PutPolicy("Writer", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:PutObject"],"Resource":"arn:aws:s3:::worm/*"}]}`)) + require.NoError(t, iam.PutPolicy("Locked", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:PutObject","s3:PutObjectRetention","s3:PutObjectLegalHold"],"Resource":"arn:aws:s3:::worm/*"}]}`)) + s3a := &S3ApiServer{iam: iam} + + retainUntil := time.Now().Add(24 * time.Hour).Format(time.RFC3339) + for _, tc := range []struct { + name string + policy string + headers map[string]string + wantErr error + }{ + {"writer sets retention", "Writer", + map[string]string{s3_constants.AmzObjectLockMode: "COMPLIANCE", s3_constants.AmzObjectLockRetainUntilDate: retainUntil}, + ErrObjectLockNotAuthorized}, + {"writer sets legal hold", "Writer", + map[string]string{s3_constants.AmzObjectLockLegalHold: s3_constants.LegalHoldOn}, + ErrObjectLockNotAuthorized}, + {"writer plain put", "Writer", nil, nil}, + {"locked principal sets both", "Locked", + map[string]string{s3_constants.AmzObjectLockMode: "GOVERNANCE", s3_constants.AmzObjectLockRetainUntilDate: retainUntil, s3_constants.AmzObjectLockLegalHold: s3_constants.LegalHoldOn}, + nil}, + } { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPut, "http://localhost:8333/worm/doc.txt", nil) + req = mux.SetURLVars(req, map[string]string{"bucket": "worm", "object": "doc.txt"}) + for k, v := range tc.headers { + req.Header.Set(k, v) + } + req = req.WithContext(s3_constants.SetIdentityInContext(req.Context(), &Identity{ + Name: "caller", + PolicyNames: []string{tc.policy}, + Account: &Account{Id: "test-account"}, + })) + if err := s3a.validateObjectLockHeaders(req, "worm", "doc.txt", true); !errors.Is(err, tc.wantErr) { + t.Errorf("validateObjectLockHeaders() = %v, want %v", err, tc.wantErr) + } + }) + } +} + func TestGovernanceBypassUsesBodyObjectKey(t *testing.T) { iam := newTestIAM() iam.identities[0].Actions = []Action{ diff --git a/weed/s3api/s3api_object_lock_headers_test.go b/weed/s3api/s3api_object_lock_headers_test.go index fc8a01232..fed835523 100644 --- a/weed/s3api/s3api_object_lock_headers_test.go +++ b/weed/s3api/s3api_object_lock_headers_test.go @@ -410,7 +410,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req.Header.Set(s3_constants.AmzObjectLockMode, "COMPLIANCE") req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, retainUntilDate.Format(time.RFC3339)) - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.NoError(t, err) }) @@ -420,7 +420,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req.Header.Set(s3_constants.AmzObjectLockMode, "GOVERNANCE") req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, retainUntilDate.Format(time.RFC3339)) - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.NoError(t, err) }) @@ -428,7 +428,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req := httptest.NewRequest("PUT", "/bucket/object", nil) req.Header.Set(s3_constants.AmzObjectLockLegalHold, "ON") - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.NoError(t, err) }) @@ -436,7 +436,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req := httptest.NewRequest("PUT", "/bucket/object", nil) req.Header.Set(s3_constants.AmzObjectLockLegalHold, "OFF") - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.NoError(t, err) }) @@ -446,7 +446,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { retainUntilDate := time.Now().Add(24 * time.Hour) req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, retainUntilDate.Format(time.RFC3339)) - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrInvalidObjectLockMode)) }) @@ -455,7 +455,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req := httptest.NewRequest("PUT", "/bucket/object", nil) req.Header.Set(s3_constants.AmzObjectLockLegalHold, "INVALID_STATUS") - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrInvalidLegalHoldStatus)) }) @@ -466,7 +466,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { retainUntilDate := time.Now().Add(24 * time.Hour) req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, retainUntilDate.Format(time.RFC3339)) - err := s3a.validateObjectLockHeaders(req, false) // non-versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", false) // non-versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrObjectLockVersioningRequired)) }) @@ -476,7 +476,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req.Header.Set(s3_constants.AmzObjectLockMode, "COMPLIANCE") req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, "invalid-date-format") - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrInvalidRetentionDateFormat)) }) @@ -487,7 +487,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { pastDate := time.Now().Add(-24 * time.Hour) req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, pastDate.Format(time.RFC3339)) - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrRetentionDateMustBeFuture)) }) @@ -496,7 +496,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req := httptest.NewRequest("PUT", "/bucket/object", nil) req.Header.Set(s3_constants.AmzObjectLockMode, "COMPLIANCE") - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrObjectLockModeRequiresDate)) }) @@ -506,7 +506,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { retainUntilDate := time.Now().Add(24 * time.Hour) req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, retainUntilDate.Format(time.RFC3339)) - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrRetentionDateRequiresMode)) }) @@ -515,7 +515,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req := httptest.NewRequest("PUT", "/bucket/object", nil) req.Header.Set("x-amz-bypass-governance-retention", "true") - err := s3a.validateObjectLockHeaders(req, false) // non-versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", false) // non-versioned bucket assert.Error(t, err) assert.True(t, errors.Is(err, ErrGovernanceBypassVersioningRequired)) }) @@ -524,7 +524,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req := httptest.NewRequest("PUT", "/bucket/object", nil) req.Header.Set("x-amz-bypass-governance-retention", "true") - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.NoError(t, err) }) @@ -532,7 +532,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req := httptest.NewRequest("PUT", "/bucket/object", nil) // No object lock headers set - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.NoError(t, err) }) @@ -543,7 +543,7 @@ func TestValidateObjectLockHeaders(t *testing.T) { req.Header.Set(s3_constants.AmzObjectLockRetainUntilDate, retainUntilDate.Format(time.RFC3339)) req.Header.Set(s3_constants.AmzObjectLockLegalHold, "ON") - err := s3a.validateObjectLockHeaders(req, true) // versioned bucket + err := s3a.validateObjectLockHeaders(req, "bucket", "object", true) // versioned bucket assert.NoError(t, err) }) }