v0.333.0 (unreleased): the R-1155 check reads every token source FileBrowser reads, Basic auth included (security review)
gates / gates (push) Successful in 45s
gates / gates (push) Successful in 45s
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user