mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-17 19:48:35 +02:00
iceberg: collapse doubled /v1 prefix in catalog requests (#11767)
* s3: verify SigV4 against the request path the client sent
Internal routers that rewrite URL.Path (the Iceberg catalog doubled-/v1
collapse) broke signature verification: verifyV4Signature re-derived the
canonical URI from the rewritten path while the client had signed the
original one, so valid signed catalog requests were rejected. Stash the
client-sent path in the request context at rewrite time and prefer it when
computing pathForSignature.
* iceberg: collapse repeated /v1 prefixes before route matching
Move the doubled-prefix collapse from the catch-all into a middleware so it
also covers paths that match the {prefix} routes (/v1/v1/namespaces would
otherwise resolve "v1" as the table bucket; the name is unreachable anyway
since bucket names require 3-63 chars). Collapse all leading repetitions in
a single pass and re-dispatch once instead of recursing per segment, and
record the original path in context so signed requests still verify.
* iceberg: slice off repeated /v1 prefixes instead of concatenating
Stripping path[len("/v1"):] per iteration reuses the original string
instead of allocating a fresh copy of the remaining path each pass.
This commit is contained in:
1 parent
ad2486919c
commit
cabaf9c38d
5 files changed
+178
-2
No files matched your search
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user