From 37b6a14b0d0883f4d61193b1a0fe89e9a6570481 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 19 May 2026 17:35:08 -0700 Subject: [PATCH] feat(s3): add four bucket configuration handlers (#9570) * feat(s3): add four bucket configuration handlers - GetBucketPolicyStatus: computes IsPublic from the existing bucket policy - PutBucketRequestPayment: companion writer to the existing GET; accepts only BucketOwner - GetBucketAccelerateConfiguration: returns Suspended - GetBucketLogging: returns an empty BucketLoggingStatus Lets AWS SDK probes succeed instead of returning MethodNotAllowed. * review: route GetBucketPolicyStatus through checkBucket Mirrors the existence/auth gating used by other bucket handlers and drops the bespoke filer_pb lookup so NoSuchBucket precedence is consistent across the API surface. * review: cap PutBucketRequestPayment body with MaxBytesReader The body is unmarshalled as RequestPaymentConfiguration, which is a handful of bytes; reject excessively large payloads up front and defer Close immediately after wrapping. * review: gate static getters on checkBucket GetBucketAccelerateConfiguration and GetBucketLogging now run the standard bucket existence check before returning the static Suspended / empty-status response so a missing bucket cannot appear to have valid configuration. * review: share cache helper across misc tests; check io.ReadAll error Accelerate and Logging tests now run through newMiscTestServer like the others so the checkBucket guard sees a cached bucket; the ReadAll error is explicitly checked. --- weed/s3api/s3api_bucket_handlers_misc.go | 132 +++++++++++++++ weed/s3api/s3api_bucket_handlers_misc_test.go | 151 ++++++++++++++++++ weed/s3api/s3api_server.go | 10 ++ 3 files changed, 293 insertions(+) create mode 100644 weed/s3api/s3api_bucket_handlers_misc.go create mode 100644 weed/s3api/s3api_bucket_handlers_misc_test.go diff --git a/weed/s3api/s3api_bucket_handlers_misc.go b/weed/s3api/s3api_bucket_handlers_misc.go new file mode 100644 index 000000000..211c5c55b --- /dev/null +++ b/weed/s3api/s3api_bucket_handlers_misc.go @@ -0,0 +1,132 @@ +package s3api + +import ( + "encoding/xml" + "errors" + "io" + "net/http" + + "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" +) + +// putBucketRequestPaymentMaxBodyBytes caps the request body for PutBucketRequestPayment +// to prevent DoS via large payloads. The valid payload is a few hundred bytes. +const putBucketRequestPaymentMaxBodyBytes = 64 * 1024 + +type policyStatusResponse struct { + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ PolicyStatus"` + IsPublic bool `xml:"IsPublic"` +} + +type accelerateConfigurationResponse struct { + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ AccelerateConfiguration"` + Status string `xml:"Status"` +} + +type bucketLoggingStatusResponse struct { + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ BucketLoggingStatus"` +} + +// GetBucketPolicyStatusHandler reports whether the bucket policy grants public access. +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketPolicyStatus.html +func (s3a *S3ApiServer) GetBucketPolicyStatusHandler(w http.ResponseWriter, r *http.Request) { + bucket, _ := s3_constants.GetBucketAndObject(r) + + if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, err) + return + } + + policyDoc, err := s3a.getBucketPolicy(bucket) + if err != nil { + if errors.Is(err, ErrPolicyNotFound) { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucketPolicy) + } else if errors.Is(err, ErrBucketNotFound) { + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchBucket) + } else { + glog.Errorf("GetBucketPolicyStatusHandler load policy %s: %v", bucket, err) + s3err.WriteErrorResponse(w, r, s3err.ErrInternalError) + } + return + } + + writeSuccessResponseXML(w, r, policyStatusResponse{IsPublic: isPolicyPublic(policyDoc)}) +} + +// isPolicyPublic returns true if any Allow statement grants access to "*" without restricting conditions. +func isPolicyPublic(doc *policy_engine.PolicyDocument) bool { + if doc == nil { + return false + } + for _, st := range doc.Statement { + if st.Effect != policy_engine.PolicyEffectAllow { + continue + } + if len(st.Condition) > 0 { + continue + } + for _, p := range st.Principal.Strings() { + if p == "*" { + return true + } + } + } + return false +} + +// PutBucketRequestPaymentHandler accepts only Payer=BucketOwner; Requester is rejected. +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutBucketRequestPayment.html +func (s3a *S3ApiServer) PutBucketRequestPaymentHandler(w http.ResponseWriter, r *http.Request) { + bucket, _ := s3_constants.GetBucketAndObject(r) + + if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, err) + return + } + + r.Body = http.MaxBytesReader(w, r.Body, putBucketRequestPaymentMaxBodyBytes) + defer r.Body.Close() + body, err := io.ReadAll(r.Body) + if err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML) + return + } + + var cfg RequestPaymentConfiguration + if err := xml.Unmarshal(body, &cfg); err != nil { + s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML) + return + } + + if cfg.Payer != "BucketOwner" { + s3err.WriteErrorResponse(w, r, s3err.ErrMalformedXML) + return + } + + writeSuccessResponseEmpty(w, r) +} + +// GetBucketAccelerateConfigurationHandler returns a static Suspended status. +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketAccelerateConfiguration.html +func (s3a *S3ApiServer) GetBucketAccelerateConfigurationHandler(w http.ResponseWriter, r *http.Request) { + bucket, _ := s3_constants.GetBucketAndObject(r) + if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, err) + return + } + writeSuccessResponseXML(w, r, accelerateConfigurationResponse{Status: "Suspended"}) +} + +// GetBucketLoggingHandler returns an empty BucketLoggingStatus (logging disabled). +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketLogging.html +func (s3a *S3ApiServer) GetBucketLoggingHandler(w http.ResponseWriter, r *http.Request) { + bucket, _ := s3_constants.GetBucketAndObject(r) + if err := s3a.checkBucket(r, bucket); err != s3err.ErrNone { + s3err.WriteErrorResponse(w, r, err) + return + } + writeSuccessResponseXML(w, r, bucketLoggingStatusResponse{}) +} diff --git a/weed/s3api/s3api_bucket_handlers_misc_test.go b/weed/s3api/s3api_bucket_handlers_misc_test.go new file mode 100644 index 000000000..3293fee53 --- /dev/null +++ b/weed/s3api/s3api_bucket_handlers_misc_test.go @@ -0,0 +1,151 @@ +package s3api + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" +) + +func newMiscTestServer(t *testing.T, bucket string) *S3ApiServer { + t.Helper() + s3a := &S3ApiServer{ + iam: &IdentityAccessManagement{isAuthEnabled: true}, + bucketConfigCache: NewBucketConfigCache(time.Minute), + } + s3a.bucketConfigCache.Set(bucket, &BucketConfig{ + Name: bucket, + Entry: &filer_pb.Entry{Name: bucket}, + }) + return s3a +} + +func newBucketRequest(method, bucket, query, body string) *http.Request { + req := httptest.NewRequest(method, "/"+bucket+"?"+query, strings.NewReader(body)) + req = mux.SetURLVars(req, map[string]string{"bucket": bucket}) + return req +} + +func TestGetBucketPolicyStatusIsPublic(t *testing.T) { + cases := []struct { + name string + raw string + want bool + }{ + { + name: "public allow star", + raw: `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::b/*"}]}`, + want: true, + }, + { + name: "deny is not public", + raw: `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::b/*"}]}`, + want: false, + }, + { + name: "condition makes it non-public", + raw: `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3:GetObject","Resource":"arn:aws:s3:::b/*","Condition":{"IpAddress":{"aws:SourceIp":"10.0.0.0/8"}}}]}`, + want: false, + }, + { + name: "specific principal is not public", + raw: `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"arn:aws:iam::1:user/a","Action":"s3:GetObject","Resource":"arn:aws:s3:::b/*"}]}`, + want: false, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var doc policy_engine.PolicyDocument + if err := json.Unmarshal([]byte(tc.raw), &doc); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if got := isPolicyPublic(&doc); got != tc.want { + t.Fatalf("isPolicyPublic = %v, want %v", got, tc.want) + } + }) + } +} + +func TestPutBucketRequestPaymentBucketOwner(t *testing.T) { + s3a := newMiscTestServer(t, "b") + body := `BucketOwner` + req := newBucketRequest(http.MethodPut, "b", "requestPayment=", body) + rec := httptest.NewRecorder() + + s3a.PutBucketRequestPaymentHandler(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } +} + +func TestPutBucketRequestPaymentRequesterRejected(t *testing.T) { + s3a := newMiscTestServer(t, "b") + body := `Requester` + req := newBucketRequest(http.MethodPut, "b", "requestPayment=", body) + rec := httptest.NewRecorder() + + s3a.PutBucketRequestPaymentHandler(rec, req) + + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusBadRequest, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "MalformedXML") { + t.Fatalf("body missing MalformedXML: %s", rec.Body.String()) + } +} + +func TestGetBucketAccelerateConfiguration(t *testing.T) { + s3a := newMiscTestServer(t, "b") + req := newBucketRequest(http.MethodGet, "b", "accelerate=", "") + rec := httptest.NewRecorder() + + s3a.GetBucketAccelerateConfigurationHandler(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + body, err := io.ReadAll(rec.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + got := string(body) + if !strings.Contains(got, "Suspended") { + t.Fatalf("missing Suspended status: %s", got) + } + if !strings.Contains(got, `xmlns="http://s3.amazonaws.com/doc/2006-03-01/"`) { + t.Fatalf("missing xmlns: %s", got) + } +} + +func TestGetBucketLogging(t *testing.T) { + s3a := newMiscTestServer(t, "b") + req := newBucketRequest(http.MethodGet, "b", "logging=", "") + rec := httptest.NewRecorder() + + s3a.GetBucketLoggingHandler(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d", rec.Code, http.StatusOK) + } + got := rec.Body.String() + if !strings.Contains(got, "