From 0a764dd15d067c255a3e83382bd1370eb45704c0 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 19:26:45 +0100 Subject: [PATCH 1/4] feat(email): constrain automated company mail submission --- docs/adr/automated-mail-submission.md | 44 +++ go.mod | 2 + go.sum | 4 + scripts/mail-submission/README.md | 75 +++++ scripts/mail-submission/configuration.go | 162 ++++++++++ scripts/mail-submission/configuration_test.go | 237 +++++++++++++++ scripts/mail-submission/connection.go | 94 ++++++ scripts/mail-submission/connection_test.go | 121 ++++++++ scripts/mail-submission/gateway.go | 286 ++++++++++++++++++ scripts/mail-submission/gateway_test.go | 194 ++++++++++++ scripts/mail-submission/main.go | 69 +++++ scripts/mail-submission/refusals_test.go | 242 +++++++++++++++ 12 files changed, 1530 insertions(+) create mode 100644 docs/adr/automated-mail-submission.md create mode 100644 scripts/mail-submission/README.md create mode 100644 scripts/mail-submission/configuration.go create mode 100644 scripts/mail-submission/configuration_test.go create mode 100644 scripts/mail-submission/connection.go create mode 100644 scripts/mail-submission/connection_test.go create mode 100644 scripts/mail-submission/gateway.go create mode 100644 scripts/mail-submission/gateway_test.go create mode 100644 scripts/mail-submission/main.go create mode 100644 scripts/mail-submission/refusals_test.go diff --git a/docs/adr/automated-mail-submission.md b/docs/adr/automated-mail-submission.md new file mode 100644 index 0000000000..d5341451d0 --- /dev/null +++ b/docs/adr/automated-mail-submission.md @@ -0,0 +1,44 @@ +# Automated mail submission + +**Status:** Proposed + +## Context + +The business portal needs invitation messages with one enforced company sender. +Application credentials must not grant mailbox access or unrestricted submission. +An optional upstream content hook cannot be the only boundary: the application +must remain restricted when that hook is absent or unavailable. + +## Decision + +Use a private SMTP submission component between the application and company Email. +It authenticates the application over TLS, validates the configured envelope and +single message author, and forwards synchronously over verified upstream TLS. +Its application credential differs from the upstream send-only credential and is +not accepted by the upstream service. No successful acknowledgement is returned +before the upstream accepts the complete message. + +The executable has no default listener. It is not installed or enabled by this +source delivery. Deployment, private credential custody and real delivery remain +under the dedicated-sender issue. Existing mail, inbound routing and the agents' +restricted sender are unchanged; no external relay or subscription is introduced. + +## Alternatives + +- A scoped native DATA hook can reject invalid authors, but omission can skip it. + Using it alone would make application restriction depend on optional upstream + configuration and a separate activation hold. +- Replacing the company mail server would expand the work into mailbox migration + and recovery. That is unnecessary for an application submission boundary. + +## Consequences + +The component adds one maintained process and separate private credentials. It +needs bounded concurrency, sizes, deadlines, authentication attempts and rate +limits, with no message or credential logging. It performs no background retry. +A lost upstream acknowledgement can leave delivery uncertain; callers must use +bounded retries and replay-safe invitation links rather than infer non-delivery. + +Loopback protocol tests use a separately authenticated SMTP peer and generated +certificates. They are not deployed Stalwart, external delivery, SPF/DKIM/DMARC, +customer enrollment or portal-activation proof. Those remain explicit parent gates. diff --git a/go.mod b/go.mod index d7cdfd300c..1c669c0615 100644 --- a/go.mod +++ b/go.mod @@ -45,6 +45,8 @@ require ( require ( github.com/aws/aws-sdk-go-v2 v1.47.5 github.com/aws/aws-sdk-go-v2/service/s3 v1.114.6 + github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 + github.com/emersion/go-smtp v0.25.1 github.com/google/go-github/v88 v88.0.0 k8s.io/apimachinery v0.37.2 k8s.io/client-go v0.37.2 diff --git a/go.sum b/go.sum index 4d7163acfa..d1a4dc5c33 100644 --- a/go.sum +++ b/go.sum @@ -84,6 +84,10 @@ github.com/dustin/go-humanize v1.0.1 h1:GzkhY7T5VNhEkwH0PVJgjz+fX1rhBrR7pRT3mDkp github.com/dustin/go-humanize v1.0.1/go.mod h1:Mu1zIs6XwVuF/gI1OepvI0qD18qycQx+mFykh5fBlto= github.com/edsrzf/mmap-go v1.2.0 h1:hXLYlkbaPzt1SaQk+anYwKSRNhufIDCchSPkUD6dD84= github.com/edsrzf/mmap-go v1.2.0/go.mod h1:19H/e8pUPLicwkyNgOykDXkJ9F0MHE+Z52B8EIth78Q= +github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 h1:oP4q0fw+fOSWn3DfFi4EXdT+B+gTtzx8GC9xsc26Znk= +github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6/go.mod h1:iL2twTeMvZnrg54ZoPDNfJaJaqy0xIQFuBdrLsmspwQ= +github.com/emersion/go-smtp v0.25.1 h1:y6nzefT6PtFeMItFw97GX/ZCkVbh0rqSXG8b3ang8/w= +github.com/emersion/go-smtp v0.25.1/go.mod h1:ZtRRkbTyp2XTHCA+BmyTFTrj8xY4I+b4McvHxCU2gsQ= github.com/emicklei/go-restful/v3 v3.13.0 h1:C4Bl2xDndpU6nJ4bc1jXd+uTmYPVUwkD6bFY/oTyCes= github.com/emicklei/go-restful/v3 v3.13.0/go.mod h1:6n3XBCmQQb25CM2LCACGz8ukIrRry+4bhvbpWn3mrbc= github.com/frankban/quicktest v1.13.1 h1:xVm/f9seEhZFL9+n5kv5XLrGwy6elc4V9v/XFY2vmd8= diff --git a/scripts/mail-submission/README.md b/scripts/mail-submission/README.md new file mode 100644 index 0000000000..62c46619a3 --- /dev/null +++ b/scripts/mail-submission/README.md @@ -0,0 +1,75 @@ +# Automated mail submission + +This internal SMTP component constrains an application's automated mail before +submitting it to company mail. It is not a mailbox, delivery queue or relay +subscription. Nothing starts without an explicit command and private inputs. + +The application authenticates to this component over implicit TLS. Only its +configured envelope sender and one matching `From` author are accepted. The +component authenticates separately to the upstream SMTP service, using mandatory +STARTTLS with an explicit CA and server name. The application credential must +have no upstream account. Upstream credentials must belong to a dedicated, +send-only identity; deployment must verify that restriction on the real MTA. + +Success means the upstream service acknowledged the final DATA command. It does +not mean external delivery. A lost acknowledgement returns a temporary failure; +the application must tolerate possible duplicates and use stable invitation +identifiers. Permanent upstream submission refusals remain permanent failures. +No protocol body, recipient, password or peer error is logged. + +## Private configuration + +Build from the repository root with `go build -o ./scripts/mail-submission`. +Use `mail-submission check --config-dir ` to validate inputs without +starting a listener. `mail-submission serve --config-dir ` starts the +explicitly configured listener. There is no default address, credential, sender +or development mode. + +The directory contains these regular files, owned by root or the service user, +with mode `0600`, `0400`, `0640` or `0440`: + +- `settings.json`: one JSON object with string members `listenAddress`, `sender`, + `senderName`, `clientUser`, `upstreamAddress`, `upstreamServerName`, + `upstreamUser` and optional `replyTo`. +- `client-password` and `upstream-password`: distinct printable secrets of + 32–256 bytes, without a trailing newline. Usernames must also be distinct. +- `server.crt` and `server.key`: the receiving certificate chain and private key. +- `upstream-ca.pem`: only PEM CA certificates for the upstream trust boundary. + +Unknown or duplicate JSON members, unresolved substitutions, malformed trust, +leaf symlinks and unsafe file permissions are refused. Kubernetes Secret +projections are read through one anchored `..data` generation. Rotate inputs by +restarting the component; existing processes intentionally retain their selected +generation. Do not put actual settings, passwords or certificates in Git. + +`listenAddress` is an explicit numeric IP and unprivileged port. The server +requires TLS 1.2 or newer, bounds active connections to 16 and each connection to +30 seconds, accepts at most three authentication attempts per connection and one +recipient per message. Messages are limited to 1 MiB and headers to 64 KiB. +Authentication has a shared burst allowance of sixteen attempts and replenishes\none allowance per second. Submission has a shared burst allowance of ten messages and replenishes one +allowance every six seconds. RSET does not reset these budgets. The upstream +connection has an absolute ten-second lifetime. Shutdown allows five seconds for +active sessions, then closes them. + +A missing `replyTo` refuses all Reply-To headers. When configured, exactly one +matching address is required. Sender, Return-Path and Resent headers are refused. +The original accepted message bytes are submitted unchanged. + +## Validation and deployment boundary + +Run `go test -race ./scripts/mail-submission` and `go vet ./scripts/mail-submission`. +Tests use generated fictional certificates and native Go SMTP peers on loopback. +They cover author and envelope refusals, credential separation, receiving and +upstream TLS, unavailable peers, permanent refusals, lost acknowledgements, +resource bounds, private input validation and credential replacement. + +These peers are not the production MTA. This change creates no Deployment, +listener in the live platform, mailbox, real credential or external message. +Issue #5400 retains deployment, credential custody, protected recovery, +upstream send-only policy and actual invitation-delivery acceptance. Its +prerequisite recovery hold must remain intact. The portal remains closed until +its separate end-to-end sign-in and invitation checks pass. + +The SMTP and SASL libraries are MIT-licensed (`github.com/emersion/go-smtp` and +`github.com/emersion/go-sasl`). The repository's existing terms apply to this +component's own source; no new license grant is introduced. diff --git a/scripts/mail-submission/configuration.go b/scripts/mail-submission/configuration.go new file mode 100644 index 0000000000..8b41b88984 --- /dev/null +++ b/scripts/mail-submission/configuration.go @@ -0,0 +1,162 @@ +package main + +import ( + "bytes" + "crypto/tls" + "crypto/x509" + "encoding/json" + "encoding/pem" + "errors" + "io" + "os" + "syscall" +) + +// privateFile only reads regular, non-executable files in the selected directory. +// The root anchors all member reads; leaf symlinks are never followed. +func privateFile(root *os.Root, name string, limit int64) ([]byte, error) { + file, err := root.OpenFile(name, os.O_RDONLY|syscall.O_NOFOLLOW, 0) + if err != nil { + return nil, errConfiguration + } + defer func() { _ = file.Close() }() + info, err := file.Stat() + if err != nil || !info.Mode().IsRegular() || info.Mode().Perm()&0o137 != 0 || info.Size() > limit { + return nil, errConfiguration + } + owner, ok := info.Sys().(*syscall.Stat_t) + if !ok || (owner.Uid != 0 && owner.Uid != uint32(os.Geteuid())) { + return nil, errConfiguration + } + raw, err := io.ReadAll(io.LimitReader(file, limit+1)) + if err != nil || int64(len(raw)) > limit { + return nil, errConfiguration + } + if err = file.Close(); err != nil { + return nil, errConfiguration + } + return raw, nil +} + +func parseSettings(raw []byte) (settings, error) { + var result settings + fields := map[string]*string{"listenAddress": &result.ListenAddress, "sender": &result.Sender, "senderName": &result.SenderName, "clientUser": &result.ClientUser, "upstreamAddress": &result.UpstreamAddress, "upstreamServerName": &result.UpstreamServerName, "upstreamUser": &result.UpstreamUser, "replyTo": &result.ReplyTo} + decoder := json.NewDecoder(bytes.NewReader(raw)) + token, err := decoder.Token() + if err != nil || token != json.Delim('{') { + return result, errConfiguration + } + seen := make(map[string]bool) + for decoder.More() { + token, err = decoder.Token() + if err != nil { + return result, errConfiguration + } + key, ok := token.(string) + destination, known := fields[key] + if !ok || !known || seen[key] { + return result, errConfiguration + } + seen[key] = true + value, valueErr := decoder.Token() + text, stringValue := value.(string) + if valueErr != nil || !stringValue { + return result, errConfiguration + } + *destination = text + } + token, err = decoder.Token() + if err != nil || token != json.Delim('}') { + return result, errConfiguration + } + if _, err = decoder.Token(); !errors.Is(err, io.EOF) { + return result, errConfiguration + } + return result, nil +} + +func trustCertificates(raw []byte) (*x509.CertPool, error) { + pool := x509.NewCertPool() + count := 0 + for len(bytes.TrimSpace(raw)) > 0 { + // pem.Decode otherwise silently skips a non-PEM prefix. + raw = bytes.TrimSpace(raw) + if !bytes.HasPrefix(raw, []byte("-----BEGIN CERTIFICATE-----")) { + return nil, errConfiguration + } + block, remaining := pem.Decode(raw) + if block == nil || block.Type != "CERTIFICATE" || len(block.Headers) != 0 { + return nil, errConfiguration + } + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil || !cert.IsCA { + return nil, errConfiguration + } + pool.AddCert(cert) + count++ + raw = remaining + } + if count == 0 { + return nil, errConfiguration + } + return pool, nil +} + +func loadConfiguration(directory string) (cfg configuration, resultErr error) { + if directory == "" { + return cfg, errConfiguration + } + root, err := os.OpenRoot(directory) + if err != nil { + return cfg, errConfiguration + } + defer func(opened *os.Root) { + if closeErr := opened.Close(); closeErr != nil { + resultErr = errConfiguration + } + }(root) + // Kubernetes projects each Secret generation under ..data. Pin that directory + // once, before reading any members, rather than mixing two generations. + if _, err = root.Lstat("..data"); err == nil { + snapshot, e := root.OpenRoot("..data") + if e != nil { + return cfg, errConfiguration + } + defer func(opened *os.Root) { + if closeErr := opened.Close(); closeErr != nil { + resultErr = errConfiguration + } + }(snapshot) + root = snapshot + } else if !errors.Is(err, os.ErrNotExist) { + return cfg, errConfiguration + } + members := make(map[string][]byte) + for _, name := range []string{"settings.json", "client-password", "upstream-password", "server.crt", "server.key", "upstream-ca.pem"} { + limit := int64(64 << 10) + if name == "settings.json" { + limit = 4 << 10 + } + members[name], err = privateFile(root, name, limit) + if err != nil { + return cfg, errConfiguration + } + } + cfg.Settings, err = parseSettings(members["settings.json"]) + if err != nil { + return cfg, errConfiguration + } + cfg.ClientPassword = string(members["client-password"]) + cfg.UpstreamPassword = string(members["upstream-password"]) + pair, err := tls.X509KeyPair(members["server.crt"], members["server.key"]) + if err != nil { + return cfg, errConfiguration + } + roots, err := trustCertificates(members["upstream-ca.pem"]) + if err != nil { + return cfg, errConfiguration + } + cfg.ServerTLS = &tls.Config{MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{pair}} + cfg.UpstreamTLS = &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: roots, ServerName: cfg.Settings.UpstreamServerName} + return cfg, cfg.validate() +} diff --git a/scripts/mail-submission/configuration_test.go b/scripts/mail-submission/configuration_test.go new file mode 100644 index 0000000000..50d05f4e8d --- /dev/null +++ b/scripts/mail-submission/configuration_test.go @@ -0,0 +1,237 @@ +package main + +import ( + "bytes" + "context" + "crypto/tls" + "crypto/x509" + "encoding/pem" + "github.com/emersion/go-smtp" + "io" + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +const fixtureSettings = `{"listenAddress":"127.0.0.1:2465","sender":"invitations@example.invalid","senderName":"Example Business","clientUser":"application-fixture","upstreamAddress":"127.0.0.1:2587","upstreamServerName":"smtp.example.invalid","upstreamUser":"upstream-fixture"}` + +func fixtureDirectory(t *testing.T) string { + t.Helper() + directory := t.TempDir() + server, _ := fixtureTLS(t) + private, err := x509.MarshalPKCS8PrivateKey(server.Certificates[0].PrivateKey) + if err != nil { + t.Fatal(err) + } + files := map[string][]byte{ + "settings.json": []byte(fixtureSettings), "client-password": []byte("fictional-application-password-0123456789"), "upstream-password": []byte("fictional-upstream-password-0123456789"), + "server.crt": pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: server.Certificates[0].Certificate[0]}), + "server.key": pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: private}), + "upstream-ca.pem": pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: server.Certificates[0].Certificate[1]}), + } + for name, raw := range files { + if err = os.WriteFile(filepath.Join(directory, name), raw, 0o600); err != nil { + t.Fatal(err) + } + } + return directory +} + +func TestConfigurationAndCheckNeverStartListener(t *testing.T) { + dir := fixtureDirectory(t) + cfg, err := loadConfiguration(dir) + if err != nil { + t.Fatal(err) + } + if cfg.Settings.Sender != "invitations@example.invalid" { + t.Fatal("configuration not read") + } + var output bytes.Buffer + if status := run(context.Background(), []string{"check", "--config-dir", dir}, &output); status != 0 { + t.Fatal(output.String()) + } + for _, args := range [][]string{nil, {"serve"}, {"serve", "--config-dir", ""}, {"unknown", "--config-dir", dir}, {"serve", "--config-dir", dir, "extra"}} { + if status := run(context.Background(), args, &output); status == 0 { + t.Fatal("invalid invocation accepted") + } + } +} + +func TestConfigurationRefusesUnresolvedAndUnsafeValues(t *testing.T) { + mutations := map[string]func(*configuration){ + "missing client password": func(c *configuration) { c.ClientPassword = "" }, + "weak password": func(c *configuration) { c.ClientPassword = "short" }, + "reused password": func(c *configuration) { c.ClientPassword = c.UpstreamPassword }, + "reused user": func(c *configuration) { c.Settings.ClientUser = c.Settings.UpstreamUser }, + "substitution": func(c *configuration) { c.ClientPassword = "${UNRESOLVED_APPLICATION_PASSWORD_123456789}" }, + "template": func(c *configuration) { c.Settings.SenderName = "{{missing.name}}" }, + "insecure trust": func(c *configuration) { c.UpstreamTLS.InsecureSkipVerify = true }, + "no trust": func(c *configuration) { c.UpstreamTLS.RootCAs = nil }, + "different server name": func(c *configuration) { c.UpstreamTLS.ServerName = "other.example.invalid" }, + "no key": func(c *configuration) { c.ServerTLS = nil }, + "hostname listener": func(c *configuration) { c.Settings.ListenAddress = "localhost:2465" }, + "privileged port": func(c *configuration) { c.Settings.ListenAddress = "127.0.0.1:465" }, + "bad upstream": func(c *configuration) { c.Settings.UpstreamAddress = "smtp.example.invalid:0" }, + "mailbox group": func(c *configuration) { c.Settings.Sender = "group: invitations@example.invalid;" }, + "header injection": func(c *configuration) { c.Settings.SenderName = "Example\r\nFrom: other@example.invalid" }, + } + for name, mutate := range mutations { + t.Run(name, func(t *testing.T) { + cfg, _, _ := fixtureConfiguration(t) + cfg.UpstreamTLS = cfg.UpstreamTLS.Clone() + mutate(&cfg) + if _, err := newGateway(cfg, nil); err == nil { + t.Fatal("unsafe configuration accepted") + } + }) + } +} + +func TestPrivateConfigurationFileRefusals(t *testing.T) { + for _, name := range []string{"missing", "world readable", "group writable", "executable", "symlink", "directory", "oversize", "invalid trust", "trust prefix", "duplicate", "unknown", "trailing", "null", "wrong type"} { + t.Run(name, func(t *testing.T) { + dir := fixtureDirectory(t) + target := filepath.Join(dir, "settings.json") + switch name { + case "missing": + _ = os.Remove(target) + case "world readable": + _ = os.Chmod(target, 0o644) + case "group writable": + _ = os.Chmod(target, 0o620) + case "executable": + _ = os.Chmod(target, 0o700) + case "symlink": + _ = os.Remove(target) + if err := os.Symlink(filepath.Join(fixtureDirectory(t), "settings.json"), target); err != nil { + t.Fatal(err) + } + case "directory": + _ = os.Remove(target) + _ = os.Mkdir(target, 0o700) + case "oversize": + _ = os.WriteFile(target, []byte(strings.Repeat(" ", 4097)), 0o600) + case "invalid trust": + _ = os.WriteFile(filepath.Join(dir, "upstream-ca.pem"), []byte("not a certificate"), 0o600) + case "trust prefix": + p := filepath.Join(dir, "upstream-ca.pem") + raw, _ := os.ReadFile(p) + _ = os.WriteFile(p, append([]byte("unexpected prefix\n"), raw...), 0o600) + case "duplicate": + _ = os.WriteFile(target, []byte(strings.TrimSuffix(fixtureSettings, "}")+`,"sender":"other@example.invalid"}`), 0o600) + case "unknown": + _ = os.WriteFile(target, []byte(strings.TrimSuffix(fixtureSettings, "}")+`,"ignored":"private-canary"}`), 0o600) + case "trailing": + _ = os.WriteFile(target, []byte(fixtureSettings+` {}`), 0o600) + case "null": + _ = os.WriteFile(target, []byte(strings.TrimSuffix(fixtureSettings, "}")+`,"replyTo":null}`), 0o600) + case "wrong type": + _ = os.WriteFile(target, []byte(strings.TrimSuffix(fixtureSettings, "}")+`,"replyTo":[]}`), 0o600) + } + if _, err := loadConfiguration(dir); err == nil { + t.Fatal("unsafe private inputs accepted") + } + var output bytes.Buffer + if status := run(context.Background(), []string{"check", "--config-dir", dir}, &output); status == 0 { + t.Fatal("check accepted unsafe inputs") + } + if strings.Contains(output.String(), dir) || strings.Contains(output.String(), "private-canary") { + t.Fatal("private input escaped in diagnostic") + } + }) + } +} + +func TestProjectedConfigurationPinsOneGeneration(t *testing.T) { + root := t.TempDir() + generation := fixtureDirectory(t) + // A normal Kubernetes projection uses an internal relative generation link. + for _, name := range []string{"settings.json", "client-password", "upstream-password", "server.crt", "server.key", "upstream-ca.pem"} { + raw, err := os.ReadFile(filepath.Join(generation, name)) + if err != nil { + t.Fatal(err) + } + target := filepath.Join(root, "..generation") + if err = os.MkdirAll(target, 0o700); err != nil { + t.Fatal(err) + } + if err = os.WriteFile(filepath.Join(target, name), raw, 0o600); err != nil { + t.Fatal(err) + } + if err = os.Symlink(filepath.Join("..data", name), filepath.Join(root, name)); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink("..generation", filepath.Join(root, "..data")); err != nil { + t.Fatal(err) + } + if _, err := loadConfiguration(root); err != nil { + t.Fatalf("valid projected generation rejected: %v", err) + } + if err := os.Remove(filepath.Join(root, "..data")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(generation, filepath.Join(root, "..data")); err != nil { + t.Fatal(err) + } + if _, err := loadConfiguration(root); err == nil { + t.Fatal("escaping generation accepted") + } +} + +func TestServeExitsWithoutLeavingAListener(t *testing.T) { + dir := fixtureDirectory(t) + ctx, cancel := context.WithCancel(context.Background()) + cancel() + var output bytes.Buffer + if status := run(ctx, []string{"serve", "--config-dir", dir}, &output); status != 0 { + t.Fatalf("shutdown failed: %s", output.String()) + } + // A second invocation on the same address proves the first released it. + if status := run(ctx, []string{"serve", "--config-dir", dir}, &output); status != 0 { + t.Fatalf("listener leaked: %s", output.String()) + } +} + +func TestShutdownGraceEndsWithActiveConnectionClosed(t *testing.T) { + dir := fixtureDirectory(t) + raw, err := os.ReadFile(filepath.Join(dir, "upstream-ca.pem")) + if err != nil { + t.Fatal(err) + } + roots, err := trustCertificates(raw) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan int, 1) + go func() { done <- run(ctx, []string{"serve", "--config-dir", dir}, io.Discard) }() + var client *smtp.Client + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + client, err = smtp.DialTLS("127.0.0.1:2465", &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: roots, ServerName: "smtp.example.invalid"}) + if err == nil { + break + } + time.Sleep(time.Millisecond) + } + if err != nil { + t.Fatal(err) + } + defer func() { _ = client.Close() }() + cancel() + select { + case status := <-done: + if status != 0 { + t.Fatal("shutdown failed") + } + case <-time.After(6 * time.Second): + t.Fatal("shutdown exceeded grace") + } + if err = client.Noop(); err == nil { + t.Fatal("active connection survived shutdown") + } +} diff --git a/scripts/mail-submission/connection.go b/scripts/mail-submission/connection.go new file mode 100644 index 0000000000..66d751bba7 --- /dev/null +++ b/scripts/mail-submission/connection.go @@ -0,0 +1,94 @@ +package main + +import ( + "crypto/tls" + "net" + "sync" + "time" +) + +type deadlineConn struct { + net.Conn + expires time.Time + release func() + once sync.Once +} + +func (c *deadlineConn) clamp(deadline time.Time) time.Time { + if deadline.IsZero() || deadline.After(c.expires) { + return c.expires + } + return deadline +} +func (c *deadlineConn) SetDeadline(t time.Time) error { return c.Conn.SetDeadline(c.clamp(t)) } +func (c *deadlineConn) SetReadDeadline(t time.Time) error { return c.Conn.SetReadDeadline(c.clamp(t)) } +func (c *deadlineConn) SetWriteDeadline(t time.Time) error { + return c.Conn.SetWriteDeadline(c.clamp(t)) +} +func (c *deadlineConn) Close() error { + c.once.Do(func() { + if c.release != nil { + c.release() + } + }) + return c.Conn.Close() +} + +type tlsListener struct { + net.Listener + config *tls.Config + slots chan struct{} + done chan struct{} + once sync.Once + closeErr error + mu sync.Mutex + active map[*deadlineConn]struct{} +} + +func newTLSListener(listener net.Listener, config *tls.Config) *tlsListener { + return &tlsListener{Listener: listener, config: config, slots: make(chan struct{}, maxConnections), done: make(chan struct{}), active: make(map[*deadlineConn]struct{})} +} +func (l *tlsListener) Accept() (net.Conn, error) { + select { + case l.slots <- struct{}{}: + case <-l.done: + return nil, net.ErrClosed + } + conn, err := l.Listener.Accept() + if err != nil { + <-l.slots + return nil, err + } + var bounded *deadlineConn + bounded = &deadlineConn{Conn: conn, expires: time.Now().Add(sessionLifetime), release: func() { + l.mu.Lock() + delete(l.active, bounded) + l.mu.Unlock() + <-l.slots + }} + l.mu.Lock() + l.active[bounded] = struct{}{} + l.mu.Unlock() + if err = bounded.SetDeadline(time.Time{}); err != nil { + _ = bounded.Close() + return nil, err + } + return tls.Server(bounded, l.config), nil +} +func (l *tlsListener) Close() error { + l.once.Do(func() { close(l.done); l.closeErr = l.Listener.Close() }) + return l.closeErr +} + +// closeConnections is called only after Accept has stopped and grace has expired. +func (l *tlsListener) closeConnections() { + l.mu.Lock() + active := make([]*deadlineConn, 0, len(l.active)) + for conn := range l.active { + active = append(active, conn) + } + l.mu.Unlock() + for _, conn := range active { + _ = conn.Close() + } +} diff --git a/scripts/mail-submission/connection_test.go b/scripts/mail-submission/connection_test.go new file mode 100644 index 0000000000..168f22920e --- /dev/null +++ b/scripts/mail-submission/connection_test.go @@ -0,0 +1,121 @@ +package main + +import ( + "crypto/tls" + "net" + "testing" + "time" + + "github.com/emersion/go-sasl" + "github.com/emersion/go-smtp" +) + +func TestAbsoluteConnectionDeadlineCannotBeExtended(t *testing.T) { + for _, requested := range []time.Time{{}, time.Now().Add(time.Hour)} { + t.Run(requested.String(), func(t *testing.T) { + server, client := net.Pipe() + defer func() { _ = server.Close() }() + defer func() { _ = client.Close() }() + bounded := &deadlineConn{Conn: server, expires: time.Now().Add(30 * time.Millisecond)} + if err := bounded.SetReadDeadline(requested); err != nil { + t.Fatal(err) + } + start := time.Now() + _, err := bounded.Read(make([]byte, 1)) + timeout, ok := err.(net.Error) + if !ok || !timeout.Timeout() { + t.Fatalf("absolute deadline did not fail closed: %v", err) + } + if time.Since(start) > time.Second { + t.Fatal("absolute deadline extended") + } + }) + } +} + +func TestConnectionSlotsBoundUnauthenticatedPeers(t *testing.T) { + raw, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + serverTLS, _ := fixtureTLS(t) + listener := newTLSListener(raw, serverTLS) + defer func() { _ = listener.Close() }() + var accepted []net.Conn + for range maxConnections { + client, e := net.Dial("tcp", raw.Addr().String()) + if e != nil { + t.Fatal(e) + } + defer func() { _ = client.Close() }() + server, e := listener.Accept() + if e != nil { + t.Fatal(e) + } + accepted = append(accepted, server) + defer func() { _ = server.Close() }() + } + next := make(chan net.Conn, 1) + errs := make(chan error, 1) + go func() { + conn, e := listener.Accept() + if e != nil { + errs <- e + return + } + next <- conn + }() + extra, err := net.Dial("tcp", raw.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer func() { _ = extra.Close() }() + select { + case conn := <-next: + _ = conn.Close() + t.Fatal("connection limit exceeded") + case err = <-errs: + t.Fatal(err) + case <-time.After(30 * time.Millisecond): + } + _ = accepted[0].Close() + select { + case conn := <-next: + _ = conn.Close() + case err = <-errs: + t.Fatal(err) + case <-time.After(time.Second): + t.Fatal("closed connection did not release slot") + } +} + +func TestNativeReceivingTLSAndRestartedCredential(t *testing.T) { + cfg, _, trust := fixtureConfiguration(t) + address := fixtureGateway(t, cfg, nil) + old := trust.Clone() + old.MinVersion = tls.VersionTLS10 + old.MaxVersion = tls.VersionTLS11 + if client, err := smtp.DialTLS(address, old); err == nil { + _ = client.Close() + t.Fatal("obsolete receiving TLS accepted") + } + _, wrong := fixtureTLS(t) + if client, err := smtp.DialTLS(address, wrong); err == nil { + _ = client.Close() + t.Fatal("receiving trust not verified") + } + original := cfg.ClientPassword + cfg.ClientPassword = "fictional-rotated-password-01234567890123456789" + restarted := fixtureGateway(t, cfg, nil) + client, err := smtp.DialTLS(restarted, trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = client.Close() }() + if err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, original)); err == nil { + t.Fatal("retired credential accepted after restart") + } + if err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); err != nil { + t.Fatal(err) + } +} diff --git a/scripts/mail-submission/gateway.go b/scripts/mail-submission/gateway.go new file mode 100644 index 0000000000..1a8feb6693 --- /dev/null +++ b/scripts/mail-submission/gateway.go @@ -0,0 +1,286 @@ +package main + +import ( + "bytes" + "crypto/sha256" + "crypto/subtle" + "crypto/tls" + "crypto/x509" + "errors" + "io" + "log" + "net" + "net/mail" + "strconv" + "strings" + "time" + + "github.com/emersion/go-sasl" + "github.com/emersion/go-smtp" + "golang.org/x/time/rate" +) + +const ( + maxMessageBytes = 1 << 20 + maxHeaderBytes = 64 << 10 + maxConnections = 16 + maxAuthAttempts = 3 + sessionLifetime = 30 * time.Second + upstreamLifetime = 10 * time.Second +) + +var ( + errConfiguration = errors.New("submission configuration refused") + errUpstreamTemporary = errors.New("upstream temporarily unavailable") + errUpstreamPermanent = errors.New("upstream refused") +) + +type settings struct { + ListenAddress string + Sender string + SenderName string + ClientUser string + UpstreamAddress string + UpstreamServerName string + UpstreamUser string + ReplyTo string +} + +type configuration struct { + Settings settings + ClientPassword string + UpstreamPassword string + ServerTLS *tls.Config + UpstreamTLS *tls.Config +} + +type gateway struct { + server *smtp.Server + cfg configuration + limiter *rate.Limiter + authenticationLimiter *rate.Limiter + logger *log.Logger +} + +// safeSMTPLogger deliberately discards library error arguments, which may contain +// peer addresses or other protocol input. Wire debugging is never enabled. +type safeSMTPLogger struct{ logger *log.Logger } + +func (s safeSMTPLogger) Printf(_ string, _ ...any) { s.logger.Print("connection refused") } +func (s safeSMTPLogger) Println(_ ...any) { s.logger.Print("connection refused") } + +func ascii(value string, minimum, maximum int, spaces bool) bool { + if len(value) < minimum || len(value) > maximum || strings.Contains(value, "${") || strings.Contains(value, "{{") { + return false + } + for _, r := range value { + if r < 32 || r > 126 || (!spaces && r == ' ') { + return false + } + } + return strings.TrimSpace(value) != "" +} + +func mailbox(value string) bool { + if !ascii(value, 3, 254, false) || strings.ContainsAny(value, ":;,<>\"\\[]") { + return false + } + address, err := mail.ParseAddress(value) + return err == nil && address.Name == "" && address.Address == value +} + +func endpoint(value string, listening bool) bool { + host, port, err := net.SplitHostPort(value) + number, e := strconv.Atoi(port) + if err != nil || e != nil || host == "" || !ascii(host, 1, 253, false) || number < 1 || number > 65535 { + return false + } + return !listening || (net.ParseIP(host) != nil && number >= 1024) +} + +func (cfg configuration) validate() error { + s := cfg.Settings + if !endpoint(s.ListenAddress, true) || !endpoint(s.UpstreamAddress, false) || + !mailbox(s.Sender) || (s.ReplyTo != "" && !mailbox(s.ReplyTo)) || + !ascii(s.SenderName, 1, 128, true) || strings.ContainsAny(s.SenderName, ":;,") || + !ascii(s.ClientUser, 1, 254, false) || !ascii(s.UpstreamUser, 1, 254, false) || s.ClientUser == s.UpstreamUser || + !ascii(cfg.ClientPassword, 32, 256, true) || !ascii(cfg.UpstreamPassword, 32, 256, true) || cfg.ClientPassword == cfg.UpstreamPassword || + !ascii(s.UpstreamServerName, 1, 253, false) || strings.ContainsAny(s.UpstreamServerName, "/:@") || + cfg.ServerTLS == nil || len(cfg.ServerTLS.Certificates) != 1 || + cfg.UpstreamTLS == nil || cfg.UpstreamTLS.InsecureSkipVerify || cfg.UpstreamTLS.RootCAs == nil || + cfg.UpstreamTLS.RootCAs.Equal(x509.NewCertPool()) || cfg.UpstreamTLS.ServerName != s.UpstreamServerName { + return errConfiguration + } + return nil +} + +func newGateway(cfg configuration, output io.Writer) (*gateway, error) { + if err := cfg.validate(); err != nil { + return nil, err + } + if output == nil { + output = io.Discard + } + cfg.ServerTLS = cfg.ServerTLS.Clone() + cfg.ServerTLS.MinVersion = tls.VersionTLS12 + cfg.UpstreamTLS = cfg.UpstreamTLS.Clone() + cfg.UpstreamTLS.MinVersion = tls.VersionTLS12 + g := &gateway{cfg: cfg, limiter: rate.NewLimiter(rate.Every(6*time.Second), 10), authenticationLimiter: rate.NewLimiter(rate.Every(time.Second), 16), logger: log.New(output, "SMTP_SUBMISSION ", 0)} + g.server = smtp.NewServer(g) + g.server.Domain = "submission.invalid" + g.server.ReadTimeout = 10 * time.Second + g.server.WriteTimeout = 5 * time.Second + g.server.MaxMessageBytes = maxMessageBytes + g.server.MaxRecipients = 1 + g.server.ErrorLog = safeSMTPLogger{g.logger} + return g, nil +} + +// serve supplies actual tls.Conn values so the SMTP library recognizes TLS and +// never offers insecure authentication. Deadlines and connection slots apply +// beneath TLS, including handshakes and clients that never authenticate. +func (g *gateway) serve(listener net.Listener) error { + return g.server.Serve(newTLSListener(listener, g.cfg.ServerTLS)) +} + +func (g *gateway) NewSession(conn *smtp.Conn) (smtp.Session, error) { + state, ok := conn.TLSConnectionState() + if !ok || !state.HandshakeComplete || state.Version < tls.VersionTLS12 { + return nil, smtp.ErrAuthRequired + } + return &session{gateway: g}, nil +} + +type session struct { + gateway *gateway + authenticated bool + attempts int + from string + recipient string +} + +func (s *session) AuthMechanisms() []string { return []string{sasl.Plain} } +func (s *session) Auth(_ string) (sasl.Server, error) { + return sasl.NewPlainServer(func(identity, user, password string) error { + s.attempts++ + actual := sha256.Sum256([]byte(password)) + expected := sha256.Sum256([]byte(s.gateway.cfg.ClientPassword)) + if s.attempts > maxAuthAttempts || !s.gateway.authenticationLimiter.Allow() || identity != "" || user != s.gateway.cfg.Settings.ClientUser || subtle.ConstantTimeCompare(actual[:], expected[:]) != 1 { + return smtp.ErrAuthFailed + } + s.authenticated = true + return nil + }), nil +} + +func refusal(code int, message string) error { + return &smtp.SMTPError{Code: code, Message: message} +} + +func (s *session) Mail(from string, _ *smtp.MailOptions) error { + if !s.authenticated { + return smtp.ErrAuthRequired + } + if from != s.gateway.cfg.Settings.Sender { + return refusal(550, "Sender refused") + } + s.from = from + return nil +} + +func (s *session) Rcpt(to string, _ *smtp.RcptOptions) error { + if !s.authenticated || s.from == "" { + return smtp.ErrAuthRequired + } + if s.recipient != "" || !mailbox(to) { + return refusal(550, "Recipient refused") + } + s.recipient = to + return nil +} + +func oneAddress(values []string, address, name string, checkName bool) bool { + if len(values) != 1 || strings.ContainsAny(values[0], ":;,") { + return false + } + parsed, err := mail.ParseAddress(values[0]) + return err == nil && parsed.Address == address && (!checkName || parsed.Name == "" || parsed.Name == name) +} + +func (s *session) validMessage(raw []byte) bool { + end := bytes.Index(raw, []byte("\r\n\r\n")) + if end < 0 || end > maxHeaderBytes { + return false + } + message, err := mail.ReadMessage(bytes.NewReader(raw)) + if err != nil || !oneAddress(message.Header["From"], s.from, s.gateway.cfg.Settings.SenderName, true) { + return false + } + for key := range message.Header { + lower := strings.ToLower(key) + if lower == "sender" || lower == "return-path" || strings.HasPrefix(lower, "resent-") { + return false + } + } + replies := message.Header["Reply-To"] + if s.gateway.cfg.Settings.ReplyTo == "" { + return len(replies) == 0 + } + return oneAddress(replies, s.gateway.cfg.Settings.ReplyTo, "", false) +} + +func (s *session) Data(reader io.Reader) error { + if !s.authenticated || s.from == "" || s.recipient == "" { + return smtp.ErrAuthRequired + } + permitted := s.gateway.limiter.Allow() + raw, err := io.ReadAll(io.LimitReader(reader, maxMessageBytes+1)) + if err != nil || len(raw) > maxMessageBytes || !s.validMessage(raw) { + return refusal(554, "Message refused") + } + if !permitted { + return refusal(451, "Submission temporarily unavailable") + } + if err = s.forward(raw); err != nil { + s.gateway.logger.Print("upstream refused") + if errors.Is(err, errUpstreamPermanent) { + return refusal(554, "Submission refused") + } + return refusal(451, "Submission temporarily unavailable") + } + s.gateway.logger.Print("accepted") + return nil +} + +func (s *session) forward(raw []byte) error { + conn, err := net.DialTimeout("tcp", s.gateway.cfg.Settings.UpstreamAddress, 3*time.Second) + if err != nil { + return errUpstreamTemporary + } + bounded := &deadlineConn{Conn: conn, expires: time.Now().Add(upstreamLifetime)} + defer func() { _ = bounded.Close() }() + client, err := smtp.NewClientStartTLS(bounded, s.gateway.cfg.UpstreamTLS) + if err != nil { + return errUpstreamTemporary + } + defer func() { _ = client.Close() }() + client.CommandTimeout = 5 * time.Second + client.SubmissionTimeout = 5 * time.Second + if err = client.Auth(sasl.NewPlainClient("", s.gateway.cfg.Settings.UpstreamUser, s.gateway.cfg.UpstreamPassword)); err != nil { + return errUpstreamTemporary + } + // SendMail waits for the final DATA acknowledgement. A later QUIT failure + // must not turn an accepted message into a retry; closing the session suffices. + err = client.SendMail(s.from, []string{s.recipient}, bytes.NewReader(raw)) + if err == nil { + return nil + } + var protocolError *smtp.SMTPError + if errors.As(err, &protocolError) && protocolError.Code >= 500 && protocolError.Code < 600 { + return errUpstreamPermanent + } + return errUpstreamTemporary +} + +func (s *session) Reset() { s.from = ""; s.recipient = "" } +func (s *session) Logout() error { return nil } diff --git a/scripts/mail-submission/gateway_test.go b/scripts/mail-submission/gateway_test.go new file mode 100644 index 0000000000..d85905bda7 --- /dev/null +++ b/scripts/mail-submission/gateway_test.go @@ -0,0 +1,194 @@ +package main + +import ( + "bytes" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "io" + "math/big" + "net" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/emersion/go-sasl" + "github.com/emersion/go-smtp" +) + +func fixtureTLS(t *testing.T) (*tls.Config, *tls.Config) { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + ca := &x509.Certificate{SerialNumber: big.NewInt(1), Subject: pkix.Name{CommonName: "Fictional SMTP fixture"}, NotBefore: time.Now().Add(-time.Minute), NotAfter: time.Now().Add(time.Hour), IsCA: true, BasicConstraintsValid: true, KeyUsage: x509.KeyUsageCertSign} + caDER, err := x509.CreateCertificate(rand.Reader, ca, ca, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + ca, err = x509.ParseCertificate(caDER) + if err != nil { + t.Fatal(err) + } + leaf := &x509.Certificate{SerialNumber: big.NewInt(2), DNSNames: []string{"smtp.example.invalid"}, NotBefore: ca.NotBefore, NotAfter: ca.NotAfter, KeyUsage: x509.KeyUsageDigitalSignature, ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}} + der, err := x509.CreateCertificate(rand.Reader, leaf, ca, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + private, err := x509.MarshalPKCS8PrivateKey(key) + if err != nil { + t.Fatal(err) + } + pair, err := tls.X509KeyPair(pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), pem.EncodeToMemory(&pem.Block{Type: "PRIVATE KEY", Bytes: private})) + if err != nil { + t.Fatal(err) + } + roots := x509.NewCertPool() + roots.AddCert(ca) + return &tls.Config{MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{{Certificate: [][]byte{der, caDER}, PrivateKey: pair.PrivateKey}}}, &tls.Config{MinVersion: tls.VersionTLS12, RootCAs: roots, ServerName: "smtp.example.invalid"} +} + +type peerMessage struct { + from string + to []string + body []byte +} +type peerBackend struct { + messages chan peerMessage + reject atomic.Bool + drop atomic.Bool + authAttempts atomic.Int64 +} + +func (p *peerBackend) NewSession(conn *smtp.Conn) (smtp.Session, error) { + return &peerSession{peer: p, conn: conn}, nil +} + +type peerSession struct { + peer *peerBackend + conn *smtp.Conn + authenticated bool + from string + to []string +} + +func (p *peerSession) AuthMechanisms() []string { return []string{sasl.Plain} } +func (p *peerSession) Auth(_ string) (sasl.Server, error) { + return sasl.NewPlainServer(func(identity, user, password string) error { + p.peer.authAttempts.Add(1) + if identity != "" || user != "upstream-fixture" || password != "fictional-upstream-password-0123456789" { + return smtp.ErrAuthFailed + } + p.authenticated = true + return nil + }), nil +} +func (p *peerSession) Mail(from string, _ *smtp.MailOptions) error { + if !p.authenticated { + return smtp.ErrAuthRequired + } + p.from = from + return nil +} +func (p *peerSession) Rcpt(to string, _ *smtp.RcptOptions) error { p.to = append(p.to, to); return nil } +func (p *peerSession) Data(r io.Reader) error { + if p.peer.reject.Load() { + return refusal(550, "private-upstream-canary") + } + body, err := io.ReadAll(r) + if err != nil { + return err + } + p.peer.messages <- peerMessage{p.from, append([]string(nil), p.to...), body} + if p.peer.drop.Load() { + _ = p.conn.Close() + return io.EOF + } + return nil +} +func (p *peerSession) Reset() { p.from = ""; p.to = nil } +func (p *peerSession) Logout() error { return nil } + +func fixturePeer(t *testing.T, serverTLS *tls.Config) (string, *peerBackend) { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + peer := &peerBackend{messages: make(chan peerMessage, 20)} + server := smtp.NewServer(peer) + server.Domain = "smtp.example.invalid" + server.TLSConfig = serverTLS + go func() { _ = server.Serve(listener) }() + t.Cleanup(func() { _ = server.Close() }) + return listener.Addr().String(), peer +} + +func fixtureConfiguration(t *testing.T) (configuration, *peerBackend, *tls.Config) { + t.Helper() + serverTLS, clientTLS := fixtureTLS(t) + address, peer := fixturePeer(t, serverTLS) + return configuration{Settings: settings{ListenAddress: "127.0.0.1:2465", Sender: "invitations@example.invalid", SenderName: "Example Business", ClientUser: "application-fixture", UpstreamAddress: address, UpstreamServerName: "smtp.example.invalid", UpstreamUser: "upstream-fixture"}, ClientPassword: "fictional-application-password-0123456789", UpstreamPassword: "fictional-upstream-password-0123456789", ServerTLS: serverTLS, UpstreamTLS: clientTLS}, peer, clientTLS +} + +func fixtureGateway(t *testing.T, cfg configuration, logs io.Writer, configure ...func(*gateway)) string { + t.Helper() + g, err := newGateway(cfg, logs) + if err != nil { + t.Fatalf("valid submission configuration refused: %v", err) + } + for _, change := range configure { + change(g) + } + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + go func() { _ = g.serve(listener) }() + t.Cleanup(func() { _ = g.server.Close() }) + return listener.Addr().String() +} + +func TestNativeSubmissionPreservesMessage(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + var logs bytes.Buffer + address := fixtureGateway(t, cfg, &logs) + client, err := smtp.DialTLS(address, trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = client.Close() }() + if err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); err != nil { + t.Fatal(err) + } + message := []byte("From: Example Business \r\nTo: client@example.invalid\r\nSubject: Fictional invitation\r\n\r\nfixture-message-canary\r\n") + if err = client.SendMail(cfg.Settings.Sender, []string{"client@example.invalid"}, bytes.NewReader(message)); err != nil { + t.Fatal(err) + } + select { + case got := <-peer.messages: + if got.from != cfg.Settings.Sender || len(got.to) != 1 || got.to[0] != "client@example.invalid" || !bytes.Equal(got.body, message) { + t.Fatal("message or envelope changed") + } + case <-time.After(time.Second): + t.Fatal("upstream did not receive submission") + } +} + +type lockedLog struct { + mu sync.Mutex + buffer bytes.Buffer +} + +func (l *lockedLog) Write(p []byte) (int, error) { + l.mu.Lock() + defer l.mu.Unlock() + return l.buffer.Write(p) +} +func (l *lockedLog) String() string { l.mu.Lock(); defer l.mu.Unlock(); return l.buffer.String() } diff --git a/scripts/mail-submission/main.go b/scripts/mail-submission/main.go new file mode 100644 index 0000000000..a5088dceb8 --- /dev/null +++ b/scripts/mail-submission/main.go @@ -0,0 +1,69 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io" + "net" + "os" + "os/signal" + "syscall" + "time" + + "github.com/emersion/go-smtp" +) + +func run(ctx context.Context, args []string, output io.Writer) int { + if len(args) != 3 || (args[0] != "check" && args[0] != "serve") || args[1] != "--config-dir" || args[2] == "" { + _, _ = fmt.Fprintln(output, "Usage: mail-submission --config-dir ") + return 1 + } + cfg, err := loadConfiguration(args[2]) + if err != nil { + _, _ = fmt.Fprintln(output, "Submission configuration refused") + return 1 + } + if args[0] == "check" { + _, _ = fmt.Fprintln(output, "Submission configuration valid; no listener started") + return 0 + } + g, err := newGateway(cfg, output) + if err != nil { + _, _ = fmt.Fprintln(output, "Submission configuration refused") + return 1 + } + listener, err := net.Listen("tcp", cfg.Settings.ListenAddress) + if err != nil { + _, _ = fmt.Fprintln(output, "Submission listener unavailable") + return 1 + } + secureListener := newTLSListener(listener, g.cfg.ServerTLS) + finished := make(chan error, 1) + go func() { finished <- g.server.Serve(secureListener) }() + select { + case err = <-finished: + if errors.Is(err, smtp.ErrServerClosed) { + return 0 + } + _, _ = fmt.Fprintln(output, "Submission listener stopped") + return 1 + case <-ctx.Done(): + // Stop Accept and join Serve before waiting for active connections. + // This also handles cancellation before the library registers its listener. + _ = secureListener.Close() + <-finished + shutdown, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err = g.server.Shutdown(shutdown); err != nil { + secureListener.closeConnections() + } + return 0 + } +} + +func main() { + ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + defer cancel() + os.Exit(run(ctx, os.Args[1:], os.Stderr)) +} diff --git a/scripts/mail-submission/refusals_test.go b/scripts/mail-submission/refusals_test.go new file mode 100644 index 0000000000..f65564c302 --- /dev/null +++ b/scripts/mail-submission/refusals_test.go @@ -0,0 +1,242 @@ +package main + +import ( + "bytes" + "crypto/tls" + "errors" + "strings" + "testing" + "time" + + "github.com/emersion/go-sasl" + "github.com/emersion/go-smtp" + "golang.org/x/time/rate" +) + +const validMessage = "From: Example Business \r\nTo: client@example.invalid\r\nSubject: Fictional invitation\r\n\r\nfixture-message-canary\r\n" + +func authenticatedClient(t *testing.T, address string, trust *tls.Config, cfg configuration) *smtp.Client { + t.Helper() + client, err := smtp.DialTLS(address, trust) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = client.Close() }) + if err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); err != nil { + t.Fatal(err) + } + return client +} + +func TestNativeMessageRefusals(t *testing.T) { + from := "From: Example Business \r\n" + cases := map[string]string{ + "other author": strings.Replace(validMessage, "invitations@example.invalid", "someone@example.invalid", 1), + "other display name": strings.Replace(validMessage, "Example Business", "Fictional Impersonator", 1), + "missing author": strings.TrimPrefix(validMessage, from), + "duplicate author": from + validMessage, + "case duplicate": "fRoM: invitations@example.invalid\r\n" + validMessage, + "multiple authors": strings.Replace(validMessage, from, "From: invitations@example.invalid, someone@example.invalid\r\n", 1), + "group author": strings.Replace(validMessage, from, "From: Trusted: invitations@example.invalid;\r\n", 1), + "folded extra author": strings.Replace(validMessage, from, "From: invitations@example.invalid,\r\n someone@example.invalid\r\n", 1), + "sender": "Sender: someone@example.invalid\r\n" + validMessage, + "resent author": "Resent-From: someone@example.invalid\r\n" + validMessage, + "return path": "Return-Path: someone@example.invalid\r\n" + validMessage, + "reply to": "Reply-To: someone@example.invalid\r\n" + validMessage, + "missing header boundary": from + "Subject: fixture\r\n", + "header bound": from + "X-Fixture: a\r\n" + strings.Repeat(" aaaaaaaaaaaaaaaaaaaa\r\n", 3200) + "\r\nbody\r\n", + "message bound": validMessage + strings.Repeat("x\r\n", maxMessageBytes/3), + } + for name, body := range cases { + t.Run(name, func(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + logs := &lockedLog{} + client := authenticatedClient(t, fixtureGateway(t, cfg, logs), trust, cfg) + if err := client.SendMail(cfg.Settings.Sender, []string{"client@example.invalid"}, strings.NewReader(body)); err == nil { + t.Fatal("invalid message accepted") + } + select { + case <-peer.messages: + t.Fatal("invalid message reached upstream") + default: + } + for _, canary := range []string{cfg.ClientPassword, cfg.UpstreamPassword, "fixture-message-canary", "client@example.invalid"} { + if strings.Contains(logs.String(), canary) { + t.Fatal("private protocol data logged") + } + } + }) + } +} + +func TestNativeAuthenticationAndEnvelopeRefusals(t *testing.T) { + for _, name := range []string{"anonymous", "wrong password", "wrong user", "delegated identity", "wrong envelope", "second recipient", "attempt limit"} { + t.Run(name, func(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + client, err := smtp.DialTLS(fixtureGateway(t, cfg, nil), trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = client.Close() }() + switch name { + case "anonymous": + err = client.Mail(cfg.Settings.Sender, nil) + case "wrong password": + err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, "wrong-password")) + case "wrong user": + err = client.Auth(sasl.NewPlainClient("", "other-user", cfg.ClientPassword)) + case "delegated identity": + err = client.Auth(sasl.NewPlainClient("delegation", cfg.Settings.ClientUser, cfg.ClientPassword)) + case "attempt limit": + for range maxAuthAttempts { + if e := client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, "wrong")); e == nil { + t.Fatal("bad authentication accepted") + } + } + err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)) + default: + if e := client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); e != nil { + t.Fatal(e) + } + if name == "wrong envelope" { + err = client.Mail("someone@example.invalid", nil) + } else { + if e := client.Mail(cfg.Settings.Sender, nil); e != nil { + t.Fatal(e) + } + if e := client.Rcpt("client@example.invalid", nil); e != nil { + t.Fatal(e) + } + err = client.Rcpt("another@example.invalid", nil) + } + } + if err == nil { + t.Fatal("unauthorized protocol command accepted") + } + select { + case <-peer.messages: + t.Fatal("unauthorized message forwarded") + default: + } + }) + } +} + +func TestNativeUpstreamRefusals(t *testing.T) { + for _, name := range []string{"wrong CA", "wrong name", "missing STARTTLS", "unavailable", "permanent refusal", "lost acknowledgement"} { + t.Run(name, func(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + cfg.UpstreamTLS = cfg.UpstreamTLS.Clone() + logs := &lockedLog{} + switch name { + case "wrong CA": + _, other := fixtureTLS(t) + cfg.UpstreamTLS.RootCAs = other.RootCAs + case "wrong name": + cfg.Settings.UpstreamServerName = "other.example.invalid" + cfg.UpstreamTLS.ServerName = cfg.Settings.UpstreamServerName + case "missing STARTTLS": + cfg.Settings.UpstreamAddress, peer = fixturePeer(t, nil) + case "unavailable": + cfg.Settings.UpstreamAddress = "127.0.0.1:1" + case "permanent refusal": + peer.reject.Store(true) + case "lost acknowledgement": + peer.drop.Store(true) + } + client := authenticatedClient(t, fixtureGateway(t, cfg, logs), trust, cfg) + err := client.SendMail(cfg.Settings.Sender, []string{"client@example.invalid"}, strings.NewReader(validMessage)) + var protocol *smtp.SMTPError + if !errors.As(err, &protocol) { + t.Fatalf("expected classified refusal, got %v", err) + } + expected := 451 + if name == "permanent refusal" { + expected = 554 + } + if protocol.Code != expected { + t.Fatalf("refusal %d, want %d", protocol.Code, expected) + } + if strings.Contains(err.Error(), "private-upstream-canary") || strings.Contains(logs.String(), "private-upstream-canary") { + t.Fatal("private upstream error escaped") + } + if name == "wrong CA" || name == "wrong name" || name == "missing STARTTLS" { + if peer.authAttempts.Load() != 0 { + t.Fatal("upstream received credentials before verified TLS") + } + } + if name == "lost acknowledgement" { + select { + case <-peer.messages: + default: + t.Fatal("fixture did not lose a real final acknowledgement") + } + } + }) + } +} + +func TestApplicationCredentialCannotAuthenticateUpstream(t *testing.T) { + cfg, _, trust := fixtureConfiguration(t) + client, err := smtp.DialStartTLS(cfg.Settings.UpstreamAddress, trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = client.Close() }() + if err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); err == nil { + t.Fatal("application credential bypasses submission boundary") + } +} + +func TestResetDoesNotResetSubmissionBudget(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + address := fixtureGateway(t, cfg, nil, func(g *gateway) { g.limiter = rate.NewLimiter(rate.Every(time.Hour), 1) }) + client := authenticatedClient(t, address, trust, cfg) + if err := client.SendMail(cfg.Settings.Sender, []string{"client@example.invalid"}, strings.NewReader(validMessage)); err != nil { + t.Fatal(err) + } + if err := client.Reset(); err != nil { + t.Fatal(err) + } + if err := client.SendMail(cfg.Settings.Sender, []string{"client@example.invalid"}, strings.NewReader(validMessage)); err == nil { + t.Fatal("reset bypasses submission budget") + } + <-peer.messages + select { + case <-peer.messages: + t.Fatal("over-budget submission forwarded") + default: + } +} + +func TestNativePermittedAuthorFormsPreserveBytes(t *testing.T) { + for _, from := range []string{"From: invitations@example.invalid\r\n", "From: Example Business\r\n \r\n"} { + t.Run(from, func(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + client := authenticatedClient(t, fixtureGateway(t, cfg, nil), trust, cfg) + body := strings.Replace(validMessage, "From: Example Business \r\n", from, 1) + if err := client.SendMail(cfg.Settings.Sender, []string{"client@example.invalid"}, strings.NewReader(body)); err != nil { + t.Fatal(err) + } + got := <-peer.messages + if !bytes.Equal(got.body, []byte(body)) { + t.Fatal("permitted message rewritten") + } + }) + } +} + +func TestAuthenticationBudgetSurvivesNewConnections(t *testing.T) { + cfg, _, trust := fixtureConfiguration(t) + address := fixtureGateway(t, cfg, nil, func(g *gateway) { g.authenticationLimiter = rate.NewLimiter(rate.Every(time.Hour), 1) }) + first := authenticatedClient(t, address, trust, cfg) + _ = first.Close() + client, err := smtp.DialTLS(address, trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = client.Close() }() + if err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); err == nil { + t.Fatal("new connection reset shared authentication budget") + } +} From 883719e14860250197cdc5280c8c8cf529193474 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 19:50:02 +0100 Subject: [PATCH 2/4] fix(email): compare provisioned credentials without password hashing --- scripts/mail-submission/gateway.go | 18 ++++++++++++++---- scripts/mail-submission/refusals_test.go | 12 ++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/scripts/mail-submission/gateway.go b/scripts/mail-submission/gateway.go index 1a8feb6693..68a43afb0a 100644 --- a/scripts/mail-submission/gateway.go +++ b/scripts/mail-submission/gateway.go @@ -2,7 +2,6 @@ package main import ( "bytes" - "crypto/sha256" "crypto/subtle" "crypto/tls" "crypto/x509" @@ -159,13 +158,24 @@ type session struct { recipient string } +// credentialsEqual compares provisioned machine secrets, not password hashes. +// Padding keeps comparison work independent of either credential length. +func credentialsEqual(actual, expected string) bool { + const maximum = 256 + if len(actual) > maximum || len(expected) > maximum { + return false + } + var a, b [maximum]byte + copy(a[:], actual) + copy(b[:], expected) + return subtle.ConstantTimeEq(int32(len(actual)), int32(len(expected)))&subtle.ConstantTimeCompare(a[:], b[:]) == 1 +} + func (s *session) AuthMechanisms() []string { return []string{sasl.Plain} } func (s *session) Auth(_ string) (sasl.Server, error) { return sasl.NewPlainServer(func(identity, user, password string) error { s.attempts++ - actual := sha256.Sum256([]byte(password)) - expected := sha256.Sum256([]byte(s.gateway.cfg.ClientPassword)) - if s.attempts > maxAuthAttempts || !s.gateway.authenticationLimiter.Allow() || identity != "" || user != s.gateway.cfg.Settings.ClientUser || subtle.ConstantTimeCompare(actual[:], expected[:]) != 1 { + if s.attempts > maxAuthAttempts || !s.gateway.authenticationLimiter.Allow() || identity != "" || user != s.gateway.cfg.Settings.ClientUser || !credentialsEqual(password, s.gateway.cfg.ClientPassword) { return smtp.ErrAuthFailed } s.authenticated = true diff --git a/scripts/mail-submission/refusals_test.go b/scripts/mail-submission/refusals_test.go index f65564c302..08f6159f8e 100644 --- a/scripts/mail-submission/refusals_test.go +++ b/scripts/mail-submission/refusals_test.go @@ -240,3 +240,15 @@ func TestAuthenticationBudgetSurvivesNewConnections(t *testing.T) { t.Fatal("new connection reset shared authentication budget") } } + +func TestCredentialComparisonRejectsLengthAndPaddingCollisions(t *testing.T) { + expected := "fictional-application-password-0123456789" + for _, candidate := range []string{expected[:len(expected)-1], expected + "x", expected + "\x00", strings.Repeat("x", 257), strings.Repeat("x", 256) + expected} { + if credentialsEqual(candidate, expected) { + t.Fatal("different credential accepted") + } + } + if !credentialsEqual(expected, expected) { + t.Fatal("matching credential refused") + } +} From 5c099254b80767aab8be9666c107756f1b978cb1 Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 21:03:10 +0100 Subject: [PATCH 3/4] fix(email): validate certificates and count malformed authentication --- scripts/mail-submission/configuration.go | 9 ++ scripts/mail-submission/gateway.go | 7 +- scripts/mail-submission/review_test.go | 129 +++++++++++++++++++++++ 3 files changed, 143 insertions(+), 2 deletions(-) create mode 100644 scripts/mail-submission/review_test.go diff --git a/scripts/mail-submission/configuration.go b/scripts/mail-submission/configuration.go index 8b41b88984..18b542b872 100644 --- a/scripts/mail-submission/configuration.go +++ b/scripts/mail-submission/configuration.go @@ -10,6 +10,7 @@ import ( "io" "os" "syscall" + "time" ) // privateFile only reads regular, non-executable files in the selected directory. @@ -152,6 +153,14 @@ func loadConfiguration(directory string) (cfg configuration, resultErr error) { if err != nil { return cfg, errConfiguration } + leaf, err := x509.ParseCertificate(pair.Certificate[0]) + if err != nil { + return cfg, errConfiguration + } + now := time.Now() + if now.Before(leaf.NotBefore) || !now.Before(leaf.NotAfter) { + return cfg, errConfiguration + } roots, err := trustCertificates(members["upstream-ca.pem"]) if err != nil { return cfg, errConfiguration diff --git a/scripts/mail-submission/gateway.go b/scripts/mail-submission/gateway.go index 68a43afb0a..55d3c17af0 100644 --- a/scripts/mail-submission/gateway.go +++ b/scripts/mail-submission/gateway.go @@ -173,9 +173,12 @@ func credentialsEqual(actual, expected string) bool { func (s *session) AuthMechanisms() []string { return []string{sasl.Plain} } func (s *session) Auth(_ string) (sasl.Server, error) { + s.attempts++ + if s.attempts > maxAuthAttempts || !s.gateway.authenticationLimiter.Allow() { + return nil, smtp.ErrAuthFailed + } return sasl.NewPlainServer(func(identity, user, password string) error { - s.attempts++ - if s.attempts > maxAuthAttempts || !s.gateway.authenticationLimiter.Allow() || identity != "" || user != s.gateway.cfg.Settings.ClientUser || !credentialsEqual(password, s.gateway.cfg.ClientPassword) { + if identity != "" || user != s.gateway.cfg.Settings.ClientUser || !credentialsEqual(password, s.gateway.cfg.ClientPassword) { return smtp.ErrAuthFailed } s.authenticated = true diff --git a/scripts/mail-submission/review_test.go b/scripts/mail-submission/review_test.go new file mode 100644 index 0000000000..3dcae2563d --- /dev/null +++ b/scripts/mail-submission/review_test.go @@ -0,0 +1,129 @@ +package main + +import ( + "bytes" + "context" + "crypto" + "crypto/rand" + "crypto/x509" + "encoding/pem" + "errors" + "os" + "path/filepath" + "testing" + "time" + + "github.com/emersion/go-sasl" + "github.com/emersion/go-smtp" + "golang.org/x/time/rate" +) + +func TestConfigurationRefusesOutOfDateReceivingCertificate(t *testing.T) { + for _, future := range []bool{false, true} { + name := "expired" + if future { + name = "not yet valid" + } + t.Run(name, func(t *testing.T) { + dir := fixtureDirectory(t) + keyPEM, err := os.ReadFile(filepath.Join(dir, "server.key")) + if err != nil { + t.Fatal(err) + } + block, _ := pem.Decode(keyPEM) + if block == nil { + t.Fatal("fixture key missing") + } + key, err := x509.ParsePKCS8PrivateKey(block.Bytes) + if err != nil { + t.Fatal(err) + } + signer, ok := key.(crypto.Signer) + if !ok { + t.Fatal("fixture key is not a signer") + } + certPEM, err := os.ReadFile(filepath.Join(dir, "server.crt")) + if err != nil { + t.Fatal(err) + } + block, _ = pem.Decode(certPEM) + if block == nil { + t.Fatal("fixture certificate missing") + } + leaf, err := x509.ParseCertificate(block.Bytes) + if err != nil { + t.Fatal(err) + } + now := time.Now() + leaf.NotBefore, leaf.NotAfter = now.Add(-2*time.Hour), now.Add(-time.Hour) + if future { + leaf.NotBefore, leaf.NotAfter = now.Add(time.Hour), now.Add(2*time.Hour) + } + der, err := x509.CreateCertificate(rand.Reader, leaf, leaf, signer.Public(), signer) + if err != nil { + t.Fatal(err) + } + if err = os.WriteFile(filepath.Join(dir, "server.crt"), pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}), 0o600); err != nil { + t.Fatal(err) + } + if _, err = loadConfiguration(dir); err == nil { + t.Error("out-of-date receiving certificate accepted") + } + var output bytes.Buffer + if status := run(context.Background(), []string{"check", "--config-dir", dir}, &output); status == 0 { + t.Error("check accepted out-of-date receiving certificate") + } + }) + } +} + +// These bytes are valid on the SMTP wire, but not a three-field PLAIN response. +type malformedPlainClient struct{} + +func (malformedPlainClient) Start() (string, []byte, error) { + return sasl.Plain, []byte("not-a-plain-response"), nil +} +func (malformedPlainClient) Next([]byte) ([]byte, error) { + return nil, errors.New("unexpected authentication challenge") +} + +func TestMalformedPlainConsumesConnectionBudget(t *testing.T) { + cfg, _, trust := fixtureConfiguration(t) + address := fixtureGateway(t, cfg, nil) + client, err := smtp.DialTLS(address, trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = client.Close() }() + for range maxAuthAttempts { + if err = client.Auth(malformedPlainClient{}); err == nil { + t.Fatal("malformed PLAIN accepted") + } + } + if err = client.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); err == nil { + t.Fatal("malformed PLAIN bypassed connection authentication budget") + } +} + +func TestMalformedPlainConsumesSharedBudget(t *testing.T) { + cfg, _, trust := fixtureConfiguration(t) + address := fixtureGateway(t, cfg, nil, func(g *gateway) { + g.authenticationLimiter = rate.NewLimiter(rate.Every(time.Hour), 1) + }) + first, err := smtp.DialTLS(address, trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = first.Close() }() + if err = first.Auth(malformedPlainClient{}); err == nil { + t.Fatal("malformed PLAIN accepted") + } + second, err := smtp.DialTLS(address, trust) + if err != nil { + t.Fatal(err) + } + defer func() { _ = second.Close() }() + if err = second.Auth(sasl.NewPlainClient("", cfg.Settings.ClientUser, cfg.ClientPassword)); err == nil { + t.Fatal("malformed PLAIN bypassed shared authentication budget") + } +} From 1a8c9e802b70820158f931349200dc8c2077cecd Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Wed, 11 Nov 2026 23:27:49 +0100 Subject: [PATCH 4/4] fix(email): isolate transactions and honor the payload bound --- scripts/mail-submission/configuration.go | 2 +- scripts/mail-submission/gateway.go | 19 ++- scripts/mail-submission/transaction_test.go | 128 ++++++++++++++++++++ 3 files changed, 144 insertions(+), 5 deletions(-) create mode 100644 scripts/mail-submission/transaction_test.go diff --git a/scripts/mail-submission/configuration.go b/scripts/mail-submission/configuration.go index 18b542b872..a8ceef4203 100644 --- a/scripts/mail-submission/configuration.go +++ b/scripts/mail-submission/configuration.go @@ -26,7 +26,7 @@ func privateFile(root *os.Root, name string, limit int64) ([]byte, error) { return nil, errConfiguration } owner, ok := info.Sys().(*syscall.Stat_t) - if !ok || (owner.Uid != 0 && owner.Uid != uint32(os.Geteuid())) { + if !ok || (owner.Uid != 0 && int64(owner.Uid) != int64(os.Geteuid())) { return nil, errConfiguration } raw, err := io.ReadAll(io.LimitReader(file, limit+1)) diff --git a/scripts/mail-submission/gateway.go b/scripts/mail-submission/gateway.go index 55d3c17af0..d26113a2fa 100644 --- a/scripts/mail-submission/gateway.go +++ b/scripts/mail-submission/gateway.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "context" "crypto/subtle" "crypto/tls" "crypto/x509" @@ -129,7 +130,9 @@ func newGateway(cfg configuration, output io.Writer) (*gateway, error) { g.server.Domain = "submission.invalid" g.server.ReadTimeout = 10 * time.Second g.server.WriteTimeout = 5 * time.Second - g.server.MaxMessageBytes = maxMessageBytes + // The DATA reader needs one extra byte to observe its terminator. The + // application still enforces the exact payload bound in Mail and Data. + g.server.MaxMessageBytes = maxMessageBytes + 1 g.server.MaxRecipients = 1 g.server.ErrorLog = safeSMTPLogger{g.logger} return g, nil @@ -190,13 +193,17 @@ func refusal(code int, message string) error { return &smtp.SMTPError{Code: code, Message: message} } -func (s *session) Mail(from string, _ *smtp.MailOptions) error { +func (s *session) Mail(from string, options *smtp.MailOptions) error { if !s.authenticated { return smtp.ErrAuthRequired } + s.Reset() if from != s.gateway.cfg.Settings.Sender { return refusal(550, "Sender refused") } + if options != nil && options.Size > maxMessageBytes { + return smtp.ErrDataTooLarge + } s.from = from return nil } @@ -266,11 +273,15 @@ func (s *session) Data(reader io.Reader) error { } func (s *session) forward(raw []byte) error { - conn, err := net.DialTimeout("tcp", s.gateway.cfg.Settings.UpstreamAddress, 3*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), upstreamLifetime) + defer cancel() + dialer := &net.Dialer{Timeout: 3 * time.Second} + conn, err := dialer.DialContext(ctx, "tcp", s.gateway.cfg.Settings.UpstreamAddress) if err != nil { return errUpstreamTemporary } - bounded := &deadlineConn{Conn: conn, expires: time.Now().Add(upstreamLifetime)} + deadline, _ := ctx.Deadline() + bounded := &deadlineConn{Conn: conn, expires: deadline} defer func() { _ = bounded.Close() }() client, err := smtp.NewClientStartTLS(bounded, s.gateway.cfg.UpstreamTLS) if err != nil { diff --git a/scripts/mail-submission/transaction_test.go b/scripts/mail-submission/transaction_test.go new file mode 100644 index 0000000000..14de010d83 --- /dev/null +++ b/scripts/mail-submission/transaction_test.go @@ -0,0 +1,128 @@ +package main + +import ( + "bytes" + "fmt" + "io" + "strings" + "testing" + "time" + + "github.com/emersion/go-smtp" +) + +func boundedMessage(size int) string { + remaining := size - len(validMessage) - 2 + const line = "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx\r\n" + return validMessage + strings.Repeat(line, remaining/len(line)) + strings.Repeat("x", remaining%len(line)) + "\r\n" +} + +func TestNativeNewMailCannotReuseRecipient(t *testing.T) { + for _, refusedSender := range []bool{false, true} { + t.Run(fmt.Sprintf("refused_sender_%t", refusedSender), func(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + client := authenticatedClient(t, fixtureGateway(t, cfg, nil), trust, cfg) + if err := client.Mail(cfg.Settings.Sender, nil); err != nil { + t.Fatal(err) + } + if err := client.Rcpt("earlier@example.invalid", nil); err != nil { + t.Fatal(err) + } + from := cfg.Settings.Sender + if refusedSender { + from = "refused@example.invalid" + } + if err := client.Mail(from, nil); (err != nil) != refusedSender { + t.Fatalf("unexpected new MAIL result: %v", err) + } + writer, err := client.Data() + if err == nil { + _, _ = io.WriteString(writer, validMessage) + err = writer.Close() + } + if err == nil { + t.Fatal("new MAIL reused the earlier recipient without RCPT") + } + select { + case <-peer.messages: + t.Fatal("incomplete transaction reached upstream") + default: + } + if err = client.Reset(); err != nil { + t.Fatal(err) + } + if err = client.SendMail(cfg.Settings.Sender, []string{"current@example.invalid"}, strings.NewReader(validMessage)); err != nil { + t.Fatal("valid transaction after RSET refused:", err) + } + select { + case message := <-peer.messages: + if len(message.to) != 1 || message.to[0] != "current@example.invalid" { + t.Fatal("old recipient survived reset") + } + case <-time.After(time.Second): + t.Fatal("valid transaction missing upstream") + } + }) + } +} + +func TestNativeDataMessageBoundary(t *testing.T) { + for _, size := range []int{maxMessageBytes - 1, maxMessageBytes, maxMessageBytes + 1} { + t.Run(fmt.Sprint(size), func(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + client := authenticatedClient(t, fixtureGateway(t, cfg, nil), trust, cfg) + body := boundedMessage(size) + if len(body) != size { + t.Fatal("fixture size differs") + } + // No SIZE declaration: exercise the actual DATA reader and application bound. + if err := client.Mail(cfg.Settings.Sender, nil); err != nil { + t.Fatal(err) + } + if err := client.Rcpt("client@example.invalid", nil); err != nil { + t.Fatal(err) + } + writer, err := client.Data() + if err != nil { + t.Fatal(err) + } + _, writeErr := io.WriteString(writer, body) + closeErr := writer.Close() + if size > maxMessageBytes { + if writeErr == nil && closeErr == nil { + t.Fatal("oversize DATA accepted") + } + select { + case <-peer.messages: + t.Fatal("oversize DATA reached upstream") + default: + } + return + } + if writeErr != nil || closeErr != nil { + t.Fatalf("in-bound DATA refused: write=%v close=%v", writeErr, closeErr) + } + select { + case message := <-peer.messages: + if !bytes.Equal(message.body, []byte(body)) { + t.Fatal("in-bound DATA bytes changed") + } + case <-time.After(time.Second): + t.Fatal("in-bound DATA missing upstream") + } + }) + } +} + +func TestNativeDeclaredMessageBound(t *testing.T) { + cfg, peer, trust := fixtureConfiguration(t) + client := authenticatedClient(t, fixtureGateway(t, cfg, nil), trust, cfg) + if err := client.Mail(cfg.Settings.Sender, &smtp.MailOptions{Size: maxMessageBytes + 1}); err == nil { + t.Fatal("declared oversize message accepted") + } + select { + case <-peer.messages: + t.Fatal("declared oversize message reached upstream") + default: + } +}