#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 <noreply@anthropic.com>
This commit is contained in:
1 parent
c83f2f75b2
commit
7c3a2d165a
5 files changed
+88
-11
No files matched your search
@@ -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),
|
||||
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user