diff --git a/weed/s3api/s3api_object_handlers_postpolicy.go b/weed/s3api/s3api_object_handlers_postpolicy.go index 1e8705ba11..8ebcc71097 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy.go +++ b/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) } diff --git a/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go b/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go index 553ff2f0a5..a0e0f7e4ce 100644 --- a/weed/s3api/s3api_object_handlers_postpolicy_authz_test.go +++ b/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") +}