mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-02 18:22:40 +01:00
filer: scope a prefix JWT to the directory a zip extract writes under (#12305)
* filer: scope a prefix JWT to the directory a zip extract writes under Without a trailing slash, an extract request writes members beside the named path, in its parent directory, while the JWT check only saw the request path. A token scoped to /tenant1 could therefore extract into / by posting to /tenant1?extract=zip. Add the extract root to the paths a prefix-restricted token must cover. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * filer: do not keep an extracted archive under the name ".." A multipart file name of ".." passes path.Base unchanged, so extract.keep stored the archive as an entry literally named ".." in the target directory, a name that local sinks resolve to the parent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * leveldb3: keep a non-local bucket name out of the per-bucket folder findDB splits the bucket out of a /buckets/ path and uses it as a folder name under the store directory. The name cannot hold a slash and dot names already go to the default DB, so this only rejects an empty or non-local name, but it also lets static analysis see the folder cannot escape the store directory. * filer: clean the zip extract root before scoping and writing A percent-encoded dot segment survives the path cleaning in front of the mux, so a request path ending in "/" reached extractZipRoot with ".." in it. The JWT scope check cleans the path it compares, but the members were written under the raw root, so the two could disagree. * leveldb3: apply the bucket folder check to bucket create and delete events findDB already keeps dot and non-local bucket names out of the per-bucket folders, but OnBucketCreation and OnBucketDeletion used the name as is. A directory event for /buckets/.. opened a DB in the parent of the store directory, and its delete removed that parent. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * leveldb3: keep a bucket delete event off the default DB A bucket named _main maps to the default DB in findDB, so deleting /buckets/_main closed the default DB and removed its folder, taking every non-bucket entry with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * leveldb3: keep bucket names with a path separator out of the store dir A bucket event named x/../_main passed the local-name check, and its delete removed the default DB folder once store.dir/x existed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * leveldb3: only reject the platform separator in a bucket name A backslash is an ordinary file name character on Unix, and a bucket folder named that way already exists for anyone who created one. Rejecting it on every platform would route those entries to the default DB after an upgrade. The slash can never appear in a bucket name here, and on Windows the backslash is the separator, so rejecting os.PathSeparator alongside the slash keeps the folder flat without changing Unix routing. * leveldb3: check for an escaped bucket db before its delete event --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
d84ae4f2ab
commit
533f5060b9
7 files changed
+184
-14
No files matched your search
@@ -8,6 +8,7 @@ import (
|
||||
"io"
|
||||
"math"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
|
||||
@@ -167,9 +168,7 @@ func (store *LevelDB3Store) findDB(fullpath weed_util.FullPath, isForChildren bo
|
||||
shortPath = weed_util.FullPath(bucketAndObjectKey[t:])
|
||||
}
|
||||
|
||||
// Dot-prefixed entries directly under /buckets (e.g. .system) are internal
|
||||
// folders, not S3 buckets; keep them in the default DB by full path.
|
||||
if strings.HasPrefix(bucket, ".") {
|
||||
if !isBucketFolder(bucket) {
|
||||
store.dbsLock.RUnlock()
|
||||
return defaultDB, DEFAULT, fullpath, nil
|
||||
}
|
||||
@@ -207,6 +206,14 @@ func (store *LevelDB3Store) emptyBucketDB() (*leveldb.DB, error) {
|
||||
return store.emptyDB, nil
|
||||
}
|
||||
|
||||
// Dot-prefixed entries directly under /buckets (e.g. .system) are internal
|
||||
// folders, not S3 buckets; they stay in the default DB by full path. A bucket
|
||||
// also names a single folder under store.dir, so it cannot hold a separator;
|
||||
// "\" is an ordinary name character on Unix and only rejected where it separates.
|
||||
func isBucketFolder(bucket string) bool {
|
||||
return !strings.HasPrefix(bucket, ".") && filepath.IsLocal(bucket) && !strings.ContainsAny(bucket, "/"+string(os.PathSeparator))
|
||||
}
|
||||
|
||||
func (store *LevelDB3Store) createDB(bucket string) (*leveldb.DB, error) {
|
||||
|
||||
store.dbsLock.Lock()
|
||||
|
||||
@@ -9,14 +9,18 @@ import (
|
||||
var _ filer.BucketAware = (*LevelDB3Store)(nil)
|
||||
|
||||
func (store *LevelDB3Store) OnBucketCreation(bucket string) {
|
||||
if !isBucketFolder(bucket) {
|
||||
return
|
||||
}
|
||||
store.createDB(bucket)
|
||||
}
|
||||
|
||||
func (store *LevelDB3Store) OnBucketDeletion(bucket string) {
|
||||
store.closeDB(bucket)
|
||||
if bucket != "" { // just to make sure
|
||||
os.RemoveAll(store.dir + "/" + bucket)
|
||||
if !isBucketFolder(bucket) || bucket == DEFAULT {
|
||||
return
|
||||
}
|
||||
store.closeDB(bucket)
|
||||
os.RemoveAll(store.dir + "/" + bucket)
|
||||
}
|
||||
|
||||
func (store *LevelDB3Store) CanDropWholeBucket() bool {
|
||||
|
||||
@@ -3,6 +3,8 @@ package leveldb
|
||||
import (
|
||||
"context"
|
||||
"math"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -207,3 +209,44 @@ func TestListDirectoryPrefixedEntries_StartBelowPrefix(t *testing.T) {
|
||||
t.Fatalf("want [item-b.versions], got %v", names)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBucketEventsStayInsideStoreDir(t *testing.T) {
|
||||
parent := t.TempDir()
|
||||
dir := filepath.Join(parent, "store")
|
||||
store := &LevelDB3Store{}
|
||||
if err := store.initialize(dir); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer store.Shutdown()
|
||||
|
||||
sentinel := filepath.Join(parent, "keep")
|
||||
if err := os.WriteFile(sentinel, []byte("keep"), 0644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// Checked after each create, before the matching delete could remove the
|
||||
// evidence of a db opened in the wrong place.
|
||||
assertNothingEscaped := func(bucket string) {
|
||||
t.Helper()
|
||||
for _, p := range []string{filepath.Join(parent, "outside"), filepath.Join(parent, "CURRENT"), filepath.Join(dir, "x", "y")} {
|
||||
if _, err := os.Stat(p); !os.IsNotExist(err) {
|
||||
t.Fatalf("bucket %q event created %s: %v", bucket, p, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
store.OnBucketCreation("x")
|
||||
for _, bucket := range []string{"..", ".", "../outside", "", DEFAULT, "x/../" + DEFAULT, "x/y"} {
|
||||
store.OnBucketCreation(bucket)
|
||||
assertNothingEscaped(bucket)
|
||||
store.OnBucketDeletion(bucket)
|
||||
}
|
||||
if _, err := os.Stat(sentinel); err != nil {
|
||||
t.Fatalf("bucket events removed %s: %v", sentinel, err)
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(dir, DEFAULT)); err != nil {
|
||||
t.Fatalf("default db folder missing: %v", err)
|
||||
}
|
||||
ctx := context.Background()
|
||||
if err := store.InsertEntry(ctx, &filer.Entry{FullPath: "/a/b"}); err != nil {
|
||||
t.Fatalf("default db unusable after bucket events: %v", err)
|
||||
}
|
||||
}
|
||||
@@ -38,6 +38,7 @@ func TestFilerServer_maybeCheckJwtAuthorization_Scoped(t *testing.T) {
|
||||
token string
|
||||
method string
|
||||
path string
|
||||
header map[string]string
|
||||
isWrite bool
|
||||
expectAuthorized bool
|
||||
}{
|
||||
@@ -225,6 +226,63 @@ func TestFilerServer_maybeCheckJwtAuthorization_Scoped(t *testing.T) {
|
||||
isWrite: true,
|
||||
expectAuthorized: false,
|
||||
},
|
||||
{
|
||||
name: "write at the prefix itself allowed",
|
||||
token: genToken([]string{"/tenant1"}, nil),
|
||||
method: "POST",
|
||||
path: "/tenant1",
|
||||
isWrite: true,
|
||||
expectAuthorized: true,
|
||||
},
|
||||
{
|
||||
name: "extract into the prefix directory allowed",
|
||||
token: genToken([]string{"/tenant1"}, nil),
|
||||
method: "POST",
|
||||
path: "/tenant1/?extract=zip",
|
||||
isWrite: true,
|
||||
expectAuthorized: true,
|
||||
},
|
||||
{
|
||||
name: "extract beside an archive inside the prefix allowed",
|
||||
token: genToken([]string{"/tenant1"}, nil),
|
||||
method: "PUT",
|
||||
path: "/tenant1/pack.zip?extract=zip",
|
||||
isWrite: true,
|
||||
expectAuthorized: true,
|
||||
},
|
||||
{
|
||||
name: "extract at the prefix itself writes to its parent, denied",
|
||||
token: genToken([]string{"/tenant1"}, nil),
|
||||
method: "POST",
|
||||
path: "/tenant1?extract=zip",
|
||||
isWrite: true,
|
||||
expectAuthorized: false,
|
||||
},
|
||||
{
|
||||
name: "extract header at the prefix itself denied",
|
||||
token: genToken([]string{"/tenant1/"}, []string{"PUT"}),
|
||||
method: "PUT",
|
||||
path: "/tenant1",
|
||||
header: map[string]string{"X-Seaweedfs-Extract": "zip"},
|
||||
isWrite: true,
|
||||
expectAuthorized: false,
|
||||
},
|
||||
{
|
||||
name: "extract at a nested prefix into the sibling parent denied",
|
||||
token: genToken([]string{"/buckets/bucket-a"}, nil),
|
||||
method: "POST",
|
||||
path: "/buckets/bucket-a?extract=zip",
|
||||
isWrite: true,
|
||||
expectAuthorized: false,
|
||||
},
|
||||
{
|
||||
name: "extract with a root prefix allowed",
|
||||
token: genToken([]string{"/"}, nil),
|
||||
method: "POST",
|
||||
path: "/tenant1?extract=zip",
|
||||
isWrite: true,
|
||||
expectAuthorized: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
@@ -233,6 +291,9 @@ func TestFilerServer_maybeCheckJwtAuthorization_Scoped(t *testing.T) {
|
||||
if tt.token != "" {
|
||||
req.Header.Set("Authorization", "Bearer "+tt.token)
|
||||
}
|
||||
for k, v := range tt.header {
|
||||
req.Header.Set(k, v)
|
||||
}
|
||||
if authorized := fs.maybeCheckJwtAuthorization(req, tt.isWrite); authorized != tt.expectAuthorized {
|
||||
t.Errorf("expected authorized=%v, got %v", tt.expectAuthorized, authorized)
|
||||
}
|
||||
|
||||
@@ -308,10 +308,14 @@ func authorizeFilerJwtPaths(r *http.Request, claims *security.SeaweedFilerClaims
|
||||
}
|
||||
|
||||
// jwtScopedRequestPaths returns every filer path a request touches that must be
|
||||
// covered by the JWT AllowedPrefixes: the write target (r.URL.Path) plus any
|
||||
// copy/move source named by the cp.from / mv.from query parameters.
|
||||
// covered by the JWT AllowedPrefixes: the write target (r.URL.Path), the
|
||||
// directory a zip extract writes under, and any copy/move source named by the
|
||||
// cp.from / mv.from query parameters.
|
||||
func jwtScopedRequestPaths(r *http.Request) []string {
|
||||
paths := []string{r.URL.Path}
|
||||
if (r.Method == http.MethodPost || r.Method == http.MethodPut) && isExtractZip(r) {
|
||||
paths = append(paths, extractZipRoot(r)+"/")
|
||||
}
|
||||
if query := r.URL.Query(); query.Has("cp.from") || query.Has("mv.from") {
|
||||
if from := query.Get("cp.from"); from != "" {
|
||||
paths = append(paths, from)
|
||||
|
||||
@@ -35,6 +35,17 @@ func isExtractZip(r *http.Request) bool {
|
||||
r.Header.Get(s3_constants.SeaweedFSExtract) == "zip"
|
||||
}
|
||||
|
||||
// extractZipRoot is the directory members land under: the request path when it
|
||||
// ends in "/", otherwise its parent, so the path can name the archive. It is
|
||||
// cleaned so the JWT scope check and the writes agree on dot segments.
|
||||
func extractZipRoot(r *http.Request) string {
|
||||
root := r.URL.Path
|
||||
if !strings.HasSuffix(root, "/") {
|
||||
root = path.Dir(root)
|
||||
}
|
||||
return strings.TrimSuffix(path.Clean(root), "/")
|
||||
}
|
||||
|
||||
// extractZipArchive writes a posted archive's members as individual entries
|
||||
// under the target directory, like AWS Snowball's auto-extract. The archive
|
||||
// itself is only stored when ?extract.keep=true.
|
||||
@@ -81,11 +92,7 @@ func (fs *FilerServer) extractZipArchive(ctx context.Context, w http.ResponseWri
|
||||
return
|
||||
}
|
||||
|
||||
root := r.URL.Path
|
||||
if !strings.HasSuffix(root, "/") {
|
||||
root = path.Dir(root)
|
||||
}
|
||||
root = strings.TrimSuffix(root, "/")
|
||||
root := extractZipRoot(r)
|
||||
|
||||
query := r.URL.Query()
|
||||
chunkSize := fs.requestChunkSize(query)
|
||||
@@ -153,7 +160,7 @@ func (fs *FilerServer) extractZipArchive(ctx context.Context, w http.ResponseWri
|
||||
}
|
||||
}
|
||||
|
||||
if query.Get("extract.keep") == "true" && archiveName != "." && archiveName != "/" {
|
||||
if query.Get("extract.keep") == "true" && archiveName != "." && archiveName != ".." && archiveName != "/" {
|
||||
mode := os.FileMode(0660)
|
||||
if modeStr := query.Get("mode"); modeStr != "" {
|
||||
if parsed, parseErr := strconv.ParseUint(modeStr, 8, 32); parseErr == nil {
|
||||
|
||||
@@ -168,6 +168,28 @@ func TestExtractZipArchiveKeep(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractZipArchiveKeepSkipsParentName(t *testing.T) {
|
||||
fs, f := newExtractTestServer(t)
|
||||
zipBytes := buildTestZip(t, map[string]string{"x.txt": "data"}, nil, nil)
|
||||
|
||||
var body bytes.Buffer
|
||||
mw := multipartWriter(t, &body, "..", zipBytes)
|
||||
r := httptest.NewRequest(http.MethodPost, "/a/b/?extract=zip&extract.keep=true", &body)
|
||||
r.Header.Set("Content-Type", mw)
|
||||
w := httptest.NewRecorder()
|
||||
fs.PostHandler(w, r, int64(body.Len()))
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("status = %d, body: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if _, err := f.FindEntry(r.Context(), util.FullPath("/a/b/..")); err == nil {
|
||||
t.Fatal("archive kept under a parent-directory name")
|
||||
}
|
||||
if _, err := f.FindEntry(r.Context(), util.FullPath("/a/b/x.txt")); err != nil {
|
||||
t.Fatalf("member missing: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractZipArchiveNotAZip(t *testing.T) {
|
||||
fs, _ := newExtractTestServer(t)
|
||||
r := httptest.NewRequest(http.MethodPut, "/dst/notzip.bin?extract=zip", bytes.NewReader([]byte("not a zip")))
|
||||
@@ -231,6 +253,28 @@ func TestExtractZipArchiveRawPost(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractZipArchiveCleansDotSegments(t *testing.T) {
|
||||
fs, f := newExtractTestServer(t)
|
||||
zipBytes := buildTestZip(t, map[string]string{"raw.txt": "raw"}, nil, nil)
|
||||
|
||||
// A percent-encoded ".." survives path cleaning in front of the mux, so the
|
||||
// handler sees it decoded and must land members under the cleaned root.
|
||||
r := httptest.NewRequest(http.MethodPost, "/other/%2e%2e/dst/?extract=zip", bytes.NewReader(zipBytes))
|
||||
r.Header.Set("Content-Type", "application/zip")
|
||||
w := httptest.NewRecorder()
|
||||
fs.PostHandler(w, r, int64(len(zipBytes)))
|
||||
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("status = %d, body: %s", w.Code, w.Body.String())
|
||||
}
|
||||
if _, err := f.FindEntry(r.Context(), util.FullPath("/dst/raw.txt")); err != nil {
|
||||
t.Fatalf("member missing under the cleaned root: %v", err)
|
||||
}
|
||||
if _, err := f.FindEntry(r.Context(), util.FullPath("/other/../dst/raw.txt")); err == nil {
|
||||
t.Fatal("member written under the raw root")
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractZipArchiveMultipartPost(t *testing.T) {
|
||||
fs, f := newExtractTestServer(t)
|
||||
zipBytes := buildTestZip(t, map[string]string{"img/1.png": "png-bytes"}, nil, nil)
|
||||
|
||||
Reference in new issue
Block a user