Add a setting to disable TLS for sending email (#10546)

This is mostly useful in case the server offers TLS, but doesn't present a valid certificate.

8 files changed, 138 insertions, 50 deletions

diff --git a/changelog.d/10546.feature b/changelog.d/10546.feature
new file mode 100644
index 0000000000..7709d010b3
--- /dev/null
+++ b/changelog.d/10546.feature
@@ -0,0 +1 @@
+Add a setting to disable TLS when sending email.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 16843dd8c9..aeebcaf45f 100644
--- a/docs/sample_config.yaml
+++ b/docs/sample_config.yaml
@@ -2242,6 +2242,14 @@ email:
   #
   #require_transport_security: true
 
+  # Uncomment the following to disable TLS for SMTP.
+  #
+  # By default, if the server supports TLS, it will be used, and the server
+  # must present a certificate that is valid for 'smtp_host'. If this option
+  # is set to false, TLS will not be used.
+  #
+  #enable_tls: false
+
   # notif_from defines the "From" address to use when sending emails.
   # It must be set if email sending is enabled.
   #
diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py
index 8d8f166e9b..42526502f0 100644
--- a/synapse/config/emailconfig.py
+++ b/synapse/config/emailconfig.py
@@ -80,6 +80,12 @@ class EmailConfig(Config):
         self.require_transport_security = email_config.get(
             "require_transport_security", False
         )
+        self.enable_smtp_tls = email_config.get("enable_tls", True)
+        if self.require_transport_security and not self.enable_smtp_tls:
+            raise ConfigError(
+                "email.require_transport_security requires email.enable_tls to be true"
+            )
+
         if "app_name" in email_config:
             self.email_app_name = email_config["app_name"]
         else:
@@ -368,6 +374,14 @@ class EmailConfig(Config):
           #
           #require_transport_security: true
 
+          # Uncomment the following to disable TLS for SMTP.
+          #
+          # By default, if the server supports TLS, it will be used, and the server
+          # must present a certificate that is valid for 'smtp_host'. If this option
+          # is set to false, TLS will not be used.
+          #
+          #enable_tls: false
+
           # notif_from defines the "From" address to use when sending emails.
           # It must be set if email sending is enabled.
           #
diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py
index e9f6aef06f..dda9659c11 100644
--- a/synapse/handlers/send_email.py
+++ b/synapse/handlers/send_email.py
@@ -16,7 +16,12 @@ import email.utils
 import logging
 from email.mime.multipart import MIMEMultipart
 from email.mime.text import MIMEText
-from typing import TYPE_CHECKING
+from io import BytesIO
+from typing import TYPE_CHECKING, Optional
+
+from twisted.internet.defer import Deferred
+from twisted.internet.interfaces import IReactorTCP
+from twisted.mail.smtp import ESMTPSenderFactory
 
 from synapse.logging.context import make_deferred_yieldable
 
@@ -26,19 +31,75 @@ if TYPE_CHECKING:
 logger = logging.getLogger(__name__)
 
 
+async def _sendmail(
+    reactor: IReactorTCP,
+    smtphost: str,
+    smtpport: int,
+    from_addr: str,
+    to_addr: str,
+    msg_bytes: bytes,
+    username: Optional[bytes] = None,
+    password: Optional[bytes] = None,
+    require_auth: bool = False,
+    require_tls: bool = False,
+    tls_hostname: Optional[str] = None,
+) -> None:
+    """A simple wrapper around ESMTPSenderFactory, to allow substitution in tests
+
+    Params:
+        reactor: reactor to use to make the outbound connection
+        smtphost: hostname to connect to
+        smtpport: port to connect to
+        from_addr: "From" address for email
+        to_addr: "To" address for email
+        msg_bytes: Message content
+        username: username to authenticate with, if auth is enabled
+        password: password to give when authenticating
+        require_auth: if auth is not offered, fail the request
+        require_tls: if TLS is not offered, fail the reqest
+        tls_hostname: TLS hostname to check for. None to disable TLS.
+    """
+    msg = BytesIO(msg_bytes)
+
+    d: "Deferred[object]" = Deferred()
+
+    factory = ESMTPSenderFactory(
+        username,
+        password,
+        from_addr,
+        to_addr,
+        msg,
+        d,
+        heloFallback=True,
+        requireAuthentication=require_auth,
+        requireTransportSecurity=require_tls,
+        hostname=tls_hostname,
+    )
+
+    # the IReactorTCP interface claims host has to be a bytes, which seems to be wrong
+    reactor.connectTCP(smtphost, smtpport, factory, timeout=30, bindAddress=None)  # type: ignore[arg-type]
+
+    await make_deferred_yieldable(d)
+
+
 class SendEmailHandler:
     def __init__(self, hs: "HomeServer"):
         self.hs = hs
 
-        self._sendmail = hs.get_sendmail()
         self._reactor = hs.get_reactor()
 
         self._from = hs.config.email.email_notif_from
         self._smtp_host = hs.config.email.email_smtp_host
         self._smtp_port = hs.config.email.email_smtp_port
-        self._smtp_user = hs.config.email.email_smtp_user
-        self._smtp_pass = hs.config.email.email_smtp_pass
+
+        user = hs.config.email.email_smtp_user
+        self._smtp_user = user.encode("utf-8") if user is not None else None
+        passwd = hs.config.email.email_smtp_pass
+        self._smtp_pass = passwd.encode("utf-8") if passwd is not None else None
         self._require_transport_security = hs.config.email.require_transport_security
+        self._enable_tls = hs.config.email.enable_smtp_tls
+
+        self._sendmail = _sendmail
 
     async def send_email(
         self,
@@ -82,17 +143,16 @@ class SendEmailHandler:
 
         logger.info("Sending email to %s" % email_address)
 
-        await make_deferred_yieldable(
-            self._sendmail(
-                self._smtp_host,
-                raw_from,
-                raw_to,
-                multipart_msg.as_string().encode("utf8"),
-                reactor=self._reactor,
-                port=self._smtp_port,
-                requireAuthentication=self._smtp_user is not None,
-                username=self._smtp_user,
-                password=self._smtp_pass,
-                requireTransportSecurity=self._require_transport_security,
-            )
+        await self._sendmail(
+            self._reactor,
+            self._smtp_host,
+            self._smtp_port,
+            raw_from,
+            raw_to,
+            multipart_msg.as_string().encode("utf8"),
+            username=self._smtp_user,
+            password=self._smtp_pass,
+            require_auth=self._smtp_user is not None,
+            require_tls=self._require_transport_security,
+            tls_hostname=self._smtp_host if self._enable_tls else None,
         )
diff --git a/synapse/server.py b/synapse/server.py
index 095dba9ad0..6c867f0f47 100644
--- a/synapse/server.py
+++ b/synapse/server.py
@@ -34,8 +34,6 @@ from typing import (
 )
 
 import twisted.internet.tcp
-from twisted.internet import defer
-from twisted.mail.smtp import sendmail
 from twisted.web.iweb import IPolicyForHTTPS
 from twisted.web.resource import IResource
 
@@ -443,10 +441,6 @@ class HomeServer(metaclass=abc.ABCMeta):
         return RoomShutdownHandler(self)
 
     @cache_in_self
-    def get_sendmail(self) -> Callable[..., defer.Deferred]:
-        return sendmail
-
-    @cache_in_self
     def get_state_handler(self) -> StateHandler:
         return StateHandler(self)
 
diff --git a/tests/push/test_email.py b/tests/push/test_email.py
index e04bc5c9a6..a487706758 100644
--- a/tests/push/test_email.py
+++ b/tests/push/test_email.py
@@ -45,14 +45,6 @@ class EmailPusherTests(HomeserverTestCase):
 
     def make_homeserver(self, reactor, clock):
 
-        # List[Tuple[Deferred, args, kwargs]]
-        self.email_attempts = []
-
-        def sendmail(*args, **kwargs):
-            d = Deferred()
-            self.email_attempts.append((d, args, kwargs))
-            return d
-
         config = self.default_config()
         config["email"] = {
             "enable_notifs": True,
@@ -75,7 +67,17 @@ class EmailPusherTests(HomeserverTestCase):
         config["public_baseurl"] = "aaa"
         config["start_pushers"] = True
 
-        hs = self.setup_test_homeserver(config=config, sendmail=sendmail)
+        hs = self.setup_test_homeserver(config=config)
+
+        # List[Tuple[Deferred, args, kwargs]]
+        self.email_attempts = []
+
+        def sendmail(*args, **kwargs):
+            d = Deferred()
+            self.email_attempts.append((d, args, kwargs))
+            return d
+
+        hs.get_send_email_handler()._sendmail = sendmail
 
         return hs
 
diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py
index 317a2287e3..e7e617e9df 100644
--- a/tests/rest/client/v2_alpha/test_account.py
+++ b/tests/rest/client/v2_alpha/test_account.py
@@ -47,12 +47,6 @@ class PasswordResetTestCase(unittest.HomeserverTestCase):
         config = self.default_config()
 
         # Email config.
-        self.email_attempts = []
-
-        async def sendmail(smtphost, from_addr, to_addrs, msg, **kwargs):
-            self.email_attempts.append(msg)
-            return
-
         config["email"] = {
             "enable_notifs": False,
             "template_dir": os.path.abspath(
@@ -67,7 +61,16 @@ class PasswordResetTestCase(unittest.HomeserverTestCase):
         }
         config["public_baseurl"] = "https://example.com"
 
-        hs = self.setup_test_homeserver(config=config, sendmail=sendmail)
+        hs = self.setup_test_homeserver(config=config)
+
+        async def sendmail(
+            reactor, smtphost, smtpport, from_addr, to_addrs, msg, **kwargs
+        ):
+            self.email_attempts.append(msg)
+
+        self.email_attempts = []
+        hs.get_send_email_handler()._sendmail = sendmail
+
         return hs
 
     def prepare(self, reactor, clock, hs):
@@ -511,11 +514,6 @@ class ThreepidEmailRestTestCase(unittest.HomeserverTestCase):
         config = self.default_config()
 
         # Email config.
-        self.email_attempts = []
-
-        async def sendmail(smtphost, from_addr, to_addrs, msg, **kwargs):
-            self.email_attempts.append(msg)
-
         config["email"] = {
             "enable_notifs": False,
             "template_dir": os.path.abspath(
@@ -530,7 +528,16 @@ class ThreepidEmailRestTestCase(unittest.HomeserverTestCase):
         }
         config["public_baseurl"] = "https://example.com"
 
-        self.hs = self.setup_test_homeserver(config=config, sendmail=sendmail)
+        self.hs = self.setup_test_homeserver(config=config)
+
+        async def sendmail(
+            reactor, smtphost, smtpport, from_addr, to_addrs, msg, **kwargs
+        ):
+            self.email_attempts.append(msg)
+
+        self.email_attempts = []
+        self.hs.get_send_email_handler()._sendmail = sendmail
+
         return self.hs
 
     def prepare(self, reactor, clock, hs):
diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py
index 1cad5f00eb..a52e5e608a 100644
--- a/tests/rest/client/v2_alpha/test_register.py
+++ b/tests/rest/client/v2_alpha/test_register.py
@@ -509,10 +509,6 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
         }
 
         # Email config.
-        self.email_attempts = []
-
-        async def sendmail(*args, **kwargs):
-            self.email_attempts.append((args, kwargs))
 
         config["email"] = {
             "enable_notifs": True,
@@ -532,7 +528,13 @@ class AccountValidityRenewalByEmailTestCase(unittest.HomeserverTestCase):
         }
         config["public_baseurl"] = "aaa"
 
-        self.hs = self.setup_test_homeserver(config=config, sendmail=sendmail)
+        self.hs = self.setup_test_homeserver(config=config)
+
+        async def sendmail(*args, **kwargs):
+            self.email_attempts.append((args, kwargs))
+
+        self.email_attempts = []
+        self.hs.get_send_email_handler()._sendmail = sendmail
 
         self.store = self.hs.get_datastore()