diff --git a/weed/command/s3.go b/weed/command/s3.go index faedfe520..1cb0b8780 100644 --- a/weed/command/s3.go +++ b/weed/command/s3.go @@ -29,6 +29,7 @@ import ( stats_collect "github.com/seaweedfs/seaweedfs/weed/stats" "github.com/seaweedfs/seaweedfs/weed/util" "github.com/seaweedfs/seaweedfs/weed/util/grace" + util_http "github.com/seaweedfs/seaweedfs/weed/util/http" "github.com/seaweedfs/seaweedfs/weed/util/version" ) @@ -524,6 +525,9 @@ func (s3opt *S3Options) startS3Server() bool { // startIcebergServer starts the Iceberg REST Catalog server on a separate port. func (s3opt *S3Options) startIcebergServer(s3ApiServer *s3api.S3ApiServer) { icebergRouter := mux.NewRouter().SkipClean(true) + // warehouse/parent query values may legally contain ';', which Go's + // url.ParseQuery would otherwise drop + icebergRouter.Use(util_http.EscapeSemicolonsInQuery) // Create Iceberg server using the S3ApiServer as filer client icebergServer := iceberg.NewServer(s3ApiServer, s3ApiServer) diff --git a/weed/iamapi/iamapi_server.go b/weed/iamapi/iamapi_server.go index 49138956a..26218587f 100644 --- a/weed/iamapi/iamapi_server.go +++ b/weed/iamapi/iamapi_server.go @@ -22,6 +22,7 @@ import ( . "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" "github.com/seaweedfs/seaweedfs/weed/util" + util_http "github.com/seaweedfs/seaweedfs/weed/util/http" "github.com/seaweedfs/seaweedfs/weed/util/request_id" "github.com/seaweedfs/seaweedfs/weed/wdclient" "google.golang.org/grpc" @@ -117,6 +118,9 @@ func NewIamApiServerWithStore(router *mux.Router, option *IamServerOption, expli } func (iama *IamApiServer) registerRouter(router *mux.Router) { + // SigV4 recomputes the canonical query from the parsed query, so raw ';' + // pairs that Go would drop must be recovered before verification. + router.Use(util_http.EscapeSemicolonsInQuery) // API Router apiRouter := router.PathPrefix("/").Subrouter() apiRouter.Use(request_id.Middleware) diff --git a/weed/s3api/auth_presigned_semicolon_test.go b/weed/s3api/auth_presigned_semicolon_test.go new file mode 100644 index 000000000..9dfc1a703 --- /dev/null +++ b/weed/s3api/auth_presigned_semicolon_test.go @@ -0,0 +1,45 @@ +package s3api + +import ( + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" + util_http "github.com/seaweedfs/seaweedfs/weed/util/http" +) + +// A presigned URL that signs content-type carries +// X-Amz-SignedHeaders=content-type%3Bhost. Some clients and proxies decode +// the %3B into a raw ';', which RFC 3986 permits in a query string and AWS +// accepts, but Go's url.ParseQuery drops the whole pair — verification then +// fails with MissingFields. The router middleware re-encodes the ';' so the +// parameter survives. +func TestPresignedPutSignedContentTypeWithRawSemicolon(t *testing.T) { + iam := newTestIAM() + + req, err := newTestRequest(http.MethodPut, "http://127.0.0.1:9000/bucket/key.png", 0, nil) + if err != nil { + t.Fatalf("newTestRequest: %v", err) + } + req.Header.Set("Content-Type", "image/png") + if err := preSignV4WithHeaders(iam, req, "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY", 600, []string{"host", "content-type"}); err != nil { + t.Fatalf("preSignV4WithHeaders: %v", err) + } + req.URL.RawQuery = strings.ReplaceAll(req.URL.RawQuery, "%3B", ";") + + if _, errCode := iam.reqSignatureV4Verify(req); errCode == s3err.ErrNone { + t.Fatal("raw-semicolon query unexpectedly verified without normalization") + } + + errCode := s3err.ErrInternalError + handler := util_http.EscapeSemicolonsInQuery(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, errCode = iam.reqSignatureV4Verify(r) + })) + handler.ServeHTTP(httptest.NewRecorder(), req) + + if errCode != s3err.ErrNone { + t.Fatalf("expected ErrNone through semicolon-normalizing middleware, got %v", errCode) + } +} diff --git a/weed/s3api/s3api_server.go b/weed/s3api/s3api_server.go index 96028a3e4..5943bfeca 100644 --- a/weed/s3api/s3api_server.go +++ b/weed/s3api/s3api_server.go @@ -724,6 +724,8 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques } func (s3a *S3ApiServer) registerRouter(router *mux.Router) { + // runs before subrouter matching so Queries() matchers see the fixed query + router.Use(util_http.EscapeSemicolonsInQuery) // API Router apiRouter := router.PathPrefix("/").Subrouter() apiRouter.Use(request_id.Middleware) diff --git a/weed/util/http/query_middleware.go b/weed/util/http/query_middleware.go new file mode 100644 index 000000000..3870e0c8a --- /dev/null +++ b/weed/util/http/query_middleware.go @@ -0,0 +1,25 @@ +package http + +import ( + "net/http" + "strings" +) + +// EscapeSemicolonsInQuery re-encodes raw ';' in the query string. RFC 3986 +// allows ';' in a query and AWS treats it as data, but Go's url.ParseQuery +// drops every key=value pair containing one. Presigned URLs carry +// X-Amz-SignedHeaders=content-type%3Bhost; clients that decode the %3B would +// otherwise lose the parameter entirely and fail signature verification. +func EscapeSemicolonsInQuery(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.Contains(r.URL.RawQuery, ";") { + u := *r.URL + u.RawQuery = strings.ReplaceAll(u.RawQuery, ";", "%3B") + r2 := new(http.Request) + *r2 = *r + r2.URL = &u + r = r2 + } + next.ServeHTTP(w, r) + }) +}