diff --git a/.env.example b/.env.example index 710a9ec..6883c88 100644 --- a/.env.example +++ b/.env.example @@ -81,6 +81,18 @@ S3_BUCKET=ekklesia-development # EKKLESIA_SMTP_PASSWORD= # EKKLESIA_SMTP_FROM_ADDRESS=noreply@example.org # EKKLESIA_SMTP_FROM_NAME=Ekklesia +# +# EKKLESIA_SMTP_HELO_NAME is the name Ekklesia announces itself as in the SMTP EHLO command. +# +# Leave it unset unless a relay refuses the connection. An SMTP client has to identify itself, and +# with nothing configured Nodemailer derives that from the machine hostname — substituting the +# address literal [127.0.0.1] whenever the hostname is not fully qualified, which is the normal +# state of a container or an unnamed VM. Some relays close the connection on that greeting with a +# 421 before any message is offered, which looks exactly like the relay being unreachable. +# +# Set it to a fully qualified domain name your deployment owns — typically the sending domain. +# No scheme, no port, no brackets, and at least one dot; anything else is refused at startup. +# EKKLESIA_SMTP_HELO_NAME=example.org # Where an emailed verification or password-reset link sends a browser (ADR 0038). # diff --git a/apps/api/tests/host-configuration-parity.test.ts b/apps/api/tests/host-configuration-parity.test.ts index 1f662e4..56c0aa5 100644 --- a/apps/api/tests/host-configuration-parity.test.ts +++ b/apps/api/tests/host-configuration-parity.test.ts @@ -33,6 +33,7 @@ function environment( const configuredInfrastructure = Object.freeze({ EKKLESIA_SMTP_FROM_ADDRESS: "noreply@example.org", + EKKLESIA_SMTP_HELO_NAME: "mail.example.org", EKKLESIA_SMTP_HOST: "smtp.example.org", EKKLESIA_SMTP_PASSWORD: smtpPassword, EKKLESIA_SMTP_USERNAME: "relay@example.org", @@ -77,6 +78,9 @@ describe("host configuration parity between startApi and Doctor", () => { expect(apiView(env).mail).toEqual(doctorView(env).mail); expect(apiView(env).mail).toMatchObject({ + // The SMTP client identity is part of that equivalence: a relay that refuses the greeting + // must fail in Doctor for the same reason it fails at runtime. + heloName: "mail.example.org", host: "smtp.example.org", port: 587, security: "starttls", @@ -123,6 +127,10 @@ describe("host configuration parity between startApi and Doctor", () => { "MAIL_USERNAME_WITHOUT_PASSWORD", ], [{ ...configuredInfrastructure, EKKLESIA_SMTP_PORT: "70000" }, "MAIL_PORT_INVALID"], + [ + { ...configuredInfrastructure, EKKLESIA_SMTP_HELO_NAME: "localhost" }, + "MAIL_HELO_NAME_INVALID", + ], ] as const) { const env = environment(overrides); expect(() => apiView(env)).toThrowError(expect.objectContaining({ code })); diff --git a/core/src/host/configuration.ts b/core/src/host/configuration.ts index 5e7a8ae..1d8c78e 100644 --- a/core/src/host/configuration.ts +++ b/core/src/host/configuration.ts @@ -133,6 +133,10 @@ export function loadHostConfiguration(environment: HostEnvironment): HostConfigu mail: { fromAddress: environment.EKKLESIA_SMTP_FROM_ADDRESS, fromName: environment.EKKLESIA_SMTP_FROM_NAME, + // The name this client announces in `EHLO`. Optional, and unset is the ordinary case: it + // matters only where the machine's own hostname is not a fully qualified name the relay will + // accept. + heloName: environment.EKKLESIA_SMTP_HELO_NAME, host: environment.EKKLESIA_SMTP_HOST, password: environment.EKKLESIA_SMTP_PASSWORD, port: environment.EKKLESIA_SMTP_PORT, diff --git a/core/src/mail/config.ts b/core/src/mail/config.ts index 3262866..cefdc0f 100644 --- a/core/src/mail/config.ts +++ b/core/src/mail/config.ts @@ -8,6 +8,7 @@ export type MailConfigErrorCode = | "MAIL_FROM_ADDRESS_INVALID" | "MAIL_FROM_ADDRESS_MISSING" | "MAIL_FROM_NAME_INVALID" + | "MAIL_HELO_NAME_INVALID" | "MAIL_HOST_INVALID" | "MAIL_PASSWORD_WITHOUT_USERNAME" | "MAIL_PORT_INVALID" @@ -19,6 +20,8 @@ const mailConfigMessages: Readonly> = Object MAIL_FROM_ADDRESS_INVALID: "The outbound email sender address is not a valid email address.", MAIL_FROM_ADDRESS_MISSING: "A sender address is required when an SMTP host is set.", MAIL_FROM_NAME_INVALID: `The sender display name must be at most ${MAIL_MAX_DISPLAY_NAME_LENGTH} characters and contain no quotes, backslashes, or control characters.`, + MAIL_HELO_NAME_INVALID: + "The SMTP HELO name must be a fully qualified domain name, with no scheme, port, brackets, or spaces.", MAIL_HOST_INVALID: "The SMTP host must be a bare hostname or IP address, with no scheme or port.", MAIL_PASSWORD_WITHOUT_USERNAME: "An SMTP password requires a username; supply both or neither for an unauthenticated relay.", @@ -70,6 +73,7 @@ export type MailSecurityMode = (typeof MAIL_SECURITY_MODES)[number]; export interface MailProviderConfigInput { readonly host?: string | undefined; + readonly heloName?: string | undefined; readonly port?: string | number | undefined; readonly security?: string | undefined; readonly username?: string | undefined; @@ -87,6 +91,24 @@ export interface MailProviderConfigInput { */ export interface MailProviderConfig { readonly host: string; + /** + * The name this client identifies itself as in the SMTP `EHLO` command, when the deployment sets + * one. + * + * This exists because of a real failure, and the failure is generic rather than tied to any one + * provider. An SMTP client must greet the server with its own identity, and when nothing is + * configured Nodemailer derives it from the machine's hostname — falling back to the address + * literal `[127.0.0.1]` whenever that hostname is not fully qualified, which is the normal state + * of a container or a VM nobody has named. Some relays refuse that greeting outright, closing the + * connection with a `421` before any message is offered, which surfaces here as a bare + * `MAIL_PROVIDER_UNAVAILABLE` and looks indistinguishable from the relay being down. + * + * Absent means Nodemailer's own derivation is used, unchanged. It is deliberately not defaulted to + * the sender's domain: a host whose machine hostname *is* a valid FQDN may already be greeting + * correctly — some relays check it against the connecting address's reverse DNS — and silently + * replacing a working identity would trade one deployment's bug for another's. + */ + readonly heloName?: string; readonly port: number; readonly security: MailSecurityMode; /** Present only for an authenticated relay. A relay authorized by source IP — the common Google @@ -119,6 +141,29 @@ function validateHost(host: string): string { return trimmed; } +/** + * A fully qualified domain name, and deliberately nothing else. + * + * Stricter than `HOST_PATTERN` on purpose, in two ways. It requires **at least one dot**, because a + * single unqualified label is precisely the input Nodemailer already refuses to use — configuring + * one would reproduce the failure this setting exists to fix. And it requires the final label to + * begin with a letter, which rejects a bare IPv4 address: `EHLO 192.0.2.1` is not valid SMTP syntax, + * and the bracketed address-literal form that *is* valid is the greeting the affected relays reject. + * + * Everything the requirement rules out falls out of the anchors and the character class rather than + * from a list of special cases: a scheme, a port, a path, whitespace, brackets, a comma, a + * carriage return or line feed, and any other prose all fail to match. + */ +const HELO_NAME_PATTERN = + /^(?=.{1,253}$)(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+[a-z](?:[a-z0-9-]{0,61}[a-z0-9])?$/iu; + +function validateHeloName(heloName: string): string { + if (!HELO_NAME_PATTERN.test(heloName)) { + throw new MailConfigError("MAIL_HELO_NAME_INVALID"); + } + return heloName; +} + function validatePort(port: string | number | undefined, security: MailSecurityMode): number { if (port === undefined || (typeof port === "string" && port.trim() === "")) { return DEFAULT_PORTS[security]; @@ -174,6 +219,16 @@ export function createMailProviderConfig( const validatedHost = validateHost(host); const port = validatePort(input?.port, security); + // Validated exactly as supplied, with no trimming. Every other value here tolerates surrounding + // whitespace as a copy-paste artefact; this one must not, because a stray character in the value + // goes straight onto the wire in a protocol command. It is refused rather than repaired, the same + // rule an address follows. + const heloNameInput = input?.heloName; + const heloName = + heloNameInput === undefined || heloNameInput === "" + ? undefined + : validateHeloName(heloNameInput); + const username = input?.username?.trim() === "" ? undefined : input?.username?.trim(); const password = input?.password === "" ? undefined : input?.password; if (username !== undefined && password === undefined) { @@ -201,6 +256,7 @@ export function createMailProviderConfig( host: validatedHost, port, security, + ...(heloName === undefined ? {} : { heloName }), ...(fromName === undefined ? {} : { fromName }), ...(username === undefined || password === undefined ? {} : { password, username }), }); @@ -219,7 +275,11 @@ export function describeMailProvider(config: MailProviderConfig): string { config.fromName === undefined ? config.fromAddress : `${config.fromName} <${config.fromAddress}>`; + // The HELO name is host infrastructure, not a credential: it is the identity this client announces + // in the clear on every connection, so printing it costs nothing and is exactly what an operator + // needs when a relay has refused the greeting. + const helo = config.heloName === undefined ? "" : ` · HELO ${config.heloName}`; return `SMTP ${config.host}:${config.port} · ${config.security} · from ${sender} · ${ config.username === undefined ? "unauthenticated relay" : "authenticated" - }`; + }${helo}`; } diff --git a/core/src/mail/smtp-provider.ts b/core/src/mail/smtp-provider.ts index 323f966..a72cf05 100644 --- a/core/src/mail/smtp-provider.ts +++ b/core/src/mail/smtp-provider.ts @@ -126,6 +126,11 @@ function transportOptions( disableUrlAccess: true, greetingTimeout: bounds.greetingTimeoutMs, host: config.host, + // The identity this client announces in `EHLO`. Left to Nodemailer when the deployment + // configures none — it derives one from the machine hostname, falling back to the address + // literal `[127.0.0.1]` when that hostname is not fully qualified, which some relays refuse + // with a `421` before a message is ever offered (ADR 0037). + ...(config.heloName === undefined ? {} : { name: config.heloName }), // Deliberately not a pooled transport: `pool` is absent, which is Nodemailer's default and is // what this type permits. A pool keeps sockets open and reuses them across sends, which for a // platform sending a handful of transactional messages buys nothing and makes the failure modes diff --git a/core/tests/doctor-host-configuration.test.ts b/core/tests/doctor-host-configuration.test.ts index 032a7ee..dfa01a4 100644 --- a/core/tests/doctor-host-configuration.test.ts +++ b/core/tests/doctor-host-configuration.test.ts @@ -363,6 +363,36 @@ describe("Doctor host-configuration parity — outbound email regression", () => expect(JSON.stringify(mailCheck)).not.toContain(smtpPassword); }); + it("probes with the SMTP client identity the runtime would use, and reports it", async () => { + const services = dependencies(); + + const { report } = await diagnose( + environment({ ...mailEnvironment, EKKLESIA_SMTP_HELO_NAME: "mail.example.org" }), + services, + ); + const mailCheck = check(report, "outbound-email.configuration"); + + // Doctor builds its provider from the same validated configuration `startApi` does, so a relay + // that refuses the greeting fails here rather than only when a reset email silently never + // arrives. The name is announced in the clear on every connection, so showing it leaks nothing. + expect(services.probeMail).toHaveBeenCalledWith( + expect.objectContaining({ heloName: "mail.example.org" }), + ); + expect(mailCheck.details[0]).toContain("HELO mail.example.org"); + expect(JSON.stringify(mailCheck)).not.toContain(smtpPassword); + }); + + it("reports a malformed SMTP client identity as a typed failure, naming no value", async () => { + const { report } = await diagnose( + environment({ ...mailEnvironment, EKKLESIA_SMTP_HELO_NAME: "smtp://internal-host:2525" }), + ); + const mailCheck = check(report, "outbound-email.configuration"); + + expect(mailCheck.status).toBe("failure"); + expect(mailCheck.details[0]).toBe("The SMTP settings were rejected: MAIL_HELO_NAME_INVALID."); + expect(JSON.stringify(mailCheck)).not.toContain("internal-host"); + }); + it("leaves a malformed SMTP setting reported as a typed configuration failure", async () => { const { report } = await diagnose( environment({ ...mailEnvironment, EKKLESIA_SMTP_PORT: "70000" }), diff --git a/core/tests/host-configuration-boundaries.test.ts b/core/tests/host-configuration-boundaries.test.ts index 20c5326..63f9399 100644 --- a/core/tests/host-configuration-boundaries.test.ts +++ b/core/tests/host-configuration-boundaries.test.ts @@ -71,6 +71,7 @@ const INTERPRETED_VARIABLES = Object.freeze([ "EKKLESIA_PUBLIC_SIGNUP_ENABLED", "EKKLESIA_SMTP_FROM_ADDRESS", "EKKLESIA_SMTP_FROM_NAME", + "EKKLESIA_SMTP_HELO_NAME", "EKKLESIA_SMTP_HOST", "EKKLESIA_SMTP_PASSWORD", "EKKLESIA_SMTP_PORT", diff --git a/core/tests/host-configuration.test.ts b/core/tests/host-configuration.test.ts index 0e9eabf..39dec57 100644 --- a/core/tests/host-configuration.test.ts +++ b/core/tests/host-configuration.test.ts @@ -20,6 +20,7 @@ const productionEnvironment = Object.freeze({ EKKLESIA_MODULE_SECRET__ANNOUNCEMENTS__STORAGE_CREDENTIAL: "module-secret-material", EKKLESIA_SMTP_FROM_ADDRESS: "noreply@example.org", EKKLESIA_SMTP_FROM_NAME: "Ekklesia", + EKKLESIA_SMTP_HELO_NAME: "mail.example.org", EKKLESIA_SMTP_HOST: "smtp.example.org", EKKLESIA_SMTP_PASSWORD: "sup3r-secret-relay-password", EKKLESIA_SMTP_PORT: "587", @@ -67,6 +68,7 @@ describe("host configuration loading", () => { expect(host.mail).toEqual({ fromAddress: "noreply@example.org", fromName: "Ekklesia", + heloName: "mail.example.org", host: "smtp.example.org", password: "sup3r-secret-relay-password", port: "587", @@ -80,6 +82,9 @@ describe("host configuration loading", () => { }); expect(relayByAddress.mail.username).toBeUndefined(); expect(relayByAddress.mail.password).toBeUndefined(); + // The SMTP client identity is optional too, and unset is the ordinary case: it matters only + // where the machine's own hostname is not a fully qualified name the relay accepts. + expect(relayByAddress.mail.heloName).toBeUndefined(); // A source-IP-authorized relay has no credentials at all, and that must still validate. expect(createMailProviderConfig(relayByAddress.mail)).toBeDefined(); }); diff --git a/core/tests/mail-boundaries.test.ts b/core/tests/mail-boundaries.test.ts index 85b1829..67a34c2 100644 --- a/core/tests/mail-boundaries.test.ts +++ b/core/tests/mail-boundaries.test.ts @@ -161,15 +161,44 @@ describe("outbound email stays server-only", () => { }); it("hardcodes no provider hostname in Core", async () => { - // Google Workspace is a deployment target, not the architecture. + // Google Workspace is a deployment target, not the architecture. It is the deployment that + // exposed the generic `EHLO` defect the HELO-name setting fixes, and it is named nowhere the + // code could come to depend on it. for (const file of await readTextFiles("core/src/mail")) { - expect(file.text.toLowerCase(), file.path).not.toContain("gmail"); - expect(file.text.toLowerCase(), file.path).not.toContain("smtp-relay"); - expect(file.text.toLowerCase(), file.path).not.toContain("resend"); - expect(file.text.toLowerCase(), file.path).not.toContain("brevo"); + const text = file.text.toLowerCase(); + for (const forbidden of [ + "gmail", + "smtp-relay", + "googlemail", + "google.com", + "outlook", + "office365", + "sendgrid", + "mailgun", + "postmark", + "resend", + "brevo", + ]) { + expect(text, `${file.path} names ${forbidden}`).not.toContain(forbidden); + } } }); + it("defaults the SMTP client identity to nothing, so no deployment's name is baked in", async () => { + const config = await readFile(path.join(repositoryRoot, "core/src/mail/config.ts"), "utf8"); + const provider = await readFile( + path.join(repositoryRoot, "core/src/mail/smtp-provider.ts"), + "utf8", + ); + + // The value that fixed a real deployment is configuration, never a constant. A fallback literal + // here would be one church's domain compiled into every installation. + expect(config).not.toContain("riverofglorymn"); + expect(provider).not.toContain("riverofglorymn"); + // The provider passes it through and supplies nothing of its own when it is absent. + expect(provider).toContain("config.heloName === undefined ? {} : { name: config.heloName }"); + }); + it("never disables TLS certificate verification", async () => { for (const file of await readTextFiles("core/src/mail")) { expect(file.text, file.path).not.toContain("rejectUnauthorized"); diff --git a/core/tests/mail-contract.test.ts b/core/tests/mail-contract.test.ts index cb0d7dc..9aca67d 100644 --- a/core/tests/mail-contract.test.ts +++ b/core/tests/mail-contract.test.ts @@ -193,6 +193,116 @@ describe("host mail configuration", () => { }); }); +describe("SMTP client identity (HELO name)", () => { + /** + * A real deployment defect, and a generic one. + * + * An SMTP client must greet the server with its own identity. Nodemailer derives that from the + * machine hostname and falls back to the address literal `[127.0.0.1]` when the hostname is not + * fully qualified — the normal state of a container. Some relays close the connection on that + * greeting with a `421` before any message is offered, which reaches a caller as an + * indistinguishable `MAIL_PROVIDER_UNAVAILABLE`. The deployment can now say what to greet as. + */ + it("omits the setting entirely when the deployment configures none", () => { + const config = createMailProviderConfig(validConfig); + + // Absent means Nodemailer's own derivation, unchanged. The key is not present at all rather + // than present-and-undefined, so nothing downstream can mistake it for a configured empty name. + expect(config).not.toHaveProperty("heloName"); + expect(createMailProviderConfig({ ...validConfig, heloName: "" })).not.toHaveProperty( + "heloName", + ); + expect(createMailProviderConfig({ ...validConfig, heloName: undefined })).not.toHaveProperty( + "heloName", + ); + }); + + it.each([ + ["a two-label domain", "example.org"], + ["a subdomain", "mail.example.org"], + ["a deep subdomain", "smtp.relay.mail.example.org"], + ["hyphens inside labels", "out-bound.example-church.org"], + ["digits", "mx1.example2.org"], + ["mixed case", "Mail.Example.ORG"], + ])("accepts %s", (_label, heloName) => { + expect(createMailProviderConfig({ ...validConfig, heloName })).toMatchObject({ heloName }); + }); + + it.each([ + ["a scheme", "smtp://example.org"], + ["a port", "example.org:587"], + ["a path", "example.org/relay"], + ["a space", "example org"], + ["a leading space", " example.org"], + ["a trailing space", "example.org "], + ["a carriage return", "example.org\r"], + ["a line feed", "example.org\nEHLO evil.example.net"], + ["a tab", "example.org\t"], + ["a comma", "example.org,evil.example.net"], + ["an unqualified single label", "localhost"], + ["a machine name with no domain", "ekklesia-host"], + ["a bracketed address literal", "[127.0.0.1]"], + ["a bare IPv4 address", "192.0.2.1"], + ["a trailing dot", "example.org."], + ["a leading dot", ".example.org"], + ["consecutive dots", "example..org"], + ["a leading hyphen in a label", "-example.org"], + ["a trailing hyphen in a label", "example-.org"], + ["an email address", "noreply@example.org"], + ["prose", "our church mail server"], + ["nothing but dots", "..."], + ["an over-long name", `${"a".repeat(250)}.example.org`], + ])("refuses %s", (_label, heloName) => { + expect(() => createMailProviderConfig({ ...validConfig, heloName })).toThrow( + expect.objectContaining({ code: "MAIL_HELO_NAME_INVALID" }), + ); + }); + + it("refuses an unqualified name, because that is the input the fallback already rejects", () => { + // Configuring a single label would reproduce the very failure this setting exists to fix: + // Nodemailer discards a non-FQDN hostname and substitutes the address literal. + expect(() => createMailProviderConfig({ ...validConfig, heloName: "ekklesia" })).toThrow( + MailConfigError, + ); + }); + + it("never carries the offending value in the error", () => { + try { + createMailProviderConfig({ ...validConfig, heloName: "smtp://secret-internal-name:2525" }); + throw new Error("expected a refusal"); + } catch (error) { + const serialized = `${(error as Error).message} ${(error as Error).stack ?? ""}`; + expect(serialized).not.toContain("secret-internal-name"); + expect(serialized).not.toContain("2525"); + } + }); + + it("is described to an operator, because it is infrastructure rather than a credential", () => { + const described = describeMailProvider( + createMailProviderConfig({ + ...validConfig, + heloName: "mail.example.org", + password, + username: "relay@example.org", + }) as Parameters[0], + ); + + // It is announced in the clear on every connection, so printing it leaks nothing — and it is + // exactly what an operator needs when a relay has refused the greeting. + expect(described).toContain("HELO mail.example.org"); + expect(described).not.toContain(password); + expect(described).not.toContain("relay@example.org"); + }); + + it("says nothing about a HELO name when none is configured", () => { + const described = describeMailProvider( + createMailProviderConfig(validConfig) as Parameters[0], + ); + + expect(described).not.toContain("HELO"); + }); +}); + describe("transactional message validation", () => { const message = { subject: "Verify your email address", diff --git a/core/tests/mail-smtp.integration.test.ts b/core/tests/mail-smtp.integration.test.ts index 2b56112..9613f71 100644 --- a/core/tests/mail-smtp.integration.test.ts +++ b/core/tests/mail-smtp.integration.test.ts @@ -1,4 +1,4 @@ -import type { AddressInfo } from "node:net"; +import type { AddressInfo, Socket } from "node:net"; import { createServer, type Server } from "node:net"; import { SMTPServer } from "smtp-server"; import { afterEach, describe, expect, it } from "vitest"; @@ -25,6 +25,8 @@ interface CapturedMessage { readonly from: string; readonly to: readonly string[]; readonly data: string; + /** The argument this client actually sent in `EHLO`, lowercased by the server. */ + readonly greetedAs: string; } interface Harness { @@ -52,6 +54,14 @@ interface HarnessOptions { readonly rejectRecipients?: readonly string[]; /** Advertises STARTTLS. Off by default, so a `starttls` client must refuse to continue. */ readonly offerStartTls?: boolean; + /** + * Refuses any client whose `EHLO` argument is not exactly this name, as a relay that requires a + * fully qualified client identity does. + * + * The refusal is a `421`, the same class of response the real relay returned: a temporary failure + * that closes the connection before a message is offered. + */ + readonly requireGreeting?: string; } async function startSmtpServer(options: HarnessOptions = {}): Promise { @@ -81,11 +91,26 @@ async function startSmtpServer(options: HarnessOptions = {}): Promise { captured.push({ data, from: session.envelope.mailFrom === false ? "" : session.envelope.mailFrom.address, + greetedAs: session.hostNameAppearsAs, to: session.envelope.rcptTo.map((entry) => entry.address), }); callback(); }); }, + onMailFrom(_address, session, callback) { + if ( + options.requireGreeting !== undefined && + session.hostNameAppearsAs !== options.requireGreeting.toLowerCase() + ) { + const error = new Error("Try again later, closing connection.") as Error & { + responseCode?: number; + }; + error.responseCode = 421; + callback(error); + return; + } + callback(); + }, onRcptTo(address, _session, callback) { if (options.rejectRecipients?.includes(address.address)) { // 550 is a permanent refusal of this mailbox, which is what a real relay returns. @@ -107,6 +132,64 @@ async function startSmtpServer(options: HarnessOptions = {}): Promise { return { captured, close, port: address.port }; } +/** + * A relay that refuses the greeting itself, byte for byte as the real one did. + * + * `smtp-server` exposes no `EHLO` hook — its earliest usable one is `MAIL FROM`, which + * `transporter.verify()` never reaches — so modelling this failure at the point it actually happens + * needs a socket that speaks the handful of SMTP verbs involved. That matters: the whole reason the + * defect was hard to diagnose is that it aborts the session *before* a message is offered, which is + * also why Doctor's send-nothing probe can see it. + */ +async function startGreetingStrictRelay(expected: string): Promise { + const sockets = new Set(); + const server = createServer((socket) => { + sockets.add(socket); + socket.on("close", () => sockets.delete(socket)); + socket.on("error", () => undefined); + socket.write("220 relay.invalid ESMTP\r\n"); + let buffer = ""; + socket.on("data", (chunk: Buffer) => { + buffer += chunk.toString("utf8"); + let index = buffer.indexOf("\r\n"); + while (index !== -1) { + const line = buffer.slice(0, index); + buffer = buffer.slice(index + 2); + const [verb = "", argument = ""] = line.split(" "); + if (verb.toUpperCase() === "EHLO" || verb.toUpperCase() === "HELO") { + if (argument.toLowerCase() !== expected.toLowerCase()) { + // The exact shape the deployment saw: a 421 naming no reason, then the door shut. + socket.write("421-4.7.0 Try again later, closing connection. (EHLO)\r\n"); + socket.write("421 4.7.0 Goodbye\r\n"); + socket.end(); + return; + } + socket.write("250-relay.invalid\r\n250 OK\r\n"); + } else if (verb.toUpperCase() === "QUIT") { + socket.write("221 Bye\r\n"); + socket.end(); + return; + } else { + socket.write("250 OK\r\n"); + } + index = buffer.indexOf("\r\n"); + } + }); + }); + + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address() as AddressInfo; + const close = async (): Promise => { + // `close()` waits for open sockets, and a refused client may still be holding one. + for (const socket of sockets) { + socket.destroy(); + } + await new Promise((resolve) => server.close(() => resolve())); + }; + openHarnesses.push(close); + return { captured: [], close, port: address.port }; +} + function config(port: number, overrides: Record = {}): MailProviderConfig { const built = createMailProviderConfig( { @@ -355,3 +438,94 @@ describe.sequential("live SMTP transport", () => { expect(harness.captured).toHaveLength(1); }); }); + +describe("SMTP client identity on the wire", () => { + /** + * The regression guard for a real production defect. + * + * A deployment relaying through an IP-authorized relay could not send at all: every attempt + * surfaced as `MAIL_PROVIDER_UNAVAILABLE`. The relay was healthy — `swaks` sent from the same + * machine — and the cause was the greeting. An SMTP client identifies itself in `EHLO`, and with + * nothing configured Nodemailer derives that from the machine hostname, substituting the address + * literal `[127.0.0.1]` whenever the hostname is not fully qualified. The relay closed the + * connection on it with a `421`. + * + * These tests are written against the wire rather than against the transport options, because the + * defect was invisible in the options: everything about the configuration looked correct. + */ + it("greets with the configured name, exactly as configured", async () => { + const harness = await startSmtpServer(); + const mail = provider(config(harness.port, { heloName: "mail.example.org" })); + + await mail.send( + validateMailMessage({ subject: "Greeting", text: "Body.", to: "person@example.org" }), + ); + + expect(harness.captured[0]?.greetedAs).toBe("mail.example.org"); + }); + + it("never greets as the address literal when a name is configured", async () => { + const harness = await startSmtpServer(); + const mail = provider(config(harness.port, { heloName: "mail.example.org" })); + + await mail.send( + validateMailMessage({ subject: "Greeting", text: "Body.", to: "person@example.org" }), + ); + + // The exact greeting the relay refused. + expect(harness.captured[0]?.greetedAs).not.toBe("[127.0.0.1]"); + }); + + it("delivers to a relay that demands a fully qualified greeting", async () => { + const harness = await startSmtpServer({ requireGreeting: "mail.example.org" }); + const mail = provider(config(harness.port, { heloName: "mail.example.org" })); + + await mail.send( + validateMailMessage({ subject: "Accepted", text: "Body.", to: "person@example.org" }), + ); + + expect(harness.captured).toHaveLength(1); + expect(harness.captured[0]?.greetedAs).toBe("mail.example.org"); + }); + + it("is refused by that same relay when no name is configured", async () => { + const harness = await startGreetingStrictRelay("mail.example.org"); + const mail = provider(config(harness.port)); + + // Deterministic regardless of what this machine is called: the server accepts exactly one name, + // and Nodemailer's derivation cannot produce `mail.example.org` on any host. + await expect( + mail.send(validateMailMessage({ subject: "Refused", text: "Body.", to: "p@example.org" })), + ).rejects.toMatchObject({ code: "MAIL_PROVIDER_UNAVAILABLE" }); + expect(harness.captured).toHaveLength(0); + }); + + it("carries the refused greeting into no error a caller can see", async () => { + const harness = await startGreetingStrictRelay("mail.example.org"); + const mail = provider(config(harness.port)); + + const failure = await mail + .send(validateMailMessage({ subject: "Refused", text: "Body.", to: "p@example.org" })) + .catch((error: unknown) => error); + + // A `421` is SMTP's "try later", so it stays `MAIL_PROVIDER_UNAVAILABLE` rather than becoming a + // permanent rejection — and the server's response text never reaches the caller. + const serialized = `${(failure as Error).message} ${(failure as Error).stack ?? ""}`; + expect(serialized).not.toContain("Try again later"); + expect(serialized).not.toContain("closing connection"); + }); + + it("still verifies through the same greeting Doctor's probe will use", async () => { + const harness = await startGreetingStrictRelay("mail.example.org"); + const configured = provider(config(harness.port, { heloName: "mail.example.org" })); + const unconfigured = provider(config(harness.port)); + + // Doctor builds its provider from the identical validated configuration, so an operator whose + // relay refuses the greeting sees it in `pnpm doctor` rather than only when a reset email + // silently fails to arrive. + await expect(configured.verify()).resolves.toBeUndefined(); + await expect(unconfigured.verify()).rejects.toMatchObject({ + code: "MAIL_PROVIDER_UNAVAILABLE", + }); + }); +}); diff --git a/docs/development/email.md b/docs/development/email.md index 60e161a..c28973f 100644 --- a/docs/development/email.md +++ b/docs/development/email.md @@ -22,6 +22,7 @@ a new account or recover a password, because both need a message to arrive. See | `EKKLESIA_SMTP_USERNAME` / `EKKLESIA_SMTP_PASSWORD` | Optional **as a pair**. Half a pair is refused. | | `EKKLESIA_SMTP_FROM_ADDRESS` | Required once a host is set. The only address this platform can send as. | | `EKKLESIA_SMTP_FROM_NAME` | Optional display name. | +| `EKKLESIA_SMTP_HELO_NAME` | Optional. The name this client announces in `EHLO`. See [When a relay refuses the connection](#when-a-relay-refuses-the-connection). | ### Security modes are explicit @@ -36,6 +37,51 @@ plaintext session waiting to happen. Naming the mode removes the ambiguity: Certificate verification is always on and there is no setting that turns it off. +### When a relay refuses the connection + +If a relay closes the connection before accepting anything — a `421`, or an +`MAIL_PROVIDER_UNAVAILABLE` from a relay you can reach perfectly well with another client — suspect +the **greeting** before suspecting the network. + +An SMTP client has to identify itself in the `EHLO` command, and Ekklesia does not invent that name. +With nothing configured, Nodemailer derives it from the machine's own hostname, and substitutes the +address literal `[127.0.0.1]` whenever that hostname is not fully qualified — which is the normal +state of a container, or of a VM nobody has named. A relay that requires a resolvable client identity +rejects that greeting outright, and because the rejection lands before any message is offered it is +indistinguishable from the relay being down. + +This was a real deployment defect, and a generic one: the relay worked, `swaks` sent from the same +machine, and only the greeting differed. + +```text +C: EHLO [127.0.0.1] +S: 421-4.7.0 Try again later, closing connection. (EHLO) +``` + +Set the name explicitly: + +```bash +EKKLESIA_SMTP_HELO_NAME=example.org +``` + +```text +C: EHLO example.org +S: 250 ... +``` + +Use a fully qualified domain name the deployment owns, typically its sending domain. The value is +validated at startup: no scheme, no port, no brackets, no whitespace, no trailing dot, and at least +one label plus a domain — a single unqualified name such as `localhost` is refused, because that is +exactly the input Nodemailer already discards. + +It is **not** a credential. It travels in the clear on every connection, so it appears in +`pnpm doctor` output where an operator can check it against what the relay expects. + +Leaving it unset preserves Nodemailer's own behaviour exactly. It is deliberately not defaulted to +the sender's domain: a machine whose hostname *is* a valid FQDN may already be greeting correctly — +some relays check the greeting against the connecting address's reverse DNS — and silently replacing +a working identity would trade one deployment's bug for another's. + ### Google Workspace SMTP Relay The expected first production deployment, and a compatibility target rather than a dependency — no @@ -58,6 +104,11 @@ controls which envelope senders are accepted. deployment relies on IP authorization, the server's egress address has to be registered, and a change of hosting provider or NAT address will break sending until it is updated. +Such a relay is also likely to require a fully qualified `EHLO` name — this is the deployment that +exposed the generic issue described in +[When a relay refuses the connection](#when-a-relay-refuses-the-connection) — so +`EKKLESIA_SMTP_HELO_NAME` is usually needed alongside the variables above. + The same variables address Microsoft 365, a transactional provider's SMTP endpoint, a local Postfix, or anything else that speaks the protocol. Switching between them is configuration, not code. diff --git a/docs/development/repository-workflow.md b/docs/development/repository-workflow.md index e1a1442..7525aad 100644 --- a/docs/development/repository-workflow.md +++ b/docs/development/repository-workflow.md @@ -99,7 +99,10 @@ Doctor reports a fixed, deterministic set of checks: logical bucket are reported separately, because binding is not health. - Outbound email: skipped when no `EKKLESIA_SMTP_HOST` is set, a blocking failure when the `EKKLESIA_SMTP_*` settings are structurally invalid, and otherwise a connection that authenticates - where credentials are set and **sends nothing**. + where credentials are set and **sends nothing**. That probe performs the same `EHLO` greeting the + runtime performs, so a relay that refuses the greeting is reported here rather than discovered when + a message silently fails to arrive — see + [Outbound email](email.md#when-a-relay-refuses-the-connection). No check prints a credential: not the storage access or secret key, not the SMTP password, not the identity secret, not a module secret value. diff --git a/docs/development/security.md b/docs/development/security.md index 86642ec..d627ff5 100644 --- a/docs/development/security.md +++ b/docs/development/security.md @@ -614,6 +614,10 @@ becoming a liability: disables it. - **The password is a deployment secret.** Never in PostgreSQL, never in module configuration, never in an API response, never in Doctor output, never in an error, and no value in `.env.example`. +- **The SMTP client identity is not a secret.** `EKKLESIA_SMTP_HELO_NAME` is announced in the clear + in `EHLO` on every connection, so it is shown in Doctor output where an operator can compare it + with what the relay expects. It is validated as a fully qualified domain name — no scheme, port, + brackets, whitespace, or control characters — so nothing arbitrary reaches the wire. - **Nothing is logged.** The transport logs nothing at all. Transactional mail carries verification and reset links, which are exactly the credentials those flows depend on. - **No blind retry.** One bounded attempt. A failure after the message data has been sent is