From 7c3a2d165a64f94a48f9cac13c4a0f1d66d879a5 Mon Sep 17 00:00:00 2001 From: Lucas Rayan Date: Sun, 2 Aug 2026 20:24:11 -0300 Subject: [PATCH] #26: let the relay's SPF mechanism be configured, not guessed SpfInclude derived the mechanism by reducing the relay hostname to its organisational domain. That is unsound, and Brevo is the case that proves it: the relay is smtp-relay.brevo.com, so the reduction yields "include:brevo.com" - but brevo.com publishes the SPF of Brevo's own corporate mail (Google Workspace and Zendesk). The sending ranges live at spf.brevo.com. Publishing the wrong include is worse than publishing none. It authorises Google to send for the domain while leaving the relay unauthorised, so every message the relay forwards fails SPF - and against the "-all" policy this reconciler generates, is rejected rather than merely marked. RELAY_SPF_INCLUDE now carries whatever the provider documents. A bare domain gets the "include:" prefix; any other SPF term (ip4:, a:, mx:) passes through untouched, since not every provider publishes an include. The old reduction stays as a fallback for relays whose hostname does happen to match, because publishing something plausible beats publishing nothing at all. Caught before configuring a relay for real, by checking what brevo.com and spf.brevo.com actually serve rather than trusting the heuristic. Co-Authored-By: Claude Opus 5 --- .env.production.example | 6 +++ .../cmd/lambdamail-protocols/config.go | 13 ++++-- .../cmd/lambdamail-protocols/main.go | 9 ++-- .../internal/application/usecase/relay.go | 28 ++++++++++-- .../application/usecase/relay_test.go | 43 +++++++++++++++++++ 5 files changed, 88 insertions(+), 11 deletions(-) diff --git a/.env.production.example b/.env.production.example index 95350ee..5698db7 100644 --- a/.env.production.example +++ b/.env.production.example @@ -77,6 +77,12 @@ RELAY_HOST= RELAY_PORT=587 RELAY_USER= RELAY_PASS= +# The SPF term that authorises the relay, exactly as your provider documents +# it. Set this: it cannot be derived from RELAY_HOST, and getting it wrong is +# worse than leaving it out. Brevo's relay is smtp-relay.brevo.com, but +# brevo.com publishes the SPF of their corporate mail - the sending ranges are +# at spf.brevo.com, so the value here is "include:spf.brevo.com". +RELAY_SPF_INCLUDE= # --- Storage ---------------------------------------------------------------- STORAGE_DRIVER=local diff --git a/services/protocols/cmd/lambdamail-protocols/config.go b/services/protocols/cmd/lambdamail-protocols/config.go index 45d0397..0e1f4c7 100644 --- a/services/protocols/cmd/lambdamail-protocols/config.go +++ b/services/protocols/cmd/lambdamail-protocols/config.go @@ -37,6 +37,10 @@ type config struct { RelayPort int RelayUser string RelayPass string + // RelaySpfInclude overrides the SPF mechanism published for the relay. + // Providers document their own, and it is rarely the relay hostname's + // organisational domain - see RelayConfig.SpfInclude. + RelaySpfInclude string // ArcSealEnabled adds an ARC set to accepted messages. It is off by // default because it only earns its keep on a host that forwards mail @@ -85,10 +89,11 @@ func loadConfig() config { DnssecResolver: envOrDefault("DNSSEC_RESOLVER", "1.1.1.1:53"), MaxMessageBytes: int64(envInt("MAX_MESSAGE_BYTES", 52428800)), - RelayHost: os.Getenv("RELAY_HOST"), - RelayPort: envInt("RELAY_PORT", 587), - RelayUser: os.Getenv("RELAY_USER"), - RelayPass: os.Getenv("RELAY_PASS"), + RelayHost: os.Getenv("RELAY_HOST"), + RelayPort: envInt("RELAY_PORT", 587), + RelayUser: os.Getenv("RELAY_USER"), + RelayPass: os.Getenv("RELAY_PASS"), + RelaySpfInclude: os.Getenv("RELAY_SPF_INCLUDE"), ArcSealEnabled: envBool("ARC_SEAL_ENABLED", false), diff --git a/services/protocols/cmd/lambdamail-protocols/main.go b/services/protocols/cmd/lambdamail-protocols/main.go index 75fbe2d..9596a5f 100644 --- a/services/protocols/cmd/lambdamail-protocols/main.go +++ b/services/protocols/cmd/lambdamail-protocols/main.go @@ -306,10 +306,11 @@ func watcherDegradedCheck(watcher *tlsprovider.AcmeCertWatcher, cfg config) func func relayConfig(cfg config) usecase.RelayConfig { return usecase.RelayConfig{ - Host: cfg.RelayHost, - Port: cfg.RelayPort, - Username: cfg.RelayUser, - Password: cfg.RelayPass, + Host: cfg.RelayHost, + Port: cfg.RelayPort, + Username: cfg.RelayUser, + Password: cfg.RelayPass, + SpfMechanism: cfg.RelaySpfInclude, } } diff --git a/services/protocols/internal/application/usecase/relay.go b/services/protocols/internal/application/usecase/relay.go index eefc38e..80d7458 100644 --- a/services/protocols/internal/application/usecase/relay.go +++ b/services/protocols/internal/application/usecase/relay.go @@ -22,6 +22,10 @@ type RelayConfig struct { Port int Username string Password string + // SpfMechanism is the SPF term that authorises this relay, as the + // provider documents it ("include:spf.brevo.com"). It has to be given + // because it cannot be derived - see SpfInclude. + SpfMechanism string // RootCAs trusts an internal relay presenting a certificate from a // private CA. Left nil, the system trust store is used. There is // deliberately no option to skip verification: the credentials sent over @@ -46,13 +50,31 @@ func (c RelayConfig) Address() string { // SpfInclude renders the mechanism to add to the domain's SPF record. Without // it the relay's own IP is not authorised to send for the domain and every // message it forwards fails SPF (PLAN.md section 10.4). +// +// SpfMechanism is used when set, and it normally has to be: the provider's +// documented include is rarely derivable from the relay hostname. Brevo is the +// example that proves it - the relay is smtp-relay.brevo.com, but brevo.com +// publishes the SPF of their *corporate* mail (Google Workspace and Zendesk), +// while the sending ranges live at spf.brevo.com. Reducing the host to its +// organisational domain would authorise Google to send for the domain and not +// the relay, so every message it forwarded would fail SPF - and against a +// "-all" policy, be rejected outright. +// +// Falling back to that reduction is still better than publishing nothing, so +// it remains for relays whose hostname does happen to match, but a deployment +// should set RELAY_SPF_INCLUDE to whatever its provider documents. func (c RelayConfig) SpfInclude() string { if !c.Configured() { return "" } - // A relay host is normally a subdomain of the provider's sending - // infrastructure; including its organisational domain is what the - // provider documents. The host itself is used when it cannot be reduced. + if mechanism := strings.TrimSpace(c.SpfMechanism); mechanism != "" { + // Accepted with or without the "include:" prefix, and any other SPF + // term ("a:", "ip4:", "mx:") is passed through untouched. + if strings.Contains(mechanism, ":") { + return mechanism + } + return "include:" + mechanism + } host := strings.TrimSuffix(strings.ToLower(c.Host), ".") labels := strings.Split(host, ".") if len(labels) > 2 { diff --git a/services/protocols/internal/application/usecase/relay_test.go b/services/protocols/internal/application/usecase/relay_test.go index 1ab3bd3..2689a82 100644 --- a/services/protocols/internal/application/usecase/relay_test.go +++ b/services/protocols/internal/application/usecase/relay_test.go @@ -307,3 +307,46 @@ func relayTestCertificate(t *testing.T) tls.Certificate { // relayTestRoots holds the CA pool for the certificate issued above. var relayTestRoots *x509.CertPool + +// The provider's documented include is not derivable from the relay hostname, +// and guessing it wrong is worse than useless against a "-all" policy. +// +// Brevo is the case that proves it: the relay is smtp-relay.brevo.com, but +// brevo.com publishes the SPF of their corporate mail (Google Workspace, +// Zendesk) while the sending ranges live at spf.brevo.com. Reducing the host +// to its organisational domain authorises Google to send for the domain and +// not the relay. +func TestRelayConfig_SpfIncludeUsesTheConfiguredMechanism(t *testing.T) { + relay := RelayConfig{Host: "smtp-relay.brevo.com", SpfMechanism: "include:spf.brevo.com"} + if got := relay.SpfInclude(); got != "include:spf.brevo.com" { + t.Errorf("SpfInclude() = %q, want the configured mechanism", got) + } + + // The reduction would have produced this, which is the wrong answer. + if got := (RelayConfig{Host: "smtp-relay.brevo.com"}).SpfInclude(); got != "include:brevo.com" { + t.Errorf("fallback = %q, want include:brevo.com", got) + } +} + +func TestRelayConfig_SpfMechanismAcceptsBareDomainsAndOtherTerms(t *testing.T) { + cases := map[string]string{ + "spf.brevo.com": "include:spf.brevo.com", + "include:spf.brevo.com": "include:spf.brevo.com", + "ip4:198.51.100.0/24": "ip4:198.51.100.0/24", + "a:relay.example.test": "a:relay.example.test", + } + for mechanism, want := range cases { + got := RelayConfig{Host: "relay.example.test", SpfMechanism: mechanism}.SpfInclude() + if got != want { + t.Errorf("SpfInclude(%q) = %q, want %q", mechanism, got, want) + } + } +} + +// An unconfigured relay publishes nothing, or the SPF record would authorise +// a host that is not being used. +func TestRelayConfig_SpfIncludeEmptyWithoutRelay(t *testing.T) { + if got := (RelayConfig{SpfMechanism: "include:spf.brevo.com"}).SpfInclude(); got != "" { + t.Errorf("SpfInclude() = %q with no relay host, want empty", got) + } +}