Technical review (Opus) and security review (Fable) of 5be109b..5154bba;
reports in the session's scratchpad. Nothing above low from security; one
bug and three risks from the technical review. Migration 0009 is not
deployed, so it was extended.
- Bug: two refusals of one second swapped their NOQUEUE lines (each line
went to the newest lineless row). AttachRefusalLine now matches the
client address too (send_log.client_ip, written by the journal milter -
Postfix's line names the client and the sender, never the login: the
sasl_username part of the pattern never matched and could be fed by HELO,
security F1/F2), takes the nearest row in time and the oldest of equals,
and a line already on a row goes on no other (security F6).
- Waiting lines and recoveries lived only in memory while the saved offset
moved past them: a stop within 30 s lost them. The tracker now never saves
an offset past the oldest waiting item in its file (tracker.hold,
refusalLines.oldest) - for a recovery, the client= line that names its
login - so they are read again; the "once" guards make that harmless.
- A recovered row whose login the tailer did not see was nobody's: its
domain_id falls back to the envelope domain's.
- The fingerprint's file identity is the inode alone: the device number can
change across a host reboot and make the same log look rotated.
- A refused row carries its queue id; UpdateStatus's "newest row" now skips
refused rows, so a later message with that id does not flip it to sent.
- The alive mark: its age is read first and kept from then on, so asking
supervisord for 30 s cannot make it stale; the entrypoint removes it with
the sockets - a container restart is not the check's downtime, and a mark
from before would have counted the whole outage as "≈".
- Security F3: the sender check prints no ";" of the client's in the From:
text of its reply, and the tailer reads the envelope after the first ";"
- Postfix's own text.
- A refused row with a queue id and no stored lines says so (it was told the
mail.log text of 2.2.0); the fact's em dash is the house hyphen.
- Docs: architecture (client address, no login), guide § Health (warns on
either count), help topics log and sender-check (design 1ca87f2).
- e2e: refusals_show_postfixs_line wants the refusal's own sender in the
line; refusal_recovered_from_the_log waits past the tailer's 30 s and
checks that a refusal the panel wrote got no recovered twin.
Not changed, written down: review 9 (mail.log.1 read to its end at that
moment; postlogd may still write to it until the reload), 10 (a log under
512 bytes has no fingerprint, so places fall back to time briefly after a
rotation), 6 (a 451 at MAIL FROM of another milter reads the same - the
client and sender must match too now), security F5/F12 (info). Review 12:
the refused and closed-by-the-panel variants of out-message were shown to
the owner as pictures of the real templates, not drawn in out-message.html:
the guard ties every drawn screen to a page of the engine, and a variant
has the page's outline.
Evidence on the test server for this tree: go vet clean, go test ./... 23
packages ok, make e2e exit 0 (refusal_recovered_from_the_log 70 s).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
188 lines
6.2 KiB
Go
188 lines
6.2 KiB
Go
package main
|
|
|
|
import (
|
|
"context"
|
|
"log"
|
|
"os"
|
|
"time"
|
|
|
|
"github.com/mixeme/selfpost/internal/domain"
|
|
"github.com/mixeme/selfpost/internal/health"
|
|
"github.com/mixeme/selfpost/internal/milter"
|
|
"github.com/mixeme/selfpost/internal/postfix"
|
|
"github.com/mixeme/selfpost/internal/store"
|
|
)
|
|
|
|
// aliveEvery is how often the sender check marks itself alive. The gap a
|
|
// restart finds since the last mark is the check's downtime, so the mark's
|
|
// age is the error of that figure.
|
|
const aliveEvery = 2 * time.Second
|
|
|
|
// serveSenderCheck runs the sender check (internal/milter/sender.go) on a
|
|
// socket of its own. Postfix attaches that socket strictly, so while this
|
|
// role is not serving, mail is deferred: what that costs is counted here and
|
|
// by the log tailer, and shown on Server > Health
|
|
// (docs/plans/from-header-binding.md § Statistics).
|
|
func serveSenderCheck(ctx context.Context, cfg config, st *store.Store) error {
|
|
ln, err := listenMilter(cfg.senderSocket)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
log.Printf("sender-check listening on %s", cfg.senderSocket)
|
|
|
|
// Statistics are the database's business and supervisord's, and the check
|
|
// is neither: they are kept beside it, and nothing of them is waited for
|
|
// before it serves.
|
|
now := time.Now()
|
|
mark := cfg.senderSocket + ".alive"
|
|
// The mark's age is read before the mark is kept again: asking supervisord
|
|
// may take half a minute, and the mark must not go stale meanwhile, or a
|
|
// restart in that half minute would count it as downtime.
|
|
var markAt time.Time
|
|
if fi, err := os.Stat(mark); err == nil {
|
|
markAt = fi.ModTime()
|
|
}
|
|
go keepAlive(ctx, mark, st)
|
|
go func() {
|
|
if err := st.StartSenderCheckStats(now); err != nil {
|
|
log.Printf("sender-check: statistics: %v", err)
|
|
}
|
|
if down, estimated := senderCheckDowntime(markAt, now, postfixStarted); down > 0 {
|
|
if estimated {
|
|
log.Printf("sender-check: was not answering for at most %s (supervisord did not say since when Postfix ran)", down.Round(time.Second))
|
|
} else {
|
|
log.Printf("sender-check: was not answering for %s while Postfix was running", down.Round(time.Second))
|
|
}
|
|
for _, part := range byDay(now.Add(-down), now) {
|
|
if err := st.AddSenderCheckDown(part.day, part.down, estimated); err != nil {
|
|
log.Printf("sender-check: statistics: %v", err)
|
|
}
|
|
}
|
|
}
|
|
}()
|
|
|
|
// The two tables the mail path itself goes by: whom Postfix lets send as
|
|
// what, and what OpenDKIM signs.
|
|
bind := milter.NewBindings(postfix.New(cfg.postfixDir).SenderLoginMapsPath())
|
|
signers := milter.NewSigners(domain.NewOpenDKIM(cfg.opendkimDir).SigningTablePath())
|
|
return milter.ServeSender(ctx, ln, bind, signers, st)
|
|
}
|
|
|
|
// postfixState is what supervisord said about Postfix: since when it runs, if
|
|
// it does; known is false when supervisord did not answer.
|
|
type postfixState struct {
|
|
started time.Time
|
|
running, known bool
|
|
}
|
|
|
|
// senderCheckDowntime is for how long the check was not serving before now
|
|
// while Postfix was: the time since the mark was last set (markAt; zero
|
|
// when there is none), or since
|
|
// Postfix started if that was later. No mark means the container is new, and
|
|
// a Postfix that is not running means mail was stopped for another reason -
|
|
// a rollout, a reboot - and neither is the check's downtime. When supervisord
|
|
// could not say, the time since the mark is taken and estimated is true: an
|
|
// upper bound, since Postfix may not have been running all of it (audit
|
|
// 2026-10-12, stage 2b - until then such a start counted nothing).
|
|
func senderCheckDowntime(markAt, now time.Time, ask func() postfixState) (down time.Duration, estimated bool) {
|
|
if markAt.IsZero() {
|
|
return 0, false
|
|
}
|
|
from := markAt
|
|
p := ask()
|
|
switch {
|
|
case !p.known:
|
|
estimated = true
|
|
case !p.running:
|
|
return 0, false
|
|
case p.started.After(from):
|
|
from = p.started
|
|
}
|
|
if d := now.Sub(from); d > 0 {
|
|
return d, estimated
|
|
}
|
|
return 0, false
|
|
}
|
|
|
|
// dayPart is the part of a stretch of downtime that fell on one UTC day.
|
|
type dayPart struct {
|
|
day time.Time
|
|
down time.Duration
|
|
}
|
|
|
|
// byDay cuts the time from 'from' to 'to' at the UTC midnights between them,
|
|
// so that a night's outage is not all booked on the morning it ended.
|
|
func byDay(from, to time.Time) []dayPart {
|
|
from, to = from.UTC(), to.UTC()
|
|
var parts []dayPart
|
|
for from.Before(to) {
|
|
next := time.Date(from.Year(), from.Month(), from.Day(), 0, 0, 0, 0, time.UTC).AddDate(0, 0, 1)
|
|
if next.After(to) {
|
|
next = to
|
|
}
|
|
parts = append(parts, dayPart{day: from, down: next.Sub(from)})
|
|
from = next
|
|
}
|
|
return parts
|
|
}
|
|
|
|
// postfixAskFor is how long postfixStarted keeps asking a supervisord that
|
|
// does not answer - at the container's start it may not be listening yet.
|
|
var postfixAskFor, postfixAskEvery = 30 * time.Second, 2 * time.Second
|
|
|
|
// postfixStarted asks supervisord since when Postfix has been running, again
|
|
// and again for postfixAskFor while it does not answer.
|
|
func postfixStarted() postfixState {
|
|
deadline := time.Now().Add(postfixAskFor)
|
|
for {
|
|
procs, err := health.Processes()
|
|
if err == nil {
|
|
for _, p := range procs {
|
|
if p.Name != "postfix" || p.State != "RUNNING" {
|
|
continue
|
|
}
|
|
if up, ok := health.Uptime(p.Detail); ok {
|
|
return postfixState{started: time.Now().Add(-up), running: true, known: true}
|
|
}
|
|
}
|
|
return postfixState{known: true}
|
|
}
|
|
if time.Now().After(deadline) {
|
|
// Said aloud: this start's downtime is then an estimate.
|
|
log.Printf("sender-check: statistics: when Postfix started is not known: %v", err)
|
|
return postfixState{}
|
|
}
|
|
time.Sleep(postfixAskEvery)
|
|
}
|
|
}
|
|
|
|
// keepAlive sets the mark until ctx ends, and once a day drops the days of
|
|
// the statistics that are older than a year.
|
|
func keepAlive(ctx context.Context, path string, st *store.Store) {
|
|
touch := func() {
|
|
now := time.Now()
|
|
if err := os.Chtimes(path, now, now); err != nil {
|
|
if err := os.WriteFile(path, nil, 0o644); err != nil {
|
|
log.Printf("sender-check: alive mark: %v", err)
|
|
}
|
|
}
|
|
}
|
|
touch()
|
|
tick := time.NewTicker(aliveEvery)
|
|
defer tick.Stop()
|
|
prune := time.NewTicker(24 * time.Hour)
|
|
defer prune.Stop()
|
|
for {
|
|
select {
|
|
case <-ctx.Done():
|
|
return
|
|
case <-tick.C:
|
|
touch()
|
|
case <-prune.C:
|
|
if err := st.PruneSenderCheckDays(time.Now()); err != nil {
|
|
log.Printf("sender-check: statistics: %v", err)
|
|
}
|
|
}
|
|
}
|
|
}
|