auth: reset links pause for a day after five (audit A3)
Security audit 2026-10-17, finding A3 (low), stage 3. sendReset spaced reset mails ten minutes apart and capped them at five an hour per account, so a stranger who repeated a username kept a live 30-minute link arriving in the administrator's mailbox five times an hour, for ever. Now five reset links a day per account (resetDayLimiter); the sixth request sends one notice instead - "Reset links paused for a day", no link, saying further requests are ignored for 24 hours and that a global administrator can set a password on Server > Users - and every later one in the window sends nothing (resetNoticeLimiter), logged either way. In memory, like every limiter: a restart clears them. Test: TestResetLinksPauseForADayAfterFive. Docs: security.md, guide § Account; CHANGELOG; plan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -22,6 +22,10 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
|
|||||||
and name, and an IPv6 client is one per /64. With every request coming
|
and name, and an IPv6 client is one per /64. With every request coming
|
||||||
from one private address with `X-Forwarded-For` and `TRUSTED_PROXY_CIDR`
|
from one private address with `X-Forwarded-For` and `TRUSTED_PROXY_CIDR`
|
||||||
empty, the log says what to set. Refused sign-ins are logged.
|
empty, the log says what to set. Refused sign-ins are logged.
|
||||||
|
- **Reset links pause for a day after five** (audit A3): anyone who knew a
|
||||||
|
username could keep a live reset link arriving in that mailbox five times
|
||||||
|
an hour. After five in a day the owner is told once that further requests
|
||||||
|
are ignored for 24 hours.
|
||||||
- **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
|
||||||
|
|||||||
+2
-1
@@ -605,7 +605,8 @@ Notifications go to the address either way. Only a confirmed address can be
|
|||||||
sent a reset link: *Forgot your password?* on the sign-in page asks for the
|
sent a reset link: *Forgot your password?* on the sign-in page asks for the
|
||||||
username and always answers the same - whether the name exists, and whether
|
username and always answers the same - whether the name exists, and whether
|
||||||
its address is confirmed, is not said; another link is not sent within ten
|
its address is confirmed, is not said; another link is not sent within ten
|
||||||
minutes of the last - and the link
|
minutes of the last, and after five in a day the owner is mailed once that
|
||||||
|
further requests are ignored for 24 hours - and the link
|
||||||
(`https://<SELFPOST_HOSTNAME>/login/reset/…`) works once, for 30 minutes, and
|
(`https://<SELFPOST_HOSTNAME>/login/reset/…`) works once, for 30 minutes, and
|
||||||
only while the account's address is still the one it was sent to. Setting the
|
only while the account's address is still the one it was sent to. Setting the
|
||||||
new password signs every session of the account out. The links name the
|
new password signs every session of the account out. The links name the
|
||||||
|
|||||||
@@ -787,8 +787,9 @@ the argument.
|
|||||||
key. — **Sonnet**, 0.5 h. *Done 2026-10-17* (`refusalLog`).
|
key. — **Sonnet**, 0.5 h. *Done 2026-10-17* (`refusalLog`).
|
||||||
- [x] **A9.** IPv6 keyed by /64. — **Sonnet**, 0.5 h. *Done 2026-10-17:*
|
- [x] **A9.** IPv6 keyed by /64. — **Sonnet**, 0.5 h. *Done 2026-10-17:*
|
||||||
TestIPv6IsLimitedPerSlash64.
|
TestIPv6IsLimitedPerSlash64.
|
||||||
- [ ] **A3.** After five unsolicited reset links in a day, one mail saying
|
- [x] **A3.** After five unsolicited reset links in a day, one mail saying
|
||||||
further requests are ignored for 24 h, then silence. — **Opus**, 1.5 h.
|
further requests are ignored for 24 h, then silence. — **Opus**, 1.5 h.
|
||||||
|
*Done 2026-10-17:* TestResetLinksPauseForADayAfterFive.
|
||||||
- [ ] **I6.** `/healthz` answered only to loopback and trusted-proxy peers;
|
- [ ] **I6.** `/healthz` answered only to loopback and trusted-proxy peers;
|
||||||
the guide's "Container health" says the proxy may expose it on purpose
|
the guide's "Container health" says the proxy may expose it on purpose
|
||||||
with the fragment line. — **Sonnet**, 0.5 h.
|
with the fragment line. — **Sonnet**, 0.5 h.
|
||||||
|
|||||||
+4
-1
@@ -176,7 +176,10 @@ The panel is exposed to the internet — the items below are **not optional**.
|
|||||||
after answering, so neither the words nor the time tell whether a name
|
after answering, so neither the words nor the time tell whether a name
|
||||||
exists or is confirmed; requests are limited to five in 15 minutes per
|
exists or is confirmed; requests are limited to five in 15 minutes per
|
||||||
client address and username (fifty per address alone), mails to five an
|
client address and username (fifty per address alone), mails to five an
|
||||||
hour per account, and a link's page to 30 requests in 15 minutes per link
|
hour per account and five reset links a day - after which the owner is
|
||||||
|
told once that further requests are ignored for 24 hours, and nothing
|
||||||
|
more is sent (`2.14.4`; audit A3: anyone who knew a username kept a live
|
||||||
|
link in that mailbox five times an hour) - and a link's page to 30 requests in 15 minutes per link
|
||||||
(300 per address alone), so one stranger behind a shared proxy cannot stop
|
(300 per address alone), so one stranger behind a shared proxy cannot stop
|
||||||
the others asking for or opening a link (`2.14.4`; audit A1). A link's page changes nothing
|
the others asking for or opening a link (`2.14.4`; audit A1). A link's page changes nothing
|
||||||
on `GET` (a mail scanner opens links): the button does. Setting a password
|
on `GET` (a mail scanner opens links): the button does. Setting a password
|
||||||
|
|||||||
@@ -49,6 +49,11 @@ type Module struct {
|
|||||||
linkLimiter *rateLimiter
|
linkLimiter *rateLimiter
|
||||||
linkAddrLimiter *rateLimiter
|
linkAddrLimiter *rateLimiter
|
||||||
mailLimiter *rateLimiter
|
mailLimiter *rateLimiter
|
||||||
|
// resetDayLimiter counts the reset links mailed to one account in a
|
||||||
|
// day, resetNoticeLimiter the one notice that further requests are
|
||||||
|
// ignored (links.go, sendReset).
|
||||||
|
resetDayLimiter *rateLimiter
|
||||||
|
resetNoticeLimiter *rateLimiter
|
||||||
|
|
||||||
proxyWatch proxyWatch
|
proxyWatch proxyWatch
|
||||||
refusals *refusalLog
|
refusals *refusalLog
|
||||||
@@ -89,6 +94,10 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo
|
|||||||
// asked for and a couple of confirmations is ordinary use, a mailbox
|
// asked for and a couple of confirmations is ordinary use, a mailbox
|
||||||
// full of them is not.
|
// full of them is not.
|
||||||
mailLimiter: newRateLimiter(5, time.Hour),
|
mailLimiter: newRateLimiter(5, time.Hour),
|
||||||
|
// Five reset links a day to one account, then one notice and
|
||||||
|
// silence for the rest of the day (security audit 2026-10-17, A3).
|
||||||
|
resetDayLimiter: newRateLimiter(resetsPerDay, 24*time.Hour),
|
||||||
|
resetNoticeLimiter: newRateLimiter(1, 24*time.Hour),
|
||||||
trustedProxies: cfg.TrustedProxyCIDRs,
|
trustedProxies: cfg.TrustedProxyCIDRs,
|
||||||
}
|
}
|
||||||
m.setupLimiter.startSweeper()
|
m.setupLimiter.startSweeper()
|
||||||
@@ -101,6 +110,8 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo
|
|||||||
m.resetLimiter.startSweeper()
|
m.resetLimiter.startSweeper()
|
||||||
m.linkLimiter.startSweeper()
|
m.linkLimiter.startSweeper()
|
||||||
m.mailLimiter.startSweeper()
|
m.mailLimiter.startSweeper()
|
||||||
|
m.resetDayLimiter.startSweeper()
|
||||||
|
m.resetNoticeLimiter.startSweeper()
|
||||||
// Made now rather than at the first unknown name, which would otherwise
|
// Made now rather than at the first unknown name, which would otherwise
|
||||||
// pay for two bcrypt runs (review of audit stage 4).
|
// pay for two bcrypt runs (review of audit stage 4).
|
||||||
go noSuchUserHash()
|
go noSuchUserHash()
|
||||||
|
|||||||
@@ -29,6 +29,12 @@ const (
|
|||||||
// spend the account's links an hour in a minute (security review,
|
// spend the account's links an hour in a minute (security review,
|
||||||
// 2026-10-17).
|
// 2026-10-17).
|
||||||
resetSpacing = 10 * time.Minute
|
resetSpacing = 10 * time.Minute
|
||||||
|
// resetsPerDay is how many reset links one account is mailed in a day.
|
||||||
|
// Anyone who knows a username can ask for them; past this the owner is
|
||||||
|
// told once that further requests are ignored for a day, and then nothing
|
||||||
|
// more is sent - rather than a live link in the mailbox five times an
|
||||||
|
// hour for ever (security audit 2026-10-17, A3).
|
||||||
|
resetsPerDay = 5
|
||||||
)
|
)
|
||||||
|
|
||||||
// later runs f after the answer, counted for Wait.
|
// later runs f after the answer, counted for Wait.
|
||||||
@@ -182,7 +188,22 @@ func (m *Module) sendReset(username string) {
|
|||||||
logf("panel: user %d: a reset link went less than %s ago; none was sent", u.ID, resetSpacing)
|
logf("panel: user %d: a reset link went less than %s ago; none was sent", u.ID, resetSpacing)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if !m.mailLimiter.Allow("reset:" + strconv.FormatInt(u.ID, 10)) {
|
key := "reset:" + strconv.FormatInt(u.ID, 10)
|
||||||
|
if !m.resetDayLimiter.Allow(key) {
|
||||||
|
if !m.resetNoticeLimiter.Allow(key) {
|
||||||
|
logf("panel: user %d: reset links asked for again after today's %d; nothing was sent", u.ID, resetsPerDay)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
logf("panel: user %d: reset links asked for more than %d times today; the owner is told further requests are ignored for 24 hours", u.ID, resetsPerDay)
|
||||||
|
body := "Reset links for the SelfPost account " + u.Username + " at " + m.cfg.Hostname + " have been asked for more than " +
|
||||||
|
strconv.Itoa(resetsPerDay) + " times today. Further requests are ignored for 24 hours, and no more mail of this kind will come before then.\n\n" +
|
||||||
|
"If it was not you, nothing needs doing: the password stays as it is. A global administrator can set a new password on Server > Users at any time.\n"
|
||||||
|
if err := m.mailer.Send(u.Email, "[SelfPost] Reset links paused for a day", body); err != nil {
|
||||||
|
logf("panel: user %d: notice not sent: %v", u.ID, err)
|
||||||
|
}
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if !m.mailLimiter.Allow(key) {
|
||||||
logf("panel: user %d: reset links asked for too often; none was sent", u.ID)
|
logf("panel: user %d: reset links asked for too often; none was sent", u.ID)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package auth
|
package auth
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"github.com/mixeme/selfpost/internal/store"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"net/url"
|
"net/url"
|
||||||
@@ -299,3 +300,33 @@ func TestLinkPathsWithoutAToken(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Five reset links a day to one account, then one notice that further
|
||||||
|
// requests are ignored for a day, then nothing (security audit 2026-10-17,
|
||||||
|
// A3). The ten-minute spacing is stepped over by removing the last link.
|
||||||
|
func TestResetLinksPauseForADayAfterFive(t *testing.T) {
|
||||||
|
m, ml := linkModule(t, true)
|
||||||
|
u, _ := m.store.GetUserByUsername("admin")
|
||||||
|
for i := 0; i < resetsPerDay+3; i++ {
|
||||||
|
if err := m.store.DeleteUserLinks(u.ID, store.LinkReset); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
m.sendReset("admin")
|
||||||
|
got, sent := ml.next(t)
|
||||||
|
switch {
|
||||||
|
case i < resetsPerDay:
|
||||||
|
if !sent || !resetLinkRe.MatchString(got.body) {
|
||||||
|
t.Fatalf("link %d: sent %v, %+v", i+1, sent, got)
|
||||||
|
}
|
||||||
|
case i == resetsPerDay:
|
||||||
|
if !sent || got.subject != "[SelfPost] Reset links paused for a day" || resetLinkRe.MatchString(got.body) ||
|
||||||
|
!strings.Contains(got.body, "ignored for 24 hours") {
|
||||||
|
t.Fatalf("the notice: sent %v, %+v", sent, got)
|
||||||
|
}
|
||||||
|
default:
|
||||||
|
if sent {
|
||||||
|
t.Fatalf("request %d after the notice sent %+v", i+1, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user