mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-11 06:18:27 +01:00
s3tables: confine caller-supplied metadataLocation to the table's namespace
CreateTable/RegisterTable/UpdateTable (and the view equivalents) validate the location only for bucket and traversal, so a tenant can register a table whose location resolves into another namespace's directory and DeleteTable will purge it. Require the location to live under the caller's namespace subtree, the boundary per-namespace delegation uses. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
73949daa98
commit
6f93f87446
4 files changed
+40
-28
No files matched your search
@@ -100,7 +100,7 @@ func (h *S3TablesHandler) handleCreateTable(w http.ResponseWriter, r *http.Reque
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil {
|
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
|
||||||
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -233,7 +233,7 @@ func (h *S3TablesHandler) handleRegisterTable(w http.ResponseWriter, r *http.Req
|
|||||||
}
|
}
|
||||||
bucketName, namespaceName, tableName := target.bucketName, target.namespaceName, target.tableName
|
bucketName, namespaceName, tableName := target.bucketName, target.namespaceName, target.tableName
|
||||||
|
|
||||||
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil {
|
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
|
||||||
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -1494,7 +1494,7 @@ func (h *S3TablesHandler) handleUpdateTable(w http.ResponseWriter, r *http.Reque
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil {
|
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
|
||||||
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -121,7 +121,7 @@ func (h *S3TablesHandler) handleCreateView(w http.ResponseWriter, r *http.Reques
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil {
|
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
|
||||||
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -410,7 +410,7 @@ func (h *S3TablesHandler) handleUpdateView(w http.ResponseWriter, r *http.Reques
|
|||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName); err != nil {
|
if err := ValidateMetadataLocation(req.MetadataLocation, bucketName, namespaceName); err != nil {
|
||||||
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error())
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -127,11 +127,13 @@ func TableDataDirFromMetadataLocation(metadataLocation string) string {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// ValidateMetadataLocation checks that an s3:// metadata location stays within
|
// ValidateMetadataLocation checks that an s3:// metadata location stays within
|
||||||
// the authorized table bucket and rejects traversal segments that path.Join
|
// the authorized table bucket and inside the caller's own namespace subtree,
|
||||||
// would collapse to escape the bucket directory. Empty locations are allowed
|
// the boundary per-tenant policies are delegated on. Without the namespace
|
||||||
// (the catalog derives one). A non-empty location must include a table path so
|
// check a caller could register a location inside another tenant's data
|
||||||
// its metadata directory is table-specific, not shared at the bucket level.
|
// directory and DeleteTable would purge it. Empty locations are allowed (the
|
||||||
func ValidateMetadataLocation(metadataLocation, bucketName string) error {
|
// catalog derives one). A non-empty location must include a table path so its
|
||||||
|
// metadata directory is table-specific, not shared at the bucket level.
|
||||||
|
func ValidateMetadataLocation(metadataLocation, bucketName, namespace string) error {
|
||||||
if metadataLocation == "" {
|
if metadataLocation == "" {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -155,6 +157,9 @@ func ValidateMetadataLocation(metadataLocation, bucketName string) error {
|
|||||||
if !hasSegment {
|
if !hasSegment {
|
||||||
return fmt.Errorf("metadata location must include a table path")
|
return fmt.Errorf("metadata location must include a table path")
|
||||||
}
|
}
|
||||||
|
if !strings.HasPrefix(tablePath, namespace+"/") {
|
||||||
|
return fmt.Errorf("metadata location must be within namespace %s", namespace)
|
||||||
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -4,29 +4,36 @@ import "testing"
|
|||||||
|
|
||||||
func TestValidateMetadataLocation(t *testing.T) {
|
func TestValidateMetadataLocation(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
loc string
|
loc string
|
||||||
bucket string
|
bucket string
|
||||||
wantErr bool
|
namespace string
|
||||||
|
wantErr bool
|
||||||
}{
|
}{
|
||||||
{"empty allowed", "", "bkt", false},
|
{"empty allowed", "", "bkt", "ns", false},
|
||||||
{"same bucket", "s3://bkt/ns/t/metadata/v1.metadata.json", "bkt", false},
|
{"own namespace", "s3://bkt/ns/t/metadata/v1.metadata.json", "bkt", "ns", false},
|
||||||
{"cross bucket rejected", "s3://other/ns/t/metadata/v1.metadata.json", "bkt", true},
|
{"own namespace arbitrary layout", "s3://bkt/ns/warehouse/t/metadata/v1.metadata.json", "bkt", "ns", false},
|
||||||
{"bucket only rejected", "s3://bkt", "bkt", true},
|
{"cross bucket rejected", "s3://other/ns/t/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
{"bucket with trailing slash rejected", "s3://bkt/", "bkt", true},
|
{"cross namespace rejected", "s3://bkt/nsa/t/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
{"slash-only path rejected", "s3://bkt///", "bkt", true},
|
{"cross namespace data dir rejected", "s3://bkt/nsa/vtab/data/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
{"dotdot in path rejected", "s3://bkt/../victim/metadata/v1.metadata.json", "bkt", true},
|
{"namespace prefix rejected", "s3://bkt/nsa/vtab/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
{"dotdot mid path rejected", "s3://bkt/ns/../../victim/metadata/v1.metadata.json", "bkt", true},
|
{"outside namespace tree rejected", "s3://bkt/shared/t/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
{"dot segment rejected", "s3://bkt/ns/./t/metadata/v1.metadata.json", "bkt", true},
|
{"namespace dir without table path rejected", "s3://bkt/ns", "bkt", "ns", true},
|
||||||
{"backslash segment rejected", "s3://bkt/ns/\\t/metadata/v1.metadata.json", "bkt", true},
|
{"bucket only rejected", "s3://bkt", "bkt", "ns", true},
|
||||||
{"non-s3 scheme rejected", "file:///bkt/ns/t", "bkt", true},
|
{"bucket with trailing slash rejected", "s3://bkt/", "bkt", "ns", true},
|
||||||
{"empty location string", "s3://", "bkt", true},
|
{"slash-only path rejected", "s3://bkt///", "bkt", "ns", true},
|
||||||
|
{"dotdot in path rejected", "s3://bkt/../victim/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
|
{"dotdot mid path rejected", "s3://bkt/ns/../../victim/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
|
{"dot segment rejected", "s3://bkt/ns/./t/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
|
{"backslash segment rejected", "s3://bkt/ns/\\t/metadata/v1.metadata.json", "bkt", "ns", true},
|
||||||
|
{"non-s3 scheme rejected", "file:///bkt/ns/t", "bkt", "ns", true},
|
||||||
|
{"empty location string", "s3://", "bkt", "ns", true},
|
||||||
}
|
}
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
t.Run(tt.name, func(t *testing.T) {
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
err := ValidateMetadataLocation(tt.loc, tt.bucket)
|
err := ValidateMetadataLocation(tt.loc, tt.bucket, tt.namespace)
|
||||||
if (err != nil) != tt.wantErr {
|
if (err != nil) != tt.wantErr {
|
||||||
t.Fatalf("ValidateMetadataLocation(%q, %q) err = %v, wantErr = %v", tt.loc, tt.bucket, err, tt.wantErr)
|
t.Fatalf("ValidateMetadataLocation(%q, %q, %q) err = %v, wantErr = %v", tt.loc, tt.bucket, tt.namespace, err, tt.wantErr)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user