diff --git a/weed/filer/leveldb3/leveldb3_store.go b/weed/filer/leveldb3/leveldb3_store.go index 3111e2a5ad..5484383713 100644 --- a/weed/filer/leveldb3/leveldb3_store.go +++ b/weed/filer/leveldb3/leveldb3_store.go @@ -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() diff --git a/weed/filer/leveldb3/leveldb3_store_bucket.go b/weed/filer/leveldb3/leveldb3_store_bucket.go index feb2a4b049..2b56cad6ca 100644 --- a/weed/filer/leveldb3/leveldb3_store_bucket.go +++ b/weed/filer/leveldb3/leveldb3_store_bucket.go @@ -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 { diff --git a/weed/filer/leveldb3/leveldb3_store_test.go b/weed/filer/leveldb3/leveldb3_store_test.go index a9f3be10da..1f57acd4c1 100644 --- a/weed/filer/leveldb3/leveldb3_store_test.go +++ b/weed/filer/leveldb3/leveldb3_store_test.go @@ -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) + } +} diff --git a/weed/server/filer_jwt_test.go b/weed/server/filer_jwt_test.go index 74f5bb9bcc..efd4b99c60 100644 --- a/weed/server/filer_jwt_test.go +++ b/weed/server/filer_jwt_test.go @@ -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) } diff --git a/weed/server/filer_server_handlers.go b/weed/server/filer_server_handlers.go index 6fd9bfbf3e..4bdd5d87f9 100644 --- a/weed/server/filer_server_handlers.go +++ b/weed/server/filer_server_handlers.go @@ -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) diff --git a/weed/server/filer_server_handlers_extract.go b/weed/server/filer_server_handlers_extract.go index c51d7b9c6f..97057ee3fc 100644 --- a/weed/server/filer_server_handlers_extract.go +++ b/weed/server/filer_server_handlers_extract.go @@ -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 { diff --git a/weed/server/filer_server_handlers_extract_test.go b/weed/server/filer_server_handlers_extract_test.go index 9461e79e5a..3040a9ad4a 100644 --- a/weed/server/filer_server_handlers_extract_test.go +++ b/weed/server/filer_server_handlers_extract_test.go @@ -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)