diff --git a/weed/s3api/s3_existing_object_tag_test.go b/weed/s3api/s3_existing_object_tag_test.go index b3cb54f101..78c24bd4a3 100644 --- a/weed/s3api/s3_existing_object_tag_test.go +++ b/weed/s3api/s3_existing_object_tag_test.go @@ -576,3 +576,64 @@ 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)) +} diff --git a/weed/s3api/s3api_object_handlers_attributes.go b/weed/s3api/s3api_object_handlers_attributes.go index d8c2bbf3bd..e37eee7748 100644 --- a/weed/s3api/s3api_object_handlers_attributes.go +++ b/weed/s3api/s3api_object_handlers_attributes.go @@ -231,13 +231,7 @@ func (s3a *S3ApiServer) GetObjectAttributesHandler(w http.ResponseWriter, r *htt // AWS also asks for the version read grant on a versioned attributes // request: s3:GetObjectVersionAttributes alone is not sufficient. if versionId != "" { - if identity, ok := s3_constants.GetIdentityFromContext(r).(*Identity); ok && identity != nil { - if errCode := s3a.iam.VerifyActionPermission(r, identity, Action(s3_constants.S3_ACTION_GET_OBJECT_VERSION), bucket, object); errCode != s3err.ErrNone { - s3err.WriteErrorResponse(w, r, errCode) - return - } - } - if errCode := s3a.recheckPolicyWithObjectEntry(r, bucket, object, s3_constants.S3_ACTION_GET_OBJECT_VERSION, entry.Extended, "GetObjectAttributesHandler"); errCode != s3err.ErrNone { + if errCode := s3a.checkVersionReadGrant(r, bucket, object, versionId, entry.Extended); errCode != s3err.ErrNone { s3err.WriteErrorResponse(w, r, errCode) return } diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 126cb124de..3156d506b7 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -678,6 +678,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