postfix: no reload for a map that did not change; DNS re-checks limited (audit W6/M9)

Security audit 2026-10-17, findings W6/M9 (low), stage 4. Every
application, upstream, recipient or report-address save reloaded Postfix
through supervisord, and POST .../dns-recheck bypassed the five-minute
cache for up to ~14 resolver queries - both at whatever rate an
authenticated user pressed.

- writeChanged: each generated map is written only when its content
  differs from the file on disk, and Postfix is reloaded only when one was
  (sender map, inbound maps, DMARC maps). Health's Reload configuration
  ends with an explicit reload (app.Service.ReloadPostfix, SenderMaps
  gains Reload), so it still re-reads whatever is on disk. No debounce:
  with unchanged saves gone, a burst is real changes Postfix must see.
- auth.AllowDNSRecheck: twenty forced re-checks in 15 minutes per user;
  past that the outbound, inbound and settings re-check show the cached
  result without the "re-checked" flash, and the log says why.

Tests: TestARebuildThatChangesNothingDoesNotReload,
TestDNSRechecksArePerUser; the fakes gain Reload. Docs: guide § Overview
and Health; CHANGELOG; plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-17 22:25:20 +03:00
co-authored by Claude Opus 5.5
parent dd950364a3
commit 668e299c69
18 changed files with 153 additions and 22 deletions
+12
View File
@@ -54,6 +54,8 @@ type Module struct {
// ignored (links.go, sendReset).
resetDayLimiter *rateLimiter
resetNoticeLimiter *rateLimiter
// recheckLimiter counts the DNS re-checks a user forces (AllowDNSRecheck).
recheckLimiter *rateLimiter
proxyWatch proxyWatch
refusals *refusalLog
@@ -98,6 +100,7 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo
// 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),
recheckLimiter: newRateLimiter(20, 15*time.Minute),
trustedProxies: cfg.TrustedProxyCIDRs,
}
m.setupLimiter.startSweeper()
@@ -112,6 +115,7 @@ func New(st *store.Store, cfg Config, v *view.Engine, setupTokenPath string) *Mo
m.mailLimiter.startSweeper()
m.resetDayLimiter.startSweeper()
m.resetNoticeLimiter.startSweeper()
m.recheckLimiter.startSweeper()
// Made now rather than at the first unknown name, which would otherwise
// pay for two bcrypt runs (review of audit stage 4).
go noSuchUserHash()
@@ -131,6 +135,14 @@ func (m *Module) AllowPasswordChange(userID int64) bool {
return m.passwordLimiter.Allow("user:" + strconv.FormatInt(userID, 10))
}
// AllowDNSRecheck is whether a user may make the DNS checks again, past the
// five-minute cache: each re-check is up to fourteen resolver queries, and a
// domain user could press it without end (security audit 2026-10-17, M9).
// Twenty in fifteen minutes per user.
func (m *Module) AllowDNSRecheck(userID int64) bool {
return m.recheckLimiter.Allow("user:" + strconv.FormatInt(userID, 10))
}
// PasswordCheckPassed gives back the attempt AllowPasswordChange took for a
// check that found the right password: only wrong guesses count, so an
// administrator who adds ten users in a quarter of an hour is not locked out
+18
View File
@@ -114,3 +114,21 @@ func TestProxyWatchAndRefusalsAreLogged(t *testing.T) {
t.Errorf("refusals logged %d times (want 5), or the name unescaped:\n%s", n, got)
}
}
// A user forces the DNS checks twenty times in fifteen minutes, then sees
// the cached result (security audit 2026-10-17, M9); another user is not
// affected.
func TestDNSRechecksArePerUser(t *testing.T) {
m := testModule(t, false)
for i := 0; i < 20; i++ {
if !m.AllowDNSRecheck(1) {
t.Fatalf("re-check %d refused", i+1)
}
}
if m.AllowDNSRecheck(1) {
t.Error("the 21st re-check was allowed")
}
if !m.AllowDNSRecheck(2) {
t.Error("another user's first re-check was refused")
}
}
+16
View File
@@ -273,10 +273,26 @@ func (h *Handlers) HandleDomainDNSRecheck(w http.ResponseWriter, r *http.Request
http.Error(w, "internal error", http.StatusInternalServerError)
return
}
if !h.allowRecheck(r) {
http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d", d.ID), http.StatusSeeOther)
return
}
h.domainDNS(d, record, d.DMARCRua, true)
http.Redirect(w, r, fmt.Sprintf("/outbound/domains/%d?rechecked=1", d.ID), http.StatusSeeOther)
}
// allowRecheck is whether the signed-in user may force the DNS checks now
// (auth.AllowDNSRecheck). Past the limit the page shows the cached result,
// without saying it re-checked; the log says why.
func (h *Handlers) allowRecheck(r *http.Request) bool {
p, ok := h.principal(r)
if !ok || h.auth == nil || h.auth.AllowDNSRecheck(p.ID) {
return true
}
logf("panel: user %d: DNS re-checks asked for too often; the cached result is shown", p.ID)
return false
}
// limitChoice names which of the form's three limit choices an application has:
// the domain limit (none of its own), manual, or auto.
func limitChoice(rl store.RateLimit, configured bool) string {
@@ -20,6 +20,8 @@ var offlineDNS = dnscheck.New([]string{"127.0.0.1:9"})
type nopSenderMaps struct{}
func (nopSenderMaps) Reload() error { return nil }
func (nopSenderMaps) RebuildSenderLoginMaps([]postfix.Binding) error { return nil }
// appStand is a panel with one sending domain and a working application
@@ -206,6 +206,9 @@ func (h *Handlers) HandleReload(w http.ResponseWriter, r *http.Request) {
if h.dmarc != nil && h.dmarc.Enabled() {
steps = append(steps, step{"the DMARC report maps", h.dmarc.Resync})
}
// A rebuild that changed nothing asked no reload (security audit
// 2026-10-17, W6): this button re-reads whatever is on disk.
steps = append(steps, step{"Postfix's reload", h.apps.ReloadPostfix})
for _, s := range steps {
if err := s.run(); err != nil {
logf("panel: manual reload (%s): %v", s.name, err)
@@ -26,6 +26,8 @@ const savedElsewhere = "Check the logs and try again."
type failingSenderMaps struct{}
func (failingSenderMaps) Reload() error { return nil }
func (failingSenderMaps) RebuildSenderLoginMaps([]postfix.Binding) error {
return errors.New("write /srv/" + leak + "/sender_login_maps: permission denied")
}
@@ -275,6 +275,10 @@ func (h *Handlers) HandleInboundDNSRecheck(w http.ResponseWriter, r *http.Reques
if !ok {
return
}
if !h.allowRecheck(r) {
http.Redirect(w, r, fmt.Sprintf("/inbound/domains/%d", d.ID), http.StatusSeeOther)
return
}
if h.dns != nil && h.cfg.Hostname != "" {
h.dns.InboundMX(d.Name, h.cfg.Hostname, true)
}
@@ -203,6 +203,10 @@ func (h *Handlers) HandleSettingsDNSRecheck(w http.ResponseWriter, r *http.Reque
if _, ok := h.requireGlobal(w, r); !ok {
return
}
if !h.allowRecheck(r) {
http.Redirect(w, r, "/server/settings", http.StatusSeeOther)
return
}
h.dns.Server(h.cfg.Hostname, true)
h.serverMail(true)
http.Redirect(w, r, "/server/settings?rechecked=1", http.StatusSeeOther)