From c7689a3c55dfcf34c12fbb9d9d51b8946b80cff4 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Thu, 3 Aug 2023 17:08:03 +0200 Subject: [PATCH 1/2] Allows to choose SSL context for SMTP provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change add two options to choose from when SSL SMTP connection is created: * default - for balance between compatibility and security * none - in case compatibility with existing infrastructure is   preferred The fallback is: * The Airflow "email", "ssl_context" * "default" --- airflow/providers/smtp/CHANGELOG.rst | 12 ++ airflow/providers/smtp/hooks/smtp.py | 20 +++- airflow/providers/smtp/provider.yaml | 24 ++++ .../configurations-ref.rst | 18 +++ docs/apache-airflow-providers-smtp/index.rst | 1 + docs/apache-airflow/configurations-ref.rst | 1 + tests/providers/smtp/hooks/test_smtp.py | 103 ++++++++++++++++-- 7 files changed, 171 insertions(+), 8 deletions(-) create mode 100644 docs/apache-airflow-providers-smtp/configurations-ref.rst diff --git a/airflow/providers/smtp/CHANGELOG.rst b/airflow/providers/smtp/CHANGELOG.rst index 6e0a955e56b97..040d092d2b55a 100644 --- a/airflow/providers/smtp/CHANGELOG.rst +++ b/airflow/providers/smtp/CHANGELOG.rst @@ -27,6 +27,18 @@ Changelog --------- +In case of SMTP SSL connection, the default context now uses "default" context + +The "default" context is Python's ``default_ssl_contest`` instead of previously used "none". The +``default_ssl_context`` provides a balance between security and compatibility but in some cases, +when certificates are old, self-signed or misconfigured, it might not work. This can be configured +by setting "ssl_context" in "smtp_provider" configuration of the provider. If it is not explicitly set, +it will default to "email", "ssl_context" setting in Airflow. + +Setting it to "none" brings back the "none" setting that was used in previous versions of the provider, +but it is not recommended due to security reasons ad this setting disables validation +of certificates and allows MITM attacks. + 1.2.0 ..... diff --git a/airflow/providers/smtp/hooks/smtp.py b/airflow/providers/smtp/hooks/smtp.py index 6279ea4f940bf..c196db1680f0c 100644 --- a/airflow/providers/smtp/hooks/smtp.py +++ b/airflow/providers/smtp/hooks/smtp.py @@ -26,6 +26,7 @@ import os import re import smtplib +import ssl from email.mime.application import MIMEApplication from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText @@ -87,7 +88,6 @@ def get_conn(self) -> SmtpHook: if attempt < self.smtp_retry_limit: continue raise AirflowException("Unable to connect to smtp server") - if self.smtp_starttls: self.smtp_client.starttls() if self.smtp_user and self.smtp_password: @@ -109,6 +109,24 @@ def _build_client(self) -> smtplib.SMTP_SSL | smtplib.SMTP: smtp_kwargs["port"] = self.port smtp_kwargs["timeout"] = self.timeout + if self.use_ssl: + from airflow.configuration import conf + + ssl_context_string = conf.get("smtp_provider", "SSL_CONTEXT", fallback=None) + if ssl_context_string is None: + ssl_context_string = conf.get("email", "SSL_CONTEXT", fallback=None) + if ssl_context_string is None: + ssl_context_string = "default" + if ssl_context_string == "default": + ssl_context = ssl.create_default_context() + elif ssl_context_string == "none": + ssl_context = None + else: + raise RuntimeError( + f"The email.ssl_context configuration variable must " + f"be set to 'default' or 'none' and is '{ssl_context_string}'." + ) + smtp_kwargs["context"] = ssl_context return SMTP(**smtp_kwargs) @classmethod diff --git a/airflow/providers/smtp/provider.yaml b/airflow/providers/smtp/provider.yaml index 9303246608692..6132969a2c23e 100644 --- a/airflow/providers/smtp/provider.yaml +++ b/airflow/providers/smtp/provider.yaml @@ -54,3 +54,27 @@ connection-types: notifications: - airflow.providers.smtp.notifications.smtp.SmtpNotifier + +config: + smtp_provider: + description: "Options for SMTP provider." + options: + ssl_context: + description: | + ssl context to use when using SMTP and IMAP SSL connections. By default, the context is "default" + which sets it to ``ssl.create_default_context()`` which provides the right balance between + compatibility and security, it however requires that certificates in your operating system are + updated and that SMTP/IMAP servers of yours have valid certificates that have corresponding public + keys installed on your machines. You can switch it to "none" if you want to disable checking + of the certificates, but it is not recommended as it allows MITM (man-in-the-middle) attacks + if your infrastructure is not sufficiently secured. It should only be set temporarily while you + are fixing your certificate configuration. This can be typically done by upgrading to newer + version of the operating system you run Airflow components on,by upgrading/refreshing proper + certificates in the OS or by updating certificates for your mail servers. + + If you do not set this option explicitly, it will use Airflow "email.ssl_context" configuration, + but if this configuration is not present, it will use "default" value. + type: string + version_added: 1.3.0 + example: "default" + default: ~ diff --git a/docs/apache-airflow-providers-smtp/configurations-ref.rst b/docs/apache-airflow-providers-smtp/configurations-ref.rst new file mode 100644 index 0000000000000..5885c9d91b6e8 --- /dev/null +++ b/docs/apache-airflow-providers-smtp/configurations-ref.rst @@ -0,0 +1,18 @@ + .. Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + .. http://www.apache.org/licenses/LICENSE-2.0 + + .. Unless required by applicable law or agreed to in writing, + software distributed under the License is distributed on an + "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + KIND, either express or implied. See the License for the + specific language governing permissions and limitations + under the License. + +.. include:: ../exts/includes/providers-configurations-ref.rst diff --git a/docs/apache-airflow-providers-smtp/index.rst b/docs/apache-airflow-providers-smtp/index.rst index 0cc35fa4e1f92..f188d1d08a59a 100644 --- a/docs/apache-airflow-providers-smtp/index.rst +++ b/docs/apache-airflow-providers-smtp/index.rst @@ -34,6 +34,7 @@ :maxdepth: 1 :caption: References + Configuration Connection types SMTP Notifications Python API <_api/airflow/providers/smtp/index> diff --git a/docs/apache-airflow/configurations-ref.rst b/docs/apache-airflow/configurations-ref.rst index c4882a8b903cf..42ff5b9a6b099 100644 --- a/docs/apache-airflow/configurations-ref.rst +++ b/docs/apache-airflow/configurations-ref.rst @@ -38,6 +38,7 @@ in the provider's documentation. The pre-installed providers that you may want t * :doc:`Configuration Reference for Celery Provider ` * :doc:`Configuration Reference for Apache Hive Provider ` * :doc:`Configuration Reference for CNCF Kubernetes Provider ` +* :doc:`Configuration Reference for SMTP Provider ` .. note:: For more information see :doc:`/howto/set-config`. diff --git a/tests/providers/smtp/hooks/test_smtp.py b/tests/providers/smtp/hooks/test_smtp.py index 10fe5df6731ac..3b8032df18168 100644 --- a/tests/providers/smtp/hooks/test_smtp.py +++ b/tests/providers/smtp/hooks/test_smtp.py @@ -30,6 +30,7 @@ from airflow.providers.smtp.hooks.smtp import SmtpHook from airflow.utils import db from airflow.utils.session import create_session +from tests.test_utils.config import conf_vars smtplib_string = "airflow.providers.smtp.hooks.smtp.smtplib" @@ -75,13 +76,16 @@ def setup_method(self): ) @patch(smtplib_string) - def test_connect_and_disconnect(self, mock_smtplib): + @patch("ssl.create_default_context") + def test_connect_and_disconnect(self, create_default_context, mock_smtplib): mock_conn = _create_fake_smtp(mock_smtplib) with SmtpHook(): pass - - mock_smtplib.SMTP_SSL.assert_called_once_with(host="smtp_server_address", port=465, timeout=30) + assert create_default_context.called + mock_smtplib.SMTP_SSL.assert_called_once_with( + host="smtp_server_address", port=465, timeout=30, context=create_default_context.return_value + ) mock_conn.login.assert_called_once_with("smtp_user", "smtp_password") assert mock_conn.close.call_count == 1 @@ -201,12 +205,90 @@ def test_hook_conn(self, mock_smtplib, mock_hook_conn): @patch("smtplib.SMTP_SSL") @patch("smtplib.SMTP") - def test_send_mime_ssl(self, mock_smtp, mock_smtp_ssl): + @patch("ssl.create_default_context") + def test_send_mime_ssl(self, create_default_context, mock_smtp, mock_smtp_ssl): mock_smtp_ssl.return_value = Mock() with SmtpHook() as smtp_hook: smtp_hook.send_email_smtp(to="to", subject="subject", html_content="content", from_email="from") assert not mock_smtp.called - mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30) + assert create_default_context.called + mock_smtp_ssl.assert_called_once_with( + host="smtp_server_address", port=465, timeout=30, context=create_default_context.return_value + ) + + @patch("smtplib.SMTP_SSL") + @patch("smtplib.SMTP") + @patch("ssl.create_default_context") + def test_send_mime_ssl_none_email_context(self, create_default_context, mock_smtp, mock_smtp_ssl): + mock_smtp_ssl.return_value = Mock() + with conf_vars({("smtp", "smtp_ssl"): "True", ("email", "ssl_context"): "none"}): + with SmtpHook() as smtp_hook: + smtp_hook.send_email_smtp( + to="to", subject="subject", html_content="content", from_email="from" + ) + assert not mock_smtp.called + assert not create_default_context.called + mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30, context=None) + + @patch("smtplib.SMTP_SSL") + @patch("smtplib.SMTP") + @patch("ssl.create_default_context") + def test_send_mime_ssl_none_smtp_provider_context(self, create_default_context, mock_smtp, mock_smtp_ssl): + mock_smtp_ssl.return_value = Mock() + with conf_vars({("smtp", "smtp_ssl"): "True", ("smtp_provider", "ssl_context"): "none"}): + with SmtpHook() as smtp_hook: + smtp_hook.send_email_smtp( + to="to", subject="subject", html_content="content", from_email="from" + ) + assert not mock_smtp.called + assert not create_default_context.called + mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30, context=None) + + @patch("smtplib.SMTP_SSL") + @patch("smtplib.SMTP") + @patch("ssl.create_default_context") + def test_send_mime_ssl_none_smtp_provider_default_email_context( + self, create_default_context, mock_smtp, mock_smtp_ssl + ): + mock_smtp_ssl.return_value = Mock() + with conf_vars( + { + ("smtp", "smtp_ssl"): "True", + ("email", "ssl_context"): "default", + ("smtp_provider", "ssl_context"): "none", + } + ): + with SmtpHook() as smtp_hook: + smtp_hook.send_email_smtp( + to="to", subject="subject", html_content="content", from_email="from" + ) + assert not mock_smtp.called + assert not create_default_context.called + mock_smtp_ssl.assert_called_once_with(host="smtp_server_address", port=465, timeout=30, context=None) + + @patch("smtplib.SMTP_SSL") + @patch("smtplib.SMTP") + @patch("ssl.create_default_context") + def test_send_mime_ssl_default_smtp_provider_none_email_context( + self, create_default_context, mock_smtp, mock_smtp_ssl + ): + mock_smtp_ssl.return_value = Mock() + with conf_vars( + { + ("smtp", "smtp_ssl"): "True", + ("email", "ssl_context"): "none", + ("smtp_provider", "ssl_context"): "default", + } + ): + with SmtpHook() as smtp_hook: + smtp_hook.send_email_smtp( + to="to", subject="subject", html_content="content", from_email="from" + ) + assert not mock_smtp.called + assert create_default_context.called + mock_smtp_ssl.assert_called_once_with( + host="smtp_server_address", port=465, timeout=30, context=create_default_context.return_value + ) @patch("smtplib.SMTP_SSL") @patch("smtplib.SMTP") @@ -269,7 +351,10 @@ def test_send_mime_partial_failure(self, mock_smtp_ssl, mime_message_mock): @patch("airflow.models.connection.Connection") @patch("smtplib.SMTP_SSL") - def test_send_mime_custom_timeout_retrylimit(self, mock_smtp_ssl, connection_mock): + @patch("ssl.create_default_context") + def test_send_mime_custom_timeout_retrylimit( + self, create_default_context, mock_smtp_ssl, connection_mock + ): mock_smtp_ssl().sendmail.side_effect = smtplib.SMTPServerDisconnected() custom_retry_limit = 10 custom_timeout = 60 @@ -287,6 +372,10 @@ def test_send_mime_custom_timeout_retrylimit(self, mock_smtp_ssl, connection_moc with pytest.raises(smtplib.SMTPServerDisconnected): smtp_hook.send_email_smtp(to="to", subject="subject", html_content="content") mock_smtp_ssl.assert_any_call( - host=fake_conn.host, port=fake_conn.port, timeout=fake_conn.extra_dejson["timeout"] + host=fake_conn.host, + port=fake_conn.port, + timeout=fake_conn.extra_dejson["timeout"], + context=create_default_context.return_value, ) + assert create_default_context.called assert mock_smtp_ssl().sendmail.call_count == 10 From dba5b2329730a65babf5a9b903e29539845ed11a Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Thu, 3 Aug 2023 18:32:55 +0200 Subject: [PATCH 2/2] Update airflow/providers/smtp/CHANGELOG.rst Co-authored-by: Ephraim Anierobi --- airflow/providers/smtp/CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/providers/smtp/CHANGELOG.rst b/airflow/providers/smtp/CHANGELOG.rst index 040d092d2b55a..527af5d7a3abe 100644 --- a/airflow/providers/smtp/CHANGELOG.rst +++ b/airflow/providers/smtp/CHANGELOG.rst @@ -29,7 +29,7 @@ Changelog In case of SMTP SSL connection, the default context now uses "default" context -The "default" context is Python's ``default_ssl_contest`` instead of previously used "none". The +The "default" context is Python's ``default_ssl_context`` instead of previously used "none". The ``default_ssl_context`` provides a balance between security and compatibility but in some cases, when certificates are old, self-signed or misconfigured, it might not work. This can be configured by setting "ssl_context" in "smtp_provider" configuration of the provider. If it is not explicitly set,