mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-28 11:56:07 +00:00
s3: confine a Lance catalog table location to the caller's own bucket (#10901)
The Lance namespace gateway took the request-body location field, trimmed a trailing slash, and passed it straight to the marker sink. That location feeds TableDataDirFromMetadataLocation, which joins it under /buckets and collapses any ../ segments, and writeMarker's CreateEntry then auto-creates every missing parent. A caller could point the location at another tenant's bucket, or escape /buckets entirely, and plant a fixed-name marker (recursively creating the parents) or hide a victim's live table with .lance-deregistered. Confine the declared location the way the Iceberg gateway already does: require an s3:// URI whose bucket is the caller's own and whose path carries no traversal segment, on both the declare and register handlers.
This commit is contained in:
@@ -130,6 +130,9 @@ func (s *Server) handleDeclareTable(w http.ResponseWriter, r *http.Request) {
|
||||
location := strings.TrimSuffix(req.Location, "/")
|
||||
if location == "" {
|
||||
location = tableLocation(bucket, ns, name)
|
||||
} else if err := confineLocation(bucket, location); err != nil {
|
||||
writeError(w, r, http.StatusBadRequest, codeInvalidInput, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
if err := s.createTable(r, bucket, ns, name, location); err != nil {
|
||||
@@ -304,6 +307,10 @@ func (s *Server) handleRegisterTable(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, r, http.StatusBadRequest, codeInvalidInput, "location is required")
|
||||
return
|
||||
}
|
||||
if err := confineLocation(bucket, location); err != nil {
|
||||
writeError(w, r, http.StatusBadRequest, codeInvalidInput, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
mode := normalizeMode(req.Mode, modeCreate)
|
||||
if mode != modeCreate && mode != modeOverwrite {
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gorilla/mux"
|
||||
"github.com/seaweedfs/seaweedfs/weed/s3api/s3err"
|
||||
@@ -348,6 +349,63 @@ func TestRouteAndBodyMustAgree(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A client-supplied location must resolve inside its own bucket. Without this,
|
||||
// a "../"-laden or cross-bucket location escapes when the marker path is joined
|
||||
// under /buckets, letting the request reach another tenant's namespace, or
|
||||
// outside /buckets entirely, and auto-create the parent directories on the way.
|
||||
func TestLocationOutsideBucketIsRejected(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
location string
|
||||
}{
|
||||
{"another bucket", "s3://victim/secret"},
|
||||
{"traversal into another bucket", "s3://analytics/../victim/secret"},
|
||||
{"traversal above the buckets root", "s3://analytics/../../etc/cron.d"},
|
||||
{"not an s3 uri", "../../etc/cron.d"},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
dir := s3tables.TableDataDirFromMetadataLocation(tc.location)
|
||||
body := `{"location":"` + tc.location + `"}`
|
||||
|
||||
// declare writes .lance-reserved at the location: rejection must leave
|
||||
// nothing behind at the escaped directory.
|
||||
t.Run("declare", func(t *testing.T) {
|
||||
h := newTestHarness(t)
|
||||
h.createBucket(t, "analytics", s3tables.FormatLance)
|
||||
h.mustDo(t, http.MethodPost, "/v1/namespace/analytics$sales/create", `{}`, http.StatusOK)
|
||||
|
||||
recorder := h.mustDo(t, http.MethodPost, "/v1/table/analytics$sales$orders/declare", body, http.StatusBadRequest)
|
||||
if got := decode[errorResponse](t, recorder); got.Code != codeInvalidInput {
|
||||
t.Fatalf("error code = %d, want %d", got.Code, codeInvalidInput)
|
||||
}
|
||||
if dir != "" && h.filer.Get(dir, reservedMarker) != nil {
|
||||
t.Fatalf("declare wrote %s into %s despite rejection", reservedMarker, dir)
|
||||
}
|
||||
})
|
||||
|
||||
// register removes .lance-deregistered at the location: rejection must
|
||||
// leave a victim's marker in place rather than un-hiding their table.
|
||||
t.Run("register", func(t *testing.T) {
|
||||
h := newTestHarness(t)
|
||||
h.createBucket(t, "analytics", s3tables.FormatLance)
|
||||
h.mustDo(t, http.MethodPost, "/v1/namespace/analytics$sales/create", `{}`, http.StatusOK)
|
||||
if dir != "" {
|
||||
h.filer.PutFile(dir, deregisteredMarker, time.Unix(1, 0))
|
||||
}
|
||||
|
||||
recorder := h.mustDo(t, http.MethodPost, "/v1/table/analytics$sales$orders/register", body, http.StatusBadRequest)
|
||||
if got := decode[errorResponse](t, recorder); got.Code != codeInvalidInput {
|
||||
t.Fatalf("error code = %d, want %d", got.Code, codeInvalidInput)
|
||||
}
|
||||
if dir != "" && h.filer.Get(dir, deregisteredMarker) == nil {
|
||||
t.Fatalf("register removed %s from %s despite rejection", deregisteredMarker, dir)
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The data plane needs Lance format support that does not exist in Go, so it
|
||||
// answers with the spec's own Unsupported code rather than a bare 404.
|
||||
func TestDataPlaneIsUnsupported(t *testing.T) {
|
||||
|
||||
@@ -107,6 +107,31 @@ func tableLocation(bucket string, ns []string, name string) string {
|
||||
return fmt.Sprintf("s3://%s/%s/%s", bucket, strings.Join(ns, "."), name)
|
||||
}
|
||||
|
||||
// confineLocation rejects a client-supplied table location that does not resolve
|
||||
// inside the caller's own bucket, the way the Iceberg gateway confines the same
|
||||
// field. The location feeds TableDataDirFromMetadataLocation, which joins it
|
||||
// under /buckets and collapses any "../" segments; an unconfined location lets a
|
||||
// marker write escape into another tenant's namespace.
|
||||
func confineLocation(bucket, location string) error {
|
||||
rest, ok := strings.CutPrefix(location, "s3://")
|
||||
if !ok {
|
||||
return fmt.Errorf("location must be an s3:// URI")
|
||||
}
|
||||
name, keys, _ := strings.Cut(rest, "/")
|
||||
if name != bucket {
|
||||
return fmt.Errorf("location must be within bucket %s", bucket)
|
||||
}
|
||||
for _, segment := range strings.Split(keys, "/") {
|
||||
switch {
|
||||
case segment == "." || segment == "..":
|
||||
return fmt.Errorf("location must not contain a path traversal segment")
|
||||
case strings.ContainsAny(segment, "\\\x00"):
|
||||
return fmt.Errorf("location contains an invalid character")
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// storageOptions builds the object_store settings a Lance client needs to reach
|
||||
// the dataset. The key names are the aws_-prefixed forms Lance clients pass
|
||||
// through to object_store.
|
||||
|
||||
Reference in New Issue
Block a user