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
<ListBucketResult> 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
<ListBucketResult>. 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

@@ -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
+}

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)
+	}
+}

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"))

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.",