diff --git a/weed/s3api/auth_signature_v4.go b/weed/s3api/auth_signature_v4.go index 8caae3e97..e46cb679a 100644 --- a/weed/s3api/auth_signature_v4.go +++ b/weed/s3api/auth_signature_v4.go @@ -354,9 +354,15 @@ func (iam *IdentityAccessManagement) verifyV4Signature(r *http.Request, shouldCh // 8. Verify the signature for each plausible host value: when X-Forwarded-Host carries // no port, the client may have signed the Host header the proxy kept or the forwarded // host and port, and the headers alone cannot tell which. - pathForSignature := r.URL.EscapedPath() + // A router that rewrote the request path (e.g. the Iceberg catalog's + // doubled-/v1 collapse) records the client-sent path in the context; + // the signature covers that original path. + pathForSignature := s3_constants.GetOriginalRequestPathFromContext(r) if pathForSignature == "" { - pathForSignature = r.URL.Path + pathForSignature = r.URL.EscapedPath() + if pathForSignature == "" { + pathForSignature = r.URL.Path + } } forwardedPrefix := r.Header.Get("X-Forwarded-Prefix") var matchedHost string diff --git a/weed/s3api/auth_signature_v4_test.go b/weed/s3api/auth_signature_v4_test.go index d1601b16e..e2fc67591 100644 --- a/weed/s3api/auth_signature_v4_test.go +++ b/weed/s3api/auth_signature_v4_test.go @@ -7,9 +7,12 @@ import ( "fmt" "net/http" "slices" + "sync" "testing" "time" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" ) @@ -677,3 +680,42 @@ func TestExtractSignedHeadersCase(t *testing.T) { }) } } + +// A router that internally rewrites the request path (the Iceberg catalog +// collapses a doubled /v1 prefix) must still verify the signature against +// the path the client signed, which is kept in the request context. +func TestVerifyV4SignatureUsesOriginalRequestPath(t *testing.T) { + iam := &IdentityAccessManagement{ + hashes: make(map[string]*sync.Pool), + hashCounters: make(map[string]*int32), + } + _ = iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{ + { + Name: "someone", + Credentials: []*iam_pb.Credential{ + { + AccessKey: "access_key_1", + SecretKey: "secret_key_1", + }, + }, + Actions: []string{"Admin", "Read", "Write"}, + }, + }, + }) + + req := mustNewSignedRequest(http.MethodGet, "http://127.0.0.1:9000/v1/v1/bucket/namespaces", 0, nil, t) + + // Simulate the rewrite: the routed path is collapsed while the signature + // still covers the doubled path the client sent. + rewritten := req.Clone(req.Context()) + rewritten.URL.Path = "/v1/bucket/namespaces" + if _, errCode := iam.reqSignatureV4Verify(rewritten); errCode == s3err.ErrNone { + t.Fatal("rewritten request verified without the original path; expected a signature mismatch") + } + + rewritten = rewritten.WithContext(s3_constants.SetOriginalRequestPathInContext(rewritten.Context(), "/v1/v1/bucket/namespaces")) + if _, errCode := iam.reqSignatureV4Verify(rewritten); errCode != s3err.ErrNone { + t.Fatalf("signed request rejected after path rewrite: %d", errCode) + } +} diff --git a/weed/s3api/iceberg/server.go b/weed/s3api/iceberg/server.go index 5266b8144..8980ebc09 100644 --- a/weed/s3api/iceberg/server.go +++ b/weed/s3api/iceberg/server.go @@ -110,6 +110,14 @@ func (s *Server) RegisterRoutes(router *mux.Router) { // audit-logged. router.Use(validateRequestPath) + // Clients pointed at a catalog URI ending in /v1 still append the API + // version themselves, producing /v1/v1/... requests (iceberg-java does + // this when configured with a /v1-suffixed URI). Collapse the repeated + // segments before routing so those requests reach the intended handler; + // a table bucket can never be named "v1" (names require 3-63 chars), so + // the rewrite is unambiguous. + router.Use(collapseDoubledV1Middleware(router)) + // Configuration endpoint - no auth needed for config router.HandleFunc("/v1/config", s.handleConfig).Methods(http.MethodGet) @@ -181,6 +189,39 @@ func (s *Server) RegisterRoutes(router *mux.Router) { glog.V(2).Infof("Registered Iceberg REST Catalog routes") } +// collapseDoubledV1Middleware rewrites /v1/v1/... (and deeper repetitions) to +// the single-/v1 path and re-dispatches through the router once. The collapsed +// path never starts with /v1/v1 again, so re-entry cannot recurse. The path +// the client sent is kept in the request context for SigV4 verification, +// which must check the signature against what was actually signed. +func collapseDoubledV1Middleware(router *mux.Router) mux.MiddlewareFunc { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + if path != "/v1/v1" && !strings.HasPrefix(path, "/v1/v1/") { + next.ServeHTTP(w, r) + return + } + for path == "/v1/v1" || strings.HasPrefix(path, "/v1/v1/") { + path = path[len("/v1"):] + } + signedPath := r.URL.EscapedPath() + if signedPath == "" { + signedPath = r.URL.Path + } + r2 := r.Clone(r.Context()) + r2.URL.Path = path + r2.URL.RawPath = "" + r2.RequestURI = r2.URL.EscapedPath() + if r.URL.RawQuery != "" { + r2.RequestURI += "?" + r.URL.RawQuery + } + r2 = r2.WithContext(s3_constants.SetOriginalRequestPathInContext(r2.Context(), signedPath)) + router.ServeHTTP(w, r2) + }) + } +} + func loggingMiddleware(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { glog.V(2).Infof("Iceberg REST request: %s %s from %s", r.Method, r.RequestURI, r.RemoteAddr) diff --git a/weed/s3api/iceberg/server_routes_test.go b/weed/s3api/iceberg/server_routes_test.go index 67417afda..1d01d702a 100644 --- a/weed/s3api/iceberg/server_routes_test.go +++ b/weed/s3api/iceberg/server_routes_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/gorilla/mux" + "github.com/seaweedfs/seaweedfs/weed/s3api/s3_constants" ) // The catch-all answers anything unrouted, so a path that reaches it is a @@ -39,3 +40,68 @@ func TestRegisteredRoutesReachAHandler(t *testing.T) { } } } + +// A catalog URI configured with a trailing /v1 (a common iceberg-java client +// mistake) produces /v1/v1/... requests; the router collapses the repeated +// prefix instead of answering "Path not found" or resolving "v1" as a table +// bucket name. +func TestDoubledV1PrefixIsCollapsed(t *testing.T) { + router := mux.NewRouter().SkipClean(true) + (&Server{}).RegisterRoutes(router) + + for _, target := range []string{ + "/v1/v1/config?warehouse=bucket", + // Degenerate repetition collapses in a single pass. + "/v1/v1/v1/config", + } { + rec := httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, target, nil)) + if rec.Code != http.StatusOK { + t.Fatalf("%s: got %d, want 200: %s", target, rec.Code, rec.Body.String()) + } + } + + // /v1/v1/namespaces also matches the {prefix} routes ("v1" can never be a + // table bucket name); it must still collapse to the unprefixed endpoint, + // which requires auth here rather than answering 404. + rec := httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/v1/v1/namespaces", nil)) + if rec.Code != http.StatusUnauthorized { + t.Fatalf("/v1/v1/namespaces: got %d, want 401: %s", rec.Code, rec.Body.String()) + } + + rec = httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/v1/nonexistent/path", nil)) + if rec.Code != http.StatusNotFound { + t.Fatalf("/v1/nonexistent/path: got %d, want 404", rec.Code) + } +} + +// The rewrite must preserve the client-sent path for SigV4 verification: a +// signed request is checked against the URI it was signed with, not the +// collapsed path it was routed under. +func TestDoubledV1CollapsePreservesOriginalPath(t *testing.T) { + router := mux.NewRouter().SkipClean(true) + var gotPath, gotSignedPath, gotQuery string + router.HandleFunc("/v1/probe", func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + gotQuery = r.URL.RawQuery + gotSignedPath = s3_constants.GetOriginalRequestPathFromContext(r) + }).Methods(http.MethodGet) + router.Use(collapseDoubledV1Middleware(router)) + router.PathPrefix("/").HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + + rec := httptest.NewRecorder() + router.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/v1/v1/probe?warehouse=bucket", nil)) + if gotPath != "/v1/probe" { + t.Fatalf("routed path: got %q, want /v1/probe", gotPath) + } + if gotQuery != "warehouse=bucket" { + t.Fatalf("query: got %q, want warehouse=bucket", gotQuery) + } + if gotSignedPath != "/v1/v1/probe" { + t.Fatalf("original signed path: got %q, want /v1/v1/probe", gotSignedPath) + } +} diff --git a/weed/s3api/s3_constants/header.go b/weed/s3api/s3_constants/header.go index 5e596b4a1..24a487151 100644 --- a/weed/s3api/s3_constants/header.go +++ b/weed/s3api/s3_constants/header.go @@ -331,6 +331,7 @@ const ( contextKeyPrincipalArn contextKey = "s3-principal-arn" contextKeyIdentityClaim contextKey = "s3-identity-claim" contextKeyIgnoredSessionToken contextKey = "s3-ignored-session-token" + contextKeyOriginalRequestPath contextKey = "s3-original-request-path" ) // identityHolder is a mutable container for the authenticated identity name, @@ -453,6 +454,26 @@ func GetIdentityClaimFromContext(r *http.Request) string { return "" } +// SetOriginalRequestPathInContext records the path a client actually sent +// before an internal rewrite (e.g. the Iceberg catalog collapsing a doubled +// /v1 prefix). SigV4 verification must run against this path: the client +// signed the URI it requested, not the normalized path it was routed with. +func SetOriginalRequestPathInContext(ctx context.Context, path string) context.Context { + if path == "" { + return ctx + } + return context.WithValue(ctx, contextKeyOriginalRequestPath, path) +} + +// GetOriginalRequestPathFromContext returns the pre-rewrite request path +// recorded by SetOriginalRequestPathInContext, or "" when unset. +func GetOriginalRequestPathFromContext(r *http.Request) string { + if path, ok := r.Context().Value(contextKeyOriginalRequestPath).(string); ok { + return path + } + return "" +} + // IgnoreSessionTokenInContext marks the request's session token as tolerated // rather than authenticated: a statically configured access key signed the // request, so the token (which credential vendors like Unity Catalog emit on