diff --git a/CHANGELOG.md b/CHANGELOG.md index d621938..1d968e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Changed + +- **A backup and a domain's export ask for your password** (backup-reauth): + both downloads take the signed-in user's account password, checked like a + password change and under its limit, so a stolen session or a panel left + open no longer hands over the server's every secret. + +### Fixed + +- **The file field of a domain's import is drawn as designed**: Bulma's + file button and the name of the file chosen, not the browser's own + "Browse…". + ## [2.10.2] - 2026-10-16 The panel on a phone. No migration; schema 12. diff --git a/cmd/panel/restore_test.go b/cmd/panel/restore_test.go index 7846d0a..e2b45f1 100644 --- a/cmd/panel/restore_test.go +++ b/cmd/panel/restore_test.go @@ -360,10 +360,11 @@ func signIn(t *testing.T, h http.Handler) *http.Cookie { // downloadBackup takes a backup through the panel's own /backup route, the way // the operator does. An empty password downloads the plain archive; otherwise -// the response is a .spbk envelope, which is decrypted here. +// the response is a .spbk envelope, which is decrypted here. The account +// password goes with it, as the form asks for it (backup-reauth). func downloadBackup(t *testing.T, h http.Handler, session *http.Cookie, password string) []byte { t.Helper() - form := url.Values{} + form := url.Values{"account_password": {restorePassword}} if password != "" { form.Set("encrypt", "1") form.Set("password", password) diff --git a/docs/guide.md b/docs/guide.md index 522ecb1..238b12c 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -777,6 +777,12 @@ as themselves, the host's IPv6 address and `127.0.0.1` as `172.18.0.1`. ### Full backup and restore +Both downloads - the full backup on **Server › Backup** and a domain's +export on its settings - ask for your account password, the one you sign in +with, every time: a session left open is not enough to take them. Wrong +guesses count against the same limit as a password change (ten in fifteen +minutes). + **Full backup** is a self-contained project archive: `data/` (SQLite, all domains' DKIM keys under `data/dkim/`, all applications' SASL credentials, `manifest.json` with the version that created it), plus `docker-compose.yml`, diff --git a/docs/roadmap.md b/docs/roadmap.md index 56c1437..8e42315 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -33,7 +33,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | test-email | Send a test message from the panel (was preflight) | agreed (2026-10-13) | — | — | | inbound-antispam-panel | Inbound antispam journal + allow/deny lists | shipped `2.1.0` | 12/12 | [plans/inbound-antispam-panel.md](plans/inbound-antispam-panel.md) | | origin-strict | Refuse a state-changing request that carries neither `Sec-Fetch-Site` nor `Origin` (was csrf-tokens) | agreed (2026-10-13) | — | — | -| backup-reauth | Ask for the account password before a backup or a domain export is downloaded | agreed (2026-10-13) | — | — | +| backup-reauth | Ask for the account password before a backup or a domain export is downloaded | done 2026-10-16, unreleased | — | — | | review-2026-08-followups | Remaining findings of the 2026-08-19 code review | closed 2026-10-14 (rows placed in [audit-2026-10-12](audit-2026-10-12.md)) | — | — | | panel-notifications | E-mail notifications about important events | shipped `2.3.0` | 10/10 | [plans/panel-notifications.md](plans/panel-notifications.md) | | delivery-log-storage | Keep a message's `mail.log` lines with its send-log row | shipped `2.2.0` | 8/8 | [plans/delivery-log-storage.md](plans/delivery-log-storage.md) | @@ -258,6 +258,13 @@ container. with a test that proves it, attempts are limited, and [security.md](security.md) § Backup says so. +**Done (2026-10-16):** "Your account password" in both forms (mockup +`1888dce`), checked by `reauth` in `internal/web/handlers` with bcrypt and +`AllowPasswordChange` (ten in fifteen minutes per user, shared with the +Account form); a wrong or missing one is 401 on the same page, logged, and +nothing is sent. Handler and e2e tests; security.md § Backup and domain +export, guide § Full backup and restore, help. + **Version:** `2.x` MINOR; `agreed`. --- diff --git a/docs/security.md b/docs/security.md index 43d639f..18f3b61 100644 --- a/docs/security.md +++ b/docs/security.md @@ -428,6 +428,18 @@ operator can act on it. ### Backup and domain export +- **A session is not enough to take them** (backup-reauth, owner + 2026-10-13; built 2026-10-16). The full backup and a domain's export ask + for the signed-in user's own account password at every download, checked + with bcrypt as a password change checks it and under the same limiter - + ten checks in fifteen minutes per user, shared with the Account form - + so a stolen session cookie or a panel left open no longer hands over a + copy of the server, and the field is no place to guess a password from. + A wrong one is refused with 401 and logged; nothing is sent + (`internal/web/handlers`, `reauth`). *Encrypt with a password* is the + file's own protection, not this one: whoever downloads it chooses that + password. The import and the `selfpost-backup` CLI, which runs inside the + container, are not asked. - Both files are secrets: a full backup carries DKIM keys, `sasldb2`, the panel users' password hashes, `docker-compose.yml`, `.env`, and the TLS private key from `certs/` when present; a domain export carries the DKIM key diff --git a/internal/web/handlers/handlers_backup.go b/internal/web/handlers/handlers_backup.go index 62d133e..f1e5326 100644 --- a/internal/web/handlers/handlers_backup.go +++ b/internal/web/handlers/handlers_backup.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "strconv" "strings" "time" @@ -16,6 +17,7 @@ import ( "github.com/mixeme/selfpost/internal/store" "github.com/mixeme/selfpost/internal/web/validate" "github.com/mixeme/selfpost/internal/web/view" + "golang.org/x/crypto/bcrypt" ) // maxImportBytes caps a domain-import upload. A domain export is a small JSON @@ -65,6 +67,10 @@ func (h *Handlers) HandleBackup(w http.ResponseWriter, r *http.Request) { if _, ok := h.requireGlobal(w, r); !ok { return } + if status, msg := h.reauth(r, "full backup"); status != 0 { + h.renderBackupPageWith(w, r, status, "", msg) + return + } password, pwErr := secretFilePassword(r) if pwErr != "" { h.renderBackupPageWith(w, r, http.StatusBadRequest, "", pwErr) @@ -161,6 +167,10 @@ func (h *Handlers) HandleExportDomain(w http.ResponseWriter, r *http.Request) { if !ok { return } + if status, msg := h.reauth(r, "export of domain "+strconv.FormatInt(d.ID, 10)); status != 0 { + h.renderDomainSettings(w, r, status, d, settingsView{Err: msg}) + return + } password, pwErr := secretFilePassword(r) if pwErr != "" { h.renderDomainSettings(w, r, http.StatusBadRequest, d, settingsView{Err: pwErr}) @@ -314,6 +324,38 @@ func (h *Handlers) HandleImportDomain(w http.ResponseWriter, r *http.Request) { // form. The confirmation field is checked here rather than in the browser // because a typo in an encryption password is unrecoverable: the archive would // be sealed with a secret the operator does not know. +// reauth checks the account password a secret download asks for +// (backup-reauth): the full backup holds the whole database - the panel's +// password hashes, every application's SASL password, the DKIM keys - and a +// domain's export its key and its applications' credentials, so a session +// alone, stolen or left open, must not be enough to take them. The password +// is the signed-in user's own, checked as a password change checks it and +// under the same limiter (auth.AllowPasswordChange), so the form is no place +// to guess one. It returns 0 when it holds, else the status and the message +// for the form. what names the download in the log. +func (h *Handlers) reauth(r *http.Request, what string) (int, string) { + p, ok := h.principal(r) + if !ok { + return http.StatusUnauthorized, "Sign in again." + } + if !h.auth.AllowPasswordChange(p.ID) { + return http.StatusTooManyRequests, "Too many attempts. Please wait and try again." + } + if err := r.ParseForm(); err != nil { + return http.StatusBadRequest, "Invalid form submission." + } + u, err := h.store.GetUser(p.ID) + if err != nil { + logf("panel: %s: get user %d: %v", what, p.ID, err) + return http.StatusInternalServerError, "Internal error. Please try again." + } + if err := bcrypt.CompareHashAndPassword([]byte(u.PasswordHash), []byte(r.PostFormValue("account_password"))); err != nil { + logf("panel: %s: user %d gave a wrong account password", what, p.ID) + return http.StatusUnauthorized, "Your account password is incorrect." + } + return 0, "" +} + func secretFilePassword(r *http.Request) (password, errMsg string) { if err := r.ParseForm(); err != nil { return "", "Invalid form submission." diff --git a/internal/web/handlers/handlers_backup_test.go b/internal/web/handlers/handlers_backup_test.go index b9dacf2..7d98e3f 100644 --- a/internal/web/handlers/handlers_backup_test.go +++ b/internal/web/handlers/handlers_backup_test.go @@ -133,3 +133,42 @@ func TestEmptyCertsMessage(t *testing.T) { t.Errorf("message = %q", msg) } } + +// A session is not enough for a secret download (backup-reauth): the full +// backup and a domain's export ask for the signed-in user's own password. +// Wrong or missing, nothing is sent and the page says so; right, the request +// goes on to its own checks. Guesses share the password change's limiter. +func TestSecretDownloadsAskForTheAccountPassword(t *testing.T) { + s := newAppStand(t) + exportPath := "/outbound/domains/" + idStr(s.d.ID) + "/settings/export" + for _, pw := range []string{"", "wrong-password-entirely"} { + rec := postFormAs(s.h.HandleExportDomain, globalPrincipal, exportPath, s.paths, url.Values{"account_password": {pw}}) + if rec.Code != http.StatusUnauthorized || rec.Header().Get("Content-Disposition") != "" || + !strings.Contains(rec.Body.String(), "Your account password is incorrect.") { + t.Fatalf("export with account password %q = %d, %q:\n%.600s", pw, rec.Code, rec.Header().Get("Content-Disposition"), rec.Body.String()) + } + rec = postFormAs(s.h.HandleBackup, globalPrincipal, "/server/backup", nil, url.Values{"account_password": {pw}}) + if rec.Code != http.StatusUnauthorized || rec.Header().Get("Content-Disposition") != "" || + !strings.Contains(rec.Body.String(), "Your account password is incorrect.") || + !strings.Contains(rec.Body.String(), `name="account_password" type="password" autocomplete="current-password" required`) { + t.Fatalf("backup with account password %q = %d:\n%.600s", pw, rec.Code, rec.Body.String()) + } + } + + // The right password lets the export through: a plain JSON download. + rec := postFormAs(s.h.HandleExportDomain, globalPrincipal, exportPath, s.paths, url.Values{"account_password": {"correct-password-here!"}}) + if rec.Code != http.StatusOK || !strings.Contains(rec.Header().Get("Content-Disposition"), "selfpost-domain-example.org.json") { + t.Fatalf("export with the right password = %d, %q:\n%.600s", rec.Code, rec.Header().Get("Content-Disposition"), rec.Body.String()) + } + + // Five checks were made above, and the limiter allows ten in its window: + // after six more, a check is refused before the password is looked at, + // the right one included. + for i := 0; i < 6; i++ { + postFormAs(s.h.HandleBackup, globalPrincipal, "/server/backup", nil, url.Values{"account_password": {"guess"}}) + } + rec = postFormAs(s.h.HandleExportDomain, globalPrincipal, exportPath, s.paths, url.Values{"account_password": {"correct-password-here!"}}) + if rec.Code != http.StatusTooManyRequests || !strings.Contains(rec.Body.String(), "Too many attempts.") { + t.Fatalf("after ten checks = %d:\n%.600s", rec.Code, rec.Body.String()) + } +} diff --git a/internal/web/handlers/handlers_outbound_pages_test.go b/internal/web/handlers/handlers_outbound_pages_test.go index 3e182bf..141326d 100644 --- a/internal/web/handlers/handlers_outbound_pages_test.go +++ b/internal/web/handlers/handlers_outbound_pages_test.go @@ -189,12 +189,12 @@ func TestDomainSettingsRefusalsStayOnTheSettingsPage(t *testing.T) { } has(t, "report address refusal", body, `Choose how aggregate reports are addressed for this domain.`) - code, body = post(s.h.HandleExportDomain, "export", url.Values{"encrypt": {"1"}, "password": {"short"}, "password_confirm": {"short"}}) + code, body = post(s.h.HandleExportDomain, "export", url.Values{"encrypt": {"1"}, "password": {"short"}, "password_confirm": {"short"}, "account_password": {"correct-password-here!"}}) if code != http.StatusBadRequest { t.Fatalf("a short export password = %d", code) } has(t, "export refusal", body, `The encryption password must be at least 12 characters.`, `

Domain settings

`) - code, body = post(s.h.HandleExportDomain, "export", url.Values{"encrypt": {"1"}, "password": {"long-enough-password"}, "password_confirm": {"another-one-entirely"}}) + code, body = post(s.h.HandleExportDomain, "export", url.Values{"encrypt": {"1"}, "password": {"long-enough-password"}, "password_confirm": {"another-one-entirely"}, "account_password": {"correct-password-here!"}}) if code != http.StatusBadRequest { t.Fatalf("export passwords that differ = %d", code) } diff --git a/internal/web/view/help.go b/internal/web/view/help.go index 07ea838..f00f2c6 100644 --- a/internal/web/view/help.go +++ b/internal/web/view/help.go @@ -234,7 +234,8 @@ var serverTopics = []HelpTopic{ {ID: HelpBackup, Title: "Backup and moving a domain", Body: []Text{ Rich("Full backup downloads one archive of the whole instance: ", Code("data/"), " (the database, DKIM keys and application credentials), ", Code("docker-compose.yml"), ", ", Code(".env"), " and ", Code("certs/"), - ". The Postfix queue is not in it. The backup is taken while the server is running, and Postfix keeps moving messages through the queue, "+ + ". Both it and a domain's export ask for your account password - the one you sign in with - at every download, so a session left open is not enough to take them; wrong guesses count against the same limit as a password change."+ + " The Postfix queue is not in it. The backup is taken while the server is running, and Postfix keeps moving messages through the queue, "+ "so a copy could hold a message twice or miss it; mail still waiting for delivery is therefore not carried over, and a restored server "+ "starts with an empty queue. To keep the queue, stop the container and archive the project directory instead. The private key in ", Code("certs/"), " has to be readable by the panel: give it to group 10998 on the host (", Code("chgrp 10998"), ", ", Code("chmod 640"), diff --git a/internal/web/view/paneljs_test.go b/internal/web/view/paneljs_test.go index 382ae4d..4b489a5 100644 --- a/internal/web/view/paneljs_test.go +++ b/internal/web/view/paneljs_test.go @@ -17,7 +17,7 @@ func TestPanelJS(t *testing.T) { if err != nil { t.Skip("node is not installed") } - for _, script := range []string{"poll_test.js", "unsaved_test.js"} { + for _, script := range []string{"poll_test.js", "unsaved_test.js", "filename_test.js"} { t.Run(script, func(t *testing.T) { out, err := exec.Command(node, filepath.Join("testdata", script), filepath.Join("static", "panel.js")).CombinedOutput() if err != nil { diff --git a/internal/web/view/static/panel.js b/internal/web/view/static/panel.js index c551e78..e81a5b9 100644 --- a/internal/web/view/static/panel.js +++ b/internal/web/view/static/panel.js @@ -245,16 +245,34 @@ }); } + // A file field drawn as Bulma's .file (the import of a domain, mockup + // backup.html): the input lies transparent over "Choose a file…", and the + // name of the file chosen is written into .file-name, which says "No file + // chosen" until then. Without this script the field still works; only the + // name is not shown. + function syncFileName(input) { + var box = input.closest(".file"); + var name = box && box.querySelector(".file-name"); + if (name) { + name.textContent = input.files && input.files[0] ? input.files[0].name : "No file chosen"; + } + } + // One listener for the document rather than one per control: a form that // arrives in an htmx swap is covered without being wired up again. document.addEventListener("change", function (ev) { if (ev.target && ev.target.matches) { syncShowWhen(ev.target); + if (ev.target.matches("input.file-input")) { + syncFileName(ev.target); + } } }); + // A page the browser restores from its history may keep the file chosen. document.addEventListener("DOMContentLoaded", function () { initShowWhen(document); + document.querySelectorAll("input.file-input").forEach(syncFileName); }); // htmx:load fires for each top-level element htmx puts into the page, so diff --git a/internal/web/view/templates/backup.html b/internal/web/view/templates/backup.html index 126e0cb..3b61164 100644 --- a/internal/web/view/templates/backup.html +++ b/internal/web/view/templates/backup.html @@ -23,6 +23,8 @@

Keep this password: SelfPost does not store it, and the file cannot be opened without it.

+
+

The one you sign in with, asked at every download: the archive holds every secret of the server, and a session left open must not be enough to take it.

{{template "body_close"}} @@ -35,7 +37,7 @@

A domain export from another instance — plain .json or encrypted .spde. Its DKIM key and application passwords come across, so the published DNS needs no change. The export itself is made on the domain's settings page.

-
+

Asked only for an encrypted .spde file.

diff --git a/internal/web/view/templates/out_domain_settings.html b/internal/web/view/templates/out_domain_settings.html index bb802e0..067f5b3 100644 --- a/internal/web/view/templates/out_domain_settings.html +++ b/internal/web/view/templates/out_domain_settings.html @@ -36,6 +36,8 @@

Keep this password: SelfPost does not store it, and the file cannot be opened without it.

+
+

The one you sign in with, asked at every export: the file holds the domain's DKIM key and its applications' credentials.

{{template "body_close"}} diff --git a/internal/web/view/testdata/filename_test.js b/internal/web/view/testdata/filename_test.js new file mode 100644 index 0000000..3f8fd2e --- /dev/null +++ b/internal/web/view/testdata/filename_test.js @@ -0,0 +1,50 @@ +// The name of the file chosen in a Bulma .file field is written into its +// .file-name (the import of a domain on Server › Backup): on change, and at +// load for a page the browser restored with a file still chosen. Run by +// TestPanelJS (paneljs_test.go): node filename_test.js . Exits +// non-zero with what went wrong. +"use strict"; +const fs = require("fs"); +const vm = require("vm"); + +const failures = []; +function check(cond, what) { + if (!cond) failures.push(what); +} + +const listeners = {}; +const label = { textContent: "No file chosen" }; +const box = { querySelector: (sel) => (sel === ".file-name" ? label : null) }; +const input = { + files: [], + matches: (sel) => sel === "input.file-input", + closest: (sel) => (sel === ".file" ? box : null), +}; +const document = { + hidden: false, + addEventListener: (type, fn) => (listeners[type] = listeners[type] || []).push(fn), + body: { addEventListener() {} }, + getElementById: () => null, + querySelectorAll: (sel) => (sel === "input.file-input" ? [input] : []), +}; +vm.runInNewContext(fs.readFileSync(process.argv[2], "utf8"), { + document, window: { addEventListener() {} }, htmx: { config: {}, ajax() {} }, setTimeout, clearTimeout, Date, Object, Math, console, +}); +const fire = (type, ev) => (listeners[type] || []).forEach((fn) => fn(ev)); + +input.files = [{ name: "example.org.spde" }]; +fire("change", { target: input }); +check(label.textContent === "example.org.spde", `after choosing a file the name reads ${JSON.stringify(label.textContent)}`); + +input.files = []; +fire("change", { target: input }); +check(label.textContent === "No file chosen", `after clearing the choice the name reads ${JSON.stringify(label.textContent)}`); + +input.files = [{ name: "restored.json" }]; +fire("DOMContentLoaded", {}); +check(label.textContent === "restored.json", `a restored page shows ${JSON.stringify(label.textContent)}`); + +if (failures.length) { + console.error(failures.join("\n")); + process.exit(1); +} diff --git a/test/e2e/certbot_test.go b/test/e2e/certbot_test.go index faccd09..3e1dc75 100644 --- a/test/e2e/certbot_test.go +++ b/test/e2e/certbot_test.go @@ -2,6 +2,7 @@ package e2e import ( "bytes" + "net/url" "os" "path/filepath" "strings" @@ -76,7 +77,7 @@ func testCertbotLayout(t *testing.T, sc *scenario) { t.Fatalf("Health names a renewal waiting, but Postfix serves what certs/ leads to") } - resp, archived, err := sc.panel.postForm("/server/backup", nil) + resp, archived, err := sc.panel.postForm("/server/backup", url.Values{"account_password": {adminPassword}}) if err != nil || resp.StatusCode != 200 { t.Fatalf("full backup over certbot's layout: %v, status %v\n%.600s", err, resp, archived) } diff --git a/test/e2e/restore_test.go b/test/e2e/restore_test.go index fb0b984..1823164 100644 --- a/test/e2e/restore_test.go +++ b/test/e2e/restore_test.go @@ -30,7 +30,19 @@ func testFullBackupRestore(t *testing.T, sc *scenario) { if err := os.Chmod(keyPath, 0o600); err != nil { t.Fatal(err) } - resp, body, err := sc.panel.postForm("/server/backup", url.Values{}) + // A session is not enough (backup-reauth): without the account password, + // or with a wrong one, nothing is sent. + for _, pw := range []string{"", "not-the-password"} { + resp, body, err := sc.panel.postForm("/server/backup", url.Values{"account_password": {pw}}) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != 401 || resp.Header.Get("Content-Disposition") != "" || !strings.Contains(body, "Your account password is incorrect.") { + t.Fatalf("full backup with account password %q: status %d, want a refusal:\n%.600s", pw, resp.StatusCode, body) + } + } + + resp, body, err := sc.panel.postForm("/server/backup", url.Values{"account_password": {adminPassword}}) if err != nil { t.Fatalf("full backup with an unreadable key: %v", err) } @@ -51,7 +63,7 @@ func testFullBackupRestore(t *testing.T, sc *scenario) { t.Fatalf("write an extra map: %v\n%s", err, out) } - resp, archive, err := sc.panel.postForm("/server/backup", url.Values{}) + resp, archive, err := sc.panel.postForm("/server/backup", url.Values{"account_password": {adminPassword}}) if err != nil { t.Fatalf("full backup: %v", err) }