From 3e64326f2b19495ce947dd2b8a923b3a21f115cf Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 6 Nov 2026 09:12:00 -0800 Subject: [PATCH] s3: require their own actions to change a bucket's CORS or versioning (#12433) * s3: require their own actions to change a bucket's CORS or versioning PUT/DELETE ?cors and PUT ?versioning were authorized as Write, so an identity granted only object writes on a bucket could replace its CORS rules or suspend versioning. They now need PutBucketCors, DeleteBucketCors and PutBucketVersioning (or Admin), and inline policies granting s3:PutBucketCors, s3:DeleteBucketCors or s3:PutBucketVersioning map to those actions instead of Write. Co-Authored-By: Claude Opus 5.5 * s3: test that object Write cannot change bucket CORS or versioning Co-Authored-By: Claude Opus 5.5 * admin: offer the bucket CORS and versioning permissions Co-Authored-By: Claude Opus 5.5 * s3: test each bucket config action with its own identity Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- weed/admin/view/app/object_store_users.templ | 10 +++ .../view/app/object_store_users_templ.go | 2 +- weed/iam/helpers.go | 12 ++- weed/s3api/s3_action_resolver.go | 6 ++ weed/s3api/s3_constants/s3_actions.go | 3 + weed/s3api/s3api_server.go | 6 +- weed/s3api/s3api_server_routing_test.go | 77 +++++++++++++++++++ 7 files changed, 109 insertions(+), 7 deletions(-) diff --git a/weed/admin/view/app/object_store_users.templ b/weed/admin/view/app/object_store_users.templ index 51e2e58546..ec11b29633 100644 --- a/weed/admin/view/app/object_store_users.templ +++ b/weed/admin/view/app/object_store_users.templ @@ -245,6 +245,11 @@ templ ObjectStoreUsers(data dash.ObjectStoreUsersData) { + + + + + @@ -359,6 +364,11 @@ templ ObjectStoreUsers(data dash.ObjectStoreUsersData) { + + + + + diff --git a/weed/admin/view/app/object_store_users_templ.go b/weed/admin/view/app/object_store_users_templ.go index 811ab4a986..f627657a48 100644 --- a/weed/admin/view/app/object_store_users_templ.go +++ b/weed/admin/view/app/object_store_users_templ.go @@ -237,7 +237,7 @@ func ObjectStoreUsers(data dash.ObjectStoreUsersData) templ.Component { return templ_7745c5c3_Err } } - templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 24, "
Hold Ctrl/Cmd to select multiple permissions
Apply selected permissions to specific buckets or all buckets
Hold Ctrl/Cmd to select multiple buckets
Hold Ctrl/Cmd to select multiple policies
Edit User
Apply selected permissions to specific buckets or all buckets
Hold Ctrl/Cmd to select multiple buckets
Grants the permission and bucket pickers above cannot show. Uncheck a grant to remove it.
User Details
Manage Access Keys
Access Keys for

Leave blank to auto-generate.

") + templ_7745c5c3_Err = templruntime.WriteString(templ_7745c5c3_Buffer, 24, "
Hold Ctrl/Cmd to select multiple permissions
Apply selected permissions to specific buckets or all buckets
Hold Ctrl/Cmd to select multiple buckets
Hold Ctrl/Cmd to select multiple policies
Edit User
Apply selected permissions to specific buckets or all buckets
Hold Ctrl/Cmd to select multiple buckets
Grants the permission and bucket pickers above cannot show. Uncheck a grant to remove it.
User Details
Manage Access Keys
Access Keys for

Leave blank to auto-generate.

") if templ_7745c5c3_Err != nil { return templ_7745c5c3_Err } diff --git a/weed/iam/helpers.go b/weed/iam/helpers.go index 6d006478bb..ee34e2d5e1 100644 --- a/weed/iam/helpers.go +++ b/weed/iam/helpers.go @@ -135,10 +135,10 @@ var baseS3ActionMap = map[string]string{ "ListAllMyBuckets": s3_constants.ACTION_LIST, "GetBucketLocation": s3_constants.ACTION_READ, "GetBucketVersioning": s3_constants.ACTION_READ, - "PutBucketVersioning": s3_constants.ACTION_WRITE, + "PutBucketVersioning": s3_constants.ACTION_PUT_BUCKET_VERSIONING, "GetBucketCors": s3_constants.ACTION_READ, - "PutBucketCors": s3_constants.ACTION_WRITE, - "DeleteBucketCors": s3_constants.ACTION_WRITE, + "PutBucketCors": s3_constants.ACTION_PUT_BUCKET_CORS, + "DeleteBucketCors": s3_constants.ACTION_DELETE_BUCKET_CORS, "GetBucketNotification": s3_constants.ACTION_READ, "PutBucketNotification": s3_constants.ACTION_WRITE, "GetBucketObjectLockConfiguration": s3_constants.ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG, @@ -220,6 +220,12 @@ func MapToIdentitiesAction(action string) string { return "PutBucketPolicy" case s3_constants.ACTION_DELETE_BUCKET_POLICY: return "DeleteBucketPolicy" + case s3_constants.ACTION_PUT_BUCKET_CORS: + return "PutBucketCors" + case s3_constants.ACTION_DELETE_BUCKET_CORS: + return "DeleteBucketCors" + case s3_constants.ACTION_PUT_BUCKET_VERSIONING: + return "PutBucketVersioning" case s3_constants.ACTION_LIST_MULTIPART_UPLOADS: return "ListBucketMultipartUploads" case s3_constants.ACTION_LIST_PARTS: diff --git a/weed/s3api/s3_action_resolver.go b/weed/s3api/s3_action_resolver.go index ea33de78d0..129e3c1fbe 100644 --- a/weed/s3api/s3_action_resolver.go +++ b/weed/s3api/s3_action_resolver.go @@ -427,6 +427,12 @@ func mapBaseActionToS3Format(baseAction string) string { return s3_constants.S3_ACTION_PUT_BUCKET_POLICY case s3_constants.ACTION_DELETE_BUCKET_POLICY: return s3_constants.S3_ACTION_DELETE_BUCKET_POLICY + case s3_constants.ACTION_PUT_BUCKET_CORS: + return s3_constants.S3_ACTION_PUT_BUCKET_CORS + case s3_constants.ACTION_DELETE_BUCKET_CORS: + return s3_constants.S3_ACTION_DELETE_BUCKET_CORS + case s3_constants.ACTION_PUT_BUCKET_VERSIONING: + return s3_constants.S3_ACTION_PUT_BUCKET_VERSIONING case s3_constants.ACTION_PUT_BUCKET_QUOTA: return s3_constants.S3_ACTION_PUT_BUCKET_QUOTA case s3_constants.ACTION_GET_BUCKET_QUOTA: diff --git a/weed/s3api/s3_constants/s3_actions.go b/weed/s3api/s3_constants/s3_actions.go index c920e87cbe..cb682aafdc 100644 --- a/weed/s3api/s3_constants/s3_actions.go +++ b/weed/s3api/s3_constants/s3_actions.go @@ -18,6 +18,9 @@ const ( ACTION_PUT_BUCKET_OBJECT_LOCK_CONFIG = "PutBucketObjectLockConfiguration" ACTION_PUT_BUCKET_POLICY = "PutBucketPolicy" ACTION_DELETE_BUCKET_POLICY = "DeleteBucketPolicy" + ACTION_PUT_BUCKET_CORS = "PutBucketCors" + ACTION_DELETE_BUCKET_CORS = "DeleteBucketCors" + ACTION_PUT_BUCKET_VERSIONING = "PutBucketVersioning" ACTION_PUT_BUCKET_QUOTA = "PutBucketQuota" ACTION_GET_BUCKET_QUOTA = "GetBucketQuota" // Scoped to their single operation: the multipart listing routes are diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index dfc2c41ab4..26d6006986 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -1099,9 +1099,9 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // GetBucketCors bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketCorsHandler, ACTION_READ)), "GET")).Queries("cors", "") // PutBucketCors - bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketCorsHandler, ACTION_WRITE)), "PUT")).Queries("cors", "") + bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketCorsHandler, ACTION_PUT_BUCKET_CORS)), "PUT")).Queries("cors", "") // DeleteBucketCors - bucket.Methods(http.MethodDelete).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.DeleteBucketCorsHandler, ACTION_WRITE)), "DELETE")).Queries("cors", "") + bucket.Methods(http.MethodDelete).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.DeleteBucketCorsHandler, ACTION_DELETE_BUCKET_CORS)), "DELETE")).Queries("cors", "") // GetBucketLifecycleConfiguration bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketLifecycleConfigurationHandler, ACTION_READ)), "GET")).Queries("lifecycle", "") @@ -1134,7 +1134,7 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // GetBucketVersioning bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketVersioningHandler, ACTION_READ)), "GET")).Queries("versioning", "") - bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketVersioningHandler, ACTION_WRITE)), "PUT")).Queries("versioning", "") + bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketVersioningHandler, ACTION_PUT_BUCKET_VERSIONING)), "PUT")).Queries("versioning", "") // GetObjectLockConfiguration / PutObjectLockConfiguration (bucket-level operations) bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetObjectLockConfigurationHandler, ACTION_GET_BUCKET_OBJECT_LOCK_CONFIG)), "GET")).Queries("object-lock", "") diff --git a/weed/s3api/s3api_server_routing_test.go b/weed/s3api/s3api_server_routing_test.go index b008b18d99..07600723f8 100644 --- a/weed/s3api/s3api_server_routing_test.go +++ b/weed/s3api/s3api_server_routing_test.go @@ -588,3 +588,80 @@ func TestRouting_BucketLifecycleWritesNeedAdmin(t *testing.T) { } } } + +func TestRouting_BucketCorsAndVersioningWritesNeedTheirActions(t *testing.T) { + const bucket = "config-bucket" + configFile := filepath.Join(t.TempDir(), "s3.json") + require.NoError(t, os.WriteFile(configFile, []byte(`{ + "identities": [ + {"name":"writer","credentials":[{"accessKey":"WRITERKEY","secretKey":"writersecret"}],"actions":["Read:`+bucket+`","Write:`+bucket+`","List:`+bucket+`","Tagging:`+bucket+`"]}, + {"name":"bucketadmin","credentials":[{"accessKey":"BUCKETADMINKEY","secretKey":"bucketadminsecret"}],"actions":["Admin:`+bucket+`"]}, + {"name":"corsputter","credentials":[{"accessKey":"CORSPUTKEY","secretKey":"corsputsecret"}],"actions":["PutBucketCors:`+bucket+`"]}, + {"name":"corsdeleter","credentials":[{"accessKey":"CORSDELKEY","secretKey":"corsdelsecret"}],"actions":["DeleteBucketCors:`+bucket+`"]}, + {"name":"versioner","credentials":[{"accessKey":"VERSIONERKEY","secretKey":"versionersecret"}],"actions":["PutBucketVersioning:`+bucket+`"]}, + {"name":"policyuser","credentials":[{"accessKey":"POLICYUSERKEY","secretKey":"policyusersecret"}],"policyNames":["ConfigPolicy"]} + ], + "policies": [ + {"name":"ConfigPolicy","content":"{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Action\":[\"s3:PutBucketCors\",\"s3:DeleteBucketCors\",\"s3:PutBucketVersioning\"],\"Resource\":[\"arn:aws:s3:::`+bucket+`\"]}]}"} + ] +}`), 0600)) + + s3a := setupRoutingTestServer(t) + s3a.iam = NewIdentityAccessManagementWithStore(&S3ApiServerOption{Config: configFile}, nil, "memory") + require.True(t, s3a.iam.isEnabled()) + s3a.cb = &CircuitBreaker{s3a: s3a} + s3a.bucketConfigCache = NewBucketConfigCache(time.Minute) + s3a.bucketConfigCache.SetNegativeCache(bucket) + router := mux.NewRouter() + s3a.registerRouter(router) + + send := func(accessKey, secretKey, method, query, body string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, "http://localhost:8333/"+bucket+"?"+query, strings.NewReader(body)) + creds := credentials.NewStaticCredentials(accessKey, secretKey, "") + _, err := v4.NewSigner(creds).Sign(req, strings.NewReader(body), "s3", "us-east-1", time.Now()) + require.NoError(t, err) + rr := httptest.NewRecorder() + router.ServeHTTP(rr, req) + return rr + } + + const corsXML = `*GET` + const versioningXML = `Suspended` + singleActionKeys := map[string]string{"CORSPUTKEY": "corsputsecret", "CORSDELKEY": "corsdelsecret", "VERSIONERKEY": "versionersecret"} + for _, tc := range []struct { + method, query, body, allowedKey string + }{ + {http.MethodPut, "cors", corsXML, "CORSPUTKEY"}, + {http.MethodDelete, "cors", "", "CORSDELKEY"}, + {http.MethodPut, "versioning", versioningXML, "VERSIONERKEY"}, + } { + denied := [][2]string{{"WRITERKEY", "writersecret"}} + allowed := [][2]string{{"BUCKETADMINKEY", "bucketadminsecret"}, {"POLICYUSERKEY", "policyusersecret"}} + for key, secret := range singleActionKeys { + if key == tc.allowedKey { + allowed = append(allowed, [2]string{key, secret}) + } else { + denied = append(denied, [2]string{key, secret}) + } + } + for _, key := range denied { + rr := send(key[0], key[1], tc.method, tc.query, tc.body) + assert.Equal(t, http.StatusForbidden, rr.Code, "%s %s ?%s", key[0], tc.method, tc.query) + assert.Contains(t, rr.Body.String(), "AccessDenied", "%s %s ?%s", key[0], tc.method, tc.query) + } + for _, key := range allowed { + rr := send(key[0], key[1], tc.method, tc.query, tc.body) + assert.Equal(t, http.StatusNotFound, rr.Code, "%s %s ?%s", key[0], tc.method, tc.query) + assert.Contains(t, rr.Body.String(), "NoSuchBucket", "%s %s ?%s", key[0], tc.method, tc.query) + } + } + + for policyAction, identityAction := range map[string]string{ + "s3:PutBucketCors": s3_constants.ACTION_PUT_BUCKET_CORS, + "s3:DeleteBucketCors": s3_constants.ACTION_DELETE_BUCKET_CORS, + "s3:PutBucketVersioning": s3_constants.ACTION_PUT_BUCKET_VERSIONING, + } { + assert.Equal(t, identityAction, iamMapToStatementAction(policyAction), policyAction) + assert.Equal(t, policyAction, "s3:"+iamMapToIdentitiesAction(identityAction), identityAction) + } +}