s3: authorize POST policy uploads through the shared object check (#12047)

* s3: authorize POST policy uploads through the shared object check

doesPolicySignatureMatch gated a verified POST Object signature on the
identity's legacy Write action, which cannot represent attached IAM
policies, STS session policies, or key-scoped grants. Users authorized
only by an attached policy (e.g. s3:PutObject on bucket/*) were denied
with AccessDenied even though the equivalent PUT succeeds.

The handler already runs AuthorizeObjectWrite against the real object
key after the form's x-amz-* fields are promoted to headers, so the
coarse check only shadowed the full evaluation. Signature verification
now just returns the identity; authorization stays in one place.

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

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* test: POST Object authorizes attached-policy identities, still denies others

Cover the signature check and AuthorizeObjectWrite for an identity whose
only write grant comes from an attached IAM policy, plus the deny leg
for an identity whose attached policy omits s3:PutObject.

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

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

---------

Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>

weed/s3api/s3api_object_handlers_postpolicy.go

@@ -139,7 +139,7 @@ func (s3a *S3ApiServer) PostPolicyBucketHandler(w http.ResponseWriter, r *http.R
 	// Forward validated POST form fields to the underlying PUT as headers.
 	applyPostPolicyFormHeaders(r, formValues)
 
-	// Authorize the object like the PUT path; the coarse Write check above does not consult the bucket policy.
+	// Authorize the object like the PUT path.
 	if errCode := s3a.iam.AuthorizeObjectWrite(r, identity, bucket, object); errCode != s3err.ErrNone {
 		s3err.WriteErrorResponse(w, r, refineDenialForBucketName(r, identity, errCode))
 		return
@@ -382,25 +382,15 @@ func getRedirectPostRawQuery(bucket, key, etag string) string {
 }
 
 // Check to see if Policy is signed correctly, returning the signing identity.
-// The verified form's access key is recorded in ctx's identity holder. The
-// identity's Write permission on the bucket is checked only once the signature
-// verifies, as on the header-signed path, so a form that fails verification
-// learns nothing about the key's permissions. A verified identity that may not
-// write is returned with ErrAccessDenied.
+// The verified form's access key is recorded in ctx's identity holder.
+// Write authorization happens later in the handler via AuthorizeObjectWrite,
+// which evaluates the identity's attached IAM policies, session policies, and
+// the bucket policy against the real object key; gating here on the legacy
+// Actions projection would deny identities those mechanisms allow.
 func (iam *IdentityAccessManagement) doesPolicySignatureMatch(ctx context.Context, formValues http.Header) (*Identity, s3err.ErrorCode) {
-	var identity *Identity
-	var errCode s3err.ErrorCode
 	// For SignV2 - Signature field will be valid
 	if _, ok := formValues["Signature"]; ok {
-		identity, errCode = iam.doesPolicySignatureV2Match(ctx, formValues)
-	} else {
-		identity, errCode = iam.doesPolicySignatureV4Match(ctx, formValues)
+		return iam.doesPolicySignatureV2Match(ctx, formValues)
 	}
-	if errCode != s3err.ErrNone {
-		return nil, errCode
-	}
-	if !identity.CanDo(s3_constants.ACTION_WRITE, formValues.Get("bucket"), "") {
-		return identity, s3err.ErrAccessDenied
-	}
-	return identity, s3err.ErrNone
+	return iam.doesPolicySignatureV4Match(ctx, formValues)
 }

weed/s3api/s3api_object_handlers_postpolicy_authz_test.go

@@ -32,6 +32,11 @@ const (
 	postAclAccessKey = "AKIAACLTESTTEST"
 	postAclSecretKey = "secret-key-for-acl-tests"
 	postAclPrincipal = "arn:aws:iam::000000000000:user/aclwriter"
+
+	postPolicyAccessKey = "AKIAPOLICYONLY0000"
+	postPolicySecretKey = "secret-key-for-policy-only"
+	postReadAccessKey   = "AKIAPOLICYREAD0000"
+	postReadSecretKey   = "secret-key-for-policy-read"
 )
 
 // newPostAuthzServer builds an S3ApiServer whose non-admin "tester" identity
@@ -274,3 +279,118 @@ func TestAuthorizeObjectAclWrite(t *testing.T) {
 		s3a.iam.AuthorizeObjectAclWrite(req, admin, postAuthzBucket, "acl-denied/secret.txt"),
 		"an admin is not confined by the bucket policy")
 }
+
+// newPostPolicyOnlyServer builds an S3ApiServer with identities that hold no
+// legacy Write action: "poster" carries an attached IAM policy allowing
+// s3:PutObject on the test bucket, and "reader" one allowing only s3:GetObject.
+func newPostPolicyOnlyServer(t *testing.T) *S3ApiServer {
+	t.Helper()
+
+	iam := &IdentityAccessManagement{
+		hashes:       make(map[string]*sync.Pool),
+		hashCounters: make(map[string]*int32),
+	}
+	err := iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{
+		Identities: []*iam_pb.Identity{{
+			Name:        "poster",
+			Account:     &iam_pb.Account{Id: "000000000000", DisplayName: "poster"},
+			Credentials: []*iam_pb.Credential{{AccessKey: postPolicyAccessKey, SecretKey: postPolicySecretKey}},
+			Actions:     []string{"List:" + postAuthzBucket},
+			PolicyNames: []string{"post-write"},
+		}, {
+			Name:        "reader",
+			Account:     &iam_pb.Account{Id: "000000000000", DisplayName: "reader"},
+			Credentials: []*iam_pb.Credential{{AccessKey: postReadAccessKey, SecretKey: postReadSecretKey}},
+			Actions:     []string{"Read:" + postAuthzBucket, "List:" + postAuthzBucket},
+			PolicyNames: []string{"post-read"},
+		}},
+		Policies: []*iam_pb.Policy{{
+			Name: "post-write",
+			Content: fmt.Sprintf(`{
+				"Version": "2012-10-17",
+				"Statement": [{
+					"Effect": "Allow",
+					"Action": ["s3:PutObject", "s3:GetObject", "s3:DeleteObject"],
+					"Resource": ["arn:aws:s3:::%s/*"]
+				}, {
+					"Effect": "Allow",
+					"Action": ["s3:ListBucket", "s3:GetBucketLocation"],
+					"Resource": ["arn:aws:s3:::%s"]
+				}]
+			}`, postAuthzBucket, postAuthzBucket),
+		}, {
+			Name: "post-read",
+			Content: fmt.Sprintf(`{
+				"Version": "2012-10-17",
+				"Statement": [{
+					"Effect": "Allow",
+					"Action": ["s3:GetObject"],
+					"Resource": ["arn:aws:s3:::%s/*"]
+				}]
+			}`, postAuthzBucket),
+		}},
+	})
+	require.NoError(t, err)
+
+	s3a := &S3ApiServer{
+		option: &S3ApiServerOption{BucketsPath: "/buckets"},
+		iam:    iam,
+	}
+	s3a.bucketRegistry = NewBucketRegistry(s3a)
+	s3a.bucketRegistry.setMetadataCache(&BucketMetaData{Name: postAuthzBucket, IsTableBucket: false})
+	return s3a
+}
+
+// postFormValues canonicalizes the signed request's multipart fields the way
+// extractPostPolicyFormValues does, for driving doesPolicySignatureMatch.
+func postFormValues(t *testing.T, r *http.Request) http.Header {
+	t.Helper()
+	require.NoError(t, r.ParseMultipartForm(1<<20))
+	formValues := make(http.Header)
+	for k, v := range r.MultipartForm.Value {
+		formValues[http.CanonicalHeaderKey(k)] = v
+	}
+	return formValues
+}
+
+// TestPostPolicySignatureAllowsAttachedPolicyIdentity signs a POST Object form
+// with a key whose holder has no legacy Write action - only an attached IAM
+// policy granting s3:PutObject - and asserts the signature check accepts it
+// and the shared object-write authorization allows the key. Gating the
+// signature check on the Actions projection used to fail this with
+// AccessDenied before authorization ever ran.
+func TestPostPolicySignatureAllowsAttachedPolicyIdentity(t *testing.T) {
+	s3a := newPostPolicyOnlyServer(t)
+
+	req := newSignedPostRequestFor(t, "uploads/pic.png", postPolicyAccessKey, postPolicySecretKey, nil)
+	identity, errCode := s3a.iam.doesPolicySignatureMatch(req.Context(), postFormValues(t, req))
+	require.Equal(t, s3err.ErrNone, errCode,
+		"a verified POST signature must not require the legacy Write action")
+	require.NotNil(t, identity)
+
+	assert.Equal(t, s3err.ErrNone,
+		s3a.iam.AuthorizeObjectWrite(req, identity, postAuthzBucket, "uploads/pic.png"),
+		"an attached IAM policy granting s3:PutObject must authorize the key like the PUT path")
+}
+
+// TestPostPolicyAttachedPolicyDeny covers the deny leg: a verified identity
+// whose attached policy does not grant s3:PutObject is still refused by
+// AuthorizeObjectWrite and by the full handler.
+func TestPostPolicyAttachedPolicyDeny(t *testing.T) {
+	s3a := newPostPolicyOnlyServer(t)
+
+	req := newSignedPostRequestFor(t, "uploads/pic.png", postReadAccessKey, postReadSecretKey, nil)
+	identity, errCode := s3a.iam.doesPolicySignatureMatch(req.Context(), postFormValues(t, req))
+	require.Equal(t, s3err.ErrNone, errCode)
+	require.NotNil(t, identity)
+
+	assert.Equal(t, s3err.ErrAccessDenied,
+		s3a.iam.AuthorizeObjectWrite(req, identity, postAuthzBucket, "uploads/pic.png"),
+		"an attached policy without s3:PutObject must still deny the upload")
+
+	rec := httptest.NewRecorder()
+	s3a.PostPolicyBucketHandler(rec, newSignedPostRequestFor(t, "uploads/pic.png", postReadAccessKey, postReadSecretKey, nil))
+	assert.Equal(t, http.StatusForbidden, rec.Code,
+		"POST from a read-only identity must stay AccessDenied, body: %s", rec.Body.String())
+	assert.Contains(t, rec.Body.String(), "AccessDenied")
+}