diff --git a/weed/iam/helpers.go b/weed/iam/helpers.go index a80ecf994..ad3cd1f0f 100644 --- a/weed/iam/helpers.go +++ b/weed/iam/helpers.go @@ -143,8 +143,14 @@ var baseS3ActionMap = map[string]string{ "UploadPart": s3_constants.ACTION_WRITE, "CompleteMultipartUpload": s3_constants.ACTION_WRITE, "AbortMultipartUpload": s3_constants.ACTION_WRITE, - "ListMultipartUploads": s3_constants.ACTION_WRITE, - "ListParts": s3_constants.ACTION_WRITE, + "ListMultipartUploads": s3_constants.ACTION_LIST_MULTIPART_UPLOADS, + "ListParts": s3_constants.ACTION_LIST_PARTS, + // AWS IAM names for the multipart listing operations get their own + // action names so a policy grant cannot widen into a general Read + // or Write grant; VerifyActionPermission matches them against the + // resolved request action. + "ListBucketMultipartUploads": s3_constants.ACTION_LIST_MULTIPART_UPLOADS, + "ListMultipartUploadParts": s3_constants.ACTION_LIST_PARTS, // Retention and legal hold operations "GetObjectRetention": s3_constants.ACTION_GET_OBJECT_RETENTION, "PutObjectRetention": s3_constants.ACTION_PUT_OBJECT_RETENTION, @@ -209,6 +215,10 @@ func MapToIdentitiesAction(action string) string { return "PutBucketPolicy" case s3_constants.ACTION_DELETE_BUCKET_POLICY: return "DeleteBucketPolicy" + case s3_constants.ACTION_LIST_MULTIPART_UPLOADS: + return "ListBucketMultipartUploads" + case s3_constants.ACTION_LIST_PARTS: + return "ListMultipartUploadParts" default: return "" } diff --git a/weed/s3api/auth_credentials.go b/weed/s3api/auth_credentials.go index 2884542eb..11e575806 100644 --- a/weed/s3api/auth_credentials.go +++ b/weed/s3api/auth_credentials.go @@ -1813,6 +1813,27 @@ func isBucketListingRequest(r *http.Request, action Action) bool { return object == "" || object == "/" } +// multipartListingAction returns the scoped action for a request the router +// sends to ListObjectPartsHandler or ListMultipartUploadsHandler, or "" for +// any other request. It follows the route order in s3api_server.go and reads +// the object from the route vars, so a ?prefix promoted into the +// authorization object, or extra subresource parameters such as ?policy, do +// not change which operation the grant has to name. +func multipartListingAction(r *http.Request, action Action) Action { + if action != s3_constants.ACTION_READ || r == nil || r.Method != http.MethodGet || r.URL == nil { + return "" + } + query := r.URL.Query() + _, object := s3_constants.GetBucketAndObject(r) + if object != "" && object != "/" && query.Has("uploadId") { + return s3_constants.ACTION_LIST_PARTS + } + if query.Has("uploads") { + return s3_constants.ACTION_LIST_MULTIPART_UPLOADS + } + return "" +} + // authRequestWithAuthType authenticates and then authorizes a request for a given action. func (iam *IdentityAccessManagement) authRequestWithAuthType(r *http.Request, action Action) (*Identity, s3err.ErrorCode, authType) { identity, s3Err, reqAuthType := iam.authenticateRequestInternal(r) @@ -2826,7 +2847,12 @@ func (iam *IdentityAccessManagement) VerifyActionPermission(r *http.Request, ide } case authorizeViaLegacyActions: if !identity.CanDo(action, bucket, object) { - return s3err.ErrAccessDenied + // The multipart listing routes are gated on ACTION_READ, but a + // grant may name the listing operation itself. + listing := multipartListingAction(r, action) + if listing == "" || !identity.CanDo(listing, bucket, object) { + return s3err.ErrAccessDenied + } } return s3err.ErrNone default: diff --git a/weed/s3api/s3_constants/s3_actions.go b/weed/s3api/s3_constants/s3_actions.go index 468ae8809..d90b8cfd3 100644 --- a/weed/s3api/s3_constants/s3_actions.go +++ b/weed/s3api/s3_constants/s3_actions.go @@ -20,6 +20,11 @@ const ( ACTION_DELETE_BUCKET_POLICY = "DeleteBucketPolicy" ACTION_PUT_BUCKET_QUOTA = "PutBucketQuota" ACTION_GET_BUCKET_QUOTA = "GetBucketQuota" + // Scoped to their single operation: the multipart listing routes are + // gated on ACTION_READ for compatibility, but a policy granting one of + // these names must not widen into a general Read grant. + ACTION_LIST_MULTIPART_UPLOADS = "ListBucketMultipartUploads" + ACTION_LIST_PARTS = "ListMultipartUploadParts" SeaweedStorageDestinationHeader = "x-seaweedfs-destination" MultipartUploadsFolder = ".uploads" diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index 685d575bc..b032bd35b 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -1194,6 +1194,9 @@ func (e *EmbeddedIamApi) GetUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values resource = fmt.Sprintf("arn:aws:s3:::%s", act[1]) } s3Action := fmt.Sprintf("s3:%s", iamMapToIdentitiesAction(act[0])) + if iamMapToIdentitiesAction(act[0]) == "" { + s3Action = fmt.Sprintf("s3:%s", act[0]) + } // Dedupe actions per resource if seenAction[resource] == nil { seenAction[resource] = make(map[string]bool) diff --git a/weed/s3api/s3api_embedded_iam_test.go b/weed/s3api/s3api_embedded_iam_test.go index 5500a0e0b..55ff8ed9a 100644 --- a/weed/s3api/s3api_embedded_iam_test.go +++ b/weed/s3api/s3api_embedded_iam_test.go @@ -2160,6 +2160,114 @@ func TestEmbeddedIamGetActionsPrefixScopedResource(t *testing.T) { "PutObject outside the allowed prefix should be denied") } +// TestEmbeddedIamGetActionsMultipartListing covers the AWS names for +// listing multipart uploads and parts: they grant only the named listing +// operation, never a general Read on the bucket. +func TestEmbeddedIamGetActionsMultipartListing(t *testing.T) { + api := NewEmbeddedIamApiForTest() + iam := NewIdentityAccessManagementWithStore(&S3ApiServerOption{}, nil, "memory") + + // Check each action on its own so a correct mapping for one cannot hide + // a wrong mapping for the other. + for _, tc := range []struct { + policyAction string + grant string + url string + object string + }{ + {"s3:ListBucketMultipartUploads", "ListBucketMultipartUploads:demo", "/demo?uploads", ""}, + {"s3:ListMultipartUploadParts", "ListMultipartUploadParts:demo", "/demo/key?uploadId=u1", "key"}, + } { + t.Run(tc.policyAction, func(t *testing.T) { + policyDoc := `{ + "Version": "2012-10-17", + "Statement": [{ + "Effect": "Allow", + "Action": ["` + tc.policyAction + `"], + "Resource": ["arn:aws:s3:::demo", "arn:aws:s3:::demo/*"] + }] + }` + + policy, err := api.GetPolicyDocument(&policyDoc) + require.NoError(t, err) + + actions, err := api.getActions(&policy) + require.NoError(t, err) + require.NotEmpty(t, actions) + for _, a := range actions { + assert.Equal(t, tc.grant, a, + "%s should only grant its own listing operation", tc.policyAction) + } + + identity := &Identity{Name: "bob"} + for _, a := range actions { + identity.Actions = append(identity.Actions, Action(a)) + } + + // The listing routes are gated on ACTION_READ, but the grant must + // not authorize object reads or anything else. + assert.False(t, identity.CanDo(ACTION_READ, "demo", "key"), + "a listing grant must not allow object reads") + assert.False(t, identity.CanDo(ACTION_WRITE, "demo", "key"), + "a listing grant must not allow writes") + assert.False(t, identity.CanDo(ACTION_LIST, "demo", ""), + "a listing grant must not allow ListObjects") + + routed := func(url, object string) *http.Request { + return mux.SetURLVars(httptest.NewRequest(http.MethodGet, url, nil), + map[string]string{"bucket": "demo", "object": object}) + } + + req := routed(tc.url, tc.object) + assert.Equal(t, s3err.ErrNone, iam.VerifyActionPermission(req, identity, ACTION_READ, "demo", tc.object), + "the listing action should be authorized") + assert.Equal(t, s3err.ErrAccessDenied, iam.VerifyActionPermission(req, identity, ACTION_READ, "other", tc.object), + "the grant must stay scoped to the policy's bucket") + + getReq := routed("/demo/key", "key") + assert.Equal(t, s3err.ErrAccessDenied, iam.VerifyActionPermission(getReq, identity, ACTION_READ, "demo", "key"), + "the grant must not authorize GetObject") + + // Other ACTION_READ routes must not be authorized by a listing + // grant, whatever query parameters ride along. + policyReq := routed("/demo?policy", "") + assert.Equal(t, s3err.ErrAccessDenied, iam.VerifyActionPermission(policyReq, identity, ACTION_READ, "demo", ""), + "the grant must not authorize GetBucketPolicy") + }) + } + + grant := func(a string) *Identity { return &Identity{Name: "bob", Actions: []Action{Action(a)}} } + routed := func(url, object string) *http.Request { + return mux.SetURLVars(httptest.NewRequest(http.MethodGet, url, nil), + map[string]string{"bucket": "demo", "object": object}) + } + + // authRequestWithAuthType promotes ?prefix into the object for upload + // listings; the operation must still come from the route, not the prefix. + t.Run("upload listing with prefix", func(t *testing.T) { + req := routed("/demo?uploads&prefix=foo/", "") + assert.Equal(t, s3err.ErrNone, + iam.VerifyActionPermission(req, grant("ListBucketMultipartUploads:demo"), ACTION_READ, "demo", "foo/")) + assert.Equal(t, s3err.ErrAccessDenied, + iam.VerifyActionPermission(req, grant("ListMultipartUploadParts:demo"), ACTION_READ, "demo", "foo/")) + }) + + // The ?uploads route is registered before ?policy, so this reaches the + // upload listing handler and needs the upload listing grant. + t.Run("upload listing with extra subresource", func(t *testing.T) { + req := routed("/demo?policy&uploads", "") + assert.Equal(t, s3err.ErrNone, + iam.VerifyActionPermission(req, grant("ListBucketMultipartUploads:demo"), ACTION_READ, "demo", "")) + assert.Equal(t, s3err.ErrAccessDenied, + iam.VerifyActionPermission(req, grant("GetBucketPolicy:demo"), ACTION_READ, "demo", "")) + }) + + // Policies rebuilt from these grants must use valid action names. + for _, a := range []string{ACTION_LIST_MULTIPART_UPLOADS, ACTION_LIST_PARTS} { + assert.Equal(t, a, iamMapToIdentitiesAction(a)) + } +} + // TestEmbeddedIamPutUserPolicyAllResourceWildcard reproduces issue #9209: // an AWS-style policy using "Action":"s3:*" with a bare "Resource":"*" is // valid AWS IAM syntax (meaning "any resource") but was rejected with