added security settings
changed fixes and issues
This commit is contained in:
Philipp
+16
-6
@@ -83,10 +83,15 @@ export function resticProcessEnv() {
|
||||
|
||||
export async function readEnvSettings() {
|
||||
const fileValues = await readEnvFile();
|
||||
return editableEnv.map((field) => ({
|
||||
...field,
|
||||
value: fileValues[field.key] ?? process.env[field.key] ?? '',
|
||||
}));
|
||||
return editableEnv.map((field) => {
|
||||
const value = fileValues[field.key] ?? process.env[field.key] ?? '';
|
||||
// Secrets are write-only: never echo their value back to callers. The UI
|
||||
// only learns whether a value is currently set via `hasValue`.
|
||||
if (field.secret) {
|
||||
return { ...field, value: '', hasValue: Boolean(value) };
|
||||
}
|
||||
return { ...field, value };
|
||||
});
|
||||
}
|
||||
|
||||
export async function writeEnvSettings(values) {
|
||||
@@ -94,14 +99,19 @@ export async function writeEnvSettings(values) {
|
||||
const currentValues = await readEnvFile();
|
||||
const nextValues = { ...currentValues };
|
||||
|
||||
const secretKeys = new Set(editableEnv.filter((field) => field.secret).map((field) => field.key));
|
||||
for (const [key, value] of Object.entries(values || {})) {
|
||||
if (!allowedKeys.has(key)) continue;
|
||||
if (key === 'API_TOKEN' && String(value || '').length < minApiTokenLength) {
|
||||
const text = String(value ?? '');
|
||||
// Secrets are write-only: an empty submission keeps the existing value so
|
||||
// the masked UI does not wipe credentials when saving unrelated fields.
|
||||
if (secretKeys.has(key) && text === '') continue;
|
||||
if (key === 'API_TOKEN' && text.length < minApiTokenLength) {
|
||||
const error = new Error(`API_TOKEN must be at least ${minApiTokenLength} characters long.`);
|
||||
error.status = 400;
|
||||
throw error;
|
||||
}
|
||||
nextValues[key] = String(value ?? '');
|
||||
nextValues[key] = text;
|
||||
}
|
||||
|
||||
const body = editableEnv
|
||||
|
||||
+9
-1
@@ -1,5 +1,6 @@
|
||||
import cors from 'cors';
|
||||
import express from 'express';
|
||||
import crypto from 'node:crypto';
|
||||
import http from 'node:http';
|
||||
import https from 'node:https';
|
||||
import { readFile } from 'node:fs/promises';
|
||||
@@ -25,7 +26,7 @@ app.use((req, res, next) => {
|
||||
return;
|
||||
}
|
||||
const header = req.get('authorization') || '';
|
||||
if (header === `Bearer ${config.apiToken}`) {
|
||||
if (config.apiToken && timingSafeEqual(header, `Bearer ${config.apiToken}`)) {
|
||||
next();
|
||||
return;
|
||||
}
|
||||
@@ -36,6 +37,13 @@ function normalizeIp(value) {
|
||||
return String(value || '').replace(/^::ffff:/, '');
|
||||
}
|
||||
|
||||
function timingSafeEqual(a, b) {
|
||||
const bufA = Buffer.from(String(a));
|
||||
const bufB = Buffer.from(String(b));
|
||||
if (bufA.length !== bufB.length) return false;
|
||||
return crypto.timingSafeEqual(bufA, bufB);
|
||||
}
|
||||
|
||||
app.use('/api/health', healthRouter);
|
||||
app.use('/api/vms', vmsRouter);
|
||||
app.use('/api/snapshots', snapshotsRouter);
|
||||
|
||||
+35
-4
@@ -46,6 +46,11 @@ Sortiert nach Risikoklasse. Datei-/Zeilenreferenzen beziehen sich auf den Stand
|
||||
- [x] **8. Bearer-Token aus Frontend-`localStorage` entfernen** ✅ erledigt
|
||||
- `frontend/src/api.js` — nur noch Cookie-Auth (`withCredentials: true`). `localStorage`-Token und `VITE_API_TOKEN` entfernt.
|
||||
|
||||
- [x] **45. Secrets in `GET /settings` maskieren (Write-only)** ✅ erledigt
|
||||
- Datei: `agent/src/config.js` (`readEnvSettings`/`writeEnvSettings`), `frontend/src/components/Settings.jsx`
|
||||
- Problem: `readEnvSettings` gab den Wert *aller* Felder zurück — inkl. `RESTIC_PASSWORD`, `AWS_SECRET_ACCESS_KEY`, `API_TOKEN`. Über den Management-Proxy konnte damit jeder eingeloggte UI-User das Restic-Verschlüsselungspasswort und die S3-Credentials enthüllen (= alle Backups entschlüsseln und löschen).
|
||||
- Fix: Secret-Felder liefern beim Lesen keinen Wert mehr, nur `hasValue: true|false`. Beim Schreiben gilt ein leeres Secret-Feld als „unverändert" (bestehender Wert bleibt erhalten). Frontend zeigt Platzhalter „gesetzt – leer lassen zum Beibehalten".
|
||||
|
||||
---
|
||||
|
||||
## 🟠 Hoch
|
||||
@@ -88,6 +93,21 @@ Sortiert nach Risikoklasse. Datei-/Zeilenreferenzen beziehen sich auf den Stand
|
||||
- Datei: `agent/src/routes/settings.js`
|
||||
- Hängt mit Fix #2 zusammen — nach #2 automatisch erfüllt, hier zur Sicherheit dokumentieren/testen.
|
||||
|
||||
- [x] **46. Agent-Token-Vergleich timing-safe machen** ✅ erledigt
|
||||
- Datei: `agent/src/index.js:28`
|
||||
- Problem: `header === \`Bearer ${config.apiToken}\`` — normaler String-Vergleich am root-Agent (das wertvollste Ziel), während das Login-Passwort bereits `timingSafeEqual` nutzte.
|
||||
- Fix: Vergleich über `crypto.timingSafeEqual` mit Längen-Guard. Leerer/fehlender konfigurierter Token verweigert weiterhin (`config.apiToken &&`).
|
||||
|
||||
- [ ] **47. Pre-Restore-Volumes aufräumen (Disk-Space-GC)**
|
||||
- Datei: `agent/src/routes/restore.js`
|
||||
- Aktuell: Jeder erfolgreiche Restore behält das alte Volume als `*.pre-restore-*` Rollback-Kopie — es gibt aber keinen Cleanup. Jeder Restore verdoppelt den Plattenbedarf der VM dauerhaft; bei mehreren Restores läuft der ZFS-Pool voll.
|
||||
- Fix: Aufbewahrungsregel (z.B. „keep last N pre-restore/failed-restore Volumes pro VM") oder expliziter Cleanup-Schritt/UI-Aktion. Mindestens im Health/Operations-View sichtbar machen.
|
||||
|
||||
- [ ] **48. Agent-Tokens im Management-SQLite nicht im Klartext speichern**
|
||||
- Datei: `management/src/store.js` (`nodes.token`), `management/src/db.js`
|
||||
- Aktuell: `nodes.token` liegt im Klartext in der Management-DB. DB-Diebstahl = alle Agent-Tokens = root auf allen Hosts.
|
||||
- Fix: Verschlüsselung at-rest mit einem Management-Key (z.B. aus `SESSION_SECRET`/dediziertem Key abgeleitet), oder zumindest DB-Dateipermissions/Disk-Encryption-Anforderung im Deployment-Doc verbindlich dokumentieren.
|
||||
|
||||
---
|
||||
|
||||
## 🟡 Mittel
|
||||
@@ -142,6 +162,11 @@ Sortiert nach Risikoklasse. Datei-/Zeilenreferenzen beziehen sich auf den Stand
|
||||
- [ ] **28. Pre-Backup VM-Zustand prüfen**
|
||||
- Live-Migration, laufende interne Snapshots, fehlende Berechtigungen → klare Fehler statt halb durchgeführter Pipeline.
|
||||
|
||||
- [ ] **49. Abgelaufene Sessions serverseitig löschen + Rotation**
|
||||
- Datei: `management/src/store.js:12` (`getUserBySession`), `createSession`
|
||||
- Aktuell: Abgelaufene Sessions werden beim Lesen nur gefiltert, nie aus der DB entfernt → unbegrenztes Tabellenwachstum. Außerdem keine Session-Rotation nach erfolgreichem Login.
|
||||
- Fix: Periodischer Cleanup (`DELETE FROM sessions WHERE expires_at <= now`) und neue Session-ID nach Login ausstellen.
|
||||
|
||||
---
|
||||
|
||||
## 🟢 Niedrig / Aufräumen
|
||||
@@ -187,11 +212,17 @@ Sortiert nach Risikoklasse. Datei-/Zeilenreferenzen beziehen sich auf den Stand
|
||||
| 7 | Backup-Verifikation | ⬜ offen |
|
||||
| 8 | Bearer-Token aus localStorage | ✅ erledigt |
|
||||
| 9 | Session-Cookie `Secure`-Flag | ⚠️ teilweise |
|
||||
| 45 | Secrets in `GET /settings` maskieren | ✅ erledigt |
|
||||
| 46 | Agent-Token-Vergleich timing-safe | ✅ erledigt |
|
||||
| 47 | Pre-Restore-Volume-GC | ⬜ offen |
|
||||
| 48 | Agent-Tokens in DB verschlüsseln | ⬜ offen |
|
||||
| 49 | Session-Cleanup serverseitig | ⬜ offen |
|
||||
|
||||
## Nächste Prioritäten
|
||||
|
||||
1. **#7** Backup-Verifikation weiter härten — Pipeline-/Stream-Fehler testen und vollständiger absichern.
|
||||
2. **#9** `SESSION_COOKIE_SECURE=true` in Deployment-Doku festschreiben.
|
||||
3. **#12** Toten `SESSION_SECRET` entfernen.
|
||||
4. **#15** ENV-Escaping vervollständigen (`$`, Backticks, Newlines).
|
||||
5. **#11 Folgearbeit** ressourcenspezifische Cleanup-Recovery nach Agent-Crash definieren.
|
||||
2. **#47** Pre-Restore-Volume-GC — sonst läuft der ZFS-Pool bei wiederholten Restores voll.
|
||||
3. **#9** `SESSION_COOKIE_SECURE=true` in Deployment-Doku festschreiben.
|
||||
4. **#12** Toten `SESSION_SECRET` entfernen.
|
||||
5. **#15** ENV-Escaping vervollständigen (`$`, Backticks, Newlines).
|
||||
6. **#11 Folgearbeit** ressourcenspezifische Cleanup-Recovery nach Agent-Crash definieren.
|
||||
|
||||
+107
@@ -14,6 +14,14 @@ Several earlier backlog items have already landed in the codebase:
|
||||
|
||||
Known caveat: agent-side jobs are persisted, but subprocesses cannot survive an agent restart. Active jobs are marked `failed` on startup; deeper cleanup recovery for partially changed host resources is still future work.
|
||||
|
||||
Current pre-production priorities:
|
||||
|
||||
1. Harden backup verification and add tests for stream/pipeline failure cases.
|
||||
2. Validate the staged restore workflow on a disposable Incus VM, including rollback scenarios.
|
||||
3. Improve agent crash cleanup for partially changed ZFS/Incus resources.
|
||||
4. Surface detailed node health diagnostics in the UI.
|
||||
5. Keep the root-running agent tightly network-restricted.
|
||||
|
||||
## 1. Persist final agent job status in management
|
||||
|
||||
Status: mostly done.
|
||||
@@ -85,6 +93,8 @@ Ensure a backup is marked `success` only when the expected source data was fully
|
||||
- [x] Remove failed Restic snapshots with `forget` and `prune` on verification errors.
|
||||
- [ ] Make stream/pipeline failure handling explicit for all sources.
|
||||
- [ ] Avoid marking success if source stream closes early but Restic exits successfully.
|
||||
- [ ] Add a post-backup Restic integrity check strategy, such as targeted `restic check`/`restic stats` usage that is safe for large repositories.
|
||||
- [ ] Define an optional restore-probe workflow for critical VMs.
|
||||
- [ ] Add tests with mocked command failures and short reads.
|
||||
|
||||
### Acceptance Criteria
|
||||
@@ -92,6 +102,7 @@ Ensure a backup is marked `success` only when the expected source data was fully
|
||||
- [x] Successful backup jobs include a verifiable Restic snapshot ID in management history.
|
||||
- [x] Size mismatches fail the job.
|
||||
- [ ] Simulated source stream errors cannot produce a successful job.
|
||||
- [ ] A backup can be independently verified without trusting only the successful process exit.
|
||||
- [ ] Verification behavior is covered by automated tests.
|
||||
|
||||
## 4. Add per-VM backup policy
|
||||
@@ -139,6 +150,8 @@ Reduce the risk of accidental or unsafe restores.
|
||||
- Optionally offer “create backup before restore” when the VM is accessible.
|
||||
- [x] Log restore intent in audit log before dispatch.
|
||||
- [x] Replace direct `dd` to production ZVOL with a staged restore workflow.
|
||||
- [ ] Test staged restore on a disposable VM: backup, restore, boot, and confirm data.
|
||||
- [ ] Test failed staged restore paths: Restic dump failure before swap, rename failure after old volume rename, and VM start failure.
|
||||
- [ ] Define a safe container restore workflow with `zfs receive` or disable container restore surfaces entirely.
|
||||
|
||||
### Acceptance Criteria
|
||||
@@ -147,6 +160,7 @@ Reduce the risk of accidental or unsafe restores.
|
||||
- Restore is blocked when required preflight checks fail.
|
||||
- Audit log records restore attempts and results.
|
||||
- [x] Restore stream failures happen on the staged volume, not the production disk.
|
||||
- [ ] Staged restore behavior is validated on a real Incus/ZFS test node.
|
||||
|
||||
## 6. Add failure notifications
|
||||
|
||||
@@ -240,3 +254,96 @@ Provide a production-ready Docker/Compose deployment for the management API and
|
||||
- Management database survives container recreation.
|
||||
- Cookie login works behind HTTPS.
|
||||
- Management can connect to HTTPS node-agents using the configured CA file.
|
||||
|
||||
## 10. Add automated tests and CI
|
||||
|
||||
The project currently has no automated tests. This is the biggest engineering gap for a backup system because most dangerous failures happen in error paths, not in the happy path.
|
||||
|
||||
### Goal
|
||||
|
||||
Catch regressions in validation, locking, backup verification, restore orchestration, authentication, and management polling before deployment.
|
||||
|
||||
### Tasks
|
||||
|
||||
- Add a test runner for agent and management code.
|
||||
- Test validators for instance names, snapshot IDs, and ambiguous snapshot prefixes.
|
||||
- Test job locking, persistence, trimming, and restart behavior.
|
||||
- Test backup pipeline failure handling with mocked command/process failures.
|
||||
- Test restore staged-volume command sequencing with mocked ZFS/Incus/Restic commands.
|
||||
- Test management job polling updates `job_history` correctly for success, failed, timeout, and missing-agent cases.
|
||||
- Add CI for install, tests, frontend build, and syntax checks.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Pull requests run tests automatically.
|
||||
- Simulated backup stream failures fail the job.
|
||||
- Simulated restore failures leave the original ZVOL name restored in the command sequence.
|
||||
- Agent restart behavior is covered by tests.
|
||||
|
||||
## 11. Harden root-running agent exposure
|
||||
|
||||
The agent runs with root-level host access because it needs Incus, ZFS, `/dev/zvol`, Restic, and device operations. A compromised agent is therefore a host-level incident.
|
||||
|
||||
### Goal
|
||||
|
||||
Reduce the network and systemd blast radius of the root-running node-agent.
|
||||
|
||||
### Tasks
|
||||
|
||||
- Require private management-to-agent connectivity in production documentation, such as NetBird or a dedicated management network.
|
||||
- Prefer HTTPS agents with a private CA; document local HTTP only as development mode.
|
||||
- Enable and document `ALLOWED_MANAGEMENT_IPS` for production.
|
||||
- Add systemd hardening where compatible with Incus/ZFS access.
|
||||
- Add explicit installer warnings when `HTTPS_ENABLED=false` and no `ALLOWED_MANAGEMENT_IPS` is configured.
|
||||
- Consider replacing `npm start` in systemd with direct `node src/index.js`.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Production install docs do not expose the agent publicly by default.
|
||||
- Installer warns on insecure network exposure.
|
||||
- systemd unit has a documented minimum hardening baseline.
|
||||
|
||||
## 12. Improve agent crash cleanup and resource recovery
|
||||
|
||||
Agent jobs are persisted and active jobs are marked `failed` on restart, but a process crash can still leave host resources behind, such as temporary Incus snapshots, staged restore volumes, visible snapshot devices, or changed ZFS properties.
|
||||
|
||||
### Goal
|
||||
|
||||
Make agent startup detect and clean up known leftover resources from interrupted jobs where doing so is safe.
|
||||
|
||||
### Tasks
|
||||
|
||||
- Persist cleanup metadata for backup jobs: temporary Incus snapshot name, ZVOL, snapdev state, and backup type.
|
||||
- Persist cleanup metadata for restore jobs: staged ZVOL, backup ZVOL, failed ZVOL, and swap state.
|
||||
- On startup, scan failed active jobs and run safe cleanup actions.
|
||||
- Log cleanup results into the persisted job logs.
|
||||
- Avoid destructive cleanup when state is ambiguous; surface manual action instead.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- Crashing during backup streaming does not leave `snapdev=visible` or temporary snapshots unnoticed.
|
||||
- Crashing before restore swap removes staged restore volumes when safe.
|
||||
- Crashing after restore swap does not automatically destroy rollback copies.
|
||||
- Management can show cleanup-required states.
|
||||
|
||||
## 13. Improve health diagnostics in the UI
|
||||
|
||||
The agent health endpoint returns useful structured details, but the management UI currently summarizes this too aggressively. Operators need actionable health reasons without opening logs.
|
||||
|
||||
### Goal
|
||||
|
||||
Show per-node health detail in the UI with concrete failed checks and messages.
|
||||
|
||||
### Tasks
|
||||
|
||||
- Add a detailed health drawer or modal on the Nodes page.
|
||||
- Show command, ZFS pool, `/dev/zvol`, Restic repository, and credential check results.
|
||||
- Show last health timestamp and the management-side error if the agent is unreachable.
|
||||
- Distinguish unreachable, unauthorized, TLS failure, degraded health, and healthy states.
|
||||
- Avoid global UI timeout banners when only one enabled node is slow or unreachable.
|
||||
|
||||
### Acceptance Criteria
|
||||
|
||||
- A missing command is visible by name in the UI.
|
||||
- Wrong Restic credentials are visible as a Restic health failure.
|
||||
- TLS or connectivity failures are distinguishable from degraded agent health.
|
||||
|
||||
@@ -12,7 +12,7 @@ export function Settings({ nodes, onChanged }) {
|
||||
const [error, setError] = useState('');
|
||||
|
||||
const requiredMissing = useMemo(
|
||||
() => fields.filter((field) => field.required && !String(values[field.key] || '').trim()),
|
||||
() => fields.filter((field) => field.required && !field.hasValue && !String(values[field.key] || '').trim()),
|
||||
[fields, values],
|
||||
);
|
||||
|
||||
@@ -100,6 +100,7 @@ export function Settings({ nodes, onChanged }) {
|
||||
className="h-10 min-w-0 flex-1 bg-transparent px-3 text-zinc-100 outline-none"
|
||||
onChange={(event) => setValues((current) => ({ ...current, [field.key]: event.target.value }))}
|
||||
min={field.key.startsWith('RESTIC_KEEP_') ? '0' : undefined}
|
||||
placeholder={field.secret && field.hasValue ? 'gesetzt – leer lassen zum Beibehalten' : undefined}
|
||||
type={inputType(field, secretVisible)}
|
||||
value={values[field.key] || ''}
|
||||
/>
|
||||
|
||||
Reference in New Issue
Block a user