mirror of
https://github.com/versity/versitygw.git
synced 2026-09-25 01:14:22 +00:00
fix: authorize browser-based POST object uploads against the object ARN
`POSTObject` called `verifyAccess` without an `Object`, and `AuthorizePublicBucketAccess` took the object name from the request path, which is only `/bucket` for a `POST`. So both evaluated `s3:PutObject` against the bucket ARN `arn:aws:s3:::bucket`. `PutBucketPolicy` rejects `s3:PutObject` on a bucket resource, so no valid policy could ever match a `POST` upload. An `Allow` on `arn:aws:s3:::bucket/*` or on a key prefix never applied, and a `Deny` on a key prefix never applied either. A public-write ACL or a broad identity policy could therefore upload through `POST` to keys a `Deny` protects, which `PutObject` refuses. Identity policies evaluated by the standalone IAM service had the same mismatch. S3 treats `POST` as an alternate form of `PUT` and authorizes it as `s3:PutObject` on the ARN of the object named by the form's `key` field. `POSTObject` now passes that `key` as `Object` to `verifyAccess`, matching `PutObject`. For anonymous `POST` requests, `AuthorizePublicBucketAccess` now uses the `key` that `AuthorizePostObject` has already parsed.
This commit is contained in:
@@ -131,6 +131,9 @@ func (c S3ApiController) POSTObject(ctx fiber.Ctx) (*Response, error) {
|
||||
|
||||
key := parsed.Fields["key"]
|
||||
|
||||
// A POST upload is an s3:PutObject on the object named by the form's
|
||||
// key field, so it is authorized against that object's ARN — the same
|
||||
// resource PutObject is — not the bucket's.
|
||||
err := c.verifyAccess(ctx,
|
||||
auth.AccessOptions{
|
||||
Acl: parsedAcl,
|
||||
@@ -138,6 +141,7 @@ func (c S3ApiController) POSTObject(ctx fiber.Ctx) (*Response, error) {
|
||||
IsRoot: isRoot,
|
||||
Acc: acct,
|
||||
Bucket: bucket,
|
||||
Object: key,
|
||||
Actions: []auth.Action{auth.PutObjectAction},
|
||||
IsPublicRequest: IsBucketPublic,
|
||||
})
|
||||
|
||||
@@ -57,6 +57,15 @@ func AuthorizePublicBucketAccess(be backend.Backend, s3action string, policyPerm
|
||||
}
|
||||
|
||||
bucket, object := parsePath(ctx.Path())
|
||||
if s3action == metrics.ActionPostObject {
|
||||
// A POST upload is addressed to the bucket; the object it writes
|
||||
// is named by the form's key field instead, which
|
||||
// AuthorizePostObject has already parsed. Authorize against that
|
||||
// object's ARN, as PutObject is.
|
||||
if parsed, ok := utils.ContextKeyObjectPostResult.Get(ctx).(PostObjectResult); ok {
|
||||
object = parsed.Fields["key"]
|
||||
}
|
||||
}
|
||||
err := auth.VerifyPublicAccess(ctx, be, policyPermission, permission, bucket, object)
|
||||
if err != nil {
|
||||
if s3action == metrics.ActionHeadBucket {
|
||||
|
||||
@@ -309,6 +309,125 @@ func PostObject_access_denied(s *S3Conf) error {
|
||||
})
|
||||
}
|
||||
|
||||
// PostObject_bucket_policy_object_resource covers a user whose only upload
|
||||
// grant is a bucket policy scoped to a key prefix. A POST upload is an
|
||||
// s3:PutObject on the object its key field names, so it is authorized
|
||||
// against that object's ARN, as PutObject is: the grant on
|
||||
// "bucket/uploads/*" allows a key under uploads/ and denies any other.
|
||||
func PostObject_bucket_policy_object_resource(s *S3Conf) error {
|
||||
testName := "PostObject_bucket_policy_object_resource"
|
||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||
testuser := getUser("user")
|
||||
if err := createUsers(s, []user{testuser}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := putBucketPolicyDoc(s, bucket, bucketStatement{
|
||||
Effect: "Allow",
|
||||
Principal: testuser.access,
|
||||
Action: "s3:PutObject",
|
||||
Resource: fmt.Sprintf("arn:aws:s3:::%s/uploads/*", bucket),
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
post := func(key string) (*http.Response, error) {
|
||||
return sendPostObject(PostRequestConfig{
|
||||
bucket: bucket,
|
||||
key: key,
|
||||
access: testuser.access,
|
||||
secret: testuser.secret,
|
||||
s3Conf: s,
|
||||
fileContent: []byte("data"),
|
||||
})
|
||||
}
|
||||
|
||||
allowedKey := "uploads/my-obj"
|
||||
resp, err := post(allowedKey)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkPostObjectSuccess(resp); err != nil {
|
||||
return fmt.Errorf("POST %s: %w", allowedKey, err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
|
||||
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
|
||||
Bucket: &bucket,
|
||||
Key: &allowedKey,
|
||||
})
|
||||
cancel()
|
||||
if err != nil {
|
||||
return fmt.Errorf("expected %s to be uploaded: %w", allowedKey, err)
|
||||
}
|
||||
|
||||
resp, err = post("private/my-obj")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return checkHTTPResponseApiErr(resp, s3err.GetAPIError(s3err.ErrAccessDenied))
|
||||
})
|
||||
}
|
||||
|
||||
// PostObject_bucket_policy_explicit_deny covers a Deny statement scoped to
|
||||
// a key prefix overriding a bucket-wide Allow for POST uploads, the way it
|
||||
// does for PutObject: the Deny on "bucket/private/*" matches the object ARN
|
||||
// a POST to a private/ key is authorized against, and nothing else.
|
||||
func PostObject_bucket_policy_explicit_deny(s *S3Conf) error {
|
||||
testName := "PostObject_bucket_policy_explicit_deny"
|
||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||
testuser := getUser("user")
|
||||
if err := createUsers(s, []user{testuser}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := putBucketPolicyDoc(s, bucket,
|
||||
bucketStatement{
|
||||
Effect: "Allow",
|
||||
Principal: testuser.access,
|
||||
Action: "s3:PutObject",
|
||||
Resource: fmt.Sprintf("arn:aws:s3:::%s/*", bucket),
|
||||
},
|
||||
bucketStatement{
|
||||
Effect: "Deny",
|
||||
Principal: testuser.access,
|
||||
Action: "s3:PutObject",
|
||||
Resource: fmt.Sprintf("arn:aws:s3:::%s/private/*", bucket),
|
||||
},
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
post := func(key string) (*http.Response, error) {
|
||||
return sendPostObject(PostRequestConfig{
|
||||
bucket: bucket,
|
||||
key: key,
|
||||
access: testuser.access,
|
||||
secret: testuser.secret,
|
||||
s3Conf: s,
|
||||
fileContent: []byte("data"),
|
||||
})
|
||||
}
|
||||
|
||||
allowedKey := "public/my-obj"
|
||||
resp, err := post(allowedKey)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkPostObjectSuccess(resp); err != nil {
|
||||
return fmt.Errorf("POST %s: %w", allowedKey, err)
|
||||
}
|
||||
|
||||
deniedKey := "private/my-obj"
|
||||
resp, err = post(deniedKey)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return checkHTTPResponseApiErr(resp, s3err.GetExplicitDenyAccessErr(testuser.access, "s3:PutObject",
|
||||
fmt.Sprintf("arn:aws:s3:::%s/%s", bucket, deniedKey), "a resource-based policy"))
|
||||
})
|
||||
}
|
||||
|
||||
func PostObject_invalid_object_names(s *S3Conf) error {
|
||||
testName := "PostObject_invalid_object_names"
|
||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||
|
||||
@@ -1746,6 +1746,7 @@ func TestS3IAMAccessControl(ts *TestState) {
|
||||
ts.Run(S3IAMAccessControl_identity_policy_action_wildcards)
|
||||
ts.Run(S3IAMAccessControl_identity_policy_resource_scoping)
|
||||
ts.Run(S3IAMAccessControl_identity_policy_bucket_vs_object_arn)
|
||||
ts.Run(S3IAMAccessControl_post_object_identity_policy_resource_scoping)
|
||||
ts.Run(S3IAMAccessControl_identity_policy_not_action_and_not_resource)
|
||||
ts.Run(S3IAMAccessControl_identity_policy_explicit_deny_wins)
|
||||
ts.Run(S3IAMAccessControl_multiple_inline_policies_combine)
|
||||
@@ -1935,6 +1936,8 @@ func TestPublicBuckets(ts *TestState) {
|
||||
}
|
||||
ts.Run(PublicBucket_public_acl)
|
||||
ts.Run(PublicBucket_policy_deny_overrides_public_acl)
|
||||
ts.Run(PublicBucket_post_object_policy)
|
||||
ts.Run(PublicBucket_post_object_policy_deny_overrides_public_acl)
|
||||
ts.Run(PublicBucket_signed_streaming_payload)
|
||||
ts.Run(PublicBucket_incorrect_sha256_hash)
|
||||
}
|
||||
@@ -2118,6 +2121,8 @@ func TestPostObject(ts *TestState) {
|
||||
ts.Run(PostObject_signature_mismatch)
|
||||
ts.Run(PostObject_expired_due_to_date)
|
||||
ts.Run(PostObject_access_denied)
|
||||
ts.Run(PostObject_bucket_policy_object_resource)
|
||||
ts.Run(PostObject_bucket_policy_explicit_deny)
|
||||
ts.Run(PostObject_invalid_object_names)
|
||||
ts.Run(PostObject_policy_access_control)
|
||||
ts.Run(PostObject_policy_expired)
|
||||
@@ -2248,6 +2253,7 @@ func GetIntTests() IntTests {
|
||||
"S3IAMAccessControl_identity_policy_action_wildcards": S3IAMAccessControl_identity_policy_action_wildcards,
|
||||
"S3IAMAccessControl_identity_policy_resource_scoping": S3IAMAccessControl_identity_policy_resource_scoping,
|
||||
"S3IAMAccessControl_identity_policy_bucket_vs_object_arn": S3IAMAccessControl_identity_policy_bucket_vs_object_arn,
|
||||
"S3IAMAccessControl_post_object_identity_policy_resource_scoping": S3IAMAccessControl_post_object_identity_policy_resource_scoping,
|
||||
"S3IAMAccessControl_identity_policy_not_action_and_not_resource": S3IAMAccessControl_identity_policy_not_action_and_not_resource,
|
||||
"S3IAMAccessControl_identity_policy_explicit_deny_wins": S3IAMAccessControl_identity_policy_explicit_deny_wins,
|
||||
"S3IAMAccessControl_multiple_inline_policies_combine": S3IAMAccessControl_multiple_inline_policies_combine,
|
||||
@@ -3501,6 +3507,8 @@ func GetIntTests() IntTests {
|
||||
"PublicBucket_public_object_policy": PublicBucket_public_object_policy,
|
||||
"PublicBucket_public_acl": PublicBucket_public_acl,
|
||||
"PublicBucket_policy_deny_overrides_public_acl": PublicBucket_policy_deny_overrides_public_acl,
|
||||
"PublicBucket_post_object_policy": PublicBucket_post_object_policy,
|
||||
"PublicBucket_post_object_policy_deny_overrides_public_acl": PublicBucket_post_object_policy_deny_overrides_public_acl,
|
||||
"PublicBucket_signed_streaming_payload": PublicBucket_signed_streaming_payload,
|
||||
"PublicBucket_incorrect_sha256_hash": PublicBucket_incorrect_sha256_hash,
|
||||
"PutBucketVersioning_non_existing_bucket": PutBucketVersioning_non_existing_bucket,
|
||||
@@ -3673,6 +3681,8 @@ func GetIntTests() IntTests {
|
||||
"PostObject_signature_mismatch": PostObject_signature_mismatch,
|
||||
"PostObject_expired_due_to_date": PostObject_expired_due_to_date,
|
||||
"PostObject_access_denied": PostObject_access_denied,
|
||||
"PostObject_bucket_policy_object_resource": PostObject_bucket_policy_object_resource,
|
||||
"PostObject_bucket_policy_explicit_deny": PostObject_bucket_policy_explicit_deny,
|
||||
"PostObject_invalid_object_names": PostObject_invalid_object_names,
|
||||
"PostObject_policy_access_control": PostObject_policy_access_control,
|
||||
"PostObject_policy_expired": PostObject_policy_expired,
|
||||
|
||||
@@ -2490,6 +2490,94 @@ func PublicBucket_policy_deny_overrides_public_acl(s *S3Conf) error {
|
||||
}, withAnonymousClient(), withOwnership(types.ObjectOwnershipBucketOwnerPreferred))
|
||||
}
|
||||
|
||||
// PublicBucket_post_object_policy covers anonymous POST uploads to a bucket
|
||||
// whose policy grants public s3:PutObject on a key prefix. The POST is
|
||||
// addressed to the bucket, but it is authorized against the ARN of the
|
||||
// object its key field names, as PutObject is: a key under uploads/ is
|
||||
// allowed and any other denied.
|
||||
func PublicBucket_post_object_policy(s *S3Conf) error {
|
||||
testName := "PublicBucket_post_object_policy"
|
||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||
if err := putBucketPolicyDoc(s, bucket, bucketStatement{
|
||||
Effect: "Allow",
|
||||
Principal: "*",
|
||||
Action: "s3:PutObject",
|
||||
Resource: fmt.Sprintf("arn:aws:s3:::%s/uploads/*", bucket),
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
allowedKey := "uploads/my-obj"
|
||||
resp, err := sendAnonymousPostObject(s, bucket, allowedKey, []byte("data"))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkPostObjectSuccess(resp); err != nil {
|
||||
return fmt.Errorf("POST %s: %w", allowedKey, err)
|
||||
}
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
|
||||
_, err = s3client.HeadObject(ctx, &s3.HeadObjectInput{
|
||||
Bucket: &bucket,
|
||||
Key: &allowedKey,
|
||||
})
|
||||
cancel()
|
||||
if err != nil {
|
||||
return fmt.Errorf("expected %s to be uploaded: %w", allowedKey, err)
|
||||
}
|
||||
|
||||
resp, err = sendAnonymousPostObject(s, bucket, "private/my-obj", []byte("data"))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return checkHTTPResponseApiErr(resp, s3err.GetAPIError(s3err.ErrAccessDenied))
|
||||
})
|
||||
}
|
||||
|
||||
// PublicBucket_post_object_policy_deny_overrides_public_acl covers a public
|
||||
// Deny scoped to a key prefix on a bucket whose ACL is public-read-write:
|
||||
// an anonymous POST to a key under private/ matches the Deny on its object
|
||||
// ARN and is refused, rather than falling through to the ACL's public
|
||||
// write grant, which still allows a POST to any other key.
|
||||
func PublicBucket_post_object_policy_deny_overrides_public_acl(s *S3Conf) error {
|
||||
testName := "PublicBucket_post_object_policy_deny_overrides_public_acl"
|
||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
|
||||
_, err := s3client.PutBucketAcl(ctx, &s3.PutBucketAclInput{
|
||||
Bucket: &bucket,
|
||||
ACL: types.BucketCannedACLPublicReadWrite,
|
||||
})
|
||||
cancel()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if err := putBucketPolicyDoc(s, bucket, bucketStatement{
|
||||
Effect: "Deny",
|
||||
Principal: "*",
|
||||
Action: "s3:PutObject",
|
||||
Resource: fmt.Sprintf("arn:aws:s3:::%s/private/*", bucket),
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
allowedKey := "public/my-obj"
|
||||
resp, err := sendAnonymousPostObject(s, bucket, allowedKey, []byte("data"))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkPostObjectSuccess(resp); err != nil {
|
||||
return fmt.Errorf("POST %s: %w", allowedKey, err)
|
||||
}
|
||||
|
||||
resp, err = sendAnonymousPostObject(s, bucket, "private/my-obj", []byte("data"))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return checkHTTPResponseApiErr(resp, s3err.GetAPIError(s3err.ErrAccessDenied))
|
||||
}, withOwnership(types.ObjectOwnershipBucketOwnerPreferred))
|
||||
}
|
||||
|
||||
func PublicBucket_signed_streaming_payload(s *S3Conf) error {
|
||||
testName := "PublicBucket_signed_streaming_payload"
|
||||
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
|
||||
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
@@ -239,6 +240,50 @@ func S3IAMAccessControl_identity_policy_bucket_vs_object_arn(s *S3Conf) error {
|
||||
})
|
||||
}
|
||||
|
||||
// S3IAMAccessControl_post_object_identity_policy_resource_scoping verifies
|
||||
// a POST upload evaluates against the ARN of the object its key field
|
||||
// names, as PutObject does, not the bucket ARN the request is addressed to:
|
||||
// an s3:PutObject grant on "bucket/allowed/*" allows a POST to a key under
|
||||
// allowed/ and denies any other.
|
||||
func S3IAMAccessControl_post_object_identity_policy_resource_scoping(s *S3Conf) error {
|
||||
testName := "S3IAMAccessControl_post_object_identity_policy_resource_scoping"
|
||||
return s3IAMActionHandler(s, testName, func(root *iam.Client, bucket string) error {
|
||||
user, cleanup, err := newS3IAMUser(root, s, map[string]string{
|
||||
"p": policyDoc(accessStatement{
|
||||
Effect: "Allow", Action: actS3PutObject,
|
||||
Resource: objectArn(bucket, "allowed/*"),
|
||||
}),
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer cleanup()
|
||||
|
||||
post := func(key string) (*http.Response, error) {
|
||||
return sendPostObject(PostRequestConfig{
|
||||
bucket: bucket,
|
||||
key: key,
|
||||
s3Conf: &user.conf,
|
||||
fileContent: []byte("data"),
|
||||
})
|
||||
}
|
||||
|
||||
resp, err := post("allowed/obj")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := checkPostObjectSuccess(resp); err != nil {
|
||||
return fmt.Errorf("expected POST on the matching key to be allowed: %w", err)
|
||||
}
|
||||
|
||||
resp, err = post("denied/obj")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return checkHTTPResponseApiErr(resp, wantImplicitDeny(user.arn, actS3PutObject, objectArn(bucket, "denied/obj")))
|
||||
})
|
||||
}
|
||||
|
||||
// S3IAMAccessControl_identity_policy_not_action_and_not_resource verifies
|
||||
// NotAction and NotResource grant everything *except* what they name.
|
||||
func S3IAMAccessControl_identity_policy_not_action_and_not_resource(s *S3Conf) error {
|
||||
|
||||
@@ -3938,6 +3938,36 @@ func sendPostObject(input PostRequestConfig) (*http.Response, error) {
|
||||
return input.s3Conf.httpClient.Do(req)
|
||||
}
|
||||
|
||||
// sendAnonymousPostObject sends an unauthenticated POST object request to
|
||||
// /{bucket}: the form carries key and the file, but none of the five
|
||||
// form-based auth fields.
|
||||
func sendAnonymousPostObject(s *S3Conf, bucket, key string, fileContent []byte) (*http.Response, error) {
|
||||
return sendPostObject(PostRequestConfig{
|
||||
bucket: bucket,
|
||||
key: key,
|
||||
s3Conf: s,
|
||||
fileContent: fileContent,
|
||||
extraFields: map[string]string{
|
||||
"x-amz-algorithm": "",
|
||||
"x-amz-credential": "",
|
||||
"x-amz-date": "",
|
||||
"policy": "",
|
||||
"x-amz-signature": "",
|
||||
},
|
||||
})
|
||||
}
|
||||
|
||||
// checkPostObjectSuccess checks that resp is the 204 No Content a POST
|
||||
// object upload returns by default, reporting the response body otherwise.
|
||||
func checkPostObjectSuccess(resp *http.Response) error {
|
||||
defer resp.Body.Close()
|
||||
if resp.StatusCode != http.StatusNoContent {
|
||||
body, _ := io.ReadAll(resp.Body)
|
||||
return fmt.Errorf("expected status 204, instead got %d: %s", resp.StatusCode, body)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func newPostObjectRequest(input PostRequestConfig) (*http.Request, map[string]string, error) {
|
||||
if input.date.IsZero() {
|
||||
input.date = time.Now().UTC()
|
||||
|
||||
Reference in New Issue
Block a user