Private
Public Access
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 <noreply@anthropic.com>
This commit is contained in:
@@ -25,13 +25,33 @@ async def _deliver(cfg: SmtpSettings, to_address: str, subject: str, body: str)
|
|||||||
message.set_content(body)
|
message.set_content(body)
|
||||||
|
|
||||||
password = decrypt(cfg.password_encrypted) if cfg.password_encrypted else None
|
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(
|
await aiosmtplib.send(
|
||||||
message,
|
message,
|
||||||
hostname=cfg.host,
|
hostname=cfg.host,
|
||||||
port=cfg.port,
|
port=cfg.port,
|
||||||
username=cfg.username or None,
|
username=cfg.username or None,
|
||||||
password=password,
|
password=password,
|
||||||
use_tls=cfg.use_tls,
|
use_tls=use_implicit_tls,
|
||||||
|
start_tls=require_starttls,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -153,6 +153,91 @@ async def test_send_test_email_success(client, db_session, monkeypatch):
|
|||||||
assert calls[0]["hostname"] == "smtp.example.com"
|
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 test_send_test_email_surfaces_failure(client, db_session, monkeypatch):
|
||||||
async def fake_send(message, **kwargs):
|
async def fake_send(message, **kwargs):
|
||||||
raise ConnectionRefusedError("boom")
|
raise ConnectionRefusedError("boom")
|
||||||
|
|||||||
Reference in New Issue
Block a user