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) + } +}