From 06675142208d40beb3770558c72b72c2fa125193 Mon Sep 17 00:00:00 2001 From: niksis02 Date: Wed, 23 Sep 2026 22:53:54 +0400 Subject: [PATCH] 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. --- s3api/controllers/bucket-post.go | 4 + s3api/middlewares/public-bucket.go | 9 ++ tests/integration/PostObject.go | 119 +++++++++++++++++++++ tests/integration/group-tests.go | 10 ++ tests/integration/public_bucket.go | 88 +++++++++++++++ tests/integration/s3_iam_access_control.go | 45 ++++++++ tests/integration/utils.go | 30 ++++++ 7 files changed, 305 insertions(+) diff --git a/s3api/controllers/bucket-post.go b/s3api/controllers/bucket-post.go index 721ad7cf..1fa97d7b 100644 --- a/s3api/controllers/bucket-post.go +++ b/s3api/controllers/bucket-post.go @@ -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, }) diff --git a/s3api/middlewares/public-bucket.go b/s3api/middlewares/public-bucket.go index 6e90bc6a..69330fe9 100644 --- a/s3api/middlewares/public-bucket.go +++ b/s3api/middlewares/public-bucket.go @@ -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 { diff --git a/tests/integration/PostObject.go b/tests/integration/PostObject.go index 722aa85e..e8609f94 100644 --- a/tests/integration/PostObject.go +++ b/tests/integration/PostObject.go @@ -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 { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index cf3493c6..a8c1bbed 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -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, diff --git a/tests/integration/public_bucket.go b/tests/integration/public_bucket.go index a709bbba..efb47cb2 100644 --- a/tests/integration/public_bucket.go +++ b/tests/integration/public_bucket.go @@ -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 { diff --git a/tests/integration/s3_iam_access_control.go b/tests/integration/s3_iam_access_control.go index 5db50930..9ca1232f 100644 --- a/tests/integration/s3_iam_access_control.go +++ b/tests/integration/s3_iam_access_control.go @@ -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 { diff --git a/tests/integration/utils.go b/tests/integration/utils.go index e2a9650c..f77d4699 100644 --- a/tests/integration/utils.go +++ b/tests/integration/utils.go @@ -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()