Skip to content

Commit 37bb431

Browse files
potiukYour friendly bot
authored andcommitted
[v3-2-test] Validate SMTP server certificate on STARTTLS upgrade (apache#65346)
* Validate SMTP server certificate on STARTTLS upgrade smtplib.SMTP.starttls() does not validate the server certificate unless an SSL context is passed. airflow.utils.email.send_mime_email and the SMTP provider's SmtpHook (both sync get_conn and async aget_conn) were calling starttls() without a context, so the STARTTLS upgrade accepted any certificate and the subsequent login() call could send credentials over a connection terminated by a MITM. Pass the existing SSL-context machinery (the email.ssl_context config in core and the ssl_context connection extra in the provider) to starttls() at all three call sites. The default becomes ssl.create_default_context(), which validates against the system's trusted CAs. Users who intentionally use self-signed certificates can still opt out by setting the value to "none". Generated-by: Claude Opus 4.6 (1M context) following the guidelines at https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions * Add newsfragment and SMTP provider changelog for STARTTLS cert default Document the default behaviour change introduced by passing an SSL context to the STARTTLS upgrade: system-default CA validation now applies to both airflow.utils.email.send_email (via email.ssl_context) and the SMTP provider's SmtpHook (via the ssl_context connection extra). Users who intentionally run against self-signed SMTP servers can preserve the old behaviour by setting the value to "none". (cherry picked from commit 06981d4) Co-authored-by: Jarek Potiuk <jarek@potiuk.com> Generated-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent df98545 commit 37bb431

5 files changed

Lines changed: 79 additions & 32 deletions

File tree

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
The SMTP STARTTLS upgrade performed by ``airflow.utils.email.send_email`` now validates the SMTP server's certificate against the system's trusted CA bundle by default. Previously the ``starttls()`` call was made without an SSL context, so any certificate was accepted.
2+
3+
Deployments that intentionally point Airflow at an SMTP server with a self-signed or otherwise non-validating certificate and need to preserve the previous behaviour must set ``email.ssl_context = "none"`` in ``airflow.cfg``. The ``"default"`` value (now also the default when the option is unset) uses :func:`ssl.create_default_context`. Previously this option applied only to the ``SMTP_SSL`` path; it now applies to the STARTTLS path as well.

airflow-core/src/airflow/utils/email.py

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ def send_mime_email(
266266
raise
267267
else:
268268
if smtp_starttls:
269-
smtp_conn.starttls()
269+
smtp_conn.starttls(context=_get_ssl_context())
270270
if smtp_user and smtp_password:
271271
smtp_conn.login(smtp_user, smtp_password)
272272
log.info("Sent an alert email to %s", e_to)
@@ -292,6 +292,26 @@ def get_email_address_list(addresses: str | Iterable[str]) -> list[str]:
292292
raise TypeError(f"Unexpected argument type: Received '{type(addresses).__name__}'.")
293293

294294

295+
def _get_ssl_context() -> ssl.SSLContext | None:
296+
"""
297+
Return the SSL context configured via the ``email.ssl_context`` option.
298+
299+
``"default"`` produces :func:`ssl.create_default_context`; ``"none"``
300+
returns ``None`` so callers that explicitly want to skip certificate
301+
validation (for example, against a self-signed SMTP server in a
302+
lab environment) can still do so.
303+
"""
304+
ssl_context_string = conf.get("email", "SSL_CONTEXT")
305+
if ssl_context_string == "default":
306+
return ssl.create_default_context()
307+
if ssl_context_string == "none":
308+
return None
309+
raise RuntimeError(
310+
f"The email.ssl_context configuration variable must "
311+
f"be set to 'default' or 'none' and is '{ssl_context_string}."
312+
)
313+
314+
295315
def _get_smtp_connection(host: str, port: int, timeout: int, with_ssl: bool) -> smtplib.SMTP:
296316
"""
297317
Return an SMTP connection to the specified host and port, with optional SSL encryption.
@@ -304,17 +324,7 @@ def _get_smtp_connection(host: str, port: int, timeout: int, with_ssl: bool) ->
304324
"""
305325
if not with_ssl:
306326
return smtplib.SMTP(host=host, port=port, timeout=timeout)
307-
ssl_context_string = conf.get("email", "SSL_CONTEXT")
308-
if ssl_context_string == "default":
309-
ssl_context = ssl.create_default_context()
310-
elif ssl_context_string == "none":
311-
ssl_context = None
312-
else:
313-
raise RuntimeError(
314-
f"The email.ssl_context configuration variable must "
315-
f"be set to 'default' or 'none' and is '{ssl_context_string}."
316-
)
317-
return smtplib.SMTP_SSL(host=host, port=port, timeout=timeout, context=ssl_context)
327+
return smtplib.SMTP_SSL(host=host, port=port, timeout=timeout, context=_get_ssl_context())
318328

319329

320330
def _get_email_list_from_str(addresses: str) -> list[str]:

providers/smtp/docs/changelog.rst

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,19 @@
2727
Changelog
2828
---------
2929

30+
Breaking changes
31+
~~~~~~~~~~~~~~~~
32+
33+
The SMTP STARTTLS upgrade performed by ``SmtpHook.get_conn`` and ``SmtpHook.aget_conn`` now
34+
validates the SMTP server's certificate against the system's trusted CA bundle by default.
35+
Previously the ``starttls()`` call was made without an SSL context, so any certificate was
36+
accepted.
37+
38+
Deployments that intentionally point ``SmtpHook`` at an SMTP server with a self-signed or
39+
otherwise non-validating certificate and need to preserve the previous behaviour must set the
40+
``ssl_context`` field in the SMTP connection extras to ``"none"``. Leaving the field unset (or
41+
setting it to ``"default"``) now applies ``ssl.create_default_context()`` to the STARTTLS
42+
upgrade as well as to the existing ``SMTP_SSL`` path.
3043

3144
2.4.3
3245
.....

providers/smtp/src/airflow/providers/smtp/hooks/smtp.py

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ def get_conn(self) -> SmtpHook:
126126
raise AirflowException("Unable to connect to smtp server")
127127
else:
128128
if self.smtp_starttls:
129-
self._smtp_client.starttls()
129+
self._smtp_client.starttls(context=self._build_ssl_context())
130130
self._smtp_client.ehlo()
131131

132132
# choose auth
@@ -172,7 +172,7 @@ async def aget_conn(self) -> SmtpHook:
172172
raise AirflowException("Unable to connect to smtp server")
173173
else:
174174
if self.smtp_starttls:
175-
await async_client.starttls()
175+
await async_client.starttls(tls_context=self._build_ssl_context())
176176
await async_client.ehlo()
177177

178178
# choose auth
@@ -191,10 +191,27 @@ async def aget_conn(self) -> SmtpHook:
191191

192192
return self
193193

194-
def _build_client_kwargs(self, is_async: bool) -> dict[str, Any]:
195-
"""Build kwargs appropriate for sync or async SMTP client."""
194+
def _build_ssl_context(self) -> ssl.SSLContext | None:
195+
"""
196+
Return the SSL context configured via the ``ssl_context`` connection extra.
197+
198+
The default (unset or ``"default"``) returns
199+
:func:`ssl.create_default_context`, which validates the server
200+
certificate against the system's trusted CAs. ``"none"`` returns
201+
``None`` so callers that explicitly want to skip validation (for
202+
example, against a self-signed SMTP server in a lab environment)
203+
can opt out.
204+
"""
196205
valid_contexts = (None, "default", "none") # Values accepted for ssl_context configuration
206+
if self.ssl_context not in valid_contexts:
207+
raise RuntimeError(
208+
f"The connection extra field `ssl_context` must "
209+
f"be set to 'default' or 'none' but it is set to '{self.ssl_context}'."
210+
)
211+
return None if self.ssl_context == "none" else ssl.create_default_context()
197212

213+
def _build_client_kwargs(self, is_async: bool) -> dict[str, Any]:
214+
"""Build kwargs appropriate for sync or async SMTP client."""
198215
kwargs: dict[str, Any] = {"timeout": self.timeout}
199216

200217
if self.port:
@@ -204,15 +221,11 @@ def _build_client_kwargs(self, is_async: bool) -> dict[str, Any]:
204221
kwargs["hostname"] = self.host
205222
kwargs["use_tls"] = self.use_ssl
206223
kwargs["start_tls"] = self.smtp_starttls if not self.use_ssl else None
224+
kwargs["tls_context"] = self._build_ssl_context()
207225
else:
208226
kwargs["host"] = self.host
209227
if self.use_ssl:
210-
if self.ssl_context not in valid_contexts:
211-
raise RuntimeError(
212-
f"The connection extra field `ssl_context` must "
213-
f"be set to 'default' or 'none' but it is set to '{self.ssl_context}'."
214-
)
215-
kwargs["context"] = None if self.ssl_context == "none" else ssl.create_default_context()
228+
kwargs["context"] = self._build_ssl_context()
216229

217230
return kwargs
218231

providers/smtp/tests/unit/smtp/hooks/test_smtp.py

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import json
2121
import os
2222
import smtplib
23+
import ssl
2324
import tempfile
2425
from email.mime.application import MIMEApplication
2526
from unittest import mock
@@ -535,9 +536,15 @@ def test_ehlo_called_after_starttls(self, mock_smtplib):
535536
with SmtpHook(smtp_conn_id=CONN_ID_NONSSL):
536537
pass
537538

538-
# Verify ehlo is called after starttls and before login
539-
expected_calls = [call.starttls(), call.ehlo(), call.login(SMTP_LOGIN, SMTP_PASSWORD)]
540-
assert manager.mock_calls == expected_calls
539+
# Verify ehlo is called after starttls and before login,
540+
# and starttls is invoked with an SSL context so certificate validation
541+
# happens on the TLS upgrade.
542+
assert len(manager.mock_calls) == 3
543+
starttls_call, ehlo_call, login_call = manager.mock_calls
544+
assert starttls_call[0] == "starttls"
545+
assert isinstance(starttls_call.kwargs.get("context"), ssl.SSLContext)
546+
assert ehlo_call == call.ehlo()
547+
assert login_call == call.login(SMTP_LOGIN, SMTP_PASSWORD)
541548

542549

543550
@pytest.mark.asyncio
@@ -626,13 +633,14 @@ async def test_async_connection(
626633
async with SmtpHook(smtp_conn_id=conn_id) as hook:
627634
assert hook is not None
628635

629-
mock_smtp.assert_called_once_with(
630-
hostname=SMTP_HOST,
631-
port=expected_port,
632-
timeout=DEFAULT_TIMEOUT,
633-
use_tls=expected_ssl,
634-
start_tls=None if expected_ssl else True,
635-
)
636+
mock_smtp.assert_called_once()
637+
call_kwargs = mock_smtp.call_args.kwargs
638+
assert call_kwargs["hostname"] == SMTP_HOST
639+
assert call_kwargs["port"] == expected_port
640+
assert call_kwargs["timeout"] == DEFAULT_TIMEOUT
641+
assert call_kwargs["use_tls"] == expected_ssl
642+
assert call_kwargs["start_tls"] == (None if expected_ssl else True)
643+
assert isinstance(call_kwargs["tls_context"], ssl.SSLContext)
636644

637645
if expected_ssl:
638646
assert mock_smtp_client.starttls.await_count == 1

0 commit comments

Comments
 (0)