mirror of
https://github.com/suitenumerique/messages.git
synced 2026-08-17 21:25:41 +02:00
🐛(mda) add To header to outbound mails missing one (#712)
Mails sent without a To recipient (Bcc- or Cc-only) ship with no To header, which anti-spam filters treat as a negative signal. Inject an RFC 4356 empty-group "To: undisclosed-recipients:;" before DKIM signing so it's covered by the signature.
This commit is contained in:
@@ -13,7 +13,7 @@ from django.core.exceptions import ValidationError as DjangoValidationError
|
||||
from django.db import transaction
|
||||
|
||||
from drf_spectacular.utils import extend_schema
|
||||
from jmap_email import parse_email
|
||||
from jmap_email import find_headers, parse_email
|
||||
from rest_framework import status
|
||||
from rest_framework.exceptions import PermissionDenied
|
||||
from rest_framework.response import Response
|
||||
@@ -127,6 +127,20 @@ class SubmitRawEmailView(APIView):
|
||||
status=status.HTTP_403_FORBIDDEN,
|
||||
)
|
||||
|
||||
# Bcc must travel in the X-Rcpt-To envelope, never in the MIME: a Bcc
|
||||
# header would be signed and delivered visibly to every recipient
|
||||
# (RFC 5322 §3.6.3). Reject rather than silently rewrite caller bytes.
|
||||
if find_headers(parsed, "bcc"):
|
||||
return Response(
|
||||
{
|
||||
"detail": (
|
||||
"Bcc headers are not allowed; pass blind recipients "
|
||||
"via the X-Rcpt-To envelope."
|
||||
)
|
||||
},
|
||||
status=status.HTTP_400_BAD_REQUEST,
|
||||
)
|
||||
|
||||
# Create the message, sign it, and arm the SMTP dispatch atomically.
|
||||
# The whole thing rolls back on any failure (no orphan draft), and the
|
||||
# Celery task is dispatched via ``transaction.on_commit`` so the broker
|
||||
|
||||
@@ -13,6 +13,7 @@ from django.utils import timezone
|
||||
import rest_framework as drf
|
||||
from jmap_email import (
|
||||
compose_email,
|
||||
find_header,
|
||||
first_address_email,
|
||||
parse_email,
|
||||
)
|
||||
@@ -106,6 +107,13 @@ def validate_attachments_size(total_size: int, message_id: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
# When a message has no To recipient, emit a "To:" header using empty-group
|
||||
# syntax (RFC 4356 §3). A missing To header is a common anti-spam negative
|
||||
# signal (and resembles a DKIM-replay shape), so this keeps such sends —
|
||||
# typically Bcc-only — looking legitimate without disclosing anyone.
|
||||
UNDISCLOSED_RECIPIENTS_TO_HEADER = b"To: undisclosed-recipients:;"
|
||||
|
||||
|
||||
def compose_and_sign_mime(
|
||||
message: models.Message,
|
||||
mailbox: models.Mailbox,
|
||||
@@ -199,6 +207,11 @@ def compose_and_sign_mime(
|
||||
prepend_headers=prepend_headers,
|
||||
)
|
||||
|
||||
# Bcc/Cc-only send: the composed MIME has no To header. Add the empty-group
|
||||
# placeholder before signing so it is covered by DKIM.
|
||||
if not mime_data["to"]:
|
||||
raw_mime = UNDISCLOSED_RECIPIENTS_TO_HEADER + b"\r\n" + raw_mime
|
||||
|
||||
dkim_header = sign_message_dkim(raw_mime, mailbox.domain)
|
||||
if dkim_header:
|
||||
raw_mime = dkim_header + b"\r\n" + raw_mime
|
||||
@@ -311,6 +324,12 @@ def prepare_outbound_message(
|
||||
# atomic for just the Blob INSERT + FK-establishing save —
|
||||
# this keeps the per-sha advisory lock taken inside
|
||||
# ``create_blob`` held for ms, not for the duration of DKIM.
|
||||
# Caller-supplied MIME may also lack a To header (e.g. Bcc-only); add
|
||||
# the placeholder before signing. ``parse_email`` returns None on
|
||||
# unparseable input (already rejected upstream by the submit view).
|
||||
parsed = parse_email(raw_mime)
|
||||
if parsed is not None and not find_header(parsed, "to"):
|
||||
raw_mime = UNDISCLOSED_RECIPIENTS_TO_HEADER + b"\r\n" + raw_mime
|
||||
signed_mime = _sign_mime(mailbox_sender, raw_mime)
|
||||
validate_mime_size(len(signed_mime), message.id)
|
||||
message.sender_user = user
|
||||
|
||||
@@ -343,6 +343,33 @@ class TestSubmitValidation:
|
||||
)
|
||||
assert response.status_code == 400
|
||||
|
||||
def test_bcc_header_returns_400(self, client, auth_header, mailbox):
|
||||
"""A Bcc header in the submitted MIME is rejected: blind recipients
|
||||
belong in the X-Rcpt-To envelope, not in the (signed, delivered)
|
||||
headers."""
|
||||
mime_with_bcc = (
|
||||
b"From: contact@company.com\r\n"
|
||||
b"To: attendee@example.com\r\n"
|
||||
b"Bcc: secret@example.com\r\n"
|
||||
b"Subject: With Bcc\r\n"
|
||||
b"Message-ID: <bcc-reject@company.com>\r\n"
|
||||
b"Date: Mon, 30 Mar 2026 10:00:00 +0000\r\n"
|
||||
b"MIME-Version: 1.0\r\n"
|
||||
b"Content-Type: text/plain; charset=utf-8\r\n"
|
||||
b"\r\n"
|
||||
b"Hello world\r\n"
|
||||
)
|
||||
response = client.post(
|
||||
SUBMIT_URL,
|
||||
data=mime_with_bcc,
|
||||
content_type="message/rfc822",
|
||||
HTTP_X_MAIL_FROM=str(mailbox.id),
|
||||
HTTP_X_RCPT_TO="attendee@example.com",
|
||||
**auth_header,
|
||||
)
|
||||
assert response.status_code == 400
|
||||
assert "Bcc" in response.json()["detail"]
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# Message creation + DKIM signing + async dispatch
|
||||
@@ -594,6 +621,49 @@ class TestSubmitIntegration:
|
||||
type=MessageRecipientTypeChoices.BCC,
|
||||
).exists()
|
||||
|
||||
@patch(TASK_MOCK)
|
||||
def test_to_less_submission_gets_undisclosed_recipients(
|
||||
self, mock_task, client, auth_header, mailbox
|
||||
):
|
||||
"""A submission with no To/Cc header (every recipient travels via
|
||||
X-Rcpt-To) gets the empty-group placeholder in the stored blob, so it
|
||||
isn't flagged for a missing To."""
|
||||
mailbox_email = str(mailbox)
|
||||
# No To/Cc header at all.
|
||||
mime = (
|
||||
f"From: {mailbox_email}\r\n"
|
||||
f"Subject: No To header\r\n"
|
||||
f"Message-ID: <noto@example.com>\r\n"
|
||||
f"Date: Mon, 30 Mar 2026 10:00:00 +0000\r\n"
|
||||
f"MIME-Version: 1.0\r\n"
|
||||
f"Content-Type: text/plain\r\n"
|
||||
f"\r\n"
|
||||
f"body\r\n"
|
||||
).encode()
|
||||
|
||||
response = client.post(
|
||||
SUBMIT_URL,
|
||||
data=mime,
|
||||
content_type="message/rfc822",
|
||||
HTTP_X_MAIL_FROM=str(mailbox.id),
|
||||
HTTP_X_RCPT_TO="hidden@example.com",
|
||||
**auth_header,
|
||||
)
|
||||
|
||||
assert response.status_code == 202, response.content
|
||||
from core.enums import MessageRecipientTypeChoices
|
||||
from core.models import Message
|
||||
|
||||
message = Message.objects.get(id=response.json()["message_id"])
|
||||
# The envelope-only recipient is stored as BCC, never in the MIME.
|
||||
assert message.recipients.filter(
|
||||
contact__email="hidden@example.com",
|
||||
type=MessageRecipientTypeChoices.BCC,
|
||||
).exists()
|
||||
content = message.blob.get_content()
|
||||
assert b"To: undisclosed-recipients:;" in content
|
||||
assert b"hidden@example.com" not in content
|
||||
|
||||
@patch(TASK_MOCK)
|
||||
def test_cc_recipients_created(self, mock_task, client, auth_header, mailbox):
|
||||
"""Cc recipients from MIME headers are created as MessageRecipient rows."""
|
||||
|
||||
@@ -12,6 +12,7 @@ from django.test import TransactionTestCase, override_settings
|
||||
import dns.resolver
|
||||
import pytest
|
||||
import rest_framework as drf
|
||||
from dkim import verify as dkim_verify
|
||||
|
||||
from core import enums, factories, models
|
||||
from core.mda import outbound
|
||||
@@ -32,6 +33,9 @@ SCHEMA_CUSTOM_ATTRIBUTES = {
|
||||
"required": [],
|
||||
}
|
||||
|
||||
# Module-level DKIM keypair (1024-bit for speed) used by the signing tests.
|
||||
_TEST_DKIM_PRIVATE_KEY, _TEST_DKIM_PUBLIC_KEY = generate_dkim_key(key_size=1024)
|
||||
|
||||
|
||||
@pytest.fixture(name="user")
|
||||
def fixture_user():
|
||||
@@ -910,6 +914,185 @@ class TestPrepareOutboundMessageReadAt:
|
||||
assert access.read_at >= message.created_at
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
class TestUndisclosedRecipientsHeader:
|
||||
"""A message with no To recipient (e.g. Bcc-only) must get an empty-group
|
||||
``To: undisclosed-recipients:;`` header (RFC 4356) so receivers don't
|
||||
flag the missing To as a spam signal. Messages that already have a
|
||||
visible To must be left untouched."""
|
||||
|
||||
def _make_draft(self, mailbox_sender):
|
||||
return factories.MessageFactory(
|
||||
thread=factories.ThreadFactory(),
|
||||
sender=factories.ContactFactory(mailbox=mailbox_sender),
|
||||
is_draft=True,
|
||||
subject="Test Message",
|
||||
signature=None,
|
||||
)
|
||||
|
||||
def _add_recipient(self, message, mailbox_sender, email, kind):
|
||||
factories.MessageRecipientFactory(
|
||||
message=message,
|
||||
contact=factories.ContactFactory(mailbox=mailbox_sender, email=email),
|
||||
type=kind,
|
||||
)
|
||||
|
||||
def test_bcc_only_message_gets_undisclosed_to_header(
|
||||
self, user, mailbox_sender, mailbox_access
|
||||
):
|
||||
"""Bcc-only send → placeholder added, Bcc address never leaked."""
|
||||
message = self._make_draft(mailbox_sender)
|
||||
self._add_recipient(
|
||||
message,
|
||||
mailbox_sender,
|
||||
"bcc@example.com",
|
||||
models.MessageRecipientTypeChoices.BCC,
|
||||
)
|
||||
|
||||
assert (
|
||||
outbound.prepare_outbound_message(
|
||||
mailbox_sender, message, "Hello", "<p>Hello</p>", user
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
message.refresh_from_db()
|
||||
content = message.blob.get_content().decode()
|
||||
assert "To: undisclosed-recipients:;" in content
|
||||
# The Bcc recipient must never end up in a visible header.
|
||||
assert "bcc@example.com" not in content
|
||||
|
||||
def test_to_recipient_suppresses_placeholder(
|
||||
self, user, mailbox_sender, mailbox_access
|
||||
):
|
||||
"""A visible To recipient → no placeholder, real To preserved."""
|
||||
message = self._make_draft(mailbox_sender)
|
||||
self._add_recipient(
|
||||
message,
|
||||
mailbox_sender,
|
||||
"to@example.com",
|
||||
models.MessageRecipientTypeChoices.TO,
|
||||
)
|
||||
self._add_recipient(
|
||||
message,
|
||||
mailbox_sender,
|
||||
"bcc@example.com",
|
||||
models.MessageRecipientTypeChoices.BCC,
|
||||
)
|
||||
|
||||
outbound.prepare_outbound_message(
|
||||
mailbox_sender, message, "Hello", "<p>Hello</p>", user
|
||||
)
|
||||
|
||||
message.refresh_from_db()
|
||||
content = message.blob.get_content().decode()
|
||||
assert "undisclosed-recipients" not in content
|
||||
assert "to@example.com" in content
|
||||
|
||||
def test_cc_only_message_gets_undisclosed_to_header(
|
||||
self, user, mailbox_sender, mailbox_access
|
||||
):
|
||||
"""A Cc-only send still has no To, so it gets the placeholder while
|
||||
keeping the visible Cc recipient."""
|
||||
message = self._make_draft(mailbox_sender)
|
||||
self._add_recipient(
|
||||
message,
|
||||
mailbox_sender,
|
||||
"cc@example.com",
|
||||
models.MessageRecipientTypeChoices.CC,
|
||||
)
|
||||
|
||||
outbound.prepare_outbound_message(
|
||||
mailbox_sender, message, "Hello", "<p>Hello</p>", user
|
||||
)
|
||||
|
||||
message.refresh_from_db()
|
||||
content = message.blob.get_content().decode()
|
||||
assert "To: undisclosed-recipients:;" in content
|
||||
assert "cc@example.com" in content
|
||||
|
||||
def test_raw_mime_submission_without_to_gets_placeholder(
|
||||
self, user, mailbox_sender, mailbox_access
|
||||
):
|
||||
"""The raw-MIME submission path (e.g. a Bcc-only send whose MIME has no
|
||||
To header) is normalized too, before the blob is signed."""
|
||||
message = self._make_draft(mailbox_sender)
|
||||
self._add_recipient(
|
||||
message,
|
||||
mailbox_sender,
|
||||
"bcc@example.com",
|
||||
models.MessageRecipientTypeChoices.BCC,
|
||||
)
|
||||
raw_mime = (
|
||||
b"From: sender@example.com\r\n"
|
||||
b"Subject: Raw MIME test\r\n"
|
||||
b"Date: Mon, 16 Jun 2026 12:00:00 +0000\r\n"
|
||||
b"\r\n"
|
||||
b"Body without a To header.\r\n"
|
||||
)
|
||||
|
||||
assert (
|
||||
outbound.prepare_outbound_message(
|
||||
mailbox_sender, message, "", "", user, raw_mime=raw_mime
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
message.refresh_from_db()
|
||||
content = message.blob.get_content().decode()
|
||||
assert "To: undisclosed-recipients:;" in content
|
||||
# The Bcc recipient must never end up in a visible header.
|
||||
assert "bcc@example.com" not in content
|
||||
|
||||
def test_undisclosed_to_header_is_covered_by_dkim(
|
||||
self, user, mailbox_sender, mailbox_access
|
||||
):
|
||||
"""The placeholder is added before signing, so DKIM (which signs To)
|
||||
covers it — the key improvement over patching the blob afterwards."""
|
||||
dkim_key = models.DKIMKey.objects.create(
|
||||
selector="testselector",
|
||||
private_key=_TEST_DKIM_PRIVATE_KEY,
|
||||
public_key=_TEST_DKIM_PUBLIC_KEY,
|
||||
key_size=1024,
|
||||
is_active=True,
|
||||
domain=mailbox_sender.domain,
|
||||
)
|
||||
message = self._make_draft(mailbox_sender)
|
||||
self._add_recipient(
|
||||
message,
|
||||
mailbox_sender,
|
||||
"bcc@example.com",
|
||||
models.MessageRecipientTypeChoices.BCC,
|
||||
)
|
||||
|
||||
outbound.prepare_outbound_message(
|
||||
mailbox_sender, message, "Hello", "<p>Hello</p>", user
|
||||
)
|
||||
|
||||
message.refresh_from_db()
|
||||
email_source = message.blob.get_content()
|
||||
assert b"To: undisclosed-recipients:;" in email_source
|
||||
assert b"DKIM-Signature:" in email_source
|
||||
|
||||
domain = mailbox_sender.domain.name
|
||||
|
||||
def get_dns_txt(fqdn, **kwargs):
|
||||
if fqdn == f"testselector._domainkey.{domain}.".encode():
|
||||
return f"v=DKIM1; k=rsa; p={dkim_key.public_key}".encode()
|
||||
return None
|
||||
|
||||
assert dkim_verify(email_source, dnsfunc=get_dns_txt), (
|
||||
"DKIM verification failed"
|
||||
)
|
||||
|
||||
# Tampering with the now-signed To header must break the signature,
|
||||
# proving the placeholder is part of the signed header set.
|
||||
tampered = email_source.replace(
|
||||
b"To: undisclosed-recipients:;", b"To: attacker@evil.com"
|
||||
)
|
||||
assert not dkim_verify(tampered, dnsfunc=get_dns_txt)
|
||||
|
||||
|
||||
@pytest.mark.django_db
|
||||
class TestSendMessageDKIMVerification:
|
||||
"""Test DKIM verification in send_message."""
|
||||
|
||||
Reference in New Issue
Block a user