From 691f5975973e429f38543ee354f2ebe9289033e7 Mon Sep 17 00:00:00 2001 From: Keith Smith Date: Sat, 15 Aug 2026 23:09:39 -0600 Subject: [PATCH] Fix SMTP sending forcing implicit TLS regardless of port cfg.use_tls was passed straight through as aiosmtplib's use_tls kwarg, which means implicit TLS -- encrypted from the first byte, port 465's convention. Port 587, what most providers (including the one that surfaced this: DreamHost) document as their primary submission port, needs STARTTLS instead -- a plaintext connection that upgrades in-band. Forcing implicit TLS against a STARTTLS-only port breaks the handshake outright: [SSL: WRONG_VERSION_NUMBER], a client TLS ClientHello sent to a server still expecting a plaintext SMTP greeting. The "Use TLS" checkbox still means "encrypt this connection" -- the fix infers which of the two negotiation modes to use from the port (465 -> implicit, everything else -> STARTTLS), matching the convention every mail client uses. start_tls is passed as an explicit requirement rather than left to aiosmtplib's opportunistic default, so a server that turns out not to support STARTTLS fails loudly instead of silently sending in plaintext despite the admin asking for encryption. Co-Authored-By: Claude Sonnet 5 --- backend/app/services/email_service.py | 22 ++++++- backend/tests/test_smtp_settings.py | 85 +++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/backend/app/services/email_service.py b/backend/app/services/email_service.py index ab77f42..0d7b5f8 100644 --- a/backend/app/services/email_service.py +++ b/backend/app/services/email_service.py @@ -25,13 +25,33 @@ async def _deliver(cfg: SmtpSettings, to_address: str, subject: str, body: str) message.set_content(body) password = decrypt(cfg.password_encrypted) if cfg.password_encrypted else None + + # "Use TLS" means "encrypt this connection", but SMTP has two genuinely + # different ways to do that, and picking the wrong one breaks the + # handshake outright rather than just failing to encrypt -- aiosmtplib's + # own `use_tls` param means *implicit* TLS (encrypted from the first + # byte, port 465's convention); attempting that against a STARTTLS-only + # port produces exactly `[SSL: WRONG_VERSION_NUMBER]` (a client TLS + # ClientHello sent to a server still expecting a plaintext SMTP + # greeting). So the actual negotiation mode has to be inferred from the + # port, matching the convention every mail client uses: 465 is implicit + # TLS, everything else (587, 25, ...) is STARTTLS (plaintext connection, + # then upgrade). `start_tls=True` (rather than leaving it to + # aiosmtplib's opportunistic default) makes the requirement strict -- + # if the server doesn't actually support STARTTLS, this fails loudly + # instead of silently sending in plaintext despite the admin asking for + # encryption. + use_implicit_tls = cfg.use_tls and cfg.port == 465 + require_starttls = cfg.use_tls and cfg.port != 465 + await aiosmtplib.send( message, hostname=cfg.host, port=cfg.port, username=cfg.username or None, password=password, - use_tls=cfg.use_tls, + use_tls=use_implicit_tls, + start_tls=require_starttls, ) diff --git a/backend/tests/test_smtp_settings.py b/backend/tests/test_smtp_settings.py index 5bae548..97ee63b 100644 --- a/backend/tests/test_smtp_settings.py +++ b/backend/tests/test_smtp_settings.py @@ -153,6 +153,91 @@ async def test_send_test_email_success(client, db_session, monkeypatch): assert calls[0]["hostname"] == "smtp.example.com" +async def test_send_test_email_port_587_uses_starttls_not_implicit_tls(client, db_session, monkeypatch): + # Regression test: port 587 (what most providers, e.g. DreamHost, + # document as their primary submission port) needs STARTTLS -- a + # plaintext connection that upgrades in-band -- not implicit TLS + # (encrypted from the first byte, port 465's convention). Passing + # cfg.use_tls straight through as aiosmtplib's `use_tls` kwarg forces + # implicit TLS regardless of port, which breaks the handshake outright + # against a STARTTLS-only server ([SSL: WRONG_VERSION_NUMBER]). + calls = [] + + async def fake_send(message, **kwargs): + calls.append(kwargs) + + monkeypatch.setattr("app.services.email_service.aiosmtplib.send", fake_send) + + admin = await register_and_login(client, db_session, username="admin1") + await _make_admin(db_session, admin["id"]) + await client.put( + "/api/admin/settings/smtp", + json={ + "host": "smtp.example.com", + "port": 587, + "from_address": "noreply@example.com", + "use_tls": True, + }, + ) + + resp = await client.post("/api/admin/settings/smtp/test") + assert resp.status_code == 204 + assert calls[0]["use_tls"] is False + assert calls[0]["start_tls"] is True + + +async def test_send_test_email_port_465_uses_implicit_tls(client, db_session, monkeypatch): + calls = [] + + async def fake_send(message, **kwargs): + calls.append(kwargs) + + monkeypatch.setattr("app.services.email_service.aiosmtplib.send", fake_send) + + admin = await register_and_login(client, db_session, username="admin1") + await _make_admin(db_session, admin["id"]) + await client.put( + "/api/admin/settings/smtp", + json={ + "host": "smtp.example.com", + "port": 465, + "from_address": "noreply@example.com", + "use_tls": True, + }, + ) + + resp = await client.post("/api/admin/settings/smtp/test") + assert resp.status_code == 204 + assert calls[0]["use_tls"] is True + assert calls[0]["start_tls"] is False + + +async def test_send_test_email_tls_disabled_uses_neither_mode(client, db_session, monkeypatch): + calls = [] + + async def fake_send(message, **kwargs): + calls.append(kwargs) + + monkeypatch.setattr("app.services.email_service.aiosmtplib.send", fake_send) + + admin = await register_and_login(client, db_session, username="admin1") + await _make_admin(db_session, admin["id"]) + await client.put( + "/api/admin/settings/smtp", + json={ + "host": "smtp.example.com", + "port": 25, + "from_address": "noreply@example.com", + "use_tls": False, + }, + ) + + resp = await client.post("/api/admin/settings/smtp/test") + assert resp.status_code == 204 + assert calls[0]["use_tls"] is False + assert calls[0]["start_tls"] is False + + async def test_send_test_email_surfaces_failure(client, db_session, monkeypatch): async def fake_send(message, **kwargs): raise ConnectionRefusedError("boom")