diff --git a/weed/iam/helpers.go b/weed/iam/helpers.go index 628fa9551d..6d006478bb 100644 --- a/weed/iam/helpers.go +++ b/weed/iam/helpers.go @@ -98,11 +98,13 @@ var fineGrainedActionMap = map[string]string{ // prefixed (s3:Action) and unprefixed (Action) variants in init(). var baseS3ActionMap = map[string]string{ // Object operations - "DeleteObject": s3_constants.ACTION_WRITE, - "PutObject": s3_constants.ACTION_WRITE, - "GetObject": s3_constants.ACTION_READ, - "DeleteObjectVersion": s3_constants.ACTION_WRITE, - "GetObjectVersion": s3_constants.ACTION_READ, + "DeleteObject": s3_constants.ACTION_WRITE, + "PutObject": s3_constants.ACTION_WRITE, + "GetObject": s3_constants.ACTION_READ, + "DeleteObjectVersion": s3_constants.ACTION_WRITE, + "GetObjectVersion": s3_constants.ACTION_READ, + "GetObjectAttributes": s3_constants.ACTION_READ, + "GetObjectVersionAttributes": s3_constants.ACTION_READ, // Tagging operations "GetObjectTagging": s3_constants.ACTION_TAGGING, "GetObjectVersionTagging": s3_constants.ACTION_TAGGING, @@ -115,6 +117,7 @@ var baseS3ActionMap = map[string]string{ "DeleteBucketTagging": s3_constants.ACTION_TAGGING, // ACL operations "PutObjectAcl": s3_constants.ACTION_WRITE_ACP, + "PutObjectVersionAcl": s3_constants.ACTION_WRITE_ACP, "GetObjectAcl": s3_constants.ACTION_READ_ACP, "GetObjectVersionAcl": s3_constants.ACTION_READ_ACP, "PutBucketAcl": s3_constants.ACTION_WRITE_ACP, diff --git a/weed/s3api/auth_copy_source_test.go b/weed/s3api/auth_copy_source_test.go index 746f16009f..21415ddc9e 100644 --- a/weed/s3api/auth_copy_source_test.go +++ b/weed/s3api/auth_copy_source_test.go @@ -3,6 +3,7 @@ package s3api import ( "context" "net/http" + "net/url" "testing" "github.com/stretchr/testify/assert" @@ -251,9 +252,10 @@ func TestAuthorizeCopySource_PresignedURLSessionTokenPreserved(t *testing.T) { } // Presigned URL: session token is in the query string, no - // X-Amz-Security-Token header. The destination URL also carries unrelated - // SigV4 query params (X-Amz-Algorithm, etc.) that must not leak into the - // synthetic source action resolution. + // X-Amz-Security-Token header. The destination URL also carries the rest of + // the SigV4 presign params, which ride along so auth-derived policy + // conditions (s3:authType) match the real request; none of them steer + // ResolveS3Action. req, err := http.NewRequest(http.MethodPut, "http://s3.local/bucket-a/dst.bin?X-Amz-Algorithm=AWS4-HMAC-SHA256"+ "&X-Amz-Credential=stsKey%2F20260518%2Fus-east-1%2Fs3%2Faws4_request"+ @@ -267,8 +269,12 @@ func TestAuthorizeCopySource_PresignedURLSessionTokenPreserved(t *testing.T) { assert.Equal(t, s3err.ErrNone, errCode) assert.Equal(t, "presigned-session-token", capturedSessionToken, "session token from presigned-URL query must reach the IAM check") - // versionId omitted, only the token should be present in the synthetic query. - assert.Equal(t, "X-Amz-Security-Token=presigned-session-token", capturedRawQuery) + capturedQuery, err := url.ParseQuery(capturedRawQuery) + require.NoError(t, err) + assert.Equal(t, "presigned-session-token", capturedQuery.Get("X-Amz-Security-Token")) + assert.Equal(t, "AWS4-HMAC-SHA256", capturedQuery.Get("X-Amz-Algorithm")) + assert.False(t, capturedQuery.Has("versionId")) + assert.False(t, capturedQuery.Has("acl")) } // TestAuthorizeCopySource_PreservesCopySourceHeader verifies that the synthetic diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index bb05bb5335..65a64c2eff 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -3061,11 +3061,21 @@ func (iam *IdentityAccessManagement) bucketsNamedByAttachedPolicies(r *http.Requ // its query describe. versionId, when set, and the STS session token ride along // for policy conditions. Returns ErrNone when auth is disabled (checked by the // callers) or the identity is an admin. +// authQueryParams are the query keys carrying a request's authentication +// material (SigV2 and SigV4 presign parameters). None steer ResolveS3Action, +// so they can ride along on a synthesized request. +var authQueryParams = []string{ + "AWSAccessKeyId", "Signature", "Expires", + "X-Amz-Algorithm", "X-Amz-Credential", "X-Amz-Date", "X-Amz-Expires", + "X-Amz-Signature", "X-Amz-SignedHeaders", "X-Amz-Security-Token", +} + // objectKeyPolicyRequest synthesizes a // request // for policy evaluation against a key the real URL does not name. The query is // built from scratch so a param on the real request (delete, uploadId, // partNumber, ...) cannot steer ResolveS3Action off the intended action; the -// STS token and versionId ride along for policy conditions. +// versionId and the auth-material params ride along so auth-derived policy +// conditions (s3:authType) evaluate as they did on the real request. func objectKeyPolicyRequest(r *http.Request, method, bucket, objectKey, versionId string) *http.Request { keyReq := new(http.Request) *keyReq = *r @@ -3078,9 +3088,9 @@ func objectKeyPolicyRequest(r *http.Request, method, bucket, objectKey, versionI if versionId != "" { keyQuery.Set("versionId", versionId) } - if strings.Contains(r.URL.RawQuery, "X-Amz-Security-Token") { - if token := r.URL.Query().Get("X-Amz-Security-Token"); token != "" { - keyQuery.Set("X-Amz-Security-Token", token) + for _, key := range authQueryParams { + if vals, ok := r.URL.Query()[key]; ok { + keyQuery[key] = vals } } if len(keyQuery) > 0 { diff --git a/weed/s3api/s3_action_resolver.go b/weed/s3api/s3_action_resolver.go index 7c228ba2ab..ea33de78d0 100644 --- a/weed/s3api/s3_action_resolver.go +++ b/weed/s3api/s3_action_resolver.go @@ -166,15 +166,25 @@ func resolveFromQueryParameters(query url.Values, method string, hasObject bool) } } - // ACL operations + // ACL operations. A versionId retargets the request at a specific + // version, so the version ACL action applies, not the current-object one. if query.Has("acl") { + versioned := hasObject && query.Get("versionId") != "" switch method { - case http.MethodGet, http.MethodHead: + // HEAD has no acl route; it lands on HeadObjectHandler and should + // resolve like a read — s3:GetObject, or s3:GetObjectVersion below. + case http.MethodGet: + if versioned { + return s3_constants.S3_ACTION_GET_OBJECT_VERSION_ACL + } if hasObject { return s3_constants.S3_ACTION_GET_OBJECT_ACL } return s3_constants.S3_ACTION_GET_BUCKET_ACL case http.MethodPut: + if versioned { + return s3_constants.S3_ACTION_PUT_OBJECT_VERSION_ACL + } if hasObject { return s3_constants.S3_ACTION_PUT_OBJECT_ACL } @@ -220,6 +230,9 @@ func resolveFromQueryParameters(query url.Values, method string, hasObject bool) // Must be checked before versionId, because GET /bucket/key?attributes&versionId=xyz // is a GetObjectAttributes request, not a GetObjectVersion request if hasObject && query.Has("attributes") && method == http.MethodGet { + if query.Get("versionId") != "" { + return s3_constants.S3_ACTION_GET_OBJECT_VERSION_ATTRIBUTES + } return s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES } diff --git a/weed/s3api/s3_action_resolver_test.go b/weed/s3api/s3_action_resolver_test.go index e46c1c8b6a..d073355fd5 100644 --- a/weed/s3api/s3_action_resolver_test.go +++ b/weed/s3api/s3_action_resolver_test.go @@ -83,7 +83,7 @@ func TestResolveS3Action_AttributesBeforeVersionId(t *testing.T) { method: http.MethodGet, baseAction: s3_constants.ACTION_READ, object: "key", - want: s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES, + want: s3_constants.S3_ACTION_GET_OBJECT_VERSION_ATTRIBUTES, }, { name: "versionId only GET", @@ -146,6 +146,59 @@ func TestResolveS3Action_VersionTagging(t *testing.T) { } } +// An ACL request carrying versionId targets a specific version and must +// resolve to the version ACL action so a grant limited to the current-object +// action cannot reach historical versions. +func TestResolveS3Action_VersionAcl(t *testing.T) { + tests := []struct { + name string + method string + object string + query string + want string + }{ + {"get current acl", http.MethodGet, "key", "acl", s3_constants.S3_ACTION_GET_OBJECT_ACL}, + {"put current acl", http.MethodPut, "key", "acl", s3_constants.S3_ACTION_PUT_OBJECT_ACL}, + {"get version acl", http.MethodGet, "key", "acl&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_ACL}, + {"head acl resolves as head object", http.MethodHead, "key", "acl", s3_constants.S3_ACTION_GET_OBJECT}, + {"head version acl resolves as head version", http.MethodHead, "key", "acl&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION}, + {"put version acl", http.MethodPut, "key", "acl&versionId=v1", s3_constants.S3_ACTION_PUT_OBJECT_VERSION_ACL}, + {"empty versionId stays current", http.MethodGet, "key", "acl&versionId=", s3_constants.S3_ACTION_GET_OBJECT_ACL}, + {"bucket acl ignores versionId", http.MethodGet, "", "acl&versionId=v1", s3_constants.S3_ACTION_GET_BUCKET_ACL}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, _ := http.NewRequest(tt.method, "http://localhost/bucket/"+tt.object+"?"+tt.query, nil) + if got := ResolveS3Action(r, s3_constants.ACTION_READ, "bucket", tt.object); got != tt.want { + t.Errorf("ResolveS3Action() = %q, want %q", got, tt.want) + } + }) + } +} + +// An attributes request carrying versionId targets a specific version and +// must resolve to the version attributes action. +func TestResolveS3Action_VersionAttributes(t *testing.T) { + tests := []struct { + name string + method string + object string + query string + want string + }{ + {"get current attributes", http.MethodGet, "key", "attributes", s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES}, + {"get version attributes", http.MethodGet, "key", "attributes&versionId=v1", s3_constants.S3_ACTION_GET_OBJECT_VERSION_ATTRIBUTES}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r, _ := http.NewRequest(tt.method, "http://localhost/bucket/"+tt.object+"?"+tt.query, nil) + if got := ResolveS3Action(r, s3_constants.ACTION_READ, "bucket", tt.object); got != tt.want { + t.Errorf("ResolveS3Action() = %q, want %q", got, tt.want) + } + }) + } +} + // Bucket subresources registered with ACTION_ADMIN must resolve to their own // S3 actions so a policy granting one of them does not need s3:*, and so no // broader grant sweeps them in. diff --git a/weed/s3api/s3_constants/s3_action_strings.go b/weed/s3api/s3_constants/s3_action_strings.go index 059c6e1dd8..124a29e5d2 100644 --- a/weed/s3api/s3_constants/s3_action_strings.go +++ b/weed/s3api/s3_constants/s3_action_strings.go @@ -4,17 +4,22 @@ package s3_constants // These match the official AWS S3 action format used in IAM and bucket policies const ( // Object operations - S3_ACTION_GET_OBJECT = "s3:GetObject" - S3_ACTION_PUT_OBJECT = "s3:PutObject" - S3_ACTION_DELETE_OBJECT = "s3:DeleteObject" - S3_ACTION_DELETE_OBJECT_VERSION = "s3:DeleteObjectVersion" - S3_ACTION_GET_OBJECT_VERSION = "s3:GetObjectVersion" - S3_ACTION_GET_OBJECT_ATTRIBUTES = "s3:GetObjectAttributes" + S3_ACTION_GET_OBJECT = "s3:GetObject" + S3_ACTION_PUT_OBJECT = "s3:PutObject" + S3_ACTION_DELETE_OBJECT = "s3:DeleteObject" + S3_ACTION_DELETE_OBJECT_VERSION = "s3:DeleteObjectVersion" + S3_ACTION_GET_OBJECT_VERSION = "s3:GetObjectVersion" + S3_ACTION_GET_OBJECT_ATTRIBUTES = "s3:GetObjectAttributes" + S3_ACTION_GET_OBJECT_VERSION_ATTRIBUTES = "s3:GetObjectVersionAttributes" // Object ACL operations S3_ACTION_GET_OBJECT_ACL = "s3:GetObjectAcl" S3_ACTION_PUT_OBJECT_ACL = "s3:PutObjectAcl" + // Version-specific ACL operations (?acl&versionId) + S3_ACTION_GET_OBJECT_VERSION_ACL = "s3:GetObjectVersionAcl" + S3_ACTION_PUT_OBJECT_VERSION_ACL = "s3:PutObjectVersionAcl" + // Object tagging operations S3_ACTION_GET_OBJECT_TAGGING = "s3:GetObjectTagging" S3_ACTION_PUT_OBJECT_TAGGING = "s3:PutObjectTagging" diff --git a/weed/s3api/s3_existing_object_tag_test.go b/weed/s3api/s3_existing_object_tag_test.go index b3cb54f101..c1809d9573 100644 --- a/weed/s3api/s3_existing_object_tag_test.go +++ b/weed/s3api/s3_existing_object_tag_test.go @@ -576,3 +576,97 @@ func TestRecheckObjectAclTagConditionedPolicy(t *testing.T) { assert.Equal(t, s3err.ErrNone, s3a.recheckPolicyWithObjectEntry(putReq, "b", "secret.txt", s3_constants.S3_ACTION_PUT_OBJECT_ACL, publicEntry, "test")) } + +// A versioned GetObjectAttributes request is authorized against the version +// read grant, not the attributes action the original request resolves to: an +// explicit bucket-policy Deny on s3:GetObjectVersion blocks it, while a legacy +// identity holding only the coarse Read grant is still accepted. +func TestCheckVersionReadGrant(t *testing.T) { + s3a := &S3ApiServer{ + policyEngine: NewBucketPolicyEngine(), + iam: &IdentityAccessManagement{}, + } + + denyPolicyJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:*", + "Resource": ["arn:aws:s3:::deny-bucket", "arn:aws:s3:::deny-bucket/*"] + }, + { + "Effect": "Deny", + "Principal": "*", + "Action": "s3:GetObjectVersion", + "Resource": "arn:aws:s3:::deny-bucket/key" + } + ] + }` + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("deny-bucket", denyPolicyJSON)) + + allowPolicyJSON := `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": "*", + "Action": "s3:*", + "Resource": ["arn:aws:s3:::allow-bucket", "arn:aws:s3:::allow-bucket/*"] + } + ] + }` + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("allow-bucket", allowPolicyJSON)) + + identity := &Identity{ + Name: "reader", + Actions: []Action{s3_constants.ACTION_READ}, + PrincipalArn: "arn:aws:iam::000000000000:user/reader", + } + + attrsReq := func(bucket string) *http.Request { + req := httptest.NewRequest(http.MethodGet, "/"+bucket+"/key?attributes&versionId=v1", nil) + return req.WithContext(s3_constants.SetIdentityInContext(req.Context(), identity)) + } + + assert.Equal(t, s3err.ErrAccessDenied, + s3a.checkVersionReadGrant(attrsReq("deny-bucket"), "deny-bucket", "key", "v1", nil)) + assert.Equal(t, s3err.ErrNone, + s3a.checkVersionReadGrant(attrsReq("allow-bucket"), "allow-bucket", "key", "v1", nil)) + assert.Equal(t, s3err.ErrNone, + s3a.checkVersionReadGrant(attrsReq("no-policy-bucket"), "no-policy-bucket", "key", "v1", nil)) +} + +// Presigned-URL credentials ride along on the synthesized version-read +// request, so an s3:authType=REST-QUERY-STRING policy condition decides the +// recheck the same way it decided the real request. +func TestCheckVersionReadGrantPreservesQueryAuth(t *testing.T) { + s3a := &S3ApiServer{ + policyEngine: NewBucketPolicyEngine(), + iam: &IdentityAccessManagement{}, + } + + policyJSON := `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Principal": "*", + "Action": "s3:*", + "Resource": ["arn:aws:s3:::qs-bucket", "arn:aws:s3:::qs-bucket/*"], + "Condition": {"StringEquals": {"s3:authType": "REST-QUERY-STRING"}} + }] + }` + require.NoError(t, s3a.policyEngine.engine.SetBucketPolicy("qs-bucket", policyJSON)) + + presigned := httptest.NewRequest(http.MethodGet, + "/qs-bucket/key?attributes&versionId=v1&AWSAccessKeyId=AK&Signature=sig&Expires=9999999999", nil) + assert.Equal(t, s3err.ErrNone, + s3a.checkVersionReadGrant(presigned, "qs-bucket", "key", "v1", nil)) + + keyReq := objectKeyPolicyRequest(presigned, http.MethodGet, "qs-bucket", "key", "v1") + q := keyReq.URL.Query() + assert.Equal(t, "AK", q.Get("AWSAccessKeyId")) + assert.Equal(t, "v1", q.Get("versionId")) + assert.False(t, q.Has("attributes")) +} diff --git a/weed/s3api/s3api_object_handlers_acl.go b/weed/s3api/s3api_object_handlers_acl.go index dc5f79eae6..e832db33dc 100644 --- a/weed/s3api/s3api_object_handlers_acl.go +++ b/weed/s3api/s3api_object_handlers_acl.go @@ -30,6 +30,10 @@ func (s3a *S3ApiServer) GetObjectAclHandler(w http.ResponseWriter, r *http.Reque // Check for specific version ID in query parameters versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } // Check if versioning is configured for the bucket (Enabled or Suspended) versioningConfigured, err := s3a.isVersioningConfigured(bucket) @@ -89,7 +93,11 @@ func (s3a *S3ApiServer) GetObjectAclHandler(w http.ResponseWriter, r *http.Reque return } - if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_GET_OBJECT_ACL, entry.Extended, "GetObjectAclHandler"); errCode != s3err.ErrNone { + action := s3_constants.S3_ACTION_GET_OBJECT_ACL + if versionId != "" { + action = s3_constants.S3_ACTION_GET_OBJECT_VERSION_ACL + } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, action, entry.Extended, "GetObjectAclHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } @@ -142,6 +150,10 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque // Check for specific version ID in query parameters versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } // Check if versioning is configured for the bucket (Enabled or Suspended) versioningConfigured, err := s3a.isVersioningConfigured(bucket) @@ -201,7 +213,11 @@ func (s3a *S3ApiServer) PutObjectAclHandler(w http.ResponseWriter, r *http.Reque return } - if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_PUT_OBJECT_ACL, entry.Extended, "PutObjectAclHandler"); errCode != s3err.ErrNone { + putAction := s3_constants.S3_ACTION_PUT_OBJECT_ACL + if versionId != "" { + putAction = s3_constants.S3_ACTION_PUT_OBJECT_VERSION_ACL + } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, putAction, entry.Extended, "PutObjectAclHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } diff --git a/weed/s3api/s3api_object_handlers_attributes.go b/weed/s3api/s3api_object_handlers_attributes.go index f483c13a2f..9e8881ebc1 100644 --- a/weed/s3api/s3api_object_handlers_attributes.go +++ b/weed/s3api/s3api_object_handlers_attributes.go @@ -129,6 +129,10 @@ func (s3a *S3ApiServer) GetObjectAttributesHandler(w http.ResponseWriter, r *htt // Check for specific version ID versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } var ( entry *filer_pb.Entry @@ -219,11 +223,24 @@ func (s3a *S3ApiServer) GetObjectAttributesHandler(w http.ResponseWriter, r *htt } } - if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES, entry.Extended, "GetObjectAttributesHandler"); errCode != s3err.ErrNone { + action := s3_constants.S3_ACTION_GET_OBJECT_ATTRIBUTES + if versionId != "" { + action = s3_constants.S3_ACTION_GET_OBJECT_VERSION_ATTRIBUTES + } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, action, entry.Extended, "GetObjectAttributesHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } + // AWS also asks for the version read grant on a versioned attributes + // request: s3:GetObjectVersionAttributes alone is not sufficient. + if versionId != "" { + if errCode := s3a.checkVersionReadGrant(r, bucket, object, versionId, entry.Extended); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } + } + // Evaluate conditional headers against the resolved entry (after version resolution) // This ensures conditions are checked against the correct version, not always the latest if s3a.hasConditionalHeaders(r) { diff --git a/weed/s3api/s3api_object_handlers_tagging.go b/weed/s3api/s3api_object_handlers_tagging.go index 37761322f8..466d8bcfd4 100644 --- a/weed/s3api/s3api_object_handlers_tagging.go +++ b/weed/s3api/s3api_object_handlers_tagging.go @@ -29,6 +29,10 @@ func (s3a *S3ApiServer) GetObjectTaggingHandler(w http.ResponseWriter, r *http.R // Check for specific version ID in query parameters versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } // Check if versioning is configured for the bucket (Enabled or Suspended) versioningConfigured, err := s3a.isVersioningConfigured(bucket) @@ -87,7 +91,11 @@ func (s3a *S3ApiServer) GetObjectTaggingHandler(w http.ResponseWriter, r *http.R } } - if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_GET_OBJECT_TAGGING, entry.Extended, "GetObjectTaggingHandler"); errCode != s3err.ErrNone { + action := s3_constants.S3_ACTION_GET_OBJECT_TAGGING + if versionId != "" { + action = s3_constants.S3_ACTION_GET_OBJECT_VERSION_TAGGING + } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, action, entry.Extended, "GetObjectTaggingHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } @@ -140,6 +148,10 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R // Check for specific version ID in query parameters versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } // Check if versioning is configured for the bucket (Enabled or Suspended) versioningConfigured, err := s3a.isVersioningConfigured(bucket) @@ -185,7 +197,11 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object)) dir, name := target.DirAndName() - if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.S3_ACTION_PUT_OBJECT_TAGGING, "PutObjectTaggingHandler"); errCode != s3err.ErrNone { + putAction := s3_constants.S3_ACTION_PUT_OBJECT_TAGGING + if versionId != "" { + putAction = s3_constants.S3_ACTION_PUT_OBJECT_VERSION_TAGGING + } + if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, putAction, "PutObjectTaggingHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } @@ -206,7 +222,11 @@ func (s3a *S3ApiServer) PutObjectTaggingHandler(w http.ResponseWriter, r *http.R return } - if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_PUT_OBJECT_TAGGING, entry.Extended, "PutObjectTaggingHandler"); errCode != s3err.ErrNone { + putAction := s3_constants.S3_ACTION_PUT_OBJECT_TAGGING + if versionId != "" { + putAction = s3_constants.S3_ACTION_PUT_OBJECT_VERSION_TAGGING + } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, putAction, entry.Extended, "PutObjectTaggingHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } @@ -268,6 +288,10 @@ func (s3a *S3ApiServer) DeleteObjectTaggingHandler(w http.ResponseWriter, r *htt // Check for specific version ID in query parameters versionId := r.URL.Query().Get("versionId") + if errCode := s3a.checkVersionIdConfigured(bucket, versionId); errCode != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, errCode) + return + } // Check if versioning is configured for the bucket (Enabled or Suspended) versioningConfigured, err := s3a.isVersioningConfigured(bucket) @@ -313,7 +337,11 @@ func (s3a *S3ApiServer) DeleteObjectTaggingHandler(w http.ResponseWriter, r *htt target := util.FullPath(fmt.Sprintf("%s/%s", s3a.bucketDir(bucket), object)) dir, name := target.DirAndName() - if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone { + deleteAction := s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING + if versionId != "" { + deleteAction = s3_constants.S3_ACTION_DELETE_OBJECT_VERSION_TAGGING + } + if errCode := s3a.recheckTagConditionedPolicyWithLookup(r, bucket, object, versionId, deleteAction, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } @@ -335,7 +363,11 @@ func (s3a *S3ApiServer) DeleteObjectTaggingHandler(w http.ResponseWriter, r *htt return } - if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING, entry.Extended, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone { + deleteAction := s3_constants.S3_ACTION_DELETE_OBJECT_TAGGING + if versionId != "" { + deleteAction = s3_constants.S3_ACTION_DELETE_OBJECT_VERSION_TAGGING + } + if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, deleteAction, entry.Extended, "DeleteObjectTaggingHandler"); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index f7c0e477cf..ad88378916 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -681,6 +681,21 @@ func (s3a *S3ApiServer) recheckPolicyWithObjectEntry(r *http.Request, bucket, ob return errCode } +// checkVersionReadGrant enforces the version read grant AWS requires alongside +// a version-scoped action such as s3:GetObjectVersionAttributes. The check runs +// on a request shaped like a plain versioned GET so the s3:GetObjectVersion +// action resolves from the request instead of the caller's subresource, and a +// legacy Read grant still applies. +func (s3a *S3ApiServer) checkVersionReadGrant(r *http.Request, bucket, object, versionId string, objectEntry map[string][]byte) s3err.ErrorCode { + versionReadReq := objectKeyPolicyRequest(r, http.MethodGet, bucket, object, versionId) + if identity, ok := GetIdentityFromContext(r).(*Identity); ok && identity != nil { + if errCode := s3a.iam.VerifyActionPermission(versionReadReq, identity, ACTION_READ, bucket, object); errCode != s3err.ErrNone { + return errCode + } + } + return s3a.recheckPolicyWithObjectEntry(versionReadReq, bucket, object, ACTION_READ, objectEntry, "checkVersionReadGrant") +} + // recheckCopySourcePolicy repeats the source object's policy check once its // entry is fetched, so conditions on the object's existing tags apply to a // CopyObject / UploadPartCopy / RenameObject source the same way they do to a