panel-notifications: findings of the technical and security reviews
test / test (push) Canceled after 0s

Neither review (read-only agents, Opus and Fable, over 972a9f8..400aa76) has
a Critical finding. Both found the same serious one, and more:

- The notification about refused mail was built on what a rejected row says
  of itself. Its domain is the one in MAIL FROM, which the client typed and
  Postfix has not yet held against the login when the milter refuses: an
  application over its limit could have had the panel write a message per
  made-up domain, to another tenant's users among others. And the same
  status is written when an application is refused for its client address,
  which is no limit. Refusals are now counted by the domain the login is
  registered under, only for logins that exist and only while a limit is set
  on the login or its domain; one subject per sending domain. They are read
  through a partial index of their own (migration 0005): the query walked
  the whole send log every minute.
- Account and user e-mails were checked with the DMARC report validator,
  which refuses Gmail and the like - the owner's own kind of address. A new
  validate.AccountEmail accepts them and refuses a local part beginning "-".
- "Said" now means sent: what arises while nobody can be told, or while
  Postfix does not take the message, is told at a later check. Its ending
  with nobody left to tell is simply over.
- Two DNS checks must be 45 minutes apart as well as two: the last check
  before a restart and the first after it were minutes apart.
- The notifier's own mail waiting in the queue, and its bounce coming back
  to postmaster@ this server, are not the queue's subject. A listing cut at
  its size limit or with a line too long to read is no listing; postqueue -j
  gets 30 seconds.
- The tables must name the selector the panel shows, not only have the line.
- An a/mx lookup that did not answer inside the SPF check made the record
  "not authorising"; it is "cannot tell" now, for the pages too.
- A ceiling of 60 messages an hour; a pass stops between domains when the
  process is asked to stop; sendmail gets a WaitDelay; days are rounded as a
  person counts; a failed read of the switch shows it on, not off.

Recorded as accepted in security.md, with a roadmap candidate
(from-header-binding): an application can have a message signed as the
server's own name - or, as always, as another registered domain - by writing
it into the From: header, which nothing holds against the login.

Left as found: a recipient whose copy failed while another's went out is not
tried again; a certificate that lives under 14 days is reported at each
renewal.

On the test server, for this tree: go vet clean, go test ./... 23 packages
ok, make e2e ok (326 s).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-12 17:00:01 +03:00
co-authored by Claude Opus 5.5
parent 400aa76737
commit f09b60aaa2
30 changed files with 808 additions and 184 deletions
+23 -2
View File
@@ -15,7 +15,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
- **The panel writes to its users when something needs them.** Four things, - **The panel writes to its users when something needs them.** Four things,
each said once: a sending domain's DKIM, SPF or DMARC record stops matching each said once: a sending domain's DKIM, SPF or DMARC record stops matching
(after two hourly checks in a row, and once more when it matches again); a (after two hourly checks in a row, and once more when it matches again); a
level-2 rate limit refuses mail (at most once a day per limit); the TLS level-2 rate limit refuses mail (at most once a day per sending domain); the TLS
certificate is 14 days from expiry, and 3; a message has waited in the certificate is 14 days from expiry, and 3; a message has waited in the
queue for more than three days (and once more when none has). Global users queue for more than three days (and once more when none has). Global users
are told all of it, domain users about their own domains. Sent from are told all of it, domain users about their own domains. Sent from
@@ -24,7 +24,7 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
- **The server's own name is signed.** The panel generates a DKIM key for - **The server's own name is signed.** The panel generates a DKIM key for
`SELFPOST_HOSTNAME` and OpenDKIM signs mail submitted locally as that name `SELFPOST_HOSTNAME` and OpenDKIM signs mail submitted locally as that name
(`non_smtpd_milters`), so that notifications arrive. The name is not a (`non_smtpd_milters`), so that notifications arrive. The name is not a
sending domain: no application can send as it. sending domain: no application can use it as an envelope sender.
- **Notifications in the panel.** Account has *Send me notifications* beside - **Notifications in the panel.** Account has *Send me notifications* beside
the e-mail; Server › Settings has the switch for everyone and the two DNS the e-mail; Server › Settings has the switch for everyone and the two DNS
records of the server's own name, DKIM and SPF, with the values to publish; records of the server's own name, DKIM and SPF, with the values to publish;
@@ -32,6 +32,27 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
has a Notifications topic. Every user starts with notifications on, and so has a Notifications topic. Every user starts with notifications on, and so
does the instance. does the instance.
### Changed
- **An account e-mail may be at a public mail host.** Account and Server ›
Users refused addresses at Gmail, Outlook, Proton and the like, a rule that
belongs to DMARC report addresses. A user's own address is now where the
panel writes to them.
- **A lookup that does not answer is not an SPF mismatch.** When the address
lookup behind an `a` or `mx` mechanism timed out, a domain's SPF record was
shown as not authorising this server. It is now a warning that the check
could not tell.
### Security
- Technical and security reviews of 2026-10-12 of the notifications, before
release: no Critical finding. The notification about refused mail was built
on the domain a client writes in `MAIL FROM`; it is now built on the
registry. Recorded in [security.md](docs/security.md) as accepted, with a
roadmap candidate (from-header-binding): an application can have a message
signed as another registered domain, or as the server's own name, by
writing it into `From:` - the first part of which has always been so.
## [2.2.0] - 2026-10-12 ## [2.2.0] - 2026-10-12
### Added ### Added
+3 -1
View File
@@ -35,7 +35,9 @@ func runNotifier(ctx context.Context, cfg config, st *store.Store) error {
if err != nil { if err != nil {
return dnscheck.Domain{}, err return dnscheck.Domain{}, err
} }
srv := dns.Server(cfg.hostname, true) // The server's own addresses are cached for a minute: one lookup
// for a pass over all the domains.
srv := dns.Server(cfg.hostname, false)
return dns.Domain(dnscheck.Query{ return dns.Domain(dnscheck.Query{
Name: d.Name, Name: d.Name,
Selector: d.DKIMSelector, Selector: d.DKIMSelector,
+34 -9
View File
@@ -159,10 +159,16 @@ by the panel. Socket `/run/opendkim/opendkim.sock`.
The server's own name (`SELFPOST_HOSTNAME`) has a key and a line in both The server's own name (`SELFPOST_HOSTNAME`) has a key and a line in both
tables too, without being a row of the registry tables too, without being a row of the registry
(`domain.Service.SetServerName`): the panel's notifications are sent as (`domain.Service.SetServerName`): the panel's notifications are sent as
`postmaster@` that name and signed with it. No application can send as it - `postmaster@` that name and signed with it. No application can use it as an
the sender map is built from the registry. A sending domain of the same name envelope sender - the sender map is built from the registry - but the
shares its key and selector with the server, and deleting that domain leaves `From:` header is not held against the login, so an application can still
the key in place. have a message signed as the server ([security.md](security.md) § Accepted
risks). A sending domain of the same name shares its key and selector with
the server; deleting that domain leaves the server signed under the default
selector, which is the same key only if that was the domain's. At its start
the notifier makes sure the tables have the line and name the selector the
panel shows (`EnsureServerSigned`): tables are otherwise rebuilt only when a
domain is added or deleted.
### Panel binary ([cmd/panel](../cmd/panel)) ### Panel binary ([cmd/panel](../cmd/panel))
@@ -219,13 +225,32 @@ is sent; the operator's view is guide § Notifications.
forced past the cache (`cmd/panel/notifier.go` builds the same query a forced past the cache (`cmd/panel/notifier.go` builds the same query a
domain's page does); the certificate file; `postqueue -j` for the arrival domain's page does); the certificate file; `postqueue -j` for the arrival
time of each queued message. Once a minute: the send log's `rejected` rows time of each queued message. Once a minute: the send log's `rejected` rows
of the last day, per domain and application. of the last day, through a partial index of their own.
- **A refused row is not believed whole.** Its domain is the one in
`MAIL FROM`, which the client typed and Postfix has not yet held against
the login when the milter refuses; the same status is written when an
application is refused for its client address. So refusals are counted by
the domain the login is registered under, only for logins that exist, and
only while a level-2 limit is set on the login or its domain
(`Store.LimitRefusalsSince`). One subject per sending domain, whichever of
its applications are refused.
- **State** is a row per subject in `notify_state` (`dns:<domain>`, `cert`, - **State** is a row per subject in `notify_state` (`dns:<domain>`, `cert`,
`queue`, `limit:<domain>:<login>`), which is what makes each thing be said `queue`, `limit:<domain>`), which is what makes each thing be said
once and a restart say nothing again. A DNS subject goes *pending* on the once and a restart say nothing again. A DNS subject goes *pending* on the
first bad check and *open*, with a message, on the second; back the same first bad check and *open*, with a message, on a later one at least 45
way. The certificate's row remembers which threshold was said for which minutes after it - a count alone would let the last check before a restart
expiry. A deleted domain's row goes with it. and the first after it stand for "an hour apart"; back the same way. The
certificate's row remembers which threshold was said for which expiry. A
deleted domain's row goes with it.
- **Said means sent.** A problem that arises while no user can be told, or
while Postfix does not take the message, stays unsaid and is told at a
later check. Its ending, with nobody left to tell, is simply over. If one
recipient's copy fails and another's goes out, the subject is said.
- **The notifier's own mail is not a subject.** A notification waiting for a
mailbox that does not answer, or its bounce coming back to
`postmaster@<hostname>` - which receives no mail - is left out of the
queue check.
- **A ceiling** of 60 messages an hour stands over all of it.
- **No information is not a finding.** A lookup that did not answer, a queue - **No information is not a finding.** A lookup that did not answer, a queue
that cannot be listed, a certificate that cannot be read move no state. that cannot be listed, a certificate that cannot be read move no state.
- **Recipients** come from the store at the moment of sending: global users - **Recipients** come from the store at the moment of sending: global users
+8 -3
View File
@@ -445,7 +445,7 @@ reported, each once:
| What | When a message is sent | A second message | | What | When a message is sent | A second message |
|---|---|---| |---|---|---|
| A sending domain's DKIM, SPF or DMARC record stops matching | after two hourly checks in a row found it wrong. A resolver that did not answer is not a mismatch; a missing DMARC record is advice on the domain's page, not a notification | when two checks in a row find the records right again | | A sending domain's DKIM, SPF or DMARC record stops matching | after two hourly checks in a row found it wrong. A resolver that did not answer is not a mismatch; a missing DMARC record is advice on the domain's page, not a notification | when two checks in a row find the records right again |
| A level-2 rate limit refuses mail | within a minute of the first refusal | at most once a day for the same domain and application, while it goes on | | A level-2 rate limit refuses mail of a sending domain's applications | within a minute of the first refusal | at most once a day for the same domain, while it goes on |
| The TLS certificate is about to expire | 14 days before | 3 days before, if it is still the same certificate | | The TLS certificate is about to expire | 14 days before | 3 days before, if it is still the same certificate |
| A message has waited in the queue for more than three days | at the next hourly check | when no message that old is left | | A message has waited in the queue for more than three days | at the next hourly check | when no message that old is left |
@@ -475,8 +475,13 @@ publish under **Server › Settings › Notification mail**:
card. Without them notifications are still sent, and a receiver may file them card. Without them notifications are still sent, and a receiver may file them
as spam or, if the parent domain publishes a strict DMARC policy, refuse them. as spam or, if the parent domain publishes a strict DMARC policy, refuse them.
Mail of your sending domains depends on neither. If the hostname is itself one Mail of your sending domains depends on neither. If the hostname is itself one
of your sending domains, the records are that domain's and are already of your sending domains, the DKIM record is that domain's and is already
published. published - and whoever administers that domain can then send mail that
looks like the panel's; a hostname that is nobody's sending domain is the
better choice.
The e-mail on Account may be at a public mail host (Gmail and the like); only
a domain's DMARC report address may not.
A relay that cannot send cannot say so: none of the four is "the relay is A relay that cannot send cannot say so: none of the four is "the relay is
down". Watch that from outside ([Container health](#container-health)). down". Watch that from outside ([Container health](#container-health)).
+3 -3
View File
@@ -80,7 +80,7 @@ A background check once an hour for DNS, certificate and queue.
|---|---|---| |---|---|---|
| DNS mismatch | after two checks in a row found it (it has held for an hour). A resolver that did not answer is not a mismatch | after two good checks in a row | | DNS mismatch | after two checks in a row found it (it has held for an hour). A resolver that did not answer is not a mismatch | after two good checks in a row |
| Certificate | 14 days before expiry, and again 3 days before if it is still the same certificate | none | | Certificate | 14 days before expiry, and again 3 days before if it is still the same certificate | none |
| Level-2 limit refused mail | at the first refusal for a domain or application, then at most once a day for the same limit | none | | Level-2 limit refused mail | at the first refusal of a sending domain's applications, then at most once a day for that domain (one subject per domain, as built: what the refused row says of its domain is the client's own word, so the registry is asked) | none |
| Queue older than three days | when such a message appears | one, when none is left | | Queue older than three days | when such a message appears | one, when none is left |
Event state is kept in the database, so a restart announces nothing a second Event state is kept in the database, so a restart announces nothing a second
@@ -115,5 +115,5 @@ to the owner before any code ([panel-redesign.md](panel-redesign.md)
- [x] Panel: Account, Settings, Health card, help - [x] Panel: Account, Settings, Health card, help
- [x] guide.md, architecture.md, security.md, schema-migrations.md - [x] guide.md, architecture.md, security.md, schema-migrations.md
- [x] `go vet`, `go test`, e2e: a level-2 refusal reaches a mailbox as a signed notification (on the test server: vet ok, 23 packages ok, e2e ok; "once" and "clears" are held by the unit tests of `internal/notify`) - [x] `go vet`, `go test`, e2e: a level-2 refusal reaches a mailbox as a signed notification (on the test server: vet ok, 23 packages ok, e2e ok; "once" and "clears" are held by the unit tests of `internal/notify`)
- [ ] Technical review - [x] Technical review — **Opus** (2026-10-12; no Critical. Fixed: refusals counted by the registered domain and only under a limit, with an index of their own; account e-mails may be at public mail hosts; "said" means sent; DNS checks must be 45 minutes apart; the notifier's own mail is not a queue subject and a cut or unreadable listing is no listing; the tables must name the selector the panel shows; an unanswered `a`/`mx` lookup is not an SPF mismatch; a pass stops between domains at shutdown; days are rounded as a person counts. Left as found: a recipient whose copy failed while another's went out is not retried; a certificate that lives under 14 days is reported at each renewal)
- [ ] Security review - [x] Security review — **Fable** (2026-10-12; no Critical. Fixed: the limit notification was built on the domain a client writes in `MAIL FROM` - a mail cannon and a way to write to another tenant's users; a ceiling of 60 messages an hour; [security.md](../security.md) corrected on who can submit locally and on what the account e-mail now is. Recorded as accepted, with a roadmap candidate: an application can have a message signed as the server by writing its name into `From:` - the header is not held against the login, for sending domains either)
+37 -1
View File
@@ -35,10 +35,11 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md).
| template-data-typing | Typed structs for template data instead of `map[string]any` | candidate | — | — | | template-data-typing | Typed structs for template data instead of `map[string]any` | candidate | — | — |
| structured-logging | `log/slog` with levels and fields | candidate | — | — | | structured-logging | `log/slog` with levels and fields | candidate | — | — |
| review-2026-08-followups | Remaining findings of the 2026-08-19 code review | candidate | — | — | | review-2026-08-followups | Remaining findings of the 2026-08-19 code review | candidate | — | — |
| panel-notifications | E-mail notifications about important events | in progress | 8/10 | [plans/panel-notifications.md](plans/panel-notifications.md) | | panel-notifications | E-mail notifications about important events | in progress | 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) | | 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) |
| password-reset | Password reset by e-mail | candidate | — | — | | password-reset | Password reset by e-mail | candidate | — | — |
| phone-desktop-mode | A phone browser in "desktop site" mode gets the phone navigation | candidate | — | — | | phone-desktop-mode | A phone browser in "desktop site" mode gets the phone navigation | candidate | — | — |
| from-header-binding | Hold a message's `From:` header against the application that sends it | candidate | — | — |
| panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) |
| schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) | | schema-squash | Squash SQLite migrations into a 2.x baseline | built in panel-redesign (stage 1) | — | [plans/panel-redesign.md](plans/panel-redesign.md) |
@@ -428,6 +429,41 @@ has one coherent layout, checked on a phone in "desktop site" mode and in a
--- ---
## from-header-binding
**Problem (security review of panel-notifications, 2026-10-12):** Postfix
holds the envelope sender against the SASL login
(`reject_sender_login_mismatch`), and OpenDKIM chooses the signing key by the
`From:` header. Nothing holds the header against the login. An application
of one sending domain can therefore write another registered domain - or,
since 2.3.0, the server's own name - into `From:` and have the message signed
with that name's key: mail that passes DMARC as a domain the application has
no right to, or that looks like a notification of the panel.
**Goal:** a message whose `From:` names a domain this server signs for is
accepted only from an application of that domain.
**Boundary:** the journal milter reads `From:` (and `Sender:`) and refuses at
end-of-headers when the header's domain is a signing name other than the
login's own. Mail whose `From:` is a domain this server does not sign for is
not touched - it goes out unsigned today and would after. The journal milter
is fail-open, so with it down the hole is as it is now; whether that is
enough or the check belongs somewhere stricter is part of the item.
**Done when:** the e2e stand shows an application refused for another
tenant's `From:` and for the hostname's, and accepted for its own.
**Dependencies / risks:** it refuses mail - an application that today sends
with a `From:` of another of the operator's domains stops working until it
is given an application of that domain; parsing `From:` differently from
OpenDKIM would leave a gap, so the check has to be the cruder one (does the
header mention `@<signing name>`).
**Version:** `2.x` MINOR; `candidate`. Until then:
[security.md](security.md) § Accepted risks.
---
## panel-redesign ## panel-redesign
**Goal:** a panel an operator opens a few times a year and still finds their **Goal:** a panel an operator opens a few times a year and still finds their
+6 -5
View File
@@ -17,8 +17,8 @@ When you add or review a migration, update this file in the same change.
| Field | Value | | Field | Value |
|---|---| |---|---|
| Chain head | `user_version = 4` | | Chain head | `user_version = 5` |
| Files | `0001_init.sql` (the 2.0 baseline), `0002_inbound_spam_log.sql`, `0003_send_log_lines.sql`, `0004_notifications.sql` | | Files | `0001_init.sql` (the 2.0 baseline), `0002_inbound_spam_log.sql`, `0003_send_log_lines.sql`, `0004_notifications.sql`, `0005_send_log_rejected_index.sql` |
| File mark | `PRAGMA application_id = 0x53503230` ("SP20") | | File mark | `PRAGMA application_id = 0x53503230` ("SP20") |
| Database file | `/data/selfpost.db` (bind mount) | | Database file | `/data/selfpost.db` (bind mount) |
| Compatibility | 2.x MINOR releases must boot a `2.0.0` data directory. **Nothing is carried over from 1.x.** | | Compatibility | 2.x MINOR releases must boot a `2.0.0` data directory. **Nothing is carried over from 1.x.** |
@@ -63,8 +63,8 @@ again and no file is deleted, renamed or reordered — every 2.x database has
already run it. `2.0.0` and `2.0.1` shipped with the baseline alone; the already run it. `2.0.0` and `2.0.1` shipped with the baseline alone; the
features after them add files, the first being `0002_inbound_spam_log.sql` of features after them add files, the first being `0002_inbound_spam_log.sql` of
inbound-antispam-panel (`2.1.0`), then `0003_send_log_lines.sql` of inbound-antispam-panel (`2.1.0`), then `0003_send_log_lines.sql` of
delivery-log-storage (`2.2.0`) and `0004_notifications.sql` of delivery-log-storage (`2.2.0`), and `0004_notifications.sql` and
panel-notifications (`2.3.0`). `0005_send_log_rejected_index.sql` of panel-notifications (`2.3.0`).
--- ---
@@ -76,6 +76,7 @@ panel-notifications (`2.3.0`).
| 2 | `0002_inbound_spam_log.sql` | 2.1.0 | DDL | `inbound_spam_log`, `inbound_spam_list` and two indexes; nothing existing changes | | 2 | `0002_inbound_spam_log.sql` | 2.1.0 | DDL | `inbound_spam_log`, `inbound_spam_list` and two indexes; nothing existing changes |
| 3 | `0003_send_log_lines.sql` | 2.2.0 | DDL | `send_log_lines` and one index; nothing existing changes | | 3 | `0003_send_log_lines.sql` | 2.2.0 | DDL | `send_log_lines` and one index; nothing existing changes |
| 4 | `0004_notifications.sql` | 2.3.0 | DDL | `users.notify` (default 1: every user starts with notifications on) and `notify_state` | | 4 | `0004_notifications.sql` | 2.3.0 | DDL | `users.notify` (default 1: every user starts with notifications on) and `notify_state` |
| 5 | `0005_send_log_rejected_index.sql` | 2.3.0 | DDL | `idx_send_log_rejected`, a partial index of the refused rows of `send_log` by time; a file of its own because the test installation had run 0004 before the review asked for it |
**Kind:** *DDL* — schema only; *data* — `INSERT`/`UPDATE` that must stay correct **Kind:** *DDL* — schema only; *data* — `INSERT`/`UPDATE` that must stay correct
for operators upgrading from older 2.x images. for operators upgrading from older 2.x images.
@@ -104,7 +105,7 @@ for operators upgrading from older 2.x images.
| `dmarc_report_records` | Per-source rows inside a report | | `dmarc_report_records` | Per-source rows inside a report |
| `inbound_spam_log` | Inbound antispam journal: one row per recipient of a message offered on port 25, with the decision (`accept`, `reject`, `tempfail`, `quarantine`) and who made it (`rspamd`, `postfix`, `selfpost`). Metadata only (v2) | | `inbound_spam_log` | Inbound antispam journal: one row per recipient of a message offered on port 25, with the decision (`accept`, `reject`, `tempfail`, `quarantine`) and who made it (`rspamd`, `postfix`, `selfpost`). Metadata only (v2) |
| `inbound_spam_list` | Allow / deny entries for the external filter (`address`, `domain`, `ip`), instance-wide; unique per list (v2) | | `inbound_spam_list` | Allow / deny entries for the external filter (`address`, `domain`, `ip`), instance-wide; unique per list (v2) |
| `notify_state` | What the panel has already written to its users about, one row per subject (`dns:<domain>`, `cert`, `queue`, `limit:<domain>:<login>`), so that each thing is said once and a restart says nothing again (v4) | | `notify_state` | What the panel has already written to its users about, one row per subject (`dns:<domain>`, `cert`, `queue`, `limit:<domain>`), so that each thing is said once and a restart says nothing again (v4) |
| `send_log_lines` | The `mail.log` lines Postfix wrote under a queue id, with the line's own time; what a message's page shows once the log has rotated. No reference to `send_log`: a line belongs to a row by queue id and time, and goes when no row carries its id (v3) | | `send_log_lines` | The `mail.log` lines Postfix wrote under a queue id, with the line's own time; what a message's page shows once the log has rotated. No reference to `send_log`: a line belongs to a row by queue id and time, and goes when no row carries its id (v3) |
`TestBaselineSchema` (`internal/store/schema_test.go`) pins every table, column `TestBaselineSchema` (`internal/store/schema_test.go`) pins every table, column
+41 -8
View File
@@ -421,12 +421,40 @@ deferred item from the roadmap.
DNS check's finding, and a queued message's sender address - and both are DNS check's finding, and a queued message's sender address - and both are
reduced to printable ASCII and cut before they are printed, so a reduced to printable ASCII and cut before they are printed, so a
notification's body can contain an address a stranger chose, as a few plain notification's body can contain an address a stranger chose, as a few plain
characters and nothing else. The panel's characters and nothing else. A refused-mail notification names the domain
membership in `postdrop`, described in the image as read-side, is now what and the applications from the registry, never the domain a client wrote in
lets it submit; any local user could already do that through the `MAIL FROM`: that string is not yet held against the login when the milter
world-executable `sendmail`. Notifications are signed with a key of the refuses, and the first build took it for the subject - an application over
server's own name and bypass the level-1 and level-2 limits: they are a few its limit could have made the panel write a message per made-up domain, to
messages a day at most, to addresses only a panel user can set. another tenant's users among others (security review, 2026-10-12; fixed
before release). The panel's membership in `postdrop` is what lets it
submit: under `no-new-privileges` the setgid bit of `postdrop` does not
take, so only root, `postfix` and the panel can. Notifications are signed
with a key of the server's own name and bypass the level-1 and level-2
limits. What bounds them: the subjects are the registered domains, the
certificate and the queue; a domain's DNS can be reported and cleared about
a dozen times a day at most by someone who flaps the zone; a domain's
limits once a day; and no more than 60 messages leave in any hour whatever
the rules arrive at.
- **A notification can be forged by an application, signed as the server.**
OpenDKIM chooses the key by the `From:` header, and Postfix holds only the
envelope sender against the login. An authenticated application can
therefore send a message whose header says `From: postmaster@<hostname>` and
have it signed with the key of the server's own name - a message that
passes DMARC and looks like the panel's. The same has always let an
application of one sending domain write another registered domain into
`From:` and get that domain's signature; the server's name is one more name
it works for. Accepted for now: applications are the operator's own, and a
forged notification can ask for nothing the panel acts on - it carries no
link and the panel takes no mail. The fix is to hold the `From:` header
against the login in the journal milter, which refuses mail and is a
decision of its own ([roadmap](roadmap.md) § from-header-binding). Trigger
to revisit: applications handed to anyone the operator does not trust with
every domain of the instance.
- **When the hostname is itself a sending domain,** the server's mail is
signed with that domain's key. Whoever may export the domain has the key,
and its applications may send as `postmaster@<hostname>` by right. The
guide says so; a hostname that is nobody's sending domain avoids it.
- **A message's page can show another message's log lines after a queue-id - **A message's page can show another message's log lines after a queue-id
collision.** Postfix's short queue ids repeat: the id is the microsecond of collision.** Postfix's short queue ids repeat: the id is the microsecond of
arrival and the queue file's inode, and inodes come round within seconds. arrival and the queue file's inode, and inodes come round within seconds.
@@ -454,8 +482,13 @@ deferred item from the roadmap.
field to the global role, is the fix. field to the global role, is the fix.
- **Account's profile (username, e-mail) is saved without the password.** - **Account's profile (username, e-mail) is saved without the password.**
Saving the profile does not ask for the current password, as changing the Saving the profile does not ask for the current password, as changing the
password does. Today the e-mail is only the address the *Use my e-mail* password does. Since 2.3.0 the e-mail is where notifications go, so a stolen
button fills in, so a stolen session gains nothing it did not have. It session can turn them to another address: the thief then learns what they
say - which domain's DNS is wrong, which application is refused, when the
certificate expires, sender addresses of old queued mail - and the user
stops hearing it. Nothing in a notification gives access. A `domain` user
can likewise point signed mail of this server at a third party's address,
a dozen messages a day per assigned domain at most. It
becomes a requirement to ask for the current password when changing becomes a requirement to ask for the current password when changing
`users.email` as soon as [password-reset](roadmap.md#password-reset) mails a `users.email` as soon as [password-reset](roadmap.md#password-reset) mails a
link to it — from then on whoever can change the address could take the link to it — from then on whoever can change the address could take the
+3
View File
@@ -42,6 +42,9 @@ func (f *fakeResolver) LookupTXT(_ context.Context, name string) ([]string, erro
func (f *fakeResolver) LookupIPAddr(_ context.Context, host string) ([]net.IPAddr, error) { func (f *fakeResolver) LookupIPAddr(_ context.Context, host string) ([]net.IPAddr, error) {
f.lookups++ f.lookups++
if f.fail[host] {
return nil, &net.DNSError{Err: "server misbehaving", Name: host, IsTemporary: true}
}
if v, ok := f.addr[host]; ok { if v, ok := f.addr[host]; ok {
return v, nil return v, nil
} }
+22
View File
@@ -80,3 +80,25 @@ func TestServerMailCacheIsItsOwn(t *testing.T) {
t.Error("the server's mail check was answered from the domain's cache entry") t.Error("the server's mail check was answered from the domain's cache entry")
} }
} }
// The address lookup behind an "a" mechanism did not answer: nothing was
// learned, and the record is not called wrong for it. A name that answers
// "no such host" is a finding, and the record is wrong.
func TestSPFLookupThatDidNotAnswerIsNotAMismatch(t *testing.T) {
// The fake fails by name, so the mechanism names a host of its own: the
// TXT lookup of the server's name answers, the address lookup does not.
f := &fakeResolver{
txt: map[string][]string{"mail.example.org": {"v=spf1 a:addr.example.org -all"}},
fail: map[string]bool{"addr.example.org": true},
}
c := newTestChecker(f)
q := serverMailQuery()
if got := c.ServerMail(q, true).SPF; got.Status != health.StatusWarn {
t.Errorf("the lookup did not answer: SPF is %s (%s), want a warning", got.Status, got.Detail)
}
f.fail = nil
if got := c.ServerMail(q, true).SPF; got.Status != health.StatusError {
t.Errorf("the name does not exist: SPF is %s (%s), want an error", got.Status, got.Detail)
}
}
+35 -12
View File
@@ -2,6 +2,7 @@ package dnscheck
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"net" "net"
"strings" "strings"
@@ -133,7 +134,14 @@ func (c *Checker) evaluateSPF(ctx context.Context, record, domainName string, ip
continue continue
} }
budget-- budget--
if qualifier == '+' && c.resolvesTo(ctx, target, ips) { match, failed := c.resolvesTo(ctx, target, ips)
if failed {
// Nothing was learned about this mechanism: "cannot tell",
// not "does not authorise".
note("a (the lookup did not answer)")
continue
}
if qualifier == '+' && match {
return term, unfollowed return term, unfollowed
} }
@@ -151,7 +159,12 @@ func (c *Checker) evaluateSPF(ctx context.Context, record, domainName string, ip
continue continue
} }
budget-- budget--
if qualifier == '+' && c.mxResolvesTo(ctx, target, ips, &budget) { match, failed := c.mxResolvesTo(ctx, target, ips, &budget)
if failed {
note("mx (the lookup did not answer)")
continue
}
if qualifier == '+' && match {
return term, unfollowed return term, unfollowed
} }
@@ -176,39 +189,49 @@ func (c *Checker) evaluateSPF(ctx context.Context, record, domainName string, ip
return "", unfollowed return "", unfollowed
} }
// lookupUnanswered tells a lookup that failed from one that answered "no such
// name": only the second is a finding.
func lookupUnanswered(err error) bool {
var dnsErr *net.DNSError
return err != nil && !(errors.As(err, &dnsErr) && dnsErr.IsNotFound)
}
// resolvesTo reports whether name resolves to one of the server's addresses. // resolvesTo reports whether name resolves to one of the server's addresses.
func (c *Checker) resolvesTo(ctx context.Context, name string, ips []net.IP) bool { // failed is set when the lookup did not answer, which says nothing either way.
func (c *Checker) resolvesTo(ctx context.Context, name string, ips []net.IP) (match, failed bool) {
addrs, err := c.resolver.LookupIPAddr(ctx, name) addrs, err := c.resolver.LookupIPAddr(ctx, name)
if err != nil { if err != nil {
return false return false, lookupUnanswered(err)
} }
for _, a := range addrs { for _, a := range addrs {
for _, ip := range ips { for _, ip := range ips {
if a.IP.Equal(ip) { if a.IP.Equal(ip) {
return true return true, false
} }
} }
} }
return false return false, false
} }
// mxResolvesTo reports whether any of name's MX hosts resolves to one of the // mxResolvesTo reports whether any of name's MX hosts resolves to one of the
// server's addresses, spending at most the remaining lookup budget. // server's addresses, spending at most the remaining lookup budget.
func (c *Checker) mxResolvesTo(ctx context.Context, name string, ips []net.IP, budget *int) bool { func (c *Checker) mxResolvesTo(ctx context.Context, name string, ips []net.IP, budget *int) (match, failed bool) {
mxs, err := c.resolver.LookupMX(ctx, name) mxs, err := c.resolver.LookupMX(ctx, name)
if err != nil { if err != nil {
return false return false, lookupUnanswered(err)
} }
for _, mx := range mxs { for _, mx := range mxs {
if *budget <= 0 { if *budget <= 0 {
return false return false, failed
} }
*budget-- *budget--
if c.resolvesTo(ctx, strings.TrimSuffix(mx.Host, "."), ips) { m, f := c.resolvesTo(ctx, strings.TrimSuffix(mx.Host, "."), ips)
return true if m {
return true, false
} }
failed = failed || f
} }
return false return false, failed
} }
// coversAny reports whether an ip4:/ip6: value — a bare address or a CIDR — // coversAny reports whether an ip4:/ip6: value — a bare address or a CIDR —
+17 -6
View File
@@ -163,23 +163,34 @@ func (o *OpenDKIM) Rebuild(domains []SigningDomain) error {
return o.reload() return o.reload()
} }
// signs reports whether the SigningTable on disk has a line for name. A table // signs reports whether the tables on disk sign name with the key of
// that does not exist yet signs nothing. // selector: a line for the name in the SigningTable, and in the KeyTable the
func (o *OpenDKIM) signs(name string) (bool, error) { // line that gives it that selector. Tables that do not exist yet sign nothing.
data, err := os.ReadFile(o.signingTablePath) // The selector is part of the question because the panel shows and checks the
// record of one key; tables that name another would have OpenDKIM sign with a
// key whose record nobody is told to publish.
func (o *OpenDKIM) signs(name, selector string) (bool, error) {
has := func(path, prefix string) (bool, error) {
data, err := os.ReadFile(path)
if os.IsNotExist(err) { if os.IsNotExist(err) {
return false, nil return false, nil
} }
if err != nil { if err != nil {
return false, fmt.Errorf("read SigningTable: %w", err) return false, fmt.Errorf("read %s: %w", filepath.Base(path), err)
} }
for _, line := range strings.Split(string(data), "\n") { for _, line := range strings.Split(string(data), "\n") {
if strings.HasPrefix(line, "*@"+name+" ") { if strings.HasPrefix(line, prefix) {
return true, nil return true, nil
} }
} }
return false, nil return false, nil
} }
signed, err := has(o.signingTablePath, "*@"+name+" ")
if err != nil || !signed {
return false, err
}
return has(o.keyTablePath, name+" "+name+":"+selector+":")
}
// Reload asks OpenDKIM to re-read its tables without regenerating them. It // Reload asks OpenDKIM to re-read its tables without regenerating them. It
// backs the panel's manual reload button (architecture.md § Panel HTTP // backs the panel's manual reload button (architecture.md § Panel HTTP
+25 -1
View File
@@ -126,11 +126,35 @@ func TestEnsureServerSignedRebuildsOnce(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
other.SetServerName("mail.example.org") other.SetServerName("mail.example.org")
if signed, err := otherOdk.signs("mail.example.org"); err != nil || signed { if signed, err := otherOdk.signs("mail.example.org", "selfpost"); err != nil || signed {
t.Errorf("a longer name was taken for the server's: %v, %v", signed, err) t.Errorf("a longer name was taken for the server's: %v, %v", signed, err)
} }
} }
// Tables that sign the server's name with the key of another selector - the
// default selector was changed - are rebuilt: OpenDKIM must sign with the key
// whose record the panel shows.
func TestEnsureServerSignedFollowsTheSelector(t *testing.T) {
svc, odk := newTestService(t, &fakeApps{})
svc.SetServerName("mail.example.org")
if _, err := svc.EnsureServerSigned(); err != nil {
t.Fatal(err)
}
svc.selector = "sp2027"
if rebuilt, err := svc.EnsureServerSigned(); err != nil || !rebuilt {
t.Fatalf("after the selector changed: rebuilt = %v, %v; want a rebuild", rebuilt, err)
}
kt, _ := readTables(t, odk)
if !strings.Contains(kt, "mail.example.org mail.example.org:sp2027:") || strings.Contains(kt, ":selfpost:") {
t.Errorf("KeyTable after the rebuild:\n%s", kt)
}
mail, err := svc.ServerMail()
if err != nil || mail.Selector != "sp2027" {
t.Errorf("the panel shows selector %q (%v), the tables sign with sp2027", mail.Selector, err)
}
}
// Without a hostname nothing is added and nothing is signed for it. // Without a hostname nothing is added and nothing is signed for it.
func TestNoServerName(t *testing.T) { func TestNoServerName(t *testing.T) {
svc, odk := newTestService(t, &fakeApps{}) svc, odk := newTestService(t, &fakeApps{})
+12 -4
View File
@@ -87,12 +87,18 @@ func (s *Service) ServerMail() (ServerMail, error) {
// server's own name, rebuilding them if they do not. The tables are otherwise // server's own name, rebuilding them if they do not. The tables are otherwise
// rebuilt only when a domain is added or deleted, so an installation that was // rebuilt only when a domain is added or deleted, so an installation that was
// upgraded to a version with notifications would go on sending them unsigned // upgraded to a version with notifications would go on sending them unsigned
// until then. It reports whether it had to rebuild. // until then, and one whose DKIM_SELECTOR_DEFAULT was changed would go on
// signing with the key of the old selector while the panel shows the record
// of the new. It reports whether it had to rebuild.
func (s *Service) EnsureServerSigned() (bool, error) { func (s *Service) EnsureServerSigned() (bool, error) {
if s.serverName == "" { if s.serverName == "" {
return false, nil return false, nil
} }
signed, err := s.odk.signs(s.serverName) selector, err := s.serverSelector()
if err != nil {
return false, err
}
signed, err := s.odk.signs(s.serverName, selector)
if err != nil { if err != nil {
return false, err return false, err
} }
@@ -203,8 +209,10 @@ func (s *Service) Delete(id int64) error {
return fmt.Errorf("rebuild sender map after deleting %s: %w", d.Name, err) return fmt.Errorf("rebuild sender map after deleting %s: %w", d.Name, err)
} }
// A sending domain named like the server shared its key with the server's // A sending domain named like the server shared its key with the server's
// own mail, which goes on being signed with it (resync above kept the name // own mail. The resync above kept the name in the tables under the default
// in the tables). // selector: the same key if that was the domain's selector, a new one -
// with a new record to publish, which Settings shows - if it was not.
// Either way the directory holds the key now in use.
if d.Name == s.serverName { if d.Name == s.serverName {
return nil return nil
} }
+5
View File
@@ -42,6 +42,8 @@ func (m *Mailer) Send(to, subject, body string) error {
// a header and onto a command line all the same, so what cannot be part of // a header and onto a command line all the same, so what cannot be part of
// an address is refused here as well (security.md). // an address is refused here as well (security.md).
if to == "" || strings.ContainsAny(to, " \t\r\n<>,;\"") || strings.HasPrefix(to, "-") || !strings.Contains(to, "@") { if to == "" || strings.ContainsAny(to, " \t\r\n<>,;\"") || strings.HasPrefix(to, "-") || !strings.Contains(to, "@") {
// (validate.AccountEmail refuses the same, so a user is told at the
// form and not here, once a minute, in the log.)
return fmt.Errorf("not an address a notification can go to: %q", to) return fmt.Errorf("not an address a notification can go to: %q", to)
} }
from := From(m.hostname) from := From(m.hostname)
@@ -88,6 +90,9 @@ func sendmail(from, to string, msg []byte) error {
defer cancel() defer cancel()
cmd := exec.CommandContext(ctx, sendmailPath, "-i", "-f", from, "--", to) cmd := exec.CommandContext(ctx, sendmailPath, "-i", "-f", from, "--", to)
cmd.Stdin = bytes.NewReader(msg) cmd.Stdin = bytes.NewReader(msg)
// sendmail hands the message to postdrop; if that child outlives the
// timeout holding the output pipe, do not wait for it for ever.
cmd.WaitDelay = 5 * time.Second
out, err := cmd.CombinedOutput() out, err := cmd.CombinedOutput()
if ctx.Err() == context.DeadlineExceeded { if ctx.Err() == context.DeadlineExceeded {
return fmt.Errorf("sendmail timed out after %s", sendmailTimeout) return fmt.Errorf("sendmail timed out after %s", sendmailTimeout)
+125 -40
View File
@@ -1,6 +1,6 @@
// Package notify writes to the panel's users when something needs them // Package notify writes to the panel's users when something needs them
// (architecture.md § Notifications): a sending domain's DNS record stops // (architecture.md § Notifications): a sending domain's DNS record does not
// matching, a level-2 rate limit refuses mail, the TLS certificate is about to // match, a level-2 rate limit refuses mail, the TLS certificate is about to
// expire, a message has waited in the queue for more than three days. // expire, a message has waited in the queue for more than three days.
// //
// Each subject has a row of state in the database, so a thing is said once and // Each subject has a row of state in the database, so a thing is said once and
@@ -12,7 +12,6 @@ package notify
import ( import (
"context" "context"
"fmt"
"log" "log"
"sort" "sort"
"strings" "strings"
@@ -26,17 +25,22 @@ import (
const ( const (
// CheckInterval is how often DNS, the certificate and the queue are looked // CheckInterval is how often DNS, the certificate and the queue are looked
// at. A var-free constant: the rules below count in checks. // at.
CheckInterval = time.Hour CheckInterval = time.Hour
// LimitInterval is how often refused mail is looked for. A refusal is // LimitInterval is how often refused mail is looked for. A refusal is
// reported within the minute rather than on the hour: it is mail not being // reported within the minute rather than on the hour: it is mail not being
// sent now. The look is one indexed query. // sent now. The look reads the refused rows through an index of their own
// (migration 0005).
LimitInterval = time.Minute LimitInterval = time.Minute
// dnsChecks is how many checks in a row must agree before a DNS mismatch is // dnsChecks is how many checks in a row must agree before a DNS mismatch is
// reported, and before it is reported gone. One check is a resolver's bad // reported, and before it is reported gone; dnsApart is how far apart the
// minute; two, an hour apart, is a record that is wrong. // first and the last of them must be. One check is a resolver's bad minute;
// two, an hour apart, is a record that is wrong. The time is asked for as
// well as the count because the first check after a start is minutes after
// the last one before it.
dnsChecks = 2 dnsChecks = 2
dnsApart = 45 * time.Minute
// QueueAge is how long a message waits before the queue is reported. Postfix // QueueAge is how long a message waits before the queue is reported. Postfix
// keeps a message for five days by default, so two are left to act in. // keeps a message for five days by default, so two are left to act in.
@@ -47,10 +51,16 @@ const (
certFirstDays = 14 certFirstDays = 14
certLastDays = 3 certLastDays = 3
// limitRepeat is the shortest time between two messages about the same // limitRepeat is the shortest time between two messages about the limits of
// level-2 limit. // the same domain.
limitRepeat = 24 * time.Hour limitRepeat = 24 * time.Hour
// hourlyCeiling is the most messages handed to Postfix in an hour, whatever
// the rules above arrive at. They bypass every rate limit, and the number of
// subjects grows with the number of domains; nothing here should ever come
// near it, and a fault that does is stopped by it.
hourlyCeiling = 60
phasePending = "pending" // seen wrong, not yet said phasePending = "pending" // seen wrong, not yet said
phaseOpen = "open" // said phaseOpen = "open" // said
) )
@@ -90,17 +100,26 @@ type Notifier struct {
sender Sender sender Sender
hostname string hostname string
now func() time.Time now func() time.Time
// ctx ends a pass between two domains when the process is asked to stop.
ctx context.Context
// sentAt holds the times of the messages of the last hour, for the ceiling.
sentAt []time.Time
} }
// New builds a notifier for the server named hostname. // New builds a notifier for the server named hostname.
func New(st Store, checks Checks, sender Sender, hostname string) *Notifier { func New(st Store, checks Checks, sender Sender, hostname string) *Notifier {
return &Notifier{st: st, checks: checks, sender: sender, hostname: hostname, now: func() time.Time { return time.Now().UTC() }} return &Notifier{
st: st, checks: checks, sender: sender, hostname: hostname,
now: func() time.Time { return time.Now().UTC() },
ctx: context.Background(),
}
} }
// Run checks on both cadences until ctx is cancelled. The first hourly check // Run checks on both cadences until ctx is cancelled. The first hourly check
// runs a few minutes after the start, not at it: a container that has just // runs a few minutes after the start, not at it: a container that has just
// come up is still starting Postfix and may have no network yet. // come up is still starting Postfix and may have no network yet.
func (n *Notifier) Run(ctx context.Context) error { func (n *Notifier) Run(ctx context.Context) error {
n.ctx = ctx
first := time.NewTimer(3 * time.Minute) first := time.NewTimer(3 * time.Minute)
defer first.Stop() defer first.Stop()
hourly := time.NewTicker(CheckInterval) hourly := time.NewTicker(CheckInterval)
@@ -144,31 +163,66 @@ func (n *Notifier) Check() {
n.checkQueue() n.checkQueue()
} }
// outcome is what became of a message.
type outcome int
const (
// sent: at least one recipient's copy was handed to Postfix.
sent outcome = iota
// nobody: no user is to be told - none with an e-mail and the switch on.
nobody
// failed: there were recipients and none could be handed a copy.
failed
)
// send writes one message to every recipient of a subject, each in a message // send writes one message to every recipient of a subject, each in a message
// of their own - users are not shown each other's addresses. It reports // of their own - users are not shown each other's addresses.
// whether the subject counts as said: nobody to tell is said, a message that //
// could not be handed over to anyone is not, and is tried at the next check. // What the caller makes of the outcome is the rule that keeps a thing from
func (n *Notifier) send(domainName string, m message) bool { // being said never: a problem counts as said only when it was sent, so one
// that arose while nobody could be told, or while the mail system was down, is
// told at a later check. A recipient whose copy failed while another's went
// out is not tried again: the subject is said.
func (n *Notifier) send(domainName string, m message) outcome {
to, err := n.st.NotifyRecipients(domainName) to, err := n.st.NotifyRecipients(domainName)
if err != nil { if err != nil {
log.Printf("notify: recipients: %v", err) log.Printf("notify: recipients: %v", err)
return false return failed
} }
if len(to) == 0 { if len(to) == 0 {
return true return nobody
} }
sent := 0 handed := 0
for _, addr := range to { for _, addr := range to {
if !n.underCeiling() {
log.Printf("notify: more than %d messages in an hour; %q to %s is held back", hourlyCeiling, m.subject, addr)
continue
}
if err := n.sender.Send(addr, m.subject, m.body); err != nil { if err := n.sender.Send(addr, m.subject, m.body); err != nil {
log.Printf("notify: send %q to %s: %v", m.subject, addr, err) log.Printf("notify: send %q to %s: %v", m.subject, addr, err)
continue continue
} }
sent++ n.sentAt = append(n.sentAt, n.now())
handed++
} }
if sent > 0 { if handed == 0 {
log.Printf("notify: %q sent to %d of %d", m.subject, sent, len(to)) return failed
} }
return sent > 0 log.Printf("notify: %q sent to %d of %d", m.subject, handed, len(to))
return sent
}
// underCeiling reports whether one more message may go out this hour.
func (n *Notifier) underCeiling() bool {
cutoff := n.now().Add(-time.Hour)
keep := n.sentAt[:0]
for _, t := range n.sentAt {
if t.After(cutoff) {
keep = append(keep, t)
}
}
n.sentAt = keep
return len(n.sentAt) < hourlyCeiling
} }
func (n *Notifier) save(st store.NotifyState) { func (n *Notifier) save(st store.NotifyState) {
@@ -225,6 +279,13 @@ func (n *Notifier) checkDNS() {
names := make(map[string]bool, len(domains)) names := make(map[string]bool, len(domains))
for _, d := range domains { for _, d := range domains {
names[d.Name] = true names[d.Name] = true
}
for _, d := range domains {
// A dead resolver costs seconds per domain; a process asked to stop
// does not wait for the rest of them.
if n.ctx.Err() != nil {
return
}
res, err := n.checks.Domain(d) res, err := n.checks.Domain(d)
if err != nil { if err != nil {
log.Printf("notify: check %s: %v", d.Name, err) log.Printf("notify: check %s: %v", d.Name, err)
@@ -246,6 +307,10 @@ func (n *Notifier) checkDNS() {
} }
// dnsStep moves one domain's state by one check. // dnsStep moves one domain's state by one check.
//
// A subject is "pending" from the first bad check, with Since the time of it,
// and "open" from the message. While open, Count is the good checks in a row
// and Mark the time of the first of them.
func (n *Notifier) dnsStep(name string, res dnscheck.Domain) { func (n *Notifier) dnsStep(name string, res dnscheck.Domain) {
faults, known := dnsReading(res) faults, known := dnsReading(res)
if !known { if !known {
@@ -269,23 +334,27 @@ func (n *Notifier) dnsStep(name string, res dnscheck.Domain) {
n.forget(key) // it was the resolver's bad minute n.forget(key) // it was the resolver's bad minute
case st.Phase == phasePending && bad: case st.Phase == phasePending && bad:
st.Count++ st.Count++
if st.Count >= dnsChecks { if st.Count >= dnsChecks && now.Sub(st.Since) >= dnsApart {
if !n.send(name, dnsProblem(n.hostname, name, faults, st.Since)) { if n.send(name, dnsProblem(n.hostname, name, faults, st.Since)) == sent {
n.save(st) st.Phase, st.Count, st.Mark, st.SentAt = phaseOpen, 0, "", now
return
} }
st.Phase, st.Count, st.SentAt = phaseOpen, 0, now
} }
n.save(st) n.save(st)
case st.Phase == phaseOpen && bad: case st.Phase == phaseOpen && bad:
if st.Count != 0 { if st.Count != 0 {
st.Count = 0 // it went right for a check and wrong again: still the same trouble // It went right for a check and wrong again: still the same trouble.
st.Count, st.Mark = 0, ""
n.save(st) n.save(st)
} }
case st.Phase == phaseOpen && !bad: case st.Phase == phaseOpen && !bad:
st.Count++ st.Count++
if st.Count >= dnsChecks { if st.Count == 1 {
if n.send(name, dnsCleared(n.hostname, name, st.Since)) { st.Mark = now.Format(time.RFC3339)
}
firstGood, _ := time.Parse(time.RFC3339, st.Mark)
if st.Count >= dnsChecks && now.Sub(firstGood) >= dnsApart {
// With nobody to tell any more, the trouble is over all the same.
if n.send(name, dnsCleared(n.hostname, name, st.Since)) != failed {
n.forget(key) n.forget(key)
return return
} }
@@ -325,7 +394,7 @@ func (n *Notifier) checkCertificate() {
if level == "" || (have && (st.Phase == level || st.Phase == "last")) { if level == "" || (have && (st.Phase == level || st.Phase == "last")) {
return return
} }
if !n.send("", certExpiring(n.hostname, c.NotAfter, left)) { if n.send("", certExpiring(n.hostname, c.NotAfter, left)) != sent {
return return
} }
n.save(store.NotifyState{Key: key, Phase: level, Mark: mark, Since: n.now(), SentAt: n.now()}) n.save(store.NotifyState{Key: key, Phase: level, Mark: mark, Since: n.now(), SentAt: n.now()})
@@ -333,6 +402,24 @@ func (n *Notifier) checkCertificate() {
// ---- queue // ---- queue
// ownMail reports whether a queued message is the notifier's: a notification
// that could not be delivered yet, or the bounce of one coming back to
// postmaster@ this server, which receives no mail. Neither is the mail the
// queue is watched for, and reporting them would have one dead mailbox
// announce itself every three days.
func (n *Notifier) ownMail(m postfix.QueuedMessage) bool {
own := From(n.hostname)
if strings.EqualFold(m.Sender, own) {
return true
}
for _, rcpt := range m.Recipients {
if strings.EqualFold(rcpt, own) {
return true
}
}
return false
}
func (n *Notifier) checkQueue() { func (n *Notifier) checkQueue() {
msgs, err := n.checks.Queue() msgs, err := n.checks.Queue()
if err != nil { if err != nil {
@@ -342,7 +429,7 @@ func (n *Notifier) checkQueue() {
now := n.now() now := n.now()
var old []postfix.QueuedMessage var old []postfix.QueuedMessage
for _, m := range msgs { for _, m := range msgs {
if now.Sub(m.Arrival) > QueueAge { if now.Sub(m.Arrival) > QueueAge && !n.ownMail(m) {
old = append(old, m) old = append(old, m)
} }
} }
@@ -356,11 +443,11 @@ func (n *Notifier) checkQueue() {
} }
switch { switch {
case len(old) > 0 && !have: case len(old) > 0 && !have:
if n.send("", queueStuck(n.hostname, old, now)) { if n.send("", queueStuck(n.hostname, old, now)) == sent {
n.save(store.NotifyState{Key: key, Phase: phaseOpen, Since: now, SentAt: now}) n.save(store.NotifyState{Key: key, Phase: phaseOpen, Since: now, SentAt: now})
} }
case len(old) == 0 && have: case len(old) == 0 && have:
if n.send("", queueCleared(n.hostname, st.Since)) { if n.send("", queueCleared(n.hostname, st.Since)) != failed {
n.forget(key) n.forget(key)
} }
} }
@@ -369,7 +456,8 @@ func (n *Notifier) checkQueue() {
// ---- level-2 limits // ---- level-2 limits
// CheckLimits looks for mail refused under a level-2 rate limit and reports // CheckLimits looks for mail refused under a level-2 rate limit and reports
// each limit that refused, at most once in limitRepeat. // each sending domain whose applications were refused, at most once in
// limitRepeat.
func (n *Notifier) CheckLimits() { func (n *Notifier) CheckLimits() {
if !n.enabled() { if !n.enabled() {
return return
@@ -381,10 +469,7 @@ func (n *Notifier) CheckLimits() {
return return
} }
for _, r := range refusals { for _, r := range refusals {
if r.Domain == "" { key := "limit:" + r.Domain
continue
}
key := fmt.Sprintf("limit:%s:%s", r.Domain, r.AppLogin)
st, have, err := n.st.GetNotifyState(key) st, have, err := n.st.GetNotifyState(key)
if err != nil { if err != nil {
log.Printf("notify: %v", err) log.Printf("notify: %v", err)
@@ -396,12 +481,12 @@ func (n *Notifier) CheckLimits() {
if have && now.Sub(st.SentAt) < limitRepeat { if have && now.Sub(st.SentAt) < limitRepeat {
continue continue
} }
if n.send(r.Domain, limitRefused(n.hostname, r)) { if n.send(r.Domain, limitRefused(n.hostname, r)) == sent {
n.save(store.NotifyState{Key: key, Phase: phaseOpen, Since: r.First, SentAt: now}) n.save(store.NotifyState{Key: key, Phase: phaseOpen, Since: r.First, SentAt: now})
} }
} }
// A limit nobody has hit for two days is not a subject any more. // A domain nobody was refused under for two days is not a subject any more.
keys, err := n.st.ListNotifyStateKeys("limit:") keys, err := n.st.ListNotifyStateKeys("limit:")
if err != nil { if err != nil {
log.Printf("notify: list limit state: %v", err) log.Printf("notify: list limit state: %v", err)
+173 -15
View File
@@ -131,7 +131,7 @@ func TestDNSMismatchIsSaidOnceAndClearedOnce(t *testing.T) {
b.dns["example.com"] = badDKIM() b.dns["example.com"] = badDKIM()
want(t, "first bad check", b.hour()) want(t, "first bad check", b.hour())
problem := "[SelfPost] example.com: DKIM no longer matches" problem := "[SelfPost] example.com: DKIM does not match"
want(t, "second bad check", b.hour(), "mine@example.org|"+problem, "root@example.org|"+problem) want(t, "second bad check", b.hour(), "mine@example.org|"+problem, "root@example.org|"+problem)
want(t, "third bad check", b.hour()) want(t, "third bad check", b.hour())
@@ -279,13 +279,30 @@ func TestQueueIsSaidOnceAndClearedOnce(t *testing.T) {
want(t, "after", b.hour()) want(t, "after", b.hour())
} }
// A limit that refuses mail is said to the domain's users, at most once a day. // A limit that refuses mail is said to the domain's users, at most once a day
// for the domain, whichever of its applications are refused.
func TestLimitRefusalIsSaidOnceADay(t *testing.T) { func TestLimitRefusalIsSaidOnceADay(t *testing.T) {
b := newBench(t) b := newBench(t)
b.now = time.Now().UTC() // refusals are dated by the store's clock b.now = time.Now().UTC() // refusals are dated by the store's clock
refuse := func() { domains, _ := b.st.ListDomains()
var domainID int64
for _, d := range domains {
if d.Name == "example.com" {
domainID = d.ID
}
}
for _, login := range []string{"app1", "app2"} {
a, err := b.st.AddApplication(domainID, login, store.AddressModeWildcard, nil)
if err != nil {
t.Fatal(err)
}
if err := b.st.SetRateLimit(store.RateLimit{Scope: store.RateLimitScopeApp, RefID: a.ID, MaxMessages: 1, WindowSeconds: 3600}); err != nil {
t.Fatal(err)
}
}
refuse := func(login, mailFromDomain string) {
t.Helper() t.Helper()
if err := b.st.InsertRejected(store.SendLogEntry{Domain: "example.com", AppLogin: "app1", From: "a@example.com"}); err != nil { if err := b.st.InsertRejected(store.SendLogEntry{Domain: mailFromDomain, AppLogin: login, From: "a@" + mailFromDomain}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
@@ -293,20 +310,28 @@ func TestLimitRefusalIsSaidOnceADay(t *testing.T) {
b.n.CheckLimits() b.n.CheckLimits()
want(t, "nothing refused", b.out.take()) want(t, "nothing refused", b.out.take())
refuse() refuse("app1", "example.com")
b.n.CheckLimits() b.n.CheckLimits()
subject := "[SelfPost] example.com: a rate limit is refusing mail" subject := "[SelfPost] example.com: a rate limit is refusing mail"
want(t, "first refusal", b.out.take(), "mine@example.org|"+subject, "root@example.org|"+subject) want(t, "first refusal", b.out.take(), "mine@example.org|"+subject, "root@example.org|"+subject)
refuse() // The second application of the domain is refused, and the first goes on
// under domains it makes up in MAIL FROM - another tenant's among them.
// It is all the same subject: example.com, told today.
refuse("app2", "example.com")
refuse("app1", "other.example")
refuse("app1", "made-up.example")
b.now = b.now.Add(2 * time.Hour) b.now = b.now.Add(2 * time.Hour)
b.n.CheckLimits() b.n.CheckLimits()
want(t, "more of it two hours later", b.out.take()) want(t, "more of it two hours later", b.out.take())
if keys, _ := b.st.ListNotifyStateKeys("limit:"); strings.Join(keys, " ") != "limit:example.com" {
t.Errorf("limit subjects = %v, want the one registered domain", keys)
}
// A day after it was said, mail is still being refused: said again. The // A day after it was said, mail is still being refused: said again. The
// refusals are dated by the store's clock, so it is the telling that is // refusals are dated by the store's clock, so it is the telling that is
// moved back, not the clock forward. // moved back, not the clock forward.
st, have, err := b.st.GetNotifyState("limit:example.com:app1") st, have, err := b.st.GetNotifyState("limit:example.com")
if err != nil || !have { if err != nil || !have {
t.Fatalf("no state for the limit: %v, %v", have, err) t.Fatalf("no state for the limit: %v, %v", have, err)
} }
@@ -316,27 +341,160 @@ func TestLimitRefusalIsSaidOnceADay(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
b.n.CheckLimits() b.n.CheckLimits()
want(t, "a day later, still refusing", b.out.take(), "mine@example.org|"+subject, "root@example.org|"+subject) if len(b.out.sent) != 2 || !strings.Contains(b.out.body[0], "4 messages from the applications app1, app2 of example.com") {
if body := limitRefused("mail.example.org", store.LimitRefusal{Domain: "example.com", AppLogin: "app1", Count: 3, First: b.now}).body; !strings.Contains(body, "3 messages from the application app1 of example.com") { t.Fatalf("a day later, still refusing: %v\n%s", b.out.sent, strings.Join(b.out.body, "\n---\n"))
t.Errorf("the message does not say who was refused how much:\n%s", body)
} }
b.out.take()
// Nobody has hit the limit for two days: it is not a subject any more. // Nobody was refused for two days: it is not a subject any more.
st.SentAt = b.now.Add(-49 * time.Hour) st.SentAt = b.now.Add(-49 * time.Hour)
if err := b.st.SetNotifyState(st); err != nil { if err := b.st.SetNotifyState(st); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := b.st.LimitRefusalsSince(b.now); err != nil {
t.Fatal(err)
}
b.now = b.now.Add(25 * time.Hour) // past the window the refusals lie in b.now = b.now.Add(25 * time.Hour) // past the window the refusals lie in
b.n.CheckLimits() b.n.CheckLimits()
want(t, "long quiet", b.out.take()) want(t, "long quiet", b.out.take())
if _, have, _ := b.st.GetNotifyState("limit:example.com:app1"); have { if _, have, _ := b.st.GetNotifyState("limit:example.com"); have {
t.Error("a limit nobody hits kept its state") t.Error("a limit nobody hits kept its state")
} }
} }
// Two checks minutes apart - the last before a restart and the first after it
// - are not "an hour apart": the mismatch is said when a check finds it again
// with the time passed.
func TestDNSChecksMustBeApart(t *testing.T) {
b := newBench(t)
b.dns["example.com"] = badDKIM()
b.n.Check()
b.now = b.now.Add(3 * time.Minute) // the process restarted
b.n.Check()
want(t, "two checks three minutes apart", b.out.take())
b.now = b.now.Add(time.Hour)
b.n.Check()
if got := b.out.take(); len(got) != 2 {
t.Fatalf("an hour later: %v, want the two messages", got)
}
// The same for its going: two good checks minutes apart do not end it.
b.dns["example.com"] = good()
b.now = b.now.Add(time.Hour)
b.n.Check()
b.now = b.now.Add(3 * time.Minute)
b.n.Check()
want(t, "two good checks three minutes apart", b.out.take())
b.now = b.now.Add(time.Hour)
b.n.Check()
if got := b.out.take(); len(got) != 2 || !strings.Contains(got[0], "match again") {
t.Errorf("an hour later: %v, want the two messages that it is over", got)
}
}
// What arises while there is nobody to tell is told when there is somebody:
// a thing is said only when it was sent. Its going, with nobody left to tell,
// is simply over.
func TestNobodyToTellIsNotSaid(t *testing.T) {
b := newBench(t)
users, _ := b.st.ListUsers()
for _, u := range users {
if err := b.st.SetUserNotify(u.ID, false); err != nil {
t.Fatal(err)
}
}
expiry := b.now.Add(10 * 24 * time.Hour)
b.cert = health.Certificate{NotAfter: expiry}
b.dns["example.com"] = badDKIM()
b.q = []postfix.QueuedMessage{{ID: "OLD00001", Sender: "app@example.com", Arrival: b.now.Add(-5 * 24 * time.Hour)}}
want(t, "nobody listens", b.hour())
want(t, "nobody listens", b.hour())
for _, u := range users {
if u.Username == "root" {
if err := b.st.SetUserNotify(u.ID, true); err != nil {
t.Fatal(err)
}
}
}
got := b.hour()
if len(got) != 3 {
t.Fatalf("somebody listens now: %v, want the DNS mismatch, the certificate and the queue", got)
}
// Everything is put right while nobody listens again: no state is left to
// announce an ending later.
for _, u := range users {
if err := b.st.SetUserNotify(u.ID, false); err != nil {
t.Fatal(err)
}
}
b.dns["example.com"] = good()
b.q = nil
want(t, "put right", b.hour())
want(t, "put right", b.hour())
for _, key := range []string{"dns:example.com", "queue"} {
if _, have, _ := b.st.GetNotifyState(key); have {
t.Errorf("state %s left behind", key)
}
}
}
// The notifier's own mail in the queue - a notification to a mailbox that does
// not answer, or its bounce coming back to this server's postmaster - is not
// the mail the queue is watched for.
func TestQueueLeavesOutTheNotifiersOwnMail(t *testing.T) {
b := newBench(t)
old := b.now.Add(-4 * 24 * time.Hour)
b.q = []postfix.QueuedMessage{
{ID: "NOTE0001", Sender: "postmaster@mail.example.org", Recipients: []string{"gone@example.net"}, Arrival: old},
{ID: "BOUNCE01", Sender: "", Recipients: []string{"Postmaster@Mail.Example.Org"}, Arrival: old},
}
want(t, "only its own mail is old", b.hour())
b.q = append(b.q, postfix.QueuedMessage{ID: "APP00001", Sender: "", Recipients: []string{"app@example.com"}, Arrival: old})
if got := b.hour(); len(got) != 1 {
t.Errorf("an application's bounce is old too: %v, want one message", got)
}
}
// Whatever the rules arrive at, no more than the ceiling leaves in an hour:
// these messages bypass every rate limit.
func TestHourlyCeiling(t *testing.T) {
b := newBench(t)
for i := 0; i < hourlyCeiling+5; i++ {
b.n.send("", message{subject: "s", body: "b"})
}
if got := len(b.out.take()); got != hourlyCeiling {
t.Errorf("%d messages left in one hour, ceiling is %d", got, hourlyCeiling)
}
if b.n.send("", message{subject: "s", body: "b"}) != failed {
t.Error("a message over the ceiling was counted as said")
}
b.now = b.now.Add(61 * time.Minute)
if b.n.send("", message{subject: "s", body: "b"}) != sent {
t.Error("the ceiling did not lift after an hour")
}
}
// The subject of a certificate message counts in days as a person would:
// found an hour past the threshold it still has "14 days".
func TestCertificateSubjectRoundsDays(t *testing.T) {
for _, c := range []struct {
left time.Duration
want string
}{
{14*24*time.Hour - time.Hour, "expires in 14 days"},
{3*24*time.Hour - 20*time.Minute, "expires in 3 days"},
{30 * time.Hour, "expires in 1 day"},
{-time.Hour, "has expired"},
} {
m := certExpiring("mail.example.org", time.Date(2026, 11, 1, 0, 0, 0, 0, time.UTC), c.left)
if !strings.Contains(m.subject, c.want) {
t.Errorf("%v left: subject %q, want %q", c.left, m.subject, c.want)
}
}
}
func TestMailerComposesAndRefuses(t *testing.T) { func TestMailerComposesAndRefuses(t *testing.T) {
var from, to string var from, to string
var msg []byte var msg []byte
+11 -9
View File
@@ -53,9 +53,11 @@ func plain(s string, max int) string {
return string(b) return string(b)
} }
// days words a duration in whole days, the unit these messages count in. // days words a duration in days, the unit these messages count in, rounded
// to the nearest: a certificate found thirteen days and twenty-three hours
// from its end has fourteen days, not thirteen.
func days(d time.Duration) string { func days(d time.Duration) string {
n := int(d.Hours() / 24) n := int((d + 12*time.Hour) / (24 * time.Hour))
if n == 1 { if n == 1 {
return "1 day" return "1 day"
} }
@@ -70,8 +72,8 @@ func dnsProblem(hostname, domain string, faults []dnsFault, since time.Time) mes
lines[i] = f.Record + ": " + plain(f.Detail, 600) lines[i] = f.Record + ": " + plain(f.Detail, 600)
} }
return compose(hostname, return compose(hostname,
fmt.Sprintf("%s: %s no longer matches", domain, strings.Join(names, ", ")), fmt.Sprintf("%s: %s does not match", domain, strings.Join(names, ", ")),
fmt.Sprintf("The DNS records of %s do not match what this server sends with. Two checks an hour apart found the same, the first at %s.", domain, stamp(since)), fmt.Sprintf("The DNS records of %s do not match what this server sends with. Checks about an hour apart found the same, the first at %s.", domain, stamp(since)),
strings.Join(lines, "\n\n"), strings.Join(lines, "\n\n"),
fmt.Sprintf("Until the records are right, receivers may refuse mail from %s or file it as spam. The records to publish are in the panel under Outbound > Domains > %s.", domain, domain), fmt.Sprintf("Until the records are right, receivers may refuse mail from %s or file it as spam. The records to publish are in the panel under Outbound > Domains > %s.", domain, domain),
"One more message follows when the records match again.", "One more message follows when the records match again.",
@@ -81,7 +83,7 @@ func dnsProblem(hostname, domain string, faults []dnsFault, since time.Time) mes
func dnsCleared(hostname, domain string, since time.Time) message { func dnsCleared(hostname, domain string, since time.Time) message {
return compose(hostname, return compose(hostname,
fmt.Sprintf("%s: DNS records match again", domain), fmt.Sprintf("%s: DNS records match again", domain),
fmt.Sprintf("The DNS records of %s match what this server sends with again; two checks an hour apart agree. They had not since %s.", domain, stamp(since)), fmt.Sprintf("The DNS records of %s match what this server sends with again; checks about an hour apart agree. They had not since %s.", domain, stamp(since)),
) )
} }
@@ -132,9 +134,9 @@ func queueCleared(hostname string, since time.Time) message {
} }
func limitRefused(hostname string, r store.LimitRefusal) message { func limitRefused(hostname string, r store.LimitRefusal) message {
who := "the domain " + r.Domain who := fmt.Sprintf("the application %s of %s", strings.Join(r.Apps, ", "), r.Domain)
if r.AppLogin != "" { if len(r.Apps) > 1 {
who = fmt.Sprintf("the application %s of %s", r.AppLogin, r.Domain) who = fmt.Sprintf("the applications %s of %s", strings.Join(r.Apps, ", "), r.Domain)
} }
count := "1 message" count := "1 message"
if r.Count != 1 { if r.Count != 1 {
@@ -144,6 +146,6 @@ func limitRefused(hostname string, r store.LimitRefusal) message {
fmt.Sprintf("%s: a rate limit is refusing mail", r.Domain), fmt.Sprintf("%s: a rate limit is refusing mail", r.Domain),
fmt.Sprintf("In the last 24 hours this server refused %s from %s under a level-2 rate limit, the first at %s. Refused mail is not queued: the application has to send it again.", count, who, stamp(r.First)), fmt.Sprintf("In the last 24 hours this server refused %s from %s under a level-2 rate limit, the first at %s. Refused mail is not queued: the application has to send it again.", count, who, stamp(r.First)),
fmt.Sprintf("If the volume is expected, raise the limit in the panel: Outbound > Domains > %s, on the domain or on the application. If it is not, the application is sending more than it should.", r.Domain), fmt.Sprintf("If the volume is expected, raise the limit in the panel: Outbound > Domains > %s, on the domain or on the application. If it is not, the application is sending more than it should.", r.Domain),
"This limit is reported at most once a day.", "Refusals of this domain's mail are reported at most once a day.",
) )
} }
+30 -10
View File
@@ -16,15 +16,21 @@ type QueuedMessage struct {
ID string ID string
Queue string // "deferred", "active", "hold", "incoming", "maildrop" Queue string // "deferred", "active", "hold", "incoming", "maildrop"
Sender string Sender string
Recipients []string
Arrival time.Time Arrival time.Time
} }
// queueJSONTimeout is longer than the listing for a page gets: the notifier
// asks once an hour and can wait, and a queue large enough to be slow to list
// is the one that must not go unreported.
const queueJSONTimeout = 30 * time.Second
// readQueueJSON runs `postqueue -j`. It is a var so tests can answer without a // readQueueJSON runs `postqueue -j`. It is a var so tests can answer without a
// running Postfix, the way readRetryConf is. // running Postfix, the way readRetryConf is.
var readQueueJSON = postqueueJSON var readQueueJSON = postqueueJSON
func postqueueJSON() ([]byte, error) { func postqueueJSON() ([]byte, error) {
ctx, cancel := context.WithTimeout(context.Background(), postqueueTimeout) ctx, cancel := context.WithTimeout(context.Background(), queueJSONTimeout)
defer cancel() defer cancel()
// A fixed flag and no user input, never through a shell (security.md). // A fixed flag and no user input, never through a shell (security.md).
cmd := exec.CommandContext(ctx, "postqueue", "-j") cmd := exec.CommandContext(ctx, "postqueue", "-j")
@@ -34,13 +40,16 @@ func postqueueJSON() ([]byte, error) {
cmd.Stderr = errOut cmd.Stderr = errOut
err := cmd.Run() err := cmd.Run()
if ctx.Err() == context.DeadlineExceeded { if ctx.Err() == context.DeadlineExceeded {
return nil, fmt.Errorf("postqueue -j timed out after %s", postqueueTimeout) return nil, fmt.Errorf("postqueue -j timed out after %s", queueJSONTimeout)
} }
if err != nil { if err != nil {
return nil, fmt.Errorf("postqueue -j: %w: %s", err, strings.TrimSpace(errOut.buf.String())) return nil, fmt.Errorf("postqueue -j: %w: %s", err, strings.TrimSpace(errOut.buf.String()))
} }
// A listing cut at the size limit is still a list of whole lines up to the if out.truncated {
// cut; the last, broken one is dropped by the parser. // A listing cut at the size limit says nothing about the messages past
// the cut - the oldest may be among them. It is no listing.
return nil, fmt.Errorf("postqueue -j: listing is larger than %d bytes", maxQueueOutput)
}
return out.buf.Bytes(), nil return out.buf.Bytes(), nil
} }
@@ -52,12 +61,13 @@ func QueueMessages() ([]QueuedMessage, error) {
if err != nil { if err != nil {
return nil, err return nil, err
} }
return parseQueueJSON(out), nil return parseQueueJSON(out)
} }
// parseQueueJSON reads `postqueue -j` output: one JSON object per line. A line // parseQueueJSON reads `postqueue -j` output: one JSON object per line. A line
// that is not one - the tail of a listing cut at its size limit - is skipped. // that is not one is skipped; a line too long to read ends the listing with an
func parseQueueJSON(out []byte) []QueuedMessage { // error, since what follows it is unknown.
func parseQueueJSON(out []byte) ([]QueuedMessage, error) {
var msgs []QueuedMessage var msgs []QueuedMessage
sc := bufio.NewScanner(strings.NewReader(string(out))) sc := bufio.NewScanner(strings.NewReader(string(out)))
sc.Buffer(make([]byte, 0, 64*1024), 1<<20) sc.Buffer(make([]byte, 0, 64*1024), 1<<20)
@@ -71,16 +81,26 @@ func parseQueueJSON(out []byte) []QueuedMessage {
QueueID string `json:"queue_id"` QueueID string `json:"queue_id"`
ArrivalTime int64 `json:"arrival_time"` ArrivalTime int64 `json:"arrival_time"`
Sender string `json:"sender"` Sender string `json:"sender"`
Recipients []struct {
Address string `json:"address"`
} `json:"recipients"`
} }
if err := json.Unmarshal([]byte(line), &e); err != nil || e.QueueID == "" || e.ArrivalTime <= 0 { if err := json.Unmarshal([]byte(line), &e); err != nil || e.QueueID == "" || e.ArrivalTime <= 0 {
continue continue
} }
msgs = append(msgs, QueuedMessage{ m := QueuedMessage{
ID: e.QueueID, ID: e.QueueID,
Queue: e.QueueName, Queue: e.QueueName,
Sender: e.Sender, Sender: e.Sender,
Arrival: time.Unix(e.ArrivalTime, 0).UTC(), Arrival: time.Unix(e.ArrivalTime, 0).UTC(),
})
} }
return msgs for _, r := range e.Recipients {
m.Recipients = append(m.Recipients, r.Address)
}
msgs = append(msgs, m)
}
if err := sc.Err(); err != nil {
return nil, fmt.Errorf("read postqueue -j: %w", err)
}
return msgs, nil
} }
+15 -3
View File
@@ -14,7 +14,10 @@ not json at all
{"queue_name": "deferred", "queue_id": "", "arrival_time": 1791250000} {"queue_name": "deferred", "queue_id": "", "arrival_time": 1791250000}
{"queue_name": "deferred", "queue_id": "6C00BB34", "arrival_time": 17912`) {"queue_name": "deferred", "queue_id": "6C00BB34", "arrival_time": 17912`)
got := parseQueueJSON(out) got, err := parseQueueJSON(out)
if err != nil {
t.Fatalf("parse: %v", err)
}
if len(got) != 2 { if len(got) != 2 {
t.Fatalf("parsed %d messages, want 2: %+v", len(got), got) t.Fatalf("parsed %d messages, want 2: %+v", len(got), got)
} }
@@ -22,11 +25,20 @@ not json at all
!got[0].Arrival.Equal(time.Unix(1791000000, 0)) { !got[0].Arrival.Equal(time.Unix(1791000000, 0)) {
t.Errorf("first message = %+v", got[0]) t.Errorf("first message = %+v", got[0])
} }
if len(got[0].Recipients) != 1 || got[0].Recipients[0] != "a@example.net" {
t.Errorf("first message's recipients = %v", got[0].Recipients)
}
if got[1].ID != "5F00AA12" || got[1].Queue != "active" { if got[1].ID != "5F00AA12" || got[1].Queue != "active" {
t.Errorf("second message = %+v", got[1]) t.Errorf("second message = %+v", got[1])
} }
if len(parseQueueJSON(nil)) != 0 { if empty, err := parseQueueJSON(nil); err != nil || len(empty) != 0 {
t.Error("an empty queue is not empty") t.Errorf("an empty queue: %v, %v", empty, err)
}
// A line the reader cannot hold ends the listing with an error: what
// follows it is unknown, and "nothing old is left" must not be concluded.
long := append([]byte(`{"queue_id": "A", "arrival_time": 1, "sender": "`), make([]byte, 2<<20)...)
if _, err := parseQueueJSON(long); err == nil {
t.Error("a line longer than the reader's limit was not an error")
} }
} }
@@ -12,12 +12,13 @@ ALTER TABLE users ADD COLUMN notify INTEGER NOT NULL DEFAULT 1 CHECK (notify IN
-- dns:<domain> a sending domain's records -- dns:<domain> a sending domain's records
-- cert the TLS certificate -- cert the TLS certificate
-- queue a message older than three days -- queue a message older than three days
-- limit:<domain>:<login> a level-2 limit that refused mail -- limit:<domain> level-2 limits that refused mail of the domain
-- phase is where the subject stands ("pending": seen wrong, not yet said; -- phase is where the subject stands ("pending": seen wrong, not yet said;
-- "open": said; for the certificate, the threshold last said). count is the -- "open": said; for the certificate, the threshold last said). count is the
-- checks in a row that agree with the phase's direction, mark what the phase -- checks in a row that agree with the phase's direction, mark what the phase
-- refers to (the certificate's expiry), since when it began, sent_at when the -- refers to (the certificate's expiry; for DNS, when the records were first
-- last message about it went out. -- seen right again), since when it began, sent_at when the last message about
-- it went out.
CREATE TABLE notify_state ( CREATE TABLE notify_state (
key TEXT PRIMARY KEY, key TEXT PRIMARY KEY,
phase TEXT NOT NULL, phase TEXT NOT NULL,
@@ -0,0 +1,6 @@
-- The notifier asks every minute for the mail refused in the last day
-- (panel-notifications, 2.3.0). Refusals are a sliver of the send log; without
-- an index of their own the question walks the whole retention period each
-- time. A partial index holds only the refused rows. The query names the
-- status as a literal, which is what lets SQLite use it.
CREATE INDEX idx_send_log_rejected ON send_log (created_at) WHERE status = 'rejected';
+39 -15
View File
@@ -155,24 +155,40 @@ func (s *Store) ListNotifyStateKeys(prefix string) ([]string, error) {
return out, rows.Err() return out, rows.Err()
} }
// LimitRefusal is the mail one application was refused under a level-2 rate // LimitRefusal is the mail the applications of one sending domain were refused
// limit in a span of time: how many messages and when the first. // under a level-2 rate limit in a span of time.
type LimitRefusal struct { type LimitRefusal struct {
Domain string Domain string
AppLogin string // Apps are the logins that were refused, in order.
Apps []string
Count int Count int
First time.Time First time.Time
} }
// LimitRefusalsSince returns, per domain and application, the messages the // LimitRefusalsSince returns, per sending domain, the messages the journal
// journal milter refused under a level-2 limit at or after since (send-log // milter refused under a level-2 limit at or after since.
// rows with status "rejected"). //
// A refusal is a send-log row with status "rejected", and what such a row
// says about itself is not to be believed whole. Its domain is the one in
// MAIL FROM, which the client typed and Postfix has not yet held against the
// login when the milter refuses; so the domain here is the one the login is
// registered under, and a row whose login is no application's is not counted.
// The same status is written when an application is refused for its client
// address (applications.auth_ip_restrict), which is no limit at all; so a
// login counts only while a level-2 limit is set on it or on its domain.
func (s *Store) LimitRefusalsSince(since time.Time) ([]LimitRefusal, error) { func (s *Store) LimitRefusalsSince(since time.Time) ([]LimitRefusal, error) {
// The status is a literal so that the partial index of migration 0005
// answers the question.
rows, err := s.db.Query( rows, err := s.db.Query(
`SELECT COALESCE(domain, ''), COALESCE(app_login, ''), COUNT(*), MIN(created_at) FROM send_log `SELECT d.name, a.login, COUNT(*), MIN(l.created_at)
WHERE status = ? AND created_at >= ? FROM send_log l
GROUP BY domain, app_login ORDER BY domain, app_login`, JOIN applications a ON a.login = l.app_login
StatusRejected, since.UTC().Format(time.RFC3339), JOIN domains d ON d.id = a.domain_id
WHERE l.status = 'rejected' AND l.created_at >= ?
AND (EXISTS (SELECT 1 FROM rate_limits r WHERE r.scope = 'application' AND r.ref_id = a.id)
OR EXISTS (SELECT 1 FROM rate_limits r WHERE r.scope = 'domain' AND r.ref_id = d.id))
GROUP BY d.name, a.login ORDER BY d.name, a.login`,
since.UTC().Format(time.RFC3339),
) )
if err != nil { if err != nil {
return nil, fmt.Errorf("limit refusals: %w", err) return nil, fmt.Errorf("limit refusals: %w", err)
@@ -181,14 +197,22 @@ func (s *Store) LimitRefusalsSince(since time.Time) ([]LimitRefusal, error) {
var out []LimitRefusal var out []LimitRefusal
for rows.Next() { for rows.Next() {
var ( var (
r LimitRefusal domain, login, first string
first string count int
) )
if err := rows.Scan(&r.Domain, &r.AppLogin, &r.Count, &first); err != nil { if err := rows.Scan(&domain, &login, &count, &first); err != nil {
return nil, fmt.Errorf("limit refusals scan: %w", err) return nil, fmt.Errorf("limit refusals scan: %w", err)
} }
r.First, _ = time.Parse(time.RFC3339, first) at, _ := time.Parse(time.RFC3339, first)
out = append(out, r) if n := len(out); n > 0 && out[n-1].Domain == domain {
out[n-1].Apps = append(out[n-1].Apps, login)
out[n-1].Count += count
if at.Before(out[n-1].First) {
out[n-1].First = at
}
continue
}
out = append(out, LimitRefusal{Domain: domain, Apps: []string{login}, Count: count, First: at})
} }
return out, rows.Err() return out, rows.Err()
} }
+67 -11
View File
@@ -130,26 +130,60 @@ func TestNotifyStateRoundTrip(t *testing.T) {
} }
} }
// Refused mail is counted per domain and application from a point in time; // Refused mail is counted per sending domain from a point in time - and only
// delivered mail and older refusals are not. // what was refused under a limit. The row's own domain is what the client
// typed in MAIL FROM; the login's registered domain is what counts. A login
// that is nobody's is not counted, nor is one refused with no limit set (its
// client address was not on its list).
func TestLimitRefusalsSince(t *testing.T) { func TestLimitRefusalsSince(t *testing.T) {
s := openTestStore(t) s := openTestStore(t)
d, err := s.AddDomain("example.com", "sp")
if err != nil {
t.Fatal(err)
}
other, err := s.AddDomain("other.example", "sp")
if err != nil {
t.Fatal(err)
}
app := func(domainID int64, login string) int64 {
t.Helper()
a, err := s.AddApplication(domainID, login, AddressModeWildcard, nil)
if err != nil {
t.Fatalf("add %s: %v", login, err)
}
return a.ID
}
limit := func(scope string, id int64) {
t.Helper()
if err := s.SetRateLimit(RateLimit{Scope: scope, RefID: id, MaxMessages: 1, WindowSeconds: 3600}); err != nil {
t.Fatalf("set limit: %v", err)
}
}
limit(RateLimitScopeApp, app(d.ID, "app1"))
limit(RateLimitScopeApp, app(d.ID, "app2"))
app(d.ID, "nolimit") // refused for its client address
app(other.ID, "byitsdomain") // limited through its domain
limit(RateLimitScopeDomain, other.ID)
for _, e := range []SendLogEntry{ for _, e := range []SendLogEntry{
{Domain: "example.com", AppLogin: "app1", From: "a@example.com"}, {Domain: "example.com", AppLogin: "app1", From: "a@example.com"},
{Domain: "example.com", AppLogin: "app1", From: "a@example.com"}, {Domain: "victim.example", AppLogin: "app1", From: "a@victim.example"}, // a domain the client made up
{Domain: "example.com", AppLogin: "app2", From: "b@example.com"}, {Domain: "other.example", AppLogin: "app2", From: "b@other.example"}, // another tenant's name
{Domain: "example.com", AppLogin: "nolimit", From: "c@example.com"},
{Domain: "example.com", AppLogin: "ghost", From: "d@example.com"},
{Domain: "other.example", AppLogin: "byitsdomain", From: "e@other.example"},
} { } {
if err := s.InsertRejected(e); err != nil { if err := s.InsertRejected(e); err != nil {
t.Fatal(err) t.Fatal(err)
} }
} }
if err := s.InsertQueued(SendLogEntry{QueueID: "ABC123", Domain: "example.com", AppLogin: "app3", To: "x@example.net"}); err != nil { if err := s.InsertQueued(SendLogEntry{QueueID: "ABC123", Domain: "example.com", AppLogin: "app1", To: "x@example.net"}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if err := s.InsertRejected(SendLogEntry{Domain: "old.example.com", AppLogin: "old"}); err != nil { if err := s.InsertRejected(SendLogEntry{Domain: "example.com", AppLogin: "app2", From: "old@example.com"}); err != nil {
t.Fatal(err) t.Fatal(err)
} }
if _, err := s.db.Exec(`UPDATE send_log SET created_at = '2026-01-01T00:00:00Z' WHERE app_login = 'old'`); err != nil { if _, err := s.db.Exec(`UPDATE send_log SET created_at = '2026-01-01T00:00:00Z' WHERE from_addr = 'old@example.com'`); err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -157,10 +191,32 @@ func TestLimitRefusalsSince(t *testing.T) {
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
if len(got) != 2 || got[0].AppLogin != "app1" || got[0].Count != 2 || got[1].AppLogin != "app2" || got[1].Count != 1 { if len(got) != 2 {
t.Fatalf("refusals = %+v", got) t.Fatalf("refusals = %+v, want two domains", got)
} }
if got[0].Domain != "example.com" || got[0].First.IsZero() { if g := got[0]; g.Domain != "example.com" || strings.Join(g.Apps, " ") != "app1 app2" || g.Count != 3 || g.First.IsZero() {
t.Errorf("first refusal = %+v", got[0]) t.Errorf("example.com = %+v; want app1 (2, one under a made-up domain) and app2 (1, under another tenant's name)", g)
}
if g := got[1]; g.Domain != "other.example" || strings.Join(g.Apps, " ") != "byitsdomain" || g.Count != 1 {
t.Errorf("other.example = %+v", g)
}
// The question is answered from the index of the refused rows alone.
var plan []string
rows, err := s.db.Query(`EXPLAIN QUERY PLAN SELECT COUNT(*) FROM send_log l WHERE l.status = 'rejected' AND l.created_at >= '2026-10-01T00:00:00Z'`)
if err != nil {
t.Fatal(err)
}
for rows.Next() {
var id, parent, notused int
var detail string
if err := rows.Scan(&id, &parent, &notused, &detail); err != nil {
t.Fatal(err)
}
plan = append(plan, detail)
}
rows.Close()
if !strings.Contains(strings.Join(plan, "; "), "idx_send_log_rejected") {
t.Errorf("refused rows are not found through their index: %v", plan)
} }
} }
+5 -5
View File
@@ -104,7 +104,7 @@ func TestBaselineSchema(t *testing.T) {
wantIdx := []string{ wantIdx := []string{
"idx_dmarc_report_records_report", "idx_dmarc_reports_domain", "idx_dmarc_reports_received", "idx_dmarc_report_records_report", "idx_dmarc_reports_domain", "idx_dmarc_reports_received",
"idx_inbound_spam_log_created_at", "idx_inbound_spam_log_domain_created_at", "idx_inbound_spam_log_created_at", "idx_inbound_spam_log_domain_created_at",
"idx_send_log_app_login_created_at", "idx_send_log_created_at", "idx_send_log_domain", "idx_send_log_lines_queue_id", "idx_send_log_queue_id", "idx_send_log_app_login_created_at", "idx_send_log_created_at", "idx_send_log_domain", "idx_send_log_lines_queue_id", "idx_send_log_queue_id", "idx_send_log_rejected",
"idx_sessions_expires_at", "idx_sessions_expires_at",
} }
sort.Strings(wantIdx) sort.Strings(wantIdx)
@@ -113,8 +113,8 @@ func TestBaselineSchema(t *testing.T) {
} }
var version, appID int64 var version, appID int64
if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 4 { if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 5 {
t.Errorf("user_version = %d, %v; want 4", version, err) t.Errorf("user_version = %d, %v; want 5", version, err)
} }
if err := st.db.QueryRow("PRAGMA application_id").Scan(&appID); err != nil || appID != applicationID { if err := st.db.QueryRow("PRAGMA application_id").Scan(&appID); err != nil || appID != applicationID {
t.Errorf("application_id = %#x, %v; want %#x", appID, err, applicationID) t.Errorf("application_id = %#x, %v; want %#x", appID, err, applicationID)
@@ -190,8 +190,8 @@ func TestOpenMigratesA20Database(t *testing.T) {
} }
defer st.Close() defer st.Close()
var version int var version int
if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 4 { if err := st.db.QueryRow("PRAGMA user_version").Scan(&version); err != nil || version != 5 {
t.Errorf("user_version = %d, %v; want 4", version, err) t.Errorf("user_version = %d, %v; want 5", version, err)
} }
if ok, err := st.UserExists(); err != nil || !ok { if ok, err := st.UserExists(); err != nil || !ok {
t.Errorf("the user of the 2.0 database: exists = %v, %v", ok, err) t.Errorf("the user of the 2.0 database: exists = %v, %v", ok, err)
+1 -1
View File
@@ -102,7 +102,7 @@ func (h *Handlers) HandleAccountProfile(w http.ResponseWriter, r *http.Request)
return return
} }
} }
if err := validate.Email(f.Email); err != nil { if err := validate.AccountEmail(f.Email); err != nil {
fail(http.StatusBadRequest, err.Error()) fail(http.StatusBadRequest, err.Error())
return return
} }
@@ -54,7 +54,10 @@ func (h *Handlers) renderServerSettings(w http.ResponseWriter, r *http.Request,
page := view.NewSettings(h.shellMeta(r), strconv.Itoa(form.outbound), view.FormatRate(h.l1Messages(), h.l1Window())) page := view.NewSettings(h.shellMeta(r), strconv.Itoa(form.outbound), view.FormatRate(h.l1Messages(), h.l1Window()))
on, err := h.store.NotificationsEnabled() on, err := h.store.NotificationsEnabled()
if err != nil { if err != nil {
// Shown as on: the form saves what it shows, and a failed read must not
// become "off" at the next Save.
logf("panel: settings: read the notifications switch: %v", err) logf("panel: settings: read the notifications switch: %v", err)
on = true
} }
page.WithNotifications(on, notify.From(h.cfg.Hostname)) page.WithNotifications(on, notify.From(h.cfg.Hostname))
page.WithMail(h.serverMail(false).records) page.WithMail(h.serverMail(false).records)
+2 -2
View File
@@ -187,7 +187,7 @@ func (h *Handlers) submitUserCreate(w http.ResponseWriter, r *http.Request) {
h.renderUserForm(w, r, http.StatusBadRequest, 0, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) h.renderUserForm(w, r, http.StatusBadRequest, 0, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return return
} }
if err := validate.Email(email); err != nil { if err := validate.AccountEmail(email); err != nil {
h.renderUserForm(w, r, http.StatusBadRequest, 0, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) h.renderUserForm(w, r, http.StatusBadRequest, 0, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return return
} }
@@ -261,7 +261,7 @@ func (h *Handlers) submitUserUpdate(w http.ResponseWriter, r *http.Request, u st
h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return return
} }
if err := validate.Email(email); err != nil { if err := validate.AccountEmail(email); err != nil {
h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach}) h.renderUserForm(w, r, http.StatusBadRequest, u.ID, userFormView{FormErr: err.Error(), FormUsername: username, FormEmail: email, FormRole: string(role), Reach: reach})
return return
} }
+19 -1
View File
@@ -118,8 +118,21 @@ var freemailDomains = map[string]struct{}{
"protonmail.com": {}, "protonmail.com": {},
} }
// AccountEmail checks a panel user's own address: where the panel writes to
// them. Empty is allowed. It is the address check of Email without that
// function's refusal of public mail hosts - a mailbox at one of those is a
// perfectly good place to be notified, and for many administrators the only
// one that does not depend on the server the notification is about.
func AccountEmail(addr string) error {
return emailAddress(addr, true)
}
// Email checks a DMARC rua= mailbox. Empty is allowed (policy-only). // Email checks a DMARC rua= mailbox. Empty is allowed (policy-only).
func Email(addr string) error { func Email(addr string) error {
return emailAddress(addr, false)
}
func emailAddress(addr string, publicHosts bool) error {
addr = strings.TrimSpace(addr) addr = strings.TrimSpace(addr)
if addr == "" { if addr == "" {
return nil return nil
@@ -129,6 +142,11 @@ func Email(addr string) error {
return fmt.Errorf("enter a valid email address") return fmt.Errorf("enter a valid email address")
} }
local := addr[:at] local := addr[:at]
// An address is handed to sendmail(1) as an argument; one that begins like
// an option is refused here, where the user can be told.
if strings.HasPrefix(local, "-") {
return fmt.Errorf("email address must not begin with '-'")
}
domain := NormalizeDomain(addr[at+1:]) domain := NormalizeDomain(addr[at+1:])
if err := Domain(domain); err != nil { if err := Domain(domain); err != nil {
return fmt.Errorf("email domain is invalid: %w", err) return fmt.Errorf("email domain is invalid: %w", err)
@@ -141,7 +159,7 @@ func Email(addr string) error {
return fmt.Errorf("email address contains invalid characters") return fmt.Errorf("email address contains invalid characters")
} }
} }
if _, blocked := freemailDomains[domain]; blocked { if _, blocked := freemailDomains[domain]; blocked && !publicHosts {
return fmt.Errorf("use an address on a domain you control; public mail hosts cannot receive authorised DMARC reports") return fmt.Errorf("use an address on a domain you control; public mail hosts cannot receive authorised DMARC reports")
} }
return nil return nil
@@ -31,3 +31,23 @@ func TestValidateEmail(t *testing.T) {
t.Error("blocked freemail domain accepted") t.Error("blocked freemail domain accepted")
} }
} }
// A user's own address is where the panel writes to them, and a mailbox at a
// public host is a good place for that - the refusal of those hosts belongs to
// DMARC report addresses alone. What cannot be an address is refused for both,
// and so is one that would read as an option on sendmail's command line.
func TestAccountEmail(t *testing.T) {
for _, ok := range []string{"", "me@gmail.com", "first.last+tag@proton.me", "admin@example.org"} {
if err := AccountEmail(ok); err != nil {
t.Errorf("AccountEmail(%q) = %v, want accepted", ok, err)
}
}
for _, bad := range []string{"no-at-sign", "@example.org", "me@", "a b@example.org", "me@exa mple.org", "-oQ/tmp@example.org", "\u00fc@example.org"} {
if err := AccountEmail(bad); err == nil {
t.Errorf("AccountEmail(%q) accepted", bad)
}
}
if err := Email("-x@example.org"); err == nil {
t.Error("Email accepted an address that begins like an option")
}
}