diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a8926f..1733a8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ What counts as a breaking change — and what does not — is defined in [API st - **`MailError` says why a send failed and whether it may have been delivered.** `kind` is a new `jasil.providers.MailFailure` naming the failed step (`auth_failed`, `sender_refused`, `recipient_refused`, `message_rejected`, `unreachable`, `connection_lost`, `deadline`, ...). `delivery_unknown` is `True` only when the connection was lost, or the deadline fired, after the message body started going out. `message_id` is set on every failure after the message was built. All three are keyword-only with defaults, so existing code is unaffected. See [MailProvider](docs/providers-and-backends.md#mailprovider). ([#52](https://github.com/endurain-project/jasil/issues/52), [#53](https://github.com/endurain-project/jasil/issues/53)) +- **`MailMessage.from_address`** sets the sender of one message (`From`, envelope sender and `Message-ID` domain) in place of `mail.from_address`, which becomes the default. It is validated like the configured sender, and JASIL keeps no allowlist: take it from your own configuration, never from user input. See [MailProvider](docs/providers-and-backends.md#mailprovider). ([#54](https://github.com/endurain-project/jasil/issues/54)) + ### Changed - `SmtpMail` sends `MAIL FROM`, `RCPT TO` and `DATA` itself instead of through `smtplib`'s `send_message`, so it knows which step failed. The conversation on the wire is unchanged, including the `SIZE` declaration and success with a warning when only some recipients are refused. A refused `MAIL FROM` is now reported as a sender refusal, no longer as a rejected message. diff --git a/docs/configuration.md b/docs/configuration.md index a7772f4..b8197e2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -135,7 +135,7 @@ Settings of the `local://` backend; `s3://` takes none. |---|---|---| | `uri` | `None` | `memory://`, `smtp://host[:port]` (STARTTLS, port 587) or `smtps://host[:port]` (implicit TLS, port 465). Unset disables mail: `send` raises `MailNotConfiguredError`. No userinfo, path or query. The host is a DNS name or an IP literal. `memory://` is refused in a multi-process deployment. | | `secure` | `True` | Verified TLS. `False` allows cleartext `smtp://` and only to a host that resolves exclusively to private, loopback or link-local addresses (re-checked on every send, and the connection is pinned to the checked addresses, tried in order; the shared address space `100.64.0.0/10` used by carrier-grade NAT and Tailscale is not private for this purpose), with no `username` / `password`; anything else, including `memory://`, fails `build_platform`. | -| `from_address` | `""` | `"addr"` or `"Name "`. Required for the SMTP schemes. Independent of `username`. | +| `from_address` | `""` | `"addr"` or `"Name "`: the default sender, used when a `MailMessage` sets no `from_address`. Required for the SMTP schemes. Independent of `username`. | | `username` | `None` | SMTP login. Must be set together with `password`. | | `password` | `None` | SMTP password. Hidden from `repr`. | | `ehlo_hostname` | `None` | Name announced in `EHLO`. Defaults to the machine's hostname; set a fully qualified one for a relay that rejects a bare name. | diff --git a/docs/providers-and-backends.md b/docs/providers-and-backends.md index 7428e9f..74c71c4 100644 --- a/docs/providers-and-backends.md +++ b/docs/providers-and-backends.md @@ -563,10 +563,14 @@ One `SmtpMail` send is bounded by `MAIL_SEND_DEADLINE_SECONDS` (90 seconds; DNS lookups are not counted). Import it to size a lease or timeout around a send rather than copying the value, which may change in a minor release. -The sender is `from_address`, independent of the SMTP `username`: relays such as -Brevo log you in as a generated identity and reject a `From` that is not a -verified sender. The `From` header keeps any display name; the envelope sender is -the bare address. If a server refuses *some* recipients the send succeeds and a +The sender is the message's `from_address` when set, otherwise the configured +`mail.from_address`; either way it is independent of the SMTP `username`: relays +such as Brevo log you in as a generated identity and reject a `From` that is not +a verified sender. A per-message sender lets one platform send as several +identities (per team, brand or campaign) through the same relay; JASIL keeps no +allowlist, so take it from your own configuration, never from user input. The +`From` header keeps any display name; the envelope sender is the bare address, +and the `Message-ID` uses the sender's domain. If a server refuses *some* recipients the send succeeds and a warning logs the refused count, because failing would make a retry re-send to recipients that already received it. Every recipient of a message is visible in `To`, and a refused one is only counted in that warning — send one message per diff --git a/docs/security/integration-checklist.md b/docs/security/integration-checklist.md index 2cb1963..6ddb6b2 100644 --- a/docs/security/integration-checklist.md +++ b/docs/security/integration-checklist.md @@ -79,6 +79,7 @@ through. - [ ] Use `smtp://` (STARTTLS) or `smtps://`. Cleartext (`mail.secure = False`) is for a relay on the same host or a private network only; it is refused with credentials. - [ ] Put credentials in `MailSettings.username` / `password`, never in the URI. - [ ] Set `from_address` to an identity your relay has verified. +- [ ] Take any per-message `MailMessage.from_address` from your own configuration, never from user input. - [ ] Remember the SMTP host is **not** SSRF-checked — it is expected to be an internal relay. Keep that setting out of attacker-influenced configuration. - [ ] Do not put a rendered body or secrets in an event payload that drives a diff --git a/docs/security/threat-model.md b/docs/security/threat-model.md index 529736a..23c75ee 100644 --- a/docs/security/threat-model.md +++ b/docs/security/threat-model.md @@ -79,6 +79,12 @@ resolves to — including a cloud metadata endpoint — and taking one logs at `SMTP` email policy rejects CR/LF in a header value; and `MailMessage` has no field through which a caller could set an arbitrary header. +**Sender spoofing.** `MailMessage.from_address` lets the host choose the sender +of each message. JASIL validates it as one address and keeps no allowlist: the +relay decides which senders or domains an account may use. A host that took the +sender from user input would let a user send as any identity the relay accepts, +so it must come from the host's own configuration. + **Transport.** TLS is verified by default; `smtp://` refuses a server that does not offer STARTTLS rather than downgrading, and AUTH is sent only after TLS is up. The one opt-out, `mail.secure = False`, is cleartext and is accepted only for diff --git a/jasil/backends/mail_memory.py b/jasil/backends/mail_memory.py index 14e40f5..db703c6 100644 --- a/jasil/backends/mail_memory.py +++ b/jasil/backends/mail_memory.py @@ -45,7 +45,7 @@ class MemoryMail: Args: clock: Time source for the ``Date`` header. - from_address: Sender identity; a placeholder when empty. + from_address: Default sender identity, used when a message sets none; a placeholder when empty. """ def __init__(self, clock: ClockProvider, from_address: str = "") -> None: @@ -68,7 +68,8 @@ def send(self, message: MailMessage) -> str: """ if lease_lost(): raise MailError("Job lease lost; send abandoned", kind=MailFailure.LEASE_LOST) - mime = build_mime(message, sender=self._from_address, now=self._clock.now()) + sender = self._from_address if message.from_address is None else message.from_address + mime = build_mime(message, sender=sender, now=self._clock.now()) # Recipients copied as sent: the caller's list may be reused or mutated # after ``send`` returns, which would rewrite what the test asserts on. A # shallow copy keeps the type, so ``outbox == [message]`` still holds. diff --git a/jasil/backends/mail_smtp.py b/jasil/backends/mail_smtp.py index bf7dbcb..f7a6d3c 100644 --- a/jasil/backends/mail_smtp.py +++ b/jasil/backends/mail_smtp.py @@ -192,8 +192,9 @@ class SmtpMail: implicit_tls: Connect over TLS (``smtps``) instead of STARTTLS. tls: ``False`` sends in cleartext (no STARTTLS). Cannot be combined with credentials or ``implicit_tls``. - from_address: Sender identity (``From`` header; the envelope sender is - its bare address). Independent of ``username``. + from_address: Default sender identity (``From`` header; the envelope + sender is its bare address), used when a message sets none. + Independent of ``username``. clock: Time source for the ``Date`` header. username: Login, or ``None`` for an unauthenticated relay. password: Password for ``username``. @@ -258,7 +259,8 @@ def send(self, message: MailMessage) -> str: """ if lease_lost(): raise MailError("Job lease lost; send abandoned", kind=MailFailure.LEASE_LOST) - mime = build_mime(message, sender=self._from_address, now=self._clock.now()) + sender = self._from_address if message.from_address is None else message.from_address + mime = build_mime(message, sender=sender, now=self._clock.now()) message_id = str(mime["Message-ID"]) try: refused = self._deliver(mime) diff --git a/jasil/providers.py b/jasil/providers.py index 6c70f56..3379da6 100644 --- a/jasil/providers.py +++ b/jasil/providers.py @@ -963,6 +963,11 @@ class MailMessage: html: HTML body. With ``text`` as well, the message is ``multipart/alternative``. reply_to: Optional single ``Reply-To`` address. + from_address: The sender of this message, ``addr`` or ``"Name "``, + in place of the configured ``mail.from_address``: the ``From`` + header, the envelope sender (its bare address) and the + ``Message-ID`` domain. The relay decides which senders it accepts; + never derive this from untrusted input. """ to: Sequence[str] @@ -970,11 +975,12 @@ class MailMessage: text: str | None = None html: str | None = None reply_to: str | None = None + from_address: str | None = None def __hash__(self) -> int: # ``to`` is usually a list, which the generated hash cannot digest. recipients = (self.to,) if isinstance(self.to, str) else tuple(self.to) - return hash((recipients, self.subject, self.text, self.html, self.reply_to)) + return hash((recipients, self.subject, self.text, self.html, self.reply_to, self.from_address)) class MailFailure(StrEnum): diff --git a/jasil/settings.py b/jasil/settings.py index e417449..556f520 100644 --- a/jasil/settings.py +++ b/jasil/settings.py @@ -351,8 +351,9 @@ class MailSettings: ``memory://`` is refused in a multi-process deployment (see ``enforce_deployment_consistency``), where no message would ever be delivered. - from_address: Sender identity, ``"addr"`` or ``"Name "``. Required - for the SMTP schemes. Independent of ``username``. + from_address: Default sender identity, ``"addr"`` or ``"Name "``, + used when a ``MailMessage`` sets no ``from_address``. Required for + the SMTP schemes. Independent of ``username``. secure: ``True`` (the default) requires verified TLS. ``False`` sends over a cleartext connection and is accepted only for ``smtp://``, only when the host resolves exclusively to private, loopback or diff --git a/tests/test_mail.py b/tests/test_mail.py index 47e1a57..8579ece 100644 --- a/tests/test_mail.py +++ b/tests/test_mail.py @@ -406,6 +406,43 @@ def test_an_invalid_message_raises_value_error(self, backend, bad): assert not FakeSMTP.instances +class TestPerMessageSender: + """``MailMessage.from_address`` replaces the configured sender for one message, in every backend.""" + + def test_the_message_sender_is_used_in_place_of_the_configured_one(self, backend): + backend.send(_message(from_address="Billing ")) + + mime = _last_built(backend) + assert mime["From"] == "Billing " + assert mime["Message-ID"].endswith("@brand.example>") + + def test_the_configured_sender_is_the_default(self, backend): + backend.send(_message()) + + assert _last_built(backend)["From"] == SENDER + + def test_the_envelope_sender_is_the_bare_message_sender(self): + _smtp().send(_message(from_address="Billing ")) + + assert FakeSMTP.instances[0].envelope_sender == "billing@brand.example" + + @pytest.mark.parametrize("value", ["", "not an address", "a@b.com, c@d.com", "x\r\nBcc: victim@example.com"]) + def test_an_invalid_message_sender_is_refused_without_echoing_it(self, backend, value): + with pytest.raises(ValueError, match="sender") as caught: + backend.send(_message(from_address=value)) + + assert value not in str(caught.value) or not value + + def test_the_sender_is_part_of_the_message_identity(self): + assert hash(_message(from_address="a@example.com")) != hash(_message(from_address="b@example.com")) + assert _message(from_address="a@example.com") != _message() + + +def _last_built(backend: MailProvider): + """The MIME message a backend built last, from ``MemoryMail.built`` or the fake SMTP server.""" + return backend.built[-1] if isinstance(backend, MemoryMail) else FakeSMTP.instances[-1].sent + + class TestMemoryMail: def test_messages_are_recorded_in_order(self): mail = _memory()