auth, docs: a session from a longer window is cut; the remaining nits (audit A6, A7, A10, W7)

Security audit 2026-10-17, stage 6.

A10: a session created under a longer PANEL_SESSION_IDLE_DAYS kept its old
expiry after the setting was lowered (the renewal arithmetic took
ExpiresAt minus the new window for the last renewal). Touch now cuts such
a session to now plus the window at once. TestSessionMadeUnderALongerWindowIsCut.

A6, A7, W7 - security.md now says what the code does, which is what it
should do: a wrong method answers 405 with Allow from the router before the
role is looked at (the routes are public source); the setup token is made
anew at the first /setup/ request after expiry, a stranger's included, and
logged where only the operator reads it; two GETs make what must exist,
the server's own DKIM key on Settings and Health. README's local trial
says the compose file is the hardened deployment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-17 22:38:52 +03:00
co-authored by Claude Opus 5.5
parent 5c5f49bad1
commit 4f999f4f95
6 changed files with 65 additions and 8 deletions
+9
View File
@@ -49,6 +49,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
- **The signature covers `Content-Type`, `MIME-Version` and `Message-ID`** - **The signature covers `Content-Type`, `MIME-Version` and `Message-ID`**
when the message has them (audit M6), and a header line too long to be when the message has them (audit M6), and a header line too long to be
delivered unbroken is named in the log as a body line is (M8). delivered unbroken is named in the log as a body line is (M8).
- **A session made under a longer `PANEL_SESSION_IDLE_DAYS` is cut to the
current window** at its next request (audit A10), not kept until its old
expiry.
- **The TLS private key no longer has to be readable by Postfix** (audit - **The TLS private key no longer has to be readable by Postfix** (audit
I1): the guide gave it to group 10998 for the panel's backup, and I1): the guide gave it to group 10998 for the panel's backup, and
`postfix` is in that group, so every Postfix daemon - an smtpd talking to `postfix` is in that group, so every Postfix daemon - an smtpd talking to
@@ -103,6 +106,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
### Documentation ### Documentation
- security.md says what the code does where it said more (audit A6, A7,
W7): a wrong method answers `405` before the role is looked at; the setup
token is made anew at the first request after it expired; two GETs make
the server's own DKIM key on first view. The README's local trial says
that the compose file is the hardened deployment.
- **The upstream's "Required" TLS checks no certificate**, and the guide and - **The upstream's "Required" TLS checks no certificate**, and the guide and
Help now say so (audit M7): it guards against a listener, not against one Help now say so (audit M7): it guards against a listener, not against one
who answers in the upstream's place. who answers in the upstream's place.
+5
View File
@@ -125,6 +125,11 @@ docker run --rm -d --name selfpost-try \
ghcr.io/mixeme/selfpost:2.14.3 ghcr.io/mixeme/selfpost:2.14.3
``` ```
This trial runs with Docker's default capabilities and no
`no-new-privileges`; the compose file of a real deployment
(`deploy/docker-compose.yml`) drops every capability but the few it names,
sets `no-new-privileges` and a process ceiling.
**Get the setup URL** (pick one): **Get the setup URL** (pick one):
```sh ```sh
+5 -2
View File
@@ -850,8 +850,11 @@ the argument.
- [x] **M7.** guide: upstream `encrypt` checks no certificate. — **Sonnet**, - [x] **M7.** guide: upstream `encrypt` checks no certificate. — **Sonnet**,
0.25 h. *Done 2026-10-17:* guide and Help. (Offering `secure` is hours and a `domain`-user-facing choice; 0.25 h. *Done 2026-10-17:* guide and Help. (Offering `secure` is hours and a `domain`-user-facing choice;
propose it on the roadmap, not here.) propose it on the roadmap, not here.)
- [ ] **A6, A7, A10, W7, M6, M8** and the README note — half an hour each, - [x] **A6, A7, A10, W7, M6, M8** and the README note — half an hour each,
or the wording change named under each. — **Sonnet**, 2 h. or the wording change named under each. — **Sonnet**, 2 h. *Done 2026-10-17:* A10 and
M6, M8 in code (a session from a longer window is cut at the next check;
three headers signed; a long header line logged); A6, A7, W7 by wording
in security.md - each is what the code should do; the README note.
- [ ] **Doc drift sweep** — every line under "Doc and comment drift" above. - [ ] **Doc drift sweep** — every line under "Doc and comment drift" above.
— **Sonnet**, 1 h. — **Sonnet**, 1 h.
- **Accept, with the argument:** *the setuid/setgid binaries of the base - **Accept, with the argument:** *the setuid/setgid binaries of the base
+15 -5
View File
@@ -87,8 +87,12 @@ The panel is exposed to the internet — the items below are **not optional**.
- A one-time secret link `/setup/<token>`, **not** an env variable holding a - A one-time secret link `/setup/<token>`, **not** an env variable holding a
ready-made password hash. ready-made password hash.
- Token ≥128 bits (`crypto/rand`); mirrored to `/data/setup-token`. - Token ≥128 bits (`crypto/rand`); mirrored to `/data/setup-token`.
- Token lifetime — **10 minutes**; after expiry, or after a restart with setup - Token lifetime — **10 minutes**; a restart with setup unfinished makes a
unfinished, it is regenerated and logged again. new one, and so does the first `/setup/` request after it expired - any
request, a stranger's `GET /setup/x` included, which then logs the new
token in the container log, where only the operator reads it (audit
2026-10-17, A7: nothing reaches the requester; the wording said "after
expiry").
- Rate limiting on `/setup/<token>` per client address (an IPv6 one per - Rate limiting on `/setup/<token>` per client address (an IPv6 one per
/64), separate from login. /64), separate from login.
- Token comparison is **constant-time** (`subtle.ConstantTimeCompare`). - Token comparison is **constant-time** (`subtle.ConstantTimeCompare`).
@@ -220,7 +224,10 @@ assigned outbound domains and inbound domains **separately**, each list with
- A path a user may not reach, a domain or application that does not exist, - A path a user may not reach, a domain or application that does not exist,
and one that belongs to another tenant answer the **same 404**, so the panel and one that belongs to another tenant answer the **same 404**, so the panel
does not confirm what exists. A group with nothing assigned is absent from does not confirm what exists. A group with nothing assigned is absent from
the user's menu. the user's menu. A route asked with a method it does not take answers
`405` with `Allow`, from the router, before the role is looked at: the
routes are the public source's, so that says nothing the code does not
(audit 2026-10-17, A6).
- `/server/` is guarded once, by `globalOnly` where its routes are registered - `/server/` is guarded once, by `globalOnly` where its routes are registered
(a route outside the subtree cannot be added through that helper), and every (a route outside the subtree cannot be added through that helper), and every
handler checks again. Outbound and inbound handlers check per object handler checks again. Outbound and inbound handlers check per object
@@ -244,8 +251,11 @@ assigned outbound domains and inbound domains **separately**, each list with
### State-changing requests ### State-changing requests
- Every state change is a `POST`; a `GET` never changes state (the delete - Every state change is a `POST`; a `GET` changes no state a user chose
confirmation pages only render a form). (the delete confirmation pages only render a form). Two make what has to
exist when it does not: Server › Settings and Health generate the server's
own DKIM key on their first view, and the first `/setup/` request after
the token expired makes a new one (audit 2026-10-17, W7, A7).
- Every non-read request first passes the origin check - Every non-read request first passes the origin check
(`originAllowed`, [internal/web/security.go](../internal/web/security.go)): (`originAllowed`, [internal/web/security.go](../internal/web/security.go)):
`Sec-Fetch-Site` must be `same-origin`, or `Origin` must name the panel's own `Sec-Fetch-Site` must be `same-origin`, or `Origin` must name the panel's own
+19
View File
@@ -379,3 +379,22 @@ func TestRequireAuthDoesNotRenewASessionWhoseUserIsGone(t *testing.T) {
t.Errorf("expiry of a missing user's session moved to %v", row.ExpiresAt) t.Errorf("expiry of a missing user's session moved to %v", row.ExpiresAt)
} }
} }
// A session made under a longer idle window is cut to the current one at its
// next renewal check, not kept until its old expiry (security audit
// 2026-10-17, A10).
func TestSessionMadeUnderALongerWindowIsCut(t *testing.T) {
s := newTestSessionStore(t) // seven days
token := mustCreate(t, s, "admin")
s.idle = 24 * time.Hour // PANEL_SESSION_IDLE_DAYS lowered to one
if !s.Touch(token) {
t.Fatal("the session from the longer window was not cut")
}
row, _, err := s.store.LookupSession(hashToken(token))
if err != nil || row.ExpiresAt.After(time.Now().Add(24*time.Hour+time.Minute)) {
t.Errorf("expires %v (%v), want within a day", row.ExpiresAt, err)
}
if s.Touch(token) {
t.Error("a session within the window was renewed again at once")
}
}
+12 -1
View File
@@ -98,8 +98,19 @@ func (s *sessionStore) Touch(token string) bool {
if !found { if !found {
return false return false
} }
lastRenewal := row.ExpiresAt.Add(-s.idle)
now := time.Now() now := time.Now()
// A session made under a longer PANEL_SESSION_IDLE_DAYS expires later
// than today's window allows: it is cut to the window at once, rather
// than keep the old expiry until it passes (security audit 2026-10-17,
// A10).
if row.ExpiresAt.After(now.Add(s.idle)) {
if err := s.store.RenewSession(hash, now.Add(s.idle)); err != nil {
logf("panel: session: shorten failed: %v", err)
return false
}
return true
}
lastRenewal := row.ExpiresAt.Add(-s.idle)
if now.Sub(lastRenewal) < renewThreshold { if now.Sub(lastRenewal) < renewThreshold {
return false return false
} }