From 1354b58675fe96a7242c47fa4d6532e5934b6aaa Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 18 Aug 2026 16:03:08 -0700 Subject: [PATCH] s3: stop unrouted bucket subresources from being answered with a listing (#10814) * s3: answer GetBucketReplication, GetBucketWebsite and GetBucketNotificationConfiguration None of the three had a route, so they reached the unconstrained ListObjectsV1 catch-all and a client asking for a bucket's replication config got 200 and a back. Replication and website report their configuration as absent the way AWS does; notification returns the empty configuration AWS returns for a bucket with no events wired up. * s3: stop an unrouted bucket subresource from being answered with a listing ListObjectsV1 is the catch-all GET on a bucket, so every subresource without a route of its own - ?torrent today, whatever AWS adds next - came back 200 with a . A client that asked for a configuration and got a listing either fails its XML decode in a way that reads like corruption, or worse, tolerantly parses it. Refuse the request instead. The allow-list is the ListObjects parameters rather than the subresources, so a new one fails closed. Presigned URLs sign their credentials into the query string, so X-Amz-* and the SigV2 trio have to stay listable. --- weed/s3api/s3api_bucket_handlers_misc.go | 65 +++++++++++++++++++ .../s3api_bucket_subresource_routing_test.go | 34 ++++++++++ weed/s3api/s3api_server.go | 14 +++- weed/s3api/s3err/s3api_errors.go | 12 ++++ 4 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 weed/s3api/s3api_bucket_subresource_routing_test.go diff --git a/weed/s3api/s3api_bucket_handlers_misc.go b/weed/s3api/s3api_bucket_handlers_misc.go index 211c5c55b..ae3f61b26 100644 --- a/weed/s3api/s3api_bucket_handlers_misc.go +++ b/weed/s3api/s3api_bucket_handlers_misc.go @@ -5,6 +5,7 @@ import ( "errors" "io" "net/http" + "strings" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" @@ -30,6 +31,10 @@ type bucketLoggingStatusResponse struct { XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ BucketLoggingStatus"` } +type notificationConfigurationResponse struct { + XMLName xml.Name `xml:"http://s3.amazonaws.com/doc/2006-03-01/ NotificationConfiguration"` +} + // 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) { @@ -130,3 +135,63 @@ func (s3a *S3ApiServer) GetBucketLoggingHandler(w http.ResponseWriter, r *http.R } writeSuccessResponseXML(w, r, bucketLoggingStatusResponse{}) } + +// GetBucketNotificationConfigurationHandler returns an empty configuration; SeaweedFS +// has no bucket event notifications, and AWS answers an unconfigured bucket the same way. +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketNotificationConfiguration.html +func (s3a *S3ApiServer) GetBucketNotificationConfigurationHandler(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, notificationConfigurationResponse{}) +} + +// GetBucketReplicationHandler reports that no replication configuration exists. +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketReplication.html +func (s3a *S3ApiServer) GetBucketReplicationHandler(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 + } + s3err.WriteErrorResponse(w, r, s3err.ErrReplicationConfigurationNotFound) +} + +// GetBucketWebsiteHandler reports that no website configuration exists. +// https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetBucketWebsite.html +func (s3a *S3ApiServer) GetBucketWebsiteHandler(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 + } + s3err.WriteErrorResponse(w, r, s3err.ErrNoSuchWebsiteConfiguration) +} + +// listObjectsQueryParams are the query keys a ListObjects/ListObjectsV2 request may +// carry. x-id is stamped by the AWS SDKs and carries no meaning for the server; the +// rest of the non-listing keys are what a presigned URL signs into the query string. +var listObjectsQueryParams = map[string]bool{ + "prefix": true, "delimiter": true, "marker": true, "max-keys": true, + "encoding-type": true, "list-type": true, "continuation-token": true, + "start-after": true, "fetch-owner": true, "expected-bucket-owner": true, + "x-id": true, + // SigV2 presigned URLs. + "AWSAccessKeyId": true, "Signature": true, "Expires": true, +} + +// unroutedBucketSubresource names a query key on a bucket GET that no route claimed. +// Everything SeaweedFS implements is matched by its own route before the ListObjects +// catch-all, so anything left is a subresource it does not implement - and answering +// it with a bucket listing is worse than saying so. +func unroutedBucketSubresource(r *http.Request) (string, bool) { + for key := range r.URL.Query() { + if listObjectsQueryParams[key] || strings.HasPrefix(key, "X-Amz-") { + continue + } + return key, true + } + return "", false +} diff --git a/weed/s3api/s3api_bucket_subresource_routing_test.go b/weed/s3api/s3api_bucket_subresource_routing_test.go new file mode 100644 index 000000000..3843ae332 --- /dev/null +++ b/weed/s3api/s3api_bucket_subresource_routing_test.go @@ -0,0 +1,34 @@ +package s3api + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestUnroutedBucketSubresource guards the rule that keeps a bucket GET carrying an +// unimplemented subresource from being answered with a bucket listing: everything +// SeaweedFS implements is matched by its own route before the ListObjects catch-all, +// so anything left over that is not a listing parameter is a subresource. +func TestUnroutedBucketSubresource(t *testing.T) { + for _, query := range []string{ + "", + "prefix=a&max-keys=10", + "list-type=2&continuation-token=x", + "delimiter=/&encoding-type=url", + "x-id=ListObjectsV2", + "prefix=a&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Signature=deadbeef&X-Amz-Expires=900", + "AWSAccessKeyId=key&Signature=sig&Expires=1700000000", + } { + req, _ := http.NewRequest("GET", "http://localhost/bucket?"+query, nil) + name, found := unroutedBucketSubresource(req) + assert.False(t, found, "%q is a listing request, got %q", query, name) + } + + for _, query := range []string{"torrent=", "replication=", "prefix=a&torrent="} { + req, _ := http.NewRequest("GET", "http://localhost/bucket?"+query, nil) + _, found := unroutedBucketSubresource(req) + assert.True(t, found, "%q should be reported as a subresource", query) + } +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 22bef931b..dd60b8d59 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -923,6 +923,12 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketAccelerateConfigurationHandler, ACTION_READ)), "GET")).Queries("accelerate", "") // GetBucketLogging bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketLoggingHandler, ACTION_READ)), "GET")).Queries("logging", "") + // GetBucketNotificationConfiguration + bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketNotificationConfigurationHandler, ACTION_READ)), "GET")).Queries("notification", "") + // GetBucketReplication + bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketReplicationHandler, ACTION_READ)), "GET")).Queries("replication", "") + // GetBucketWebsite + bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketWebsiteHandler, ACTION_READ)), "GET")).Queries("website", "") // GetBucketVersioning bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.GetBucketVersioningHandler, ACTION_READ)), "GET")).Queries("versioning", "") @@ -993,8 +999,14 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) { // DeleteBucket bucket.Methods(http.MethodDelete).HandlerFunc(track(s3a.iam.Auth(s3a.cb.Limit(s3a.DeleteBucketHandler, ACTION_DELETE_BUCKET)), "DELETE")) - // ListObjectsV1 (Legacy) + // ListObjectsV1 (Legacy). This is the catch-all GET on a bucket, so a + // subresource with no route of its own would be answered with a listing. bucket.Methods(http.MethodGet).HandlerFunc(track(s3a.AuthWithPublicRead(func(w http.ResponseWriter, r *http.Request) { + if subresource, found := unroutedBucketSubresource(r); found { + glog.V(1).Infof("unimplemented bucket subresource ?%s", subresource) + s3err.WriteErrorResponse(w, r, s3err.ErrNotImplemented) + return + } limitedHandler, _ := s3a.cb.Limit(s3a.ListObjectsV1Handler, ACTION_LIST) limitedHandler(w, r) }, ACTION_LIST), "LIST")) diff --git a/weed/s3api/s3err/s3api_errors.go b/weed/s3api/s3err/s3api_errors.go index f5ce20652..87ffaeb6f 100644 --- a/weed/s3api/s3err/s3api_errors.go +++ b/weed/s3api/s3err/s3api_errors.go @@ -56,6 +56,8 @@ const ( ErrNoSuchBucketPolicy ErrNoSuchCORSConfiguration ErrNoSuchLifecycleConfiguration + ErrNoSuchWebsiteConfiguration + ErrReplicationConfigurationNotFound ErrNoSuchKey ErrNoSuchVersion ErrNoSuchUpload @@ -298,6 +300,16 @@ var errorCodeResponse = map[ErrorCode]APIError{ Description: "The lifecycle configuration does not exist", HTTPStatusCode: http.StatusNotFound, }, + ErrNoSuchWebsiteConfiguration: { + Code: "NoSuchWebsiteConfiguration", + Description: "The specified bucket does not have a website configuration", + HTTPStatusCode: http.StatusNotFound, + }, + ErrReplicationConfigurationNotFound: { + Code: "ReplicationConfigurationNotFoundError", + Description: "The replication configuration was not found", + HTTPStatusCode: http.StatusNotFound, + }, ErrNoSuchKey: { Code: "NoSuchKey", Description: "The specified key does not exist.",