mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-14 18:40:48 +02:00
* iceberg: confine commit/transaction/view-update write paths to authorized bucket The create, register, and createView handlers already confine the client- supplied metadata location to the caller table bucket and reject ".." segments. The commit, create-on-commit, transaction, and view-update paths read the stored metadataLocation back from the catalog and skipped the same guard, so a location poisoned via the raw S3Tables UpdateTable API (which persists metadataLocation verbatim) could escape the caller bucket through a ".." segment that path.Join collapses in saveMetadataBlob. Add confineMetadataLocation and apply it after parseS3Location on every commit/update/transaction/view write path, mirroring the create/register/ createView check. Reject with 400 so a poisoned stored location fails the commit instead of writing into another tenant bucket tree. * s3tables: validate metadataLocation at the store layer The raw S3Tables API (CreateTable, RegisterTable, UpdateTable, CreateView, UpdateView) persisted the client-supplied metadataLocation verbatim with no bucket-confinement or traversal check, so a caller could store a location pointing outside its own bucket. The Iceberg REST gateway commit paths then read that stored value back and wrote through it. Add ValidateMetadataLocation and call it in every s3tables store handler that accepts a metadataLocation, rejecting locations whose bucket differs from the caller table bucket or whose path contains traversal segments. This prevents a poisoned location from ever being persisted, complementing the per-write-path guard added to the Iceberg commit handlers. * iceberg/s3tables: validate location before repair and after idempotency check Address review feedback: - Move the commit-path confinement check ahead of repairManifests so a poisoned stored location cannot reach manifest repair I/O before the commit is rejected. - Move ValidateMetadataLocation in CreateTable/CreateView to after the existing-resource check so idempotent retries that do not consume the requested location are not rejected for an unused bad location. - Assert HTTP 400 in the cross-tenant reproduction tests so an unrelated failure cannot satisfy them. * iceberg: confine staged metadata location before load in create-on-commit The create-on-commit path parsed the staged metadata location from the stage-create marker and called loadMetadataFile before validating that the staged bucket/path stay within the authorized bucket. Add the same confineMetadataLocation guard before the read so a tampered marker cannot direct a cross-tenant metadata read. * iceberg/s3tables: reject bucket-only metadata locations ValidateMetadataLocation and confineMetadataLocation accepted s3://bucket with an empty table path. metadataDirPath then maps every such table to the shared <TablesPath>/<bucket>/metadata directory, so tables could overwrite or read each other's metadata files. Require a non-empty table path in both validators; the empty-location case (where the catalog derives one) is unaffected. * iceberg/s3tables: reject slash-only table paths in location validation s3://bkt/// parses to tablePath="/" which passed the empty-string check but path.Join cleans it away, mapping to the bucket-level metadata directory shared across tables. Update isValidTablePath to require at least one non-empty segment and mirror the same check in ValidateMetadataLocation, closing the gap in all callers.
140 lines
4.6 KiB
Go
140 lines
4.6 KiB
Go
package iceberg
|
|
|
|
import (
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"testing"
|
|
|
|
"github.com/gorilla/mux"
|
|
)
|
|
|
|
func TestIsValidTablePath(t *testing.T) {
|
|
tests := []struct {
|
|
tablePath string
|
|
want bool
|
|
}{
|
|
{"sales.orders", true},
|
|
{"ns/table", true},
|
|
{"ns//table", true},
|
|
{"", false},
|
|
{"/", false},
|
|
{"//", false},
|
|
{"../other", false},
|
|
{"ns/../../etc", false},
|
|
{"ns/./x", false},
|
|
{`ns\..\x`, false},
|
|
}
|
|
for _, tt := range tests {
|
|
if got := isValidTablePath(tt.tablePath); got != tt.want {
|
|
t.Errorf("isValidTablePath(%q) = %v, want %v", tt.tablePath, got, tt.want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestValidateRequestPath_RejectsTraversal(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
rawPath string
|
|
wantCode int
|
|
}{
|
|
{"clean namespace+table passes", "/v1/namespaces/sales/tables/orders", http.StatusOK},
|
|
{"clean prefixed passes", "/v1/wh/namespaces/sales/tables/orders", http.StatusOK},
|
|
{"clean namespace only passes", "/v1/namespaces/sales", http.StatusOK},
|
|
|
|
// SkipClean(true) means raw `..` survives routing — these are the
|
|
// realistic traversal shapes the middleware must catch.
|
|
{"dotdot as prefix var rejected", "/v1/../namespaces/sales", http.StatusBadRequest},
|
|
{"dotdot as namespace var rejected", "/v1/namespaces/..", http.StatusBadRequest},
|
|
{"dotdot as namespace var prefixed rejected", "/v1/wh/namespaces/..", http.StatusBadRequest},
|
|
{"dotdot as table var rejected", "/v1/namespaces/sales/tables/..", http.StatusBadRequest},
|
|
{"dot as table var rejected", "/v1/namespaces/sales/tables/.", http.StatusBadRequest},
|
|
// Iceberg clients send the 0x1F unit separator percent-encoded; mux
|
|
// decodes it before the middleware sees the namespace var.
|
|
{"unit-sep namespace with dotdot part rejected", "/v1/namespaces/sales%1F..%1Fevil", http.StatusBadRequest},
|
|
{"leading unit-sep namespace rejected", "/v1/namespaces/%1Fsales", http.StatusBadRequest},
|
|
{"trailing unit-sep namespace rejected", "/v1/namespaces/sales%1F", http.StatusBadRequest},
|
|
{"consecutive unit-sep namespace rejected", "/v1/namespaces/sales%1F%1Fevil", http.StatusBadRequest},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
router := mux.NewRouter().SkipClean(true)
|
|
router.Use(validateRequestPath)
|
|
handlerCalled := false
|
|
pass := func(w http.ResponseWriter, r *http.Request) {
|
|
handlerCalled = true
|
|
w.WriteHeader(http.StatusOK)
|
|
}
|
|
router.HandleFunc("/v1/namespaces/{namespace}", pass)
|
|
router.HandleFunc("/v1/namespaces/{namespace}/tables/{table}", pass)
|
|
router.HandleFunc("/v1/{prefix}/namespaces/{namespace}", pass)
|
|
router.HandleFunc("/v1/{prefix}/namespaces/{namespace}/tables/{table}", pass)
|
|
|
|
req := httptest.NewRequest(http.MethodGet, tt.rawPath, nil)
|
|
rr := httptest.NewRecorder()
|
|
router.ServeHTTP(rr, req)
|
|
|
|
if rr.Code != tt.wantCode {
|
|
t.Fatalf("path %q: got status %d, want %d (body=%q)", tt.rawPath, rr.Code, tt.wantCode, rr.Body.String())
|
|
}
|
|
if tt.wantCode == http.StatusBadRequest && handlerCalled {
|
|
t.Fatalf("path %q: inner handler reached despite rejection", tt.rawPath)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// Defense-in-depth: if a future route or middleware ever leaves one of the
|
|
// captured vars empty, the middleware must still reject the request. The
|
|
// default mux regex won't normally allow this.
|
|
func TestValidateRequestPath_RejectsEmptyCapturedVars(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
vars map[string]string
|
|
}{
|
|
{"empty prefix", map[string]string{"prefix": "", "namespace": "ns"}},
|
|
{"empty table", map[string]string{"namespace": "ns", "table": ""}},
|
|
{"empty namespace", map[string]string{"namespace": ""}},
|
|
}
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
handlerCalled := false
|
|
h := validateRequestPath(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
|
handlerCalled = true
|
|
}))
|
|
req := mux.SetURLVars(httptest.NewRequest(http.MethodGet, "/", nil), tt.vars)
|
|
rr := httptest.NewRecorder()
|
|
h.ServeHTTP(rr, req)
|
|
if handlerCalled {
|
|
t.Fatalf("vars %v: inner handler reached despite empty capture", tt.vars)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestIsValidNameSegment(t *testing.T) {
|
|
tests := []struct {
|
|
name string
|
|
input string
|
|
want bool
|
|
}{
|
|
{"empty ok", "", true},
|
|
{"plain", "orders", true},
|
|
{"with dot inside", "my.table", true},
|
|
{"hidden", ".hidden", true},
|
|
|
|
{"bare dot", ".", false},
|
|
{"bare dotdot", "..", false},
|
|
{"contains slash", "foo/bar", false},
|
|
{"contains backslash", "foo\\bar", false},
|
|
{"contains nul", "foo\x00bar", false},
|
|
}
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
if got := isValidNameSegment(tt.input); got != tt.want {
|
|
t.Errorf("isValidNameSegment(%q) = %v, want %v", tt.input, got, tt.want)
|
|
}
|
|
})
|
|
}
|
|
}
|