Security audit 2026-10-17, finding W1, stage 1. HandleDomainDMARC took any
address validate.Email takes; AllowedRecipients opened every stored address
on the hostname, not only the domain's hosted one; checkReportDomain
cross-checked the claimed domain only against a dmarc-reports+<tag> local
part. So a domain user of tenant A could store ops@<hostname> and from then
on anyone could mail a report claiming tenant B's domain to it, stored and
shown under B and counted toward B's 500; or store B's hosted address and
open B's mailbox without B choosing hosted. TestAllowedRecipients asserted
ops@<host> was allowed - a choice, now reversed.
Now: a custom address on the hostname is refused on the form ("choose
SelfPost hosted"); AllowedRecipients adds a domain's address only when it
is that domain's own hosted address; checkReportDomain requires a tag and
the claimed domain to equal it. The live registry (checked 2026-10-17)
holds two hosted addresses and no custom one on the hostname, so nothing
there changes.
Tests: TestCustomReportAddressOnTheHostnameIsRefused (typed, another
domain's hosted, the domain's own hosted typed as custom; an address
elsewhere still saves); TestAllowedRecipientsFollowTheDomainAddresses
inverted plus another domain's hosted address stored; untagged addresses
rejected in TestIngestMessageRejectsTagMismatch; the ingest tests deliver
to tagged addresses.
Docs: security.md § Accepted risks (DMARC), guide § DMARC reports;
CHANGELOG; plan.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
379 lines
16 KiB
Go
379 lines
16 KiB
Go
package handlers
|
|
|
|
import (
|
|
"errors"
|
|
"net/http"
|
|
"net/http/httptest"
|
|
"net/url"
|
|
"os"
|
|
"path/filepath"
|
|
"regexp"
|
|
"strconv"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/mixeme/selfpost/internal/dmarc"
|
|
"github.com/mixeme/selfpost/internal/domain"
|
|
"github.com/mixeme/selfpost/internal/health"
|
|
"github.com/mixeme/selfpost/internal/postfix"
|
|
"github.com/mixeme/selfpost/internal/store"
|
|
"github.com/mixeme/selfpost/internal/web/auth"
|
|
)
|
|
|
|
// postFormAs sends a form to a handler as the given principal.
|
|
func postFormAs(h http.HandlerFunc, p auth.Principal, target string, pathValues map[string]string, values url.Values) *httptest.ResponseRecorder {
|
|
req := httptest.NewRequest(http.MethodPost, target, strings.NewReader(values.Encode()))
|
|
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
|
for k, v := range pathValues {
|
|
req.SetPathValue(k, v)
|
|
}
|
|
rec := httptest.NewRecorder()
|
|
h(rec, auth.RequestWithPrincipal(req, p))
|
|
return rec
|
|
}
|
|
|
|
// dmarcRecordingMaps stands in for Postfix: the allow-lists a resync would write.
|
|
type dmarcRecordingMaps struct{ got []postfix.DMARCMapsConfig }
|
|
|
|
func (m *dmarcRecordingMaps) RebuildDMARCMaps(cfg postfix.DMARCMapsConfig) error {
|
|
m.got = append(m.got, cfg)
|
|
return nil
|
|
}
|
|
|
|
// hostedStand is an app stand whose server receives hosted DMARC reports.
|
|
func hostedStand(t *testing.T) (appStand, *dmarcRecordingMaps) {
|
|
t.Helper()
|
|
s := newAppStand(t)
|
|
maps := &dmarcRecordingMaps{}
|
|
s.h.cfg.DMARCEnabled, s.h.cfg.Hostname = true, "mail.example.org"
|
|
s.h.dmarc = dmarc.NewService(s.h.store, maps, "mail.example.org", true)
|
|
return s, maps
|
|
}
|
|
|
|
const hostedForExample = "dmarc-reports+example.org@mail.example.org"
|
|
|
|
// hostedEscaped is that address as html/template writes it into a page.
|
|
const hostedEscaped = "dmarc-reports+example.org@mail.example.org"
|
|
|
|
// saveReports posts the report-address form of domain 1 as the global
|
|
// administrator.
|
|
func (s appStand) saveReports(form url.Values) *httptest.ResponseRecorder {
|
|
return postFormAs(s.h.HandleDomainDMARC, globalPrincipal, "/outbound/domains/1/settings/reports", s.paths, form)
|
|
}
|
|
|
|
func (s appStand) reportAddress(t *testing.T) string {
|
|
t.Helper()
|
|
d, err := s.h.store.GetDomain(s.d.ID)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
return d.DMARCRua
|
|
}
|
|
|
|
func contains(list []string, want string) bool {
|
|
for _, s := range list {
|
|
if s == want {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
// A new domain has no report address: its settings show "No reports" selected,
|
|
// and its DMARC record says so.
|
|
func TestNewDomainHasNoReports(t *testing.T) {
|
|
s, _ := hostedStand(t)
|
|
keys := domain.NewKeys(t.TempDir())
|
|
if _, err := keys.EnsureKey("example.org", "mail"); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
s.h.domains = domain.NewService(s.h.store, keys, s.h.apps, "mail")
|
|
rec := postFormAs(s.h.HandleAddDomain, globalPrincipal, "/outbound/domains", nil, url.Values{"name": {"new.example"}})
|
|
if rec.Code != http.StatusSeeOther {
|
|
t.Fatalf("adding a domain = %d:\n%s", rec.Code, rec.Body.String())
|
|
}
|
|
domains, _ := s.h.store.ListDomains()
|
|
var added store.Domain
|
|
for _, d := range domains {
|
|
if d.Name == "new.example" {
|
|
added = d
|
|
}
|
|
}
|
|
if added.ID == 0 || added.DMARCRua != "" {
|
|
t.Fatalf("a new domain reports to %q (%+v), want no reports", added.DMARCRua, added)
|
|
}
|
|
body := send(s.h.HandleDomainSettings, &globalPrincipal, "GET", "/outbound/domains/2/settings", map[string]string{"id": idStr(added.ID)}, nil).Body.String()
|
|
has(t, "settings of a new domain", body, `<option value="none" selected>No reports</option>`)
|
|
lacks(t, "settings of a new domain", body, `<option value="hosted" selected>`, `<option value="custom" selected>`)
|
|
|
|
// The domain the stand started with has none either.
|
|
has(t, "domain page", pageOf(t, s.h.HandleDomainDetail, "/outbound/domains/1"),
|
|
`Report address: <a href="/outbound/domains/1/settings">no reports</a>`)
|
|
}
|
|
|
|
// Each of the three choices is saved, comes back selected, and is what the
|
|
// DMARC record names. Hosted is the address SelfPost derives for the domain.
|
|
func TestDomainReportAddressThreeChoices(t *testing.T) {
|
|
s, maps := hostedStand(t)
|
|
settings := func() string {
|
|
return pageOf(t, s.h.HandleDomainSettings, "/outbound/domains/1/settings")
|
|
}
|
|
// Offered in the order of the mockup, hosted because the server receives it.
|
|
has(t, "settings", settings(),
|
|
`<option value="none" selected>No reports</option>`,
|
|
`<option value="hosted">SelfPost hosted — `+hostedEscaped+`</option>`,
|
|
`<option value="custom">A specific address</option>`)
|
|
|
|
if rec := s.saveReports(url.Values{"dmarc_rua_mode": {"hosted"}}); rec.Code != http.StatusSeeOther {
|
|
t.Fatalf("hosted = %d:\n%s", rec.Code, rec.Body.String())
|
|
}
|
|
if got := s.reportAddress(t); got != hostedForExample {
|
|
t.Fatalf("hosted stored %q, want %q", got, hostedForExample)
|
|
}
|
|
has(t, "settings after hosted", settings(), `<option value="hosted" selected>`)
|
|
lacks(t, "settings after hosted", settings(), `value="`+hostedForExample+`"`) // the typed field stays empty
|
|
has(t, "domain page after hosted", pageOf(t, s.h.HandleDomainDetail, "/outbound/domains/1"),
|
|
`Report address: <a href="/outbound/domains/1/settings">`+hostedEscaped+`</a>`,
|
|
`rua=mailto:`+hostedEscaped)
|
|
// The ingest allow-list was rebuilt for the new address.
|
|
if last := maps.got[len(maps.got)-1]; !contains(last.Recipients, hostedForExample) {
|
|
t.Errorf("the allow-list after choosing hosted: %v", last.Recipients)
|
|
}
|
|
|
|
if rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {" reports@hub.example "}}); rec.Code != http.StatusSeeOther {
|
|
t.Fatalf("custom = %d:\n%s", rec.Code, rec.Body.String())
|
|
}
|
|
if got := s.reportAddress(t); got != "reports@hub.example" {
|
|
t.Fatalf("custom stored %q", got)
|
|
}
|
|
has(t, "settings after custom", settings(), `<option value="custom" selected>`, `value="reports@hub.example"`)
|
|
has(t, "domain page after custom", pageOf(t, s.h.HandleDomainDetail, "/outbound/domains/1"),
|
|
`Report address: <a href="/outbound/domains/1/settings">reports@hub.example</a>`, `rua=mailto:reports@hub.example`,
|
|
`<h3>Report authorization</h3>`) // the address is on another domain than the senders
|
|
if last := maps.got[len(maps.got)-1]; contains(last.Recipients, hostedForExample) || contains(last.Recipients, "reports@hub.example") {
|
|
t.Errorf("the allow-list after choosing another host: %v", last.Recipients)
|
|
}
|
|
|
|
if rec := s.saveReports(url.Values{"dmarc_rua_mode": {"none"}, "dmarc_rua_email": {"left-over@hub.example"}}); rec.Code != http.StatusSeeOther {
|
|
t.Fatalf("none = %d:\n%s", rec.Code, rec.Body.String())
|
|
}
|
|
if got := s.reportAddress(t); got != "" {
|
|
t.Fatalf("none stored %q; the typed address belongs to the typed choice alone", got)
|
|
}
|
|
has(t, "settings after none", settings(), `<option value="none" selected>`)
|
|
lacks(t, "settings after none", settings(), `left-over@hub.example`)
|
|
}
|
|
|
|
// Hosted reports are offered, and accepted, only while the server receives
|
|
// them; the old follow / keep choices are not accepted any more.
|
|
func TestDomainReportAddressRefusals(t *testing.T) {
|
|
s := newAppStand(t) // hosted reports are off
|
|
body := pageOf(t, s.h.HandleDomainSettings, "/outbound/domains/1/settings")
|
|
lacks(t, "settings", body, `value="hosted"`, `SelfPost hosted`, `value="inherit"`, `value="keep"`)
|
|
has(t, "settings", body, `<option value="none" selected>No reports</option>`, `<option value="custom">A specific address</option>`)
|
|
|
|
if err := s.h.store.SetDomainDMARCAddress(s.d.ID, "reports@hub.example"); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
for _, form := range []url.Values{
|
|
{"dmarc_rua_mode": {"hosted"}},
|
|
{"dmarc_rua_mode": {"inherit"}},
|
|
{"dmarc_rua_mode": {"keep"}},
|
|
{"dmarc_rua_mode": {"custom"}},
|
|
{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {"not an address<b>"}},
|
|
{},
|
|
} {
|
|
if rec := s.saveReports(form); rec.Code != http.StatusBadRequest {
|
|
t.Errorf("%v = %d, want 400", form, rec.Code)
|
|
}
|
|
if got := s.reportAddress(t); got != "reports@hub.example" {
|
|
t.Fatalf("%v changed the address to %q", form, got)
|
|
}
|
|
}
|
|
|
|
// A refused address comes back in the field, escaped, with its choice.
|
|
rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {`x"><i>@no`}})
|
|
has(t, "refusal", rec.Body.String(), `<option value="custom" selected>`, `value="x"><i>@no"`)
|
|
lacks(t, "refusal", rec.Body.String(), `<i>@no`)
|
|
}
|
|
|
|
// Saving the form as it was shown changes nothing, whichever of the three
|
|
// states the domain is in.
|
|
func TestSavingTheReportFormUntouchedChangesNothing(t *testing.T) {
|
|
selected := regexp.MustCompile(`<option value="([a-z]+)" selected>`)
|
|
typed := regexp.MustCompile(`name="dmarc_rua_email" type="email" value="([^"]*)"`)
|
|
for name, stored := range map[string]string{
|
|
"no reports": "", "hosted": hostedForExample, "typed": "reports@hub.example",
|
|
} {
|
|
s, _ := hostedStand(t)
|
|
if err := s.h.store.SetDomainDMARCAddress(s.d.ID, stored); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
page := pageOf(t, s.h.HandleDomainSettings, "/outbound/domains/1/settings")
|
|
form := url.Values{}
|
|
if m := selected.FindStringSubmatch(page[strings.Index(page, `name="dmarc_rua_mode"`):]); m != nil {
|
|
form.Set("dmarc_rua_mode", m[1])
|
|
}
|
|
if m := typed.FindStringSubmatch(page); m != nil {
|
|
form.Set("dmarc_rua_email", m[1])
|
|
}
|
|
if form.Get("dmarc_rua_mode") == "" {
|
|
t.Fatalf("%s: no choice is selected on the page:\n%s", name, page)
|
|
}
|
|
if rec := s.saveReports(form); rec.Code != http.StatusSeeOther {
|
|
t.Fatalf("%s: saving untouched %v = %d:\n%s", name, form, rec.Code, rec.Body.String())
|
|
}
|
|
if got := s.reportAddress(t); got != stored {
|
|
t.Errorf("%s: an untouched form changed %q to %q", name, stored, got)
|
|
}
|
|
}
|
|
}
|
|
|
|
// settingsAs is the settings page of domain 1 as the given principal.
|
|
func settingsAs(t *testing.T, s appStand, p auth.Principal) string {
|
|
t.Helper()
|
|
rec := send(s.h.HandleDomainSettings, &p, "GET", "/outbound/domains/1/settings", s.paths, nil)
|
|
if rec.Code != http.StatusOK {
|
|
t.Fatalf("settings as %s = %d", p.Username, rec.Code)
|
|
}
|
|
return rec.Body.String()
|
|
}
|
|
|
|
// The button that fills the typed address carries the signed-in user's profile
|
|
// e-mail, escaped; without an e-mail there is no button.
|
|
func TestFillButtonCarriesTheProfileEmail(t *testing.T) {
|
|
s := newAppStand(t)
|
|
lacks(t, "settings", settingsAs(t, s, globalPrincipal), `data-fill`, `Use my e-mail`)
|
|
|
|
admin, _ := s.h.store.GetUser(globalPrincipal.ID)
|
|
if err := s.h.store.UpdateUser(admin.ID, admin.Username, admin.PasswordHash, `o'brien&co@example.org`); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
body := settingsAs(t, s, globalPrincipal)
|
|
has(t, "settings", body, `data-fill="rua_email" data-fill-value="o'brien&co@example.org"`, `Use my e-mail`)
|
|
lacks(t, "settings", body, `o'brien`)
|
|
// It fills; the form's own button saves, so the fill button is not a submit.
|
|
has(t, "settings", body, `<button type="button" class="button" data-fill=`)
|
|
|
|
// A domain administrator is offered their own e-mail, never the global one's.
|
|
ops := domainAdmin(t, s.h.store, "ops", s.d.ID)
|
|
if got := settingsAs(t, s, ops); strings.Contains(got, "brien") || strings.Contains(got, "data-fill") {
|
|
t.Errorf("the co-administrator without an e-mail is offered someone else's:\n%s", got)
|
|
}
|
|
opsUser, _ := s.h.store.GetUser(ops.ID)
|
|
if err := s.h.store.UpdateUser(opsUser.ID, opsUser.Username, opsUser.PasswordHash, "ops@example.org"); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
has(t, "settings for ops", settingsAs(t, s, ops), `data-fill-value="ops@example.org"`)
|
|
}
|
|
|
|
// Deleting a user leaves every domain's address as it was, and a domain
|
|
// administrator never sees another user's name or address in this form.
|
|
func TestDeletingAUserLeavesDomainAddressesAndNamesNobody(t *testing.T) {
|
|
s, _ := hostedStand(t)
|
|
admin, _ := s.h.store.GetUser(globalPrincipal.ID)
|
|
if err := s.h.store.UpdateUser(admin.ID, admin.Username, admin.PasswordHash, "boss-private@example.org"); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {"reports@hub.example"}}); rec.Code != http.StatusSeeOther {
|
|
t.Fatalf("save = %d", rec.Code)
|
|
}
|
|
ops := domainAdmin(t, s.h.store, "ops", s.d.ID)
|
|
for name, body := range map[string]string{
|
|
"settings": settingsAs(t, s, ops),
|
|
"domain": send(s.h.HandleDomainDetail, &ops, "GET", "/outbound/domains/1", s.paths, nil).Body.String(),
|
|
} {
|
|
lacks(t, name+" for the co-administrator", body, `boss-private@example.org`, `admin's`, `s default`, `Another user`)
|
|
}
|
|
|
|
rec := send(s.h.HandleUserDelete, &globalPrincipal, "POST", "/server/users/"+idStr(ops.ID)+"/delete", map[string]string{"uid": idStr(ops.ID)}, url.Values{"account_password": {"correct-password-here!"}})
|
|
if rec.Code != http.StatusSeeOther {
|
|
t.Fatalf("deleting a user = %d:\n%s", rec.Code, rec.Body.String())
|
|
}
|
|
if got := s.reportAddress(t); got != "reports@hub.example" {
|
|
t.Errorf("deleting a user changed the domain's address to %q", got)
|
|
}
|
|
confirm := send(s.h.HandleUserDeleteConfirm, &globalPrincipal, "GET", "/server/users/"+idStr(admin.ID)+"/delete", map[string]string{"uid": idStr(admin.ID)}, nil).Body.String()
|
|
lacks(t, "delete confirmation", confirm, `report address`, `will have no`)
|
|
}
|
|
|
|
// failingDMARCMaps stands in for Postfix refusing the DMARC maps.
|
|
type failingDMARCMaps struct{}
|
|
|
|
func (failingDMARCMaps) RebuildDMARCMaps(postfix.DMARCMapsConfig) error {
|
|
return errors.New("disk full")
|
|
}
|
|
|
|
// Reload configuration writes what a restore's first start writes, the DMARC
|
|
// maps among it, and a failure is a message on Health that names the step,
|
|
// not a plain-text page (audit 2026-10-12, L13).
|
|
func TestReloadWritesTheDMARCMapsAndSaysWhereItFailed(t *testing.T) {
|
|
s, maps := hostedStand(t)
|
|
// Keys of its own, and a sampler for the Health page a failure is shown
|
|
// on.
|
|
s.h.domains = domain.NewService(s.h.store, domain.NewKeys(t.TempDir()), s.h.apps, "mail")
|
|
s.h.machine = &health.MachineSampler{}
|
|
rec := postFormAs(s.h.HandleReload, globalPrincipal, "/server/health/reload", nil, url.Values{})
|
|
if rec.Code != http.StatusSeeOther || rec.Header().Get("Location") != "/server/health?reloaded=1" {
|
|
t.Fatalf("reload = %d, Location %q", rec.Code, rec.Header().Get("Location"))
|
|
}
|
|
if len(maps.got) != 1 {
|
|
t.Errorf("the reload rebuilt the DMARC maps %d times, want once", len(maps.got))
|
|
}
|
|
|
|
s.h.dmarc = dmarc.NewService(s.h.store, failingDMARCMaps{}, "mail.example.org", true)
|
|
rec = postFormAs(s.h.HandleReload, globalPrincipal, "/server/health/reload", nil, url.Values{})
|
|
if rec.Code != http.StatusInternalServerError {
|
|
t.Fatalf("a failed reload = %d, want 500", rec.Code)
|
|
}
|
|
has(t, "failed reload", rec.Body.String(), "Reload failed at the DMARC report maps", "Reload configuration")
|
|
}
|
|
|
|
// A domain deleted with a rebuild failing after is said to be deleted, with
|
|
// the way to finish, not "internal error" (audit 2026-10-12, L17).
|
|
func TestDeleteDomainSaysItIsGoneWhenARebuildFails(t *testing.T) {
|
|
s, _ := hostedStand(t)
|
|
dir := t.TempDir()
|
|
s.h.domains = domain.NewService(s.h.store, domain.NewKeys(dir), s.h.apps, "mail")
|
|
d, err := s.h.domains.Add("gone.example")
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
// A directory where the selector table goes: it cannot be rewritten.
|
|
table := filepath.Join(dir, "selectors")
|
|
if err := os.Remove(table); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if err := os.MkdirAll(filepath.Join(table, "x"), 0o700); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
rec := postFormAs(s.h.HandleDeleteDomain, globalPrincipal, "/outbound/domains/x/delete", map[string]string{"id": strconv.FormatInt(d.ID, 10)}, url.Values{})
|
|
if rec.Code != http.StatusInternalServerError {
|
|
t.Fatalf("delete with a failing rebuild = %d", rec.Code)
|
|
}
|
|
has(t, "delete", rec.Body.String(), "Domain gone.example deleted, but the mail configuration was not fully rewritten", "Reload configuration")
|
|
if _, err := s.h.store.GetDomain(d.ID); !errors.Is(err, store.ErrDomainNotFound) {
|
|
t.Errorf("the domain is still there: %v", err)
|
|
}
|
|
}
|
|
|
|
// A typed report address on the server's own name is refused: the hosted
|
|
// choice is the one way to a mailbox here, tagged with the domain - a typed
|
|
// one opened an untagged mailbox, or another domain's, to the internet
|
|
// (security audit 2026-10-17, W1).
|
|
func TestCustomReportAddressOnTheHostnameIsRefused(t *testing.T) {
|
|
s, _ := hostedStand(t)
|
|
for _, addr := range []string{"ops@mail.example.org", "dmarc-reports+victim.example@MAIL.example.org", hostedForExample} {
|
|
rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {addr}})
|
|
if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "choose SelfPost hosted") {
|
|
t.Errorf("custom %s = %d:\n%.600s", addr, rec.Code, rec.Body.String())
|
|
}
|
|
if got := s.reportAddress(t); got != "" {
|
|
t.Errorf("custom %s stored %q", addr, got)
|
|
}
|
|
}
|
|
if rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {"reports@hub.example"}}); rec.Code != http.StatusSeeOther {
|
|
t.Errorf("an address elsewhere = %d", rec.Code)
|
|
}
|
|
}
|