Allows to choose SSL context for SMTP provider (#33075)

* Allows to choose SSL context for SMTP provider 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" * Update airflow/providers/smtp/CHANGELOG.rst Co-authored-by: Ephraim Anierobi <[email protected]> (cherry picked from commit e20325d)
apache · Aug 8, 2023 · dbacacb · dbacacb
1 parent 2b312cd
commit dbacacb
Show file tree

Hide file tree

Showing 7 changed files with 171 additions and 8 deletions.
diff --git 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_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,
+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
@@ -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
@@ -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
@@ -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
@@ -34,6 +34,7 @@
     :maxdepth: 1
     :caption: References
 
+    Configuration <configurations-ref>
     Connection types <connections/smtp>
     SMTP Notifications <notifications/smtp_notifier_howto_guide>
     Python API <_api/airflow/providers/smtp/index>

diff --git 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 <apache-airflow-providers-celery:configurations-ref>`
 * :doc:`Configuration Reference for Apache Hive Provider <apache-airflow-providers-apache-hive:configurations-ref>`
 * :doc:`Configuration Reference for CNCF Kubernetes Provider <apache-airflow-providers-cncf-kubernetes:configurations-ref>`
+* :doc:`Configuration Reference for SMTP Provider <apache-airflow-providers-smtp:configurations-ref>`
 
 .. 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
@@ -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