From 4f1f0dcb17b42390895c88aa9400bf8ccc63e48a Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 6 Jul 2026 18:25:12 -0700 Subject: [PATCH] filer: require JWT authorization on TUS upload endpoints (#10249) * filer: require JWT authorization on TUS upload endpoints filerHandler and readonlyFilerHandler run every request through maybeCheckJwtAuthorization, but the TUS routes were registered only behind filerGuard.WhiteList, which is a pass-through for the filer (its guard is built with an empty whitelist), and no TUS handler called the JWT check. So with jwt.filer_signing.key set, normal PUT/POST/DELETE were authorized while the TUS endpoints were not. Run the same check in tusHandler before routing: HEAD uses the read key, POST/PATCH/DELETE use the write key, and creation scopes a prefix-restricted token against the resolved target path. OPTIONS stays open for capability discovery. * filer: enforce read-only and WORM rules on TUS completion completeTusUpload wrote the final entry with CreateEntry directly, skipping the read-only and WORM checks the normal write path applies. Reject completion when the target prefix is read-only or the existing entry at the target is WORM-enforced. * filer: align TUS write path with the normal write path Resolve the create target with a guaranteed leading slash so a prefix-restricted token and the stored path stay absolute even if TusBasePath were set with a trailing slash. Reject read-only prefixes at session creation before any chunk is written, and map read-only and WORM rejections at completion to 507 and 403 instead of a generic 500. * filer: cover method-restricted TUS tokens in the auth test --- weed/server/filer_server_handlers.go | 12 ++- weed/server/filer_server_tus_handlers.go | 71 ++++++++++++++- weed/server/filer_server_tus_jwt_test.go | 111 +++++++++++++++++++++++ weed/server/filer_server_tus_session.go | 19 ++++ 4 files changed, 207 insertions(+), 6 deletions(-) create mode 100644 weed/server/filer_server_tus_jwt_test.go diff --git a/weed/server/filer_server_handlers.go b/weed/server/filer_server_handlers.go index 3062c3f72..da6a3e9d6 100644 --- a/weed/server/filer_server_handlers.go +++ b/weed/server/filer_server_handlers.go @@ -216,6 +216,16 @@ func (fs *FilerServer) maybeCheckJwtAuthorization(r *http.Request, isWrite bool) return true } + return fs.checkJwtAuthorization(r, isWrite, jwtScopedRequestPaths(r)) +} + +// checkJwtAuthorization verifies the request carries a valid filer JWT for the +// requested access level and, for prefix-restricted tokens, that every path in +// scopedPaths falls within the token's AllowedPrefixes. Callers whose write +// target is not r.URL.Path — the TUS handler, whose URL points at the /.tus +// route — pass the resolved filer path(s) here instead. +func (fs *FilerServer) checkJwtAuthorization(r *http.Request, isWrite bool, scopedPaths []string) bool { + var signingKey security.SigningKey if isWrite { @@ -256,7 +266,7 @@ func (fs *FilerServer) maybeCheckJwtAuthorization(r *http.Request, isWrite bool) // Copy and move name their source via a query parameter, not r.URL.Path. // Scope every path the request reads or relocates, or a prefix-restricted // token could reach data outside its allowed subtree. - for _, p := range jwtScopedRequestPaths(r) { + for _, p := range scopedPaths { if !anyComponentPrefixMatches(claims.AllowedPrefixes, p) { glog.V(1).Infof("jwt path not allowed from %s: %v", r.RemoteAddr, p) return false diff --git a/weed/server/filer_server_tus_handlers.go b/weed/server/filer_server_tus_handlers.go index 224eb72e9..d2d815fe5 100644 --- a/weed/server/filer_server_tus_handlers.go +++ b/weed/server/filer_server_tus_handlers.go @@ -26,13 +26,23 @@ func (fs *FilerServer) tusHandler(w http.ResponseWriter, r *http.Request) { // Set common TUS response headers w.Header().Set("Tus-Resumable", TusVersion) - // Check Tus-Resumable header for non-OPTIONS requests + // OPTIONS is capability discovery only and carries no data, so it is left + // unauthenticated like the main filer OPTIONS handler. Every other TUS method + // mutates or reads the filer namespace and must pass the same JWT check the + // rest of the filer API enforces: the TUS routes are otherwise only wrapped by + // filerGuard.WhiteList, which is a no-op for the filer, so without this they + // bypass jwt.filer_signing entirely. if r.Method != http.MethodOptions { tusVersion := r.Header.Get("Tus-Resumable") if tusVersion != TusVersion { http.Error(w, "Unsupported TUS version", http.StatusPreconditionFailed) return } + + if !fs.checkTusJwtAuthorization(r) { + writeJsonError(w, r, http.StatusUnauthorized, errors.New("wrong jwt")) + return + } } // Route based on method and path @@ -70,6 +80,50 @@ func (fs *FilerServer) tusHandler(w http.ResponseWriter, r *http.Request) { } } +// checkTusJwtAuthorization enforces filer JWT authorization for a TUS request. +// HEAD reads the current offset (read access); POST/PATCH/DELETE mutate the +// namespace (write access). Only creation (POST) names a new target path, taken +// from the request URL, so prefix-restricted tokens are scoped against that +// resolved filer path rather than the /.tus route. HEAD/PATCH/DELETE act on an +// existing session addressed by an unguessable id, whose target was already +// scope-checked at creation, so no further path scoping is applied to them. +func (fs *FilerServer) checkTusJwtAuthorization(r *http.Request) bool { + isWrite := r.Method != http.MethodHead + var scopedPaths []string + if r.Method == http.MethodPost { + if target := fs.tusTargetPath(r); target != "" && target != "/" { + scopedPaths = append(scopedPaths, target) + } + } + return fs.checkJwtAuthorization(r, isWrite, scopedPaths) +} + +// tusTargetPath resolves the filer path a TUS create request targets from the +// request URL. It guarantees a leading slash so the result matches stored +// absolute paths and JWT AllowedPrefixes even if TusBasePath were misconfigured +// with a trailing slash. +func (fs *FilerServer) tusTargetPath(r *http.Request) string { + target := strings.TrimPrefix(r.URL.Path, fs.option.TusBasePath) + if target != "" && !strings.HasPrefix(target, "/") { + target = "/" + target + } + return target +} + +// writeTusCompleteError maps a completeTusUpload failure to the same HTTP status +// the normal write path uses: a read-only prefix returns 507 and a WORM-protected +// target returns 403, rather than a generic 500. +func writeTusCompleteError(w http.ResponseWriter, err error) { + switch { + case errors.Is(err, ErrReadOnly): + http.Error(w, err.Error(), http.StatusInsufficientStorage) + case errors.Is(err, ErrWormEnforced): + http.Error(w, err.Error(), http.StatusForbidden) + default: + http.Error(w, "Failed to complete upload", http.StatusInternalServerError) + } +} + // tusOptionsHandler handles OPTIONS requests for capability discovery func (fs *FilerServer) tusOptionsHandler(w http.ResponseWriter, r *http.Request) { w.Header().Set("Tus-Version", TusVersion) @@ -107,13 +161,20 @@ func (fs *FilerServer) tusCreateHandler(w http.ResponseWriter, r *http.Request) // TusBasePath is pre-normalized in filer_server.go (leading slash, no trailing slash) tusPrefix := fs.option.TusBasePath - // Determine target path from request URL - targetPath := strings.TrimPrefix(r.URL.Path, tusPrefix) + // Determine target path from request URL (leading slash guaranteed) + targetPath := fs.tusTargetPath(r) if targetPath == "" || targetPath == "/" { http.Error(w, "Target path required", http.StatusBadRequest) return } + // Reject writes to a read-only prefix up front, before creating a session or + // uploading any chunks, matching the normal write path. + if fs.filer.FilerConf.MatchStorageRule(targetPath).ReadOnly { + http.Error(w, ErrReadOnly.Error(), http.StatusInsufficientStorage) + return + } + // Generate upload ID uploadID := uuid.New().String() @@ -168,7 +229,7 @@ func (fs *FilerServer) tusCreateHandler(w http.ResponseWriter, r *http.Request) } if err := fs.completeTusUpload(ctx, session); err != nil { glog.Errorf("Failed to complete TUS upload: %v", err) - http.Error(w, "Failed to complete upload", http.StatusInternalServerError) + writeTusCompleteError(w, err) return } } @@ -268,7 +329,7 @@ func (fs *FilerServer) tusPatchHandler(w http.ResponseWriter, r *http.Request, u if err := fs.completeTusUpload(ctx, session); err != nil { glog.Errorf("Failed to complete TUS upload: %v", err) - http.Error(w, "Failed to complete upload", http.StatusInternalServerError) + writeTusCompleteError(w, err) return } } diff --git a/weed/server/filer_server_tus_jwt_test.go b/weed/server/filer_server_tus_jwt_test.go new file mode 100644 index 000000000..53a6f49de --- /dev/null +++ b/weed/server/filer_server_tus_jwt_test.go @@ -0,0 +1,111 @@ +package weed_server + +import ( + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/golang-jwt/jwt/v5" + "github.com/seaweedfs/seaweedfs/weed/security" +) + +// signFilerToken builds a signed filer JWT for tests. +func signFilerToken(t *testing.T, signingKey string, allowedPrefixes, allowedMethods []string) string { + t.Helper() + claims := security.SeaweedFilerClaims{ + AllowedPrefixes: allowedPrefixes, + AllowedMethods: allowedMethods, + RegisteredClaims: jwt.RegisteredClaims{ + ExpiresAt: jwt.NewNumericDate(time.Now().Add(1 * time.Hour)), + }, + } + str, err := jwt.NewWithClaims(jwt.SigningMethodHS256, claims).SignedString([]byte(signingKey)) + if err != nil { + t.Fatalf("failed to sign token: %v", err) + } + return str +} + +func TestFilerServer_checkTusJwtAuthorization(t *testing.T) { + const writeKey = "write-secret" + const readKey = "read-secret" + fs := &FilerServer{ + filerGuard: security.NewGuard(nil, writeKey, 0, readKey, 0), + option: &FilerOption{TusBasePath: "/.tus"}, + } + + tests := []struct { + name string + method string + path string + token string + expectAuthorized bool + }{ + // The advisory: with a filer signing key configured, an unauthenticated + // TUS request must be rejected the same as a normal filer write. + {"create without token denied", http.MethodPost, "/.tus/buckets/secret/owned.txt", "", false}, + {"patch without token denied", http.MethodPatch, "/.tus/.uploads/abc", "", false}, + {"delete without token denied", http.MethodDelete, "/.tus/.uploads/abc", "", false}, + {"head without token denied", http.MethodHead, "/.tus/.uploads/abc", "", false}, + + // A valid token for the right access level is accepted. + {"create with write token allowed", http.MethodPost, "/.tus/buckets/data/ok.txt", signFilerToken(t, writeKey, nil, nil), true}, + {"patch with write token allowed", http.MethodPatch, "/.tus/.uploads/abc", signFilerToken(t, writeKey, nil, nil), true}, + {"head with read token allowed", http.MethodHead, "/.tus/.uploads/abc", signFilerToken(t, readKey, nil, nil), true}, + + // HEAD is a read, so a write-only token must not authorize it and a read + // token must not authorize a write. + {"head with write token denied", http.MethodHead, "/.tus/.uploads/abc", signFilerToken(t, writeKey, nil, nil), false}, + {"create with read token denied", http.MethodPost, "/.tus/buckets/data/ok.txt", signFilerToken(t, readKey, nil, nil), false}, + + // Prefix-restricted tokens are scoped against the resolved target path + // (URL minus the /.tus prefix), not the /.tus route. + {"create within allowed prefix", http.MethodPost, "/.tus/buckets/allowed/ok.txt", signFilerToken(t, writeKey, []string{"/buckets/allowed"}, nil), true}, + {"create outside allowed prefix denied", http.MethodPost, "/.tus/buckets/secret/owned.txt", signFilerToken(t, writeKey, []string{"/buckets/allowed"}, nil), false}, + + // Method-restricted tokens are checked against the actual HTTP method. + {"patch with matching method allowed", http.MethodPatch, "/.tus/.uploads/abc", signFilerToken(t, writeKey, nil, []string{"POST", "PATCH", "DELETE"}), true}, + {"patch outside allowed methods denied", http.MethodPatch, "/.tus/.uploads/abc", signFilerToken(t, writeKey, nil, []string{"POST"}), false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(tt.method, tt.path, nil) + if tt.token != "" { + req.Header.Set("Authorization", "Bearer "+tt.token) + } + if got := fs.checkTusJwtAuthorization(req); got != tt.expectAuthorized { + t.Errorf("checkTusJwtAuthorization(%s %s) = %v, want %v", tt.method, tt.path, got, tt.expectAuthorized) + } + }) + } +} + +// TestFilerServer_tusHandler_UnauthenticatedRejected exercises the full handler +// entry point: OPTIONS discovery stays open, but an unauthenticated write is +// rejected with 401 before any session is created. +func TestFilerServer_tusHandler_UnauthenticatedRejected(t *testing.T) { + fs := &FilerServer{ + filerGuard: security.NewGuard(nil, "write-secret", 0, "read-secret", 0), + option: &FilerOption{TusBasePath: "/.tus"}, + } + + // OPTIONS is capability discovery and must not require a token. + optionsReq := httptest.NewRequest(http.MethodOptions, "/.tus/buckets/secret/owned.txt", nil) + optionsRec := httptest.NewRecorder() + fs.tusHandler(optionsRec, optionsReq) + if optionsRec.Code != http.StatusOK { + t.Errorf("OPTIONS without token = %d, want %d", optionsRec.Code, http.StatusOK) + } + + // POST without a token must be rejected before touching the filer store. + postReq := httptest.NewRequest(http.MethodPost, "/.tus/buckets/secret/owned.txt", nil) + postReq.Header.Set("Tus-Resumable", TusVersion) + postReq.Header.Set("Upload-Length", "5") + postRec := httptest.NewRecorder() + fs.tusHandler(postRec, postReq) + if postRec.Code != http.StatusUnauthorized { + t.Errorf("unauthenticated POST = %d, want %d", postRec.Code, http.StatusUnauthorized) + } +} diff --git a/weed/server/filer_server_tus_session.go b/weed/server/filer_server_tus_session.go index 2b804b3d8..009d2a3eb 100644 --- a/weed/server/filer_server_tus_session.go +++ b/weed/server/filer_server_tus_session.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "encoding/json" + "errors" "fmt" "os" "sort" @@ -15,6 +16,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/util" + "github.com/seaweedfs/seaweedfs/weed/util/constants" ) const ( @@ -26,6 +28,11 @@ const ( TusExtensions = "creation,creation-with-upload,termination" ) +// ErrWormEnforced marks a TUS completion rejected because the target entry is +// WORM-protected. It shares the message the normal write path uses so it maps to +// the same client-facing status. +var ErrWormEnforced = errors.New(constants.ErrMsgOperationNotPermitted) + // TusSession represents an in-progress TUS upload session type TusSession struct { ID string `json:"id"` @@ -357,6 +364,18 @@ func (fs *FilerServer) completeTusUpload(ctx context.Context, session *TusSessio // Create the final file entry targetPath := util.FullPath(session.TargetPath) + + // Apply the same read-only / WORM protections the normal write path enforces + // before landing the entry at the client-chosen target path. + if fs.filer.FilerConf.MatchStorageRule(string(targetPath)).ReadOnly { + return fmt.Errorf("%w: %s", ErrReadOnly, targetPath) + } + if wormEnforced, err := fs.wormEnforcedForEntry(ctx, string(targetPath)); err != nil { + return fmt.Errorf("check worm: %w", err) + } else if wormEnforced { + return ErrWormEnforced + } + entry := &filer.Entry{ FullPath: targetPath, Attr: filer.Attr{