docs(roadmap): the restore password has no safe channel to the host yet
Phase 1 is built. Phase 2 hit a blocker worth deciding rather than silently working around. The WebUI cannot run restic, so a password typed in the browser has to reach the host. Both existing channels leak it. The task command string — which is what the Backup page ALREADY uses for this exact field — lands in a task JSON under frontend/data/tasks at 0644, world-readable, and shows in ps while the task runs. A file in that directory does not work either: the container writes as dockerinstall, the manager runs as libreportal, and at 0640 the manager cannot read it (verified on the live box). So this is an existing product-wide weakness that the restore branch happens to surface, not one the feature would introduce — and the restore case is its sharpest form, since that password is the key to every backup the user has. Recommends a one-shot secret drop: the ownership helper already solves the mirror-image case (_webui_bind_access chowns MANAGER:cowner 0640 so the container can read manager-owned files), so the reverse is a small, well-scoped addition — a directory owned cowner:MANAGER 0730 that the container drops a 0640 file into, which the manager reads once and unlinks. Worth doing because it also fixes the Backup page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
15ca10530d
commit
d44ebf0ca7
@ -77,6 +77,47 @@ That single fact drives two requirements:
|
||||
|
||||
Once the repo opens, ordering is already correct in the CLI and should be preserved: **system config first** (it carries every *other* location's credentials, so one password unlocks the rest), then apps.
|
||||
|
||||
### 4.1 — Where does the typed password actually travel? (blocker for phase 2)
|
||||
|
||||
Found while building phase 2, and it needs a decision before the restore branch
|
||||
can be written, because it is a security trade-off rather than an implementation
|
||||
detail.
|
||||
|
||||
The WebUI cannot run restic. So a password the user types in the browser has to
|
||||
reach the host somehow, and the two existing channels both leak it:
|
||||
|
||||
| Channel | Problem |
|
||||
|---|---|
|
||||
| Task command string (what the Backup page already does for this exact field, via `config_update CFG_BACKUP_LOC_<n>_PASSWORD=…`) | lands in the task JSON under `frontend/data/tasks/`, which is **0644** so the manager can read it — i.e. world-readable — and is visible in `ps` while the task runs |
|
||||
| A file in `frontend/data/tasks/` | the container writes as `dockerinstall`; the manager runs as `libreportal`. At 0640 the manager cannot read it (verified), and 0644 is world-readable again |
|
||||
|
||||
Note the first row is **existing behaviour**, not something this feature would
|
||||
introduce: editing a backup location's password on the Backup page already
|
||||
sends it that way. So this is a product-wide finding that phase 2 happens to
|
||||
surface, and the restore case is the sharpest version of it — that password is
|
||||
the key to every backup the user has.
|
||||
|
||||
Three ways out, roughly in order of effort:
|
||||
|
||||
1. **A one-shot secret drop.** The ownership helper already knows how to make a
|
||||
path readable across exactly this boundary (`_webui_bind_access` chowns
|
||||
`MANAGER:cowner` 0640 so the container can read manager-owned files). The
|
||||
reverse needs the same treatment: a root-helper-created directory owned
|
||||
`cowner:MANAGER` 0730, into which the container drops a 0640 file the manager
|
||||
reads once and unlinks.
|
||||
2. **Never persist it.** Hold the password only in the task processor's memory
|
||||
for the life of the restore; write it into the location config only after the
|
||||
system-config restore lands (and reconcile with what the backup contained).
|
||||
3. **Accept the existing channel** for consistency, and fix it product-wide
|
||||
later — cheapest now, and no worse than what shipping code already does, but
|
||||
it does mean a restore password sits world-readable in a task file until that
|
||||
task is pruned.
|
||||
|
||||
Recommendation: **(1)**, and apply it to the Backup page's password field at the
|
||||
same time. It is a small, well-scoped addition to a helper that already exists
|
||||
for the mirror-image case, and it fixes a live weakness rather than only
|
||||
avoiding a new one.
|
||||
|
||||
## 5. "Set up a backup server if you don't have one"
|
||||
|
||||
Same components, other direction. After a **New install**, offer: *"Where should your backups go?"* — the same location fields, then `engineInit` and a first `backup system`. That closes a real gap: today backups exist but nothing prompts you to configure them, so the people most likely to need a restore are the least likely to have one.
|
||||
@ -96,8 +137,8 @@ If a genuine single-file import is wanted, that is a **different feature**: a po
|
||||
|
||||
| Phase | Deliverable |
|
||||
|---|---|
|
||||
| **1** | Backup destination step for the New-install path (§5) — small, useful on its own, exercises the location fields inside the wizard |
|
||||
| **2** | The two blocks, and the restore branch through discovery (steps 1–3). Read-only: nothing is written, so it can ship before reconciliation exists |
|
||||
| **1** ✅ | Backup destination step for the New-install path (§5) — built |
|
||||
| **2** ⛔ | The two blocks and the restore branch. **Blocked on §4.1** — the typed repository password has no safe channel to the host yet |
|
||||
| **3** | The reconciliation screen (§3) and the restore itself, driven by `restoreFirstRunBulk` |
|
||||
| **4** | Portable per-app export/import (§6), if wanted |
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user