diff --git a/TASK.md b/TASK.md index 746c7ba..6e31a38 100644 --- a/TASK.md +++ b/TASK.md @@ -1,855 +1,843 @@ -# TASK: CSRF Protection on POST Endpoints +# TASK: Storage Namespace (`felhom-data/`) + Test Node Wipe Script (v0.26.0) -**Controller:** v0.22.3 → v0.23.0 -**Hub:** v0.3.7 → v0.3.8 +**Controller:** v0.25.0 → v0.26.0 ## Overview -Add CSRF (Cross-Site Request Forgery) protection to all browser-facing POST endpoints in both the controller and the hub. Currently, any page on the internet can craft a form that POSTs to the controller/hub if the user has a valid session cookie — no origin verification exists. +All felhom-managed data on external drives moves under a `felhom-data/` subdirectory, cleanly separating our data from user files. Plus a wipe script for repeatable testing. -**Approach: Synchronizer Token Pattern** (per-session CSRF token stored server-side, embedded in forms, validated on POST). This matches the existing setup wizard implementation in `controller/internal/setup/csrf.go` but is more secure: tokens are tied to server-side sessions rather than using the double-submit cookie pattern. - -**Key principle:** Bearer-token-authenticated API endpoints (selfupdate, config/apply) do NOT need CSRF protection — browsers never auto-send `Authorization` headers. Only cookie/session-authenticated POST endpoints need protection. +**Key design principle:** `HDD_PATH` env var stays as the mount point (e.g., `/mnt/hdd_1`). The `felhom-data` segment is embedded in path helpers and compose templates, not in `HDD_PATH`. --- -## Security Audit Summary +## Phase 1: Path Helpers (`internal/backup/paths.go`) -### Controller (current state) +This is the foundation. All other changes depend on this. -| Item | Status | Notes | -|------|--------|-------| -| Session cookie HttpOnly | OK | `HttpOnly: true` | -| Session cookie SameSite | PARTIAL | `SameSite=Lax` (good defense-in-depth but not sufficient alone) | -| Session cookie Secure | OK | Set when TLS detected | -| Random session tokens | OK | 64-char hex (32 bytes) | -| CSRF on web form POSTs | MISSING | **None — vulnerable** | -| CSRF on JS-driven API POSTs | MISSING | **None — vulnerable** | -| Bearer-only endpoints exempt | n/a | selfupdate + config endpoints accept Bearer OR session | +### File: `controller/internal/backup/paths.go` -### Hub (current state — MORE CRITICAL) +**Add constant at top of file (after imports):** +```go +// FelhomDataDir is the namespace directory on storage drives for all felhom-managed data. +const FelhomDataDir = "felhom-data" +``` -| Item | Status | Notes | -|------|--------|-------| -| Session cookie HttpOnly | OK | `HttpOnly: true` | -| Session cookie SameSite | MISSING | **Not set** (browser default = Lax in modern browsers, but should be explicit) | -| Session cookie Secure | MISSING | **Not set** | -| Random session tokens | MISSING | **Cookie value is literal string `"authenticated"`** — any cookie injection = full session | -| CSRF on web form POSTs | MISSING | **None — vulnerable** | -| CSRF on JS-driven POSTs | MISSING | **None — vulnerable** | +**Update 8 functions** to include `FelhomDataDir` in the path. Each function gets `FelhomDataDir` inserted between `drivePath` and the next segment: -**Hub session model is fundamentally weak:** The `hub_session=authenticated` cookie contains no randomness. This means: -1. Any XSS or subdomain-based cookie injection can forge a session -2. There is no way to invalidate individual sessions -3. Combined with no CSRF = highly vulnerable +| Function | Current first segment | New first segment | +|----------|----------------------|-------------------| +| `PrimaryBackupPath` | `"backups"` | `FelhomDataDir, "backups"` | +| `PrimaryResticRepoPath` | `"backups"` | `FelhomDataDir, "backups"` | +| `AppDBDumpPath` | `"backups"` | `FelhomDataDir, "backups"` | +| `SecondaryBackupPath` | `"backups"` | `FelhomDataDir, "backups"` | +| `AppSecondaryRsyncPath` | `"backups"` | `FelhomDataDir, "backups"` | +| `SecondaryResticRepoPath` | `"backups"` | `FelhomDataDir, "backups"` | +| `SecondaryInfraPath` | `"backups"` | `FelhomDataDir, "backups"` | +| `AppDataDir` | `"appdata"` | `FelhomDataDir, "appdata"` | -This task fixes both the CSRF gap AND the hub session model. +**DO NOT change `InfraBackupDir`** — it stays at drive root (DR scanner needs it). + +Example transformation for each function — replace: +```go +return filepath.Join(drivePath, "backups", "primary") +``` +with: +```go +return filepath.Join(drivePath, FelhomDataDir, "backups", "primary") +``` + +And for `AppDataDir` replace: +```go +return filepath.Join(drivePath, "appdata", stackName) +``` +with: +```go +return filepath.Join(drivePath, FelhomDataDir, "appdata", stackName) +``` --- -## Part 1: Controller CSRF (v0.23.0) +## Phase 2: Fix Hardcoded Paths -### Endpoint Map: All State-Changing (POST/DELETE) Endpoints +### 2a. `controller/internal/stacks/delete.go` — `ProtectedHDDPaths()` -#### Web Form POSTs (need CSRF token in hidden field) - -| # | Method | Path | Handler | Auth | -|---|--------|------|---------|------| -| 1 | POST | `/settings/password` | `handlePasswordChange` | Session | -| 2 | POST | `/settings/notifications` | `handleNotificationSettings` | Session | -| 3 | POST | `/settings/notifications/test` | `handleTestNotification` | Session | -| 4 | POST | `/settings/storage/add` | `handleStorageAdd` | Session | -| 5 | POST | `/settings/storage/remove` | `handleStorageRemove` | Session | -| 6 | POST | `/settings/storage/default` | `handleStorageDefault` | Session | -| 7 | POST | `/settings/storage/schedulable` | `handleStorageSchedulable` | Session | -| 8 | POST | `/settings/storage/label` | `handleStorageLabel` | Session | -| 9 | POST | `/settings/cross-backup/{name}` | `handleCrossBackupSave` | Session | -| 10 | POST | `/backup/restore` | `handleRestore` | Session | - -#### JSON API POSTs called via `fetch()` from UI pages (need X-CSRF-Token header) - -| # | Method | Path | Handler | Notes | -|---|--------|------|---------|-------| -| 11 | POST | `/api/stacks/{name}/deploy` | `handleDeploy` | Deploy page JS | -| 12 | POST | `/api/stacks/{name}/start` | `handleStart` | Dashboard buttons | -| 13 | POST | `/api/stacks/{name}/stop` | `handleStop` | Dashboard buttons | -| 14 | POST | `/api/stacks/{name}/restart` | `handleRestart` | Dashboard buttons | -| 15 | POST | `/api/stacks/{name}/update` | `handleUpdate` | Dashboard buttons | -| 16 | POST | `/api/stacks/{name}/optional-config` | `handleOptionalConfig` | App info page | -| 17 | POST | `/api/stacks/{name}/remove` | `handleRemove` | Remove modal | -| 18 | DELETE | `/api/stacks/{name}` | `handleDelete` | Delete modal | -| 19 | POST | `/api/stacks/{name}/cross-backup` | `handleCrossBackupAPI` | Deploy page | -| 20 | POST | `/api/stacks/{name}/cross-backup/run` | `handleCrossBackupRun` | Backup page | -| 21 | POST | `/api/sync` | `handleSync` | Dashboard button | -| 22 | POST | `/api/backup/run` | `handleBackupRun` | Backup page | -| 23 | POST | `/api/backup/cross-drive/run-all` | `handleCrossDriveRunAll` | Backup page | -| 24 | POST | `/api/stacks/rescan` | `handleRescan` | Internal | -| 25 | POST | `/api/storage/scan` | `handleStorageScan` | Init wizard | -| 26 | POST | `/api/storage/init` | `handleStorageInit` | Init wizard | -| 27 | POST | `/api/storage/attach/mount-raw` | `handleAttachMountRaw` | Attach wizard | -| 28 | POST | `/api/storage/attach/mkdir` | `handleAttachMkdir` | Attach wizard | -| 29 | POST | `/api/storage/attach` | `handleAttachFinalize` | Attach wizard | -| 30 | POST | `/api/storage/attach/cancel` | `handleAttachCancel` | Attach wizard | -| 31 | POST | `/api/storage/migrate` | `handleMigrate` | Migration page | -| 32 | POST | `/api/storage/stale-cleanup` | `handleStaleCleanup` | Deploy page | -| 33 | POST | `/api/storage/disconnect` | `handleDisconnect` | Settings | -| 34 | POST | `/api/storage/reconnect` | `handleReconnect` | Settings | -| 35 | POST | `/api/storage/restart-apps` | `handleRestartApps` | Settings | -| 36 | POST | `/api/storage/migrate-drive` | `handleMigrateDrive` | Drive migration | -| 37 | POST | `/api/storage/decommission/remove` | `handleDecommissionRemove` | Settings | -| 38 | POST | `/api/assets/sync` | `handleAssetSync` | Internal | -| 39 | POST | `/api/restore/all` | `handleRestoreAll` | DR page | -| 40 | POST | `/api/restore/skip` | `handleRestoreSkip` | DR page | - -#### Bearer-token API POSTs (CSRF EXEMPT — not browser-initiated) - -| # | Method | Path | Notes | -|---|--------|------|-------| -| — | POST | `/api/selfupdate/check` | Accepts Bearer token (hub/scripts) | -| — | POST | `/api/selfupdate/update` | Accepts Bearer token (hub/scripts) | -| — | POST | `/api/config/apply` | Accepts Bearer token (hub push) | - -These three endpoints accept **both** session auth and Bearer auth. When accessed with a Bearer token, CSRF is not needed. When accessed via session cookie from the UI, they should require CSRF. The middleware handles this: if `Authorization: Bearer` header is present and valid, skip CSRF check. - -#### GET-only / No-auth endpoints (no CSRF needed) - -| Method | Path | Notes | -|--------|------|-------| -| GET | `/api/health` | No auth, read-only | -| GET | `/api/stacks`, `/api/stacks/{name}`, etc. | Read-only | -| GET | `/api/backup/status`, `/api/backup/snapshots` | Read-only | -| GET | `/api/metrics/*`, `/api/system/info` | Read-only | -| GET | `/api/storage/*` (browse, status) | Read-only | -| GET | `/api/selfupdate/status` | Read-only | -| GET | `/api/config`, `/api/config/hash` | Read-only | -| GET | `/api/assets/status` | Read-only | - ---- - -### 1.1 Add CSRF token to session struct - -**File: `controller/internal/web/auth.go`** - -Change the session struct to include a CSRF token: +**Line 54-65.** Cannot import `backup` package (architectural boundary via `StackDataProvider`). +Add local constant at top of file (after imports, before types): ```go -type session struct { - expiresAt time.Time - csrfToken string +// felhomDataDir matches backup.FelhomDataDir — duplicated to avoid circular import via StackDataProvider. +const felhomDataDir = "felhom-data" +``` + +Update `ProtectedHDDPaths()`: +```go +func ProtectedHDDPaths(hddPath string) map[string]bool { + if hddPath == "" { + return nil + } + return map[string]bool{ + hddPath: true, + filepath.Join(hddPath, felhomDataDir): true, + filepath.Join(hddPath, felhomDataDir, "appdata"): true, + filepath.Join(hddPath, felhomDataDir, "backups"): true, + filepath.Join(hddPath, "media"): true, + filepath.Join(hddPath, "Dokumentumok"): true, + } } ``` -Update `createSession()` to generate a CSRF token alongside the session token: +### 2b. `controller/internal/stacks/delete.go` — `GetStackBackupData()` +**Line 355 and 359.** Replace hardcoded paths with the local constant: + +Replace: ```go -func (s *Server) createSession() string { - b := make([]byte, 32) - _, _ = rand.Read(b) - token := hex.EncodeToString(b) - - csrfB := make([]byte, 32) - _, _ = rand.Read(csrfB) - csrfToken := hex.EncodeToString(csrfB) - - s.sessionsMu.Lock() - s.sessions[token] = &session{ - expiresAt: time.Now().Add(sessionMaxAge), - csrfToken: csrfToken, - } - s.sessionsMu.Unlock() - - return token -} +dbDumpPath := filepath.Join(drivePath, "backups", "primary", name, "db-dumps") +``` +with: +```go +dbDumpPath := filepath.Join(drivePath, felhomDataDir, "backups", "primary", name, "db-dumps") ``` -Add a method to retrieve the CSRF token for a session: - +Replace: ```go -// csrfTokenForSession returns the CSRF token for the given session cookie value. -// Returns "" if the session is invalid or expired. -func (s *Server) csrfTokenForSession(sessionToken string) string { - s.sessionsMu.RLock() - defer s.sessionsMu.RUnlock() - sess, ok := s.sessions[sessionToken] - if !ok || time.Now().After(sess.expiresAt) { - return "" - } - return sess.csrfToken -} +rsyncPath := filepath.Join(drivePath, "backups", "secondary", name, "rsync") +``` +with: +```go +rsyncPath := filepath.Join(drivePath, felhomDataDir, "backups", "secondary", name, "rsync") ``` -### 1.2 Create CSRF middleware - -**File: `controller/internal/web/csrf.go`** (new file) +### 2c. `controller/internal/storage/migrate_drive.go` — Conflict check & verify +**Import `backup` package** (safe — `backup` does NOT import `storage`): ```go -package web +"gitea.dooplex.hu/admin/felhom-controller/internal/backup" +``` -import ( - "crypto/subtle" - "fmt" - "html/template" - "net/http" - "strings" +**Line 183** — conflict check before migration. Replace: +```go +destAppData := filepath.Join(req.DestPath, "appdata", app.Name) +``` +with: +```go +destAppData := backup.AppDataDir(req.DestPath, app.Name) +``` + +**Line 325** — verify after copy. Replace: +```go +destAppData := filepath.Join(req.DestPath, "appdata", app.Name) +``` +with: +```go +destAppData := backup.AppDataDir(req.DestPath, app.Name) +``` + +### 2d. `controller/internal/storage/migrate_drive.go` — rsync exclude paths + +**CRITICAL BUG FIX.** Line 254-256. The rsync copies the entire drive root. The exclude patterns are relative paths. After namespace change, restic repos are under `felhom-data/backups/*/restic/`. + +Replace: +```go +rsyncCmd := exec.CommandContext(ctx, "rsync", "-a", "--info=progress2", + "--exclude=backups/primary/restic/", + "--exclude=backups/secondary/restic/", + req.SourcePath+"/", req.DestPath+"/", +) +``` +with: +```go +rsyncCmd := exec.CommandContext(ctx, "rsync", "-a", "--info=progress2", + "--exclude=felhom-data/backups/primary/restic/", + "--exclude=felhom-data/backups/secondary/restic/", + req.SourcePath+"/", req.DestPath+"/", ) - -const csrfFormField = "_csrf" -const csrfHeaderName = "X-CSRF-Token" - -// csrfProtect validates CSRF tokens on unsafe HTTP methods (POST, PUT, DELETE, PATCH). -// Safe methods (GET, HEAD, OPTIONS) pass through — the token is made available -// to templates via the Server.csrfToken() helper. -// -// Exempt: requests with a valid Authorization: Bearer header (API key auth). -func (s *Server) csrfProtect(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Safe methods: no CSRF check needed - switch r.Method { - case http.MethodGet, http.MethodHead, http.MethodOptions: - next.ServeHTTP(w, r) - return - } - - // Skip CSRF if auth is disabled (no password set = open access) - if !s.authEnabled() { - next.ServeHTTP(w, r) - return - } - - // Skip CSRF for Bearer-token authenticated requests. - // These endpoints also accept session auth, but when a Bearer token - // is present, the request is from a script/hub, not a browser. - if auth := r.Header.Get("Authorization"); strings.HasPrefix(auth, "Bearer ") { - next.ServeHTTP(w, r) - return - } - - // Get the session's CSRF token - cookie, err := r.Cookie(sessionCookieName) - if err != nil { - s.csrfReject(w, r, "no session") - return - } - expected := s.csrfTokenForSession(cookie.Value) - if expected == "" { - s.csrfReject(w, r, "invalid session") - return - } - - // Check form field first, then header (for fetch/AJAX) - submitted := r.FormValue(csrfFormField) - if submitted == "" { - submitted = r.Header.Get(csrfHeaderName) - } - - if submitted == "" || subtle.ConstantTimeCompare([]byte(submitted), []byte(expected)) != 1 { - s.csrfReject(w, r, "token mismatch") - return - } - - next.ServeHTTP(w, r) - }) -} - -// csrfReject sends a 403 response. For API requests returns JSON, for web requests returns HTML. -func (s *Server) csrfReject(w http.ResponseWriter, r *http.Request, reason string) { - s.logger.Printf("[WARN] CSRF rejected: %s %s from %s (%s)", r.Method, r.URL.Path, r.RemoteAddr, reason) - if strings.HasPrefix(r.URL.Path, "/api/") { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusForbidden) - fmt.Fprint(w, `{"ok":false,"error":"CSRF token missing or invalid"}`) - return - } - http.Error(w, "CSRF token missing or invalid. Please reload the page and try again.", http.StatusForbidden) -} - -// csrfToken returns the CSRF token for the current request's session. -// Call from handlers to embed in templates or return to JS. -func (s *Server) csrfToken(r *http.Request) string { - cookie, err := r.Cookie(sessionCookieName) - if err != nil { - return "" - } - return s.csrfTokenForSession(cookie.Value) -} - -// csrfField returns an HTML hidden input for embedding in forms. -func (s *Server) csrfField(r *http.Request) template.HTML { - token := s.csrfToken(r) - return template.HTML(``) -} ``` -### 1.3 Wire CSRF middleware into the server +Use `backup.FelhomDataDir` constant in the string construction if preferred, but since these are string literals for rsync args, hardcoding `"felhom-data"` is acceptable. -**File: `controller/internal/web/server.go`** +### 2e. `controller/internal/storage/migrate_drive.go` — Size estimation -The controller uses a custom router in `ServeHTTP`. The CSRF middleware must wrap the handler AFTER auth but protect all POST routes. Find where `RequireAuth` wraps the server's handler and add `csrfProtect` inside it. +**BUG FIX.** Lines 196-208. This scans the drive root for size estimation. After namespace change, `entry.Name() == "backups"` never matches because backups are now under `felhom-data/`. -In `server.go`, locate the HTTP server setup. The pattern is typically: +Replace the size estimation block: ```go -handler := s.RequireAuth(s) -``` - -Change to: -```go -handler := s.RequireAuth(s.csrfProtect(s)) -``` - -This means: RequireAuth runs first (verifies session), then csrfProtect runs (verifies CSRF token on POST), then the actual handler runs. - -**Important:** Check the exact wiring in `server.go`. The auth middleware likely wraps the mux in `Start()` or `ListenAndServe()`. Find that location and insert `csrfProtect` between auth and handler. If the `RequireAuth` is applied in `main.go`, adjust there instead. - -### 1.4 Add CSRF token to all template data - -**File: `controller/internal/web/handlers.go`** (and `storage_handlers.go`, `handler_restore.go`) - -Every handler that renders an HTML page must include the CSRF token in its template data. The recommended approach: - -**Add `CSRFField` and `CSRFToken` to every template data map.** In each handler that renders a page, add these two fields: - -```go -data["CSRFField"] = s.csrfField(r) -data["CSRFToken"] = s.csrfToken(r) -``` - -Alternatively, create an `executeTemplate` wrapper method that auto-injects these: - -```go -// executeTemplate renders a template with per-request CSRF data injected. -func (s *Server) executeTemplate(w http.ResponseWriter, r *http.Request, name string, data map[string]interface{}) { - if data == nil { - data = make(map[string]interface{}) - } - data["CSRFField"] = s.csrfField(r) - data["CSRFToken"] = s.csrfToken(r) - w.Header().Set("Content-Type", "text/html; charset=utf-8") - if err := s.tmpl.ExecuteTemplate(w, name, data); err != nil { - s.logger.Printf("[ERROR] Template error (%s): %v", name, err) - http.Error(w, "Internal error", http.StatusInternalServerError) +// Estimate total size (exclude restic repos) +var totalBytes int64 +entries, _ := os.ReadDir(req.SourcePath) +for _, entry := range entries { + entryPath := filepath.Join(req.SourcePath, entry.Name()) + if entry.IsDir() { + // Skip restic repos in size estimate + if entry.Name() == "backups" { + totalBytes += dirSizeExcluding(entryPath, "restic") + } else { + totalBytes += dirSize(entryPath) + } } } ``` -Then replace all `s.tmpl.ExecuteTemplate(w, "templateName", data)` calls with `s.executeTemplate(w, r, "templateName", data)`. - -**Handlers that render pages (all need CSRFField/CSRFToken):** -- `handleDashboard` → dashboard template -- `handleStacks` → stacks template (if separate from dashboard) -- `handleDeploy` → deploy template -- `handleBackups` → backups template -- `handleMonitoring` → monitoring template -- `handleSettings` → settings template -- `handleAppInfo` → app_info template -- `handleLogs` → logs template -- `handleMigrate` → migrate template -- `handleMigrateDrive` → migrate_drive template -- `handleStorageInit` → init_disk template -- `handleStorageAttach` → attach_disk template -- `handleRestorePage` → restore template -- `renderLogin` → login template (**no CSRF needed** — no session yet) - -### 1.5 Add hidden field to all HTML form templates - -**File: `controller/internal/web/templates/*.html`** - -For every `
-``` - -Templates to update (search for `method="POST"` or `method="post"` in all template files): - -| # | Template | Form action(s) | -|---|----------|----------------| -| 1 | `settings.html` | `/settings/password`, `/settings/notifications`, `/settings/notifications/test`, `/settings/storage/add`, `/settings/storage/remove`, `/settings/storage/default`, `/settings/storage/schedulable`, `/settings/storage/label` | -| 2 | `deploy.html` | Cross-backup form (if any form POST exists) | -| 3 | `backups.html` | `/backup/restore` form, cross-backup save forms | -| 4 | `init_disk.html` | Storage init wizard (if form-based) | -| 5 | `attach_disk.html` | Attach wizard (if form-based) | -| 6 | `restore.html` | DR restore (if form-based) | - -**Note:** Many operations on these pages use JavaScript `fetch()` instead of HTML form submissions. For those, see section 1.6. - -### 1.6 Add CSRF token to JavaScript fetch() calls - -For all JavaScript code that makes POST/DELETE requests via `fetch()`, include the CSRF token in the `X-CSRF-Token` header. - -**Step A: Embed the token in a meta tag in the base layout.** - -In the template that defines the `` section (likely within `{{define "head"}}` or the shared HTML header), add: - -```html - -``` - -**Step B: Create a helper function in the shared JS.** - -At the top of the `