s3api: check the version read grant on a synthesized versioned GET

The versioned attributes check passed the literal s3:GetObjectVersion
action, but both policy paths re-resolve the action from the request, so
the attributes query parameter mapped it back to
s3:GetObjectVersionAttributes and a lone version-read Deny never fired.
The literal action also reached the legacy CanDo path, where a plain
Read grant no longer matched. Evaluate the grant on a request shaped
like a plain versioned GET instead.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
Devin
2026-10-21 14:51:55 -07:00
co-authored by Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
parent d7c608ec51
commit 8e92be099a
3 changed files with 77 additions and 7 deletions
+61
View File
@@ -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))
}
@@ -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
}
+15
View File
@@ -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