From ce292a4e5ec2da8328ff54c533dddab94041908d Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 2 Nov 2026 01:26:16 -0800 Subject: [PATCH] s3: require Admin to put or delete a bucket's lifecycle configuration (#12327) PUT and DELETE ?lifecycle were authorized as Write, so an identity granted only object writes on a bucket could add expiration rules or remove the ones the bucket administrator set. They now need Admin on the bucket, like the encryption, public access block and ownership controls writes. Identities authorized by IAM policies are unaffected: they are still checked against s3:PutLifecycleConfiguration. Co-authored-by: Claude Opus 5.5 --- weed/s3api/s3api_server.go | 4 +- weed/s3api/s3api_server_routing_test.go | 59 +++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index b6f8c1486a..dfc2c41ab4 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -1106,9 +1106,9 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // GetBucketLifecycleConfiguration bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketLifecycleConfigurationHandler, ACTION_READ)), "GET")).Queries("lifecycle", "") // PutBucketLifecycleConfiguration - bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketLifecycleConfigurationHandler, ACTION_WRITE)), "PUT")).Queries("lifecycle", "") + bucket.Methods(http.MethodPut).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.PutBucketLifecycleConfigurationHandler, ACTION_ADMIN)), "PUT")).Queries("lifecycle", "") // DeleteBucketLifecycleConfiguration - bucket.Methods(http.MethodDelete).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.DeleteBucketLifecycleHandler, ACTION_WRITE)), "DELETE")).Queries("lifecycle", "") + bucket.Methods(http.MethodDelete).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.DeleteBucketLifecycleHandler, ACTION_ADMIN)), "DELETE")).Queries("lifecycle", "") // GetBucketLocation bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketLocationHandler, ACTION_READ)), "GET")).Queries("location", "") diff --git a/weed/s3api/s3api_server_routing_test.go b/weed/s3api/s3api_server_routing_test.go index 9316174924..b008b18d99 100644 --- a/weed/s3api/s3api_server_routing_test.go +++ b/weed/s3api/s3api_server_routing_test.go @@ -5,6 +5,8 @@ import ( "net/http" "net/http/httptest" "net/url" + "os" + "path/filepath" "strings" "testing" "time" @@ -529,3 +531,60 @@ func TestRouting_BucketRootPreflightUsesBucketCORS(t *testing.T) { }) } } + +// Write covers a bucket's objects, not its configuration: an identity holding +// Write on the bucket is refused lifecycle writes, while Admin on the bucket or +// a policy granting s3:PutLifecycleConfiguration passes. The bucket is +// negatively cached, so a request that gets past authorization ends at the +// handler's NoSuchBucket. +func TestRouting_BucketLifecycleWritesNeedAdmin(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":"lifecycler","credentials":[{"accessKey":"LIFECYCLERKEY","secretKey":"lifecyclersecret"}],"policyNames":["LifecyclePolicy"]} + ], + "policies": [ + {"name":"LifecyclePolicy","content":"{\"Version\":\"2012-10-17\",\"Statement\":[{\"Effect\":\"Allow\",\"Action\":[\"s3:PutLifecycleConfiguration\"],\"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, body string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, "http://localhost:8333/"+bucket+"?lifecycle", 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 lifecycleXML = `rEnabledp/1` + for _, tc := range []struct { + method, body string + }{ + {http.MethodPut, lifecycleXML}, + {http.MethodDelete, ""}, + } { + rr := send("WRITERKEY", "writersecret", tc.method, tc.body) + assert.Equal(t, http.StatusForbidden, rr.Code, "writer %s", tc.method) + assert.Contains(t, rr.Body.String(), "AccessDenied", "writer %s", tc.method) + + for _, key := range [][2]string{{"BUCKETADMINKEY", "bucketadminsecret"}, {"LIFECYCLERKEY", "lifecyclersecret"}} { + rr = send(key[0], key[1], tc.method, tc.body) + assert.Equal(t, http.StatusNotFound, rr.Code, "%s %s", key[0], tc.method) + assert.Contains(t, rr.Body.String(), "NoSuchBucket", "%s %s", key[0], tc.method) + } + } +}