From a6fa3c01d23d11b794fc0575479a793883376aff Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 19 Oct 2026 11:43:52 +0200 Subject: [PATCH] v0.333.0 (unreleased): the R-1155 check reads every token source FileBrowser reads, Basic auth included (security review) --- CHANGELOG.md | 9 ++++--- controller/internal/web/files_sso.go | 30 ++++++++++++++++++----- controller/internal/web/files_sso_test.go | 14 +++++++++++ 3 files changed, 44 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a5c7a87..8024552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,13 +7,16 @@ claim answer the agent already gives). No golden (`09` §3 decision 250). Task: one-sign-in user's FileBrowser token (HS256 JWT; claims `belongsTo`, `Permissions{admin,…}`, `iat`, `exp` 2 h, `iss` — no user name) still opened `/api/users?id=self` (200) after the dashboard sign-out, as a cookie and as a Bearer header. The forwardAuth answer (`ServeFilesAuth`) now refuses a request that has NO valid files cookie but carries a - NON-admin FileBrowser token — in the `filebrowser_quantum_jwt` cookie, an `Authorization: Bearer` header or the - `auth` query parameter — exactly like a stale files cookie: both cookies cleared, a browser sent to the dashboard + NON-admin FileBrowser token — anywhere FileBrowser 1.5.6 reads one (`backend/http/auth.go` `extractToken`): the + `filebrowser_quantum_jwt` cookie, the `auth` query parameter, `Authorization: Bearer` (split on a space, as FileBrowser + does) and `Authorization: Basic` with the token as the PASSWORD (the commit's security review found Basic missing + from the first version, c809dd3; never released) — exactly like a stale files cookie: both cookies cleared, a browser sent to the dashboard (`/__files/open`, which re-issues the pass while the session lives), anything else 401. `admin`'s token (the password fallback) and FileBrowser's sign-in page still pass; an unreadable token passes on to FileBrowser, which refuses it. The signature is not checked here and need not be: this check can only refuse. `carriesNonAdminFileBrowserToken`; `TestFilesSSO_R1155_HouseholdTokenDiesWithTheSignOut` — red-proved both ways: without the check the household's - token alone passes after the sign-out (200); with every token refused, admin's own token gets 302. + token alone passes after the sign-out (200); without the Basic branch the token as a Basic password passes (200); with + every token refused, admin's own token gets 302. - **The recovery-code page tells the truth about when the code is live (R-1161).** Since R-1151 (agent ≥ 0.160.0) the new code becomes live only when the two words come back; the page still said „Ez mostantól az élő helyreállítási kód". It now shows „Ez a kód az ellenőrzés után lesz az élő helyreállítási kód: írd be lent a két kért szót. Addig a diff --git a/controller/internal/web/files_sso.go b/controller/internal/web/files_sso.go index f65fd66..c20bbe6 100644 --- a/controller/internal/web/files_sso.go +++ b/controller/internal/web/files_sso.go @@ -187,8 +187,10 @@ func (s *Server) filesCookieKey(r *http.Request) (present, ok bool) { return true, s.filesSessionAlive(parts[0]) } -// carriesNonAdminFileBrowserToken: does the request carry a FileBrowser token (its cookie, a Bearer header, or the -// `auth` query parameter of the forwarded URI) whose permissions are not admin's? FileBrowser 1.5.6's token is an HS256 +// carriesNonAdminFileBrowserToken: does the request carry a FileBrowser token — anywhere FileBrowser 1.5.6 reads one: +// its cookie, the `auth` query parameter of the forwarded URI, an Authorization header (Bearer, or Basic with the token +// as the password) — whose permissions are not admin's? A source missed here is a way past this check (the security +// review of 2026-10-19 found Basic missing). FileBrowser 1.5.6's token is an HS256 // JWT with the claims belongsTo, Permissions{admin,…}, iat, exp, iss (measured on demo-hp 2026-10-19); the user's NAME // is not in it, so admin-or-not is the line: the box has `admin` (password) and the header user (never admin, // infra.fileBrowserDefaultPermissions). The signature is NOT checked here and need not be: this check can only REFUSE, @@ -199,12 +201,28 @@ func carriesNonAdminFileBrowserToken(r *http.Request, uri string) bool { if c, err := r.Cookie(fileBrowserJWTCookie); err == nil && c.Value != "" { toks = append(toks, c.Value) } - if a := r.Header.Get("Authorization"); len(a) > 7 && strings.EqualFold(a[:7], "bearer ") { - toks = append(toks, strings.TrimSpace(a[7:])) + // The Authorization header exactly as FileBrowser 1.5.6 reads it (backend/http/auth.go extractToken): split on " ", + // "bearer" → the second part; "basic" → the Basic-auth PASSWORD is the token. Our trimmed reading is added too. + if a := r.Header.Get("Authorization"); a != "" { + parts := strings.Split(a, " ") + switch strings.ToLower(parts[0]) { + case "bearer": + if len(parts) > 1 { + toks = append(toks, parts[1]) + } + toks = append(toks, strings.TrimSpace(a[len(parts[0]):])) + case "basic": + if _, pw, ok := r.BasicAuth(); ok { + toks = append(toks, pw) + } + } } + // `auth` is FileBrowser's token query parameter; `jwt` is its external-JWT one (off on our boxes — read anyway). if u, err := url.Parse(uri); err == nil { - if v := u.Query().Get("auth"); v != "" { - toks = append(toks, v) + for _, k := range []string{"auth", "jwt"} { + if v := u.Query().Get(k); v != "" { + toks = append(toks, v) + } } } for _, t := range toks { diff --git a/controller/internal/web/files_sso_test.go b/controller/internal/web/files_sso_test.go index 3d983e9..56c8926 100644 --- a/controller/internal/web/files_sso_test.go +++ b/controller/internal/web/files_sso_test.go @@ -433,6 +433,20 @@ func TestFilesSSO_R1155_HouseholdTokenDiesWithTheSignOut(t *testing.T) { if b := s.filesEntry(r); b.Code != http.StatusUnauthorized { t.Fatalf("API call with the household's token as Bearer: %d, want 401", b.Code) } + // Every other way FileBrowser 1.5.6 reads a token (backend/http/auth.go extractToken) — each must be refused too. + for name, hdr := range map[string]string{ + "Basic auth, the token as the password": "Basic " + base64.StdEncoding.EncodeToString([]byte("x:"+household.Value)), + "Bearer with two spaces": "Bearer " + household.Value, + "lower-case bearer, trailing words": "bearer " + household.Value + " x", + } { + r := httptest.NewRequest(http.MethodGet, "http://felhom-controller:8080"+filesAuthPath, nil) + r.Header.Set("X-Forwarded-Host", "files.example.hu") + r.Header.Set("X-Forwarded-Uri", "/api/users?id=self") + r.Header.Set("Authorization", hdr) + if b := s.filesEntry(r); b.Code != http.StatusUnauthorized { + t.Errorf("API call with the household's token as %s: %d, want 401", name, b.Code) + } + } if q := filesAsk(s, "files.example.hu", "/api/raw?files=x&auth="+url.QueryEscape(household.Value), "*/*"); q.Code != http.StatusUnauthorized { t.Fatalf("download with the household's token in ?auth=: %d, want 401", q.Code) }