From bde6a4000ebf3a48a42b2337987aec2333f672c0 Mon Sep 17 00:00:00 2001 From: Mix Date: Sat, 17 Oct 2026 22:27:21 +0300 Subject: [PATCH] import: bounds on a domain export (audit W4, W5, W8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security audit 2026-10-17, findings W4/W5/W8 (low), stage 4 - all global-only and self-inflicted, but unbounded: - W4: ImportKey took any RSA key; a 16 384-bit key makes every signature of the domain cost seconds. Now 1024-4096 bits (MinImportKeyBits, MaxImportKeyBits); under 1024 Go's parser already refuses. - W5: an export's applications were unbounded, each a copy of sasldb2 and a saslpasswd2 run on the panel's one connection. Now at most 500 (maxImportApplications). - W8: an import named as the hostname silently replaced the server's own signing key while DNS still held the old record. Now refused with a sentence. Tests: TestImportKeyRefusesAnOversizedKey (4104 bits); the import cases. Docs: guide § Moving a domain; CHANGELOG; plan. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +++ docs/guide.md | 5 ++++- docs/security-audit-2026-10-17.md | 6 ++++-- internal/domain/keys.go | 12 +++++++++++ internal/domain/transfer_test.go | 20 +++++++++++++++++++ internal/web/handlers/handlers_backup.go | 15 ++++++++++++++ internal/web/handlers/handlers_import_test.go | 8 ++++++++ 7 files changed, 66 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4fb652..ae733fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version bypassed the cache without limit. Now only a changed file is written and reloaded (Health's *Reload configuration* always re-reads), and a user re-checks twenty times in 15 minutes. +- **A domain import is bounded** (audit W4, W5, W8): a DKIM key over 4096 + bits, more than 500 applications, and a domain named as the server + itself - whose key signs the panel's own mail - are refused. - **The TLS private key no longer has to be readable by Postfix** (audit I1): the guide gave it to group 10998 for the panel's backup, and `postfix` is in that group, so every Postfix daemon - an smtpd talking to diff --git a/docs/guide.md b/docs/guide.md index 20690f6..070b172 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -1159,7 +1159,10 @@ the file is unrecoverable, which is the entire point. *Import a domain* takes an encrypted export directly: choose a `.spde` file and the password field appears (driven by the file extension in the browser; the server also detects the envelope by its magic bytes). A plain `.json` export -needs no password. +needs no password. An import is refused, and says why, for a DKIM key outside +1024-4096 bits, more than 500 applications, a domain named as the server +itself (its key signs the panel's own mail), and a report address on the +server's name other than the domain's own hosted one. A full backup has to be turned back into a plain archive before it can be unpacked into `/data`, which the CLI does with the same password: diff --git a/docs/security-audit-2026-10-17.md b/docs/security-audit-2026-10-17.md index 4e8e1e5..25dbeef 100644 --- a/docs/security-audit-2026-10-17.md +++ b/docs/security-audit-2026-10-17.md @@ -812,9 +812,11 @@ the argument. real changes, each of which Postfix must see; twenty re-checks in 15 minutes per user, then the cached result. TestARebuildThatChangesNothingDoesNotReload, TestDNSRechecksArePerUser. -- [ ] **W4/W5/W8.** Key size 1024-4096 on import; ≤ 500 applications per +- [x] **W4/W5/W8.** Key size 1024-4096 on import; ≤ 500 applications per export; an import named as the hostname refused with a sentence. — - **Sonnet**, 1.5 h. + **Sonnet**, 1.5 h. *Done 2026-10-17:* + TestImportKeyRefusesAnOversizedKey and the import cases (a key under + 1024 bits the parser already refuses). - [ ] **S4.** rspamd pin → `4.2.x` after reading `milter_headers`/`multimap` against the 4.x docs and one e2e run (`inbound_upstream_test`, `antispam_test`); `cap_drop: [ALL]`, `no-new-privileges`, `pids_limit` on diff --git a/internal/domain/keys.go b/internal/domain/keys.go index 4bec5f9..133c2ef 100644 --- a/internal/domain/keys.go +++ b/internal/domain/keys.go @@ -120,6 +120,12 @@ func (o *Keys) ExportKey(domainName, selector string) ([]byte, error) { return pem.EncodeToMemory(block), nil } +// The sizes of an imported DKIM key ImportKey takes. +const ( + MinImportKeyBits = 1024 + MaxImportKeyBits = 4096 +) + // ImportKey writes an imported DKIM private key to disk for a domain // (architecture.md § Persistence). The PEM is parsed and re-marshalled through // the same writer used for generated keys, so only a well-formed PKCS#1 RSA @@ -138,6 +144,12 @@ func (o *Keys) ImportKey(domainName, selector string, pemKey []byte) error { if err != nil { return fmt.Errorf("import dkim key for %s: %w", domainName, err) } + // Within what receivers verify and what signing can afford: RFC 8301 § + // 3.2 asks for at least 1024 bits, and a key far above 4096 makes every + // signature of the domain cost seconds (security audit 2026-10-17, W4). + if bits := key.N.BitLen(); bits < MinImportKeyBits || bits > MaxImportKeyBits { + return fmt.Errorf("import dkim key for %s: a %d-bit key; SelfPost signs with keys of %d to %d bits", domainName, bits, MinImportKeyBits, MaxImportKeyBits) + } path := o.keyPath(domainName, selector) if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { return fmt.Errorf("create key dir: %w", err) diff --git a/internal/domain/transfer_test.go b/internal/domain/transfer_test.go index 9daaeb5..911226d 100644 --- a/internal/domain/transfer_test.go +++ b/internal/domain/transfer_test.go @@ -1,8 +1,13 @@ package domain import ( + "crypto/rand" + "crypto/rsa" + "crypto/x509" + "encoding/pem" "errors" "fmt" + "strings" "testing" "github.com/mixeme/selfpost/internal/store" @@ -196,3 +201,18 @@ func TestImportRollsBackOnAppFailure(t *testing.T) { t.Error("expected rollback to remove the imported DKIM key") } } + +// An imported key over 4096 bits is refused (security audit 2026-10-17, +// W4): every signature of the domain would cost seconds. One under 1024 is +// refused by the parser already. +func TestImportKeyRefusesAnOversizedKey(t *testing.T) { + key, err := rsa.GenerateKey(rand.Reader, 4104) + if err != nil { + t.Fatal(err) + } + pemKey := pem.EncodeToMemory(&pem.Block{Type: "RSA PRIVATE KEY", Bytes: x509.MarshalPKCS1PrivateKey(key)}) + err = NewKeys(t.TempDir()).ImportKey("example.com", "sel", pemKey) + if err == nil || !strings.Contains(err.Error(), "a 4104-bit key; SelfPost signs with keys of 1024 to 4096 bits") { + t.Errorf("a 4104-bit key: %v", err) + } +} diff --git a/internal/web/handlers/handlers_backup.go b/internal/web/handlers/handlers_backup.go index 10a1523..b6fa80a 100644 --- a/internal/web/handlers/handlers_backup.go +++ b/internal/web/handlers/handlers_backup.go @@ -440,6 +440,9 @@ func importErrorMessage(err error) (int, string) { // 2026-10-12, M10). Client addresses are written back as the form would store // them. The login, the addresses an application may send as and the password // are checked where the application is created (app.ImportApplication). +// maxImportApplications is how many applications a domain import takes. +const maxImportApplications = 500 + func (h *Handlers) checkDomainImport(exp *domain.DomainExport) string { invalid := func(what string, err error) string { return "Invalid " + what + " in export file: " + err.Error() @@ -447,6 +450,18 @@ func (h *Handlers) checkDomainImport(exp *domain.DomainExport) string { if err := validate.Selector(exp.DKIMSelector); err != nil { return invalid("DKIM selector", err) } + // The server's own name is not a domain to import: its key signs the + // panel's own mail, and an import would replace it while DNS still + // holds the old record (security audit 2026-10-17, W8). + if h.cfg.Hostname != "" && strings.EqualFold(strings.TrimSpace(exp.Domain), h.cfg.Hostname) { + return "The export is of this server's own name, whose key signs the panel's own mail. It cannot be imported as a domain." + } + // Each application is a copy of sasldb2 and a saslpasswd2 run, all on + // the panel's one connection: an export is held to what a domain plainly + // needs (security audit 2026-10-17, W5). + if n := len(exp.Applications); n > maxImportApplications { + return fmt.Sprintf("The export holds %d applications; an import takes at most %d.", n, maxImportApplications) + } if exp.DMARCRua != "" { if err := validate.Email(exp.DMARCRua); err != nil { return invalid("report address", err) diff --git a/internal/web/handlers/handlers_import_test.go b/internal/web/handlers/handlers_import_test.go index 55df510..401f48f 100644 --- a/internal/web/handlers/handlers_import_test.go +++ b/internal/web/handlers/handlers_import_test.go @@ -71,6 +71,14 @@ func TestDomainImportIsCheckedLikeTheForms(t *testing.T) { {"report address", func(e *domain.DomainExport) { e.DMARCRua = "not an address" }, "Invalid report address in export file: "}, // As on Domain settings (security audit 2026-10-17, W1). + // Security audit 2026-10-17, W8 and W5. + {"the server's own name", func(e *domain.DomainExport) { e.Domain = "MAIL.example.org" }, + "The export is of this server's own name"}, + {"too many applications", func(e *domain.DomainExport) { + for i := 0; i < 500; i++ { + e.Applications = append(e.Applications, e.Applications[0]) + } + }, "The export holds 501 applications; an import takes at most 500."}, {"report address on the hostname", func(e *domain.DomainExport) { e.DMARCRua = "ops@MAIL.example.org" }, "The export's report address is on this server's own name"}, {"another domain's hosted address", func(e *domain.DomainExport) { e.DMARCRua = "dmarc-reports+victim.example@mail.example.org" },