Add email.tlsname config option (#17849)

The existing `email.smtp_host` config option is used for two distinct
purposes: it is resolved into the IP address to connect to, and used to
(request via SNI and) validate the server's certificate if TLS is
enabled. This new option allows specifying a different name for the
second purpose.

This is especially helpful, if `email.smtp_host` isn't a global FQDN,
but something that resolves only locally (e.g. "localhost" to connect
through the loopback interface, or some other internally routed name),
that one cannot get a valid certificate for.
Alternatives would of course be to specify a global FQDN as
`email.smtp_host`, or to disable TLS entirely, both of which might be
undesirable, depending on the SMTP server configuration.

changelog.d/17849.feature

@@ -0,0 +1 @@
+Added the `email.tlsname` config option.  This allows specifying the domain name used to validate the SMTP server's TLS certificate separately from the `email.smtp_host` to connect to.

docs/usage/configuration/config_documentation.md

@@ -673,8 +673,9 @@ This setting has the following sub-options:
    TLS via STARTTLS *if the SMTP server supports it*. If this option is set,
    Synapse will refuse to connect unless the server supports STARTTLS.
 * `enable_tls`: 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
+   must present a certificate that is valid for `tlsname`. If this option
    is set to false, TLS will not be used.
+* `tlsname`: The domain name the SMTP server's TLS certificate must be valid for, defaulting to `smtp_host`.
 * `notif_from`: defines the "From" address to use when sending emails.
     It must be set if email sending is enabled. The placeholder '%(app)s' will be replaced by the application name,
     which is normally set in `app_name`, but may be overridden by the
@@ -741,6 +742,7 @@ email:
   force_tls: true
   require_transport_security: true
   enable_tls: false
+  tlsname: mail.server.example.com
   notif_from: "Your Friendly %(app)s homeserver <noreply@example.com>"
   app_name: my_branded_matrix_server
   enable_notifs: true

synapse/config/emailconfig.py

@@ -110,6 +110,7 @@ class EmailConfig(Config):
             raise ConfigError(
                 "email.require_transport_security requires email.enable_tls to be true"
             )
+        self.email_tlsname = email_config.get("tlsname", None)
 
         if "app_name" in email_config:
             self.email_app_name = email_config["app_name"]

synapse/handlers/send_email.py

@@ -47,15 +47,45 @@ logger = logging.getLogger(__name__)
 _is_old_twisted = parse_version(twisted.__version__) < parse_version("21")
 
 
-class _NoTLSESMTPSender(ESMTPSender):
-    """Extend ESMTPSender to disable TLS
+class _BackportESMTPSender(ESMTPSender):
+    """Extend old versions of ESMTPSender to configure TLS.
 
-    Unfortunately, before Twisted 21.2, ESMTPSender doesn't give an easy way to disable
-    TLS, so we override its internal method which it uses to generate a context factory.
+    Unfortunately, before Twisted 21.2, ESMTPSender doesn't give an easy way to
+    disable TLS, or to configure the hostname used for TLS certificate validation.
+    This backports the `hostname` parameter for that functionality.
     """
 
+    __hostname: Optional[str]
+
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        """"""
+        self.__hostname = kwargs.pop("hostname", None)
+        super().__init__(*args, **kwargs)
+
     def _getContextFactory(self) -> Optional[IOpenSSLContextFactory]:
-        return None
+        if self.context is not None:
+            return self.context
+        elif self.__hostname is None:
+            return None  # disable TLS if hostname is None
+        return optionsForClientTLS(self.__hostname)
+
+
+class _BackportESMTPSenderFactory(ESMTPSenderFactory):
+    """An ESMTPSenderFactory for _BackportESMTPSender.
+
+    This backports the `hostname` parameter, to disable or configure TLS.
+    """
+
+    __hostname: Optional[str]
+
+    def __init__(self, *args: Any, **kwargs: Any) -> None:
+        self.__hostname = kwargs.pop("hostname", None)
+        super().__init__(*args, **kwargs)
+
+    def protocol(self, *args: Any, **kwargs: Any) -> ESMTPSender:  # type: ignore
+        # this overrides ESMTPSenderFactory's `protocol` attribute, with a Callable
+        # instantiating our _BackportESMTPSender, providing the hostname parameter
+        return _BackportESMTPSender(*args, **kwargs, hostname=self.__hostname)
 
 
 async def _sendmail(
@@ -71,6 +101,7 @@ async def _sendmail(
     require_tls: bool = False,
     enable_tls: bool = True,
     force_tls: bool = False,
+    tlsname: Optional[str] = None,
 ) -> None:
     """A simple wrapper around ESMTPSenderFactory, to allow substitution in tests
 
@@ -88,12 +119,19 @@ async def _sendmail(
         enable_tls: True to enable STARTTLS. If this is False and require_tls is True,
            the request will fail.
         force_tls: True to enable Implicit TLS.
+        tlsname: the domain name expected as the TLS certificate's commonname,
+           defaults to smtphost.
     """
     msg = BytesIO(msg_bytes)
     d: "Deferred[object]" = Deferred()
+    if not enable_tls:
+        tlsname = None
+    elif tlsname is None:
+        tlsname = smtphost
 
-    def build_sender_factory(**kwargs: Any) -> ESMTPSenderFactory:
-        return ESMTPSenderFactory(
+    factory: IProtocolFactory = (
+        _BackportESMTPSenderFactory if _is_old_twisted else ESMTPSenderFactory
+    )(
         username,
         password,
         from_addr,
@@ -103,24 +141,11 @@ async def _sendmail(
         heloFallback=True,
         requireAuthentication=require_auth,
         requireTransportSecurity=require_tls,
-            **kwargs,
+        hostname=tlsname,
     )
 
-    factory: IProtocolFactory
-    if _is_old_twisted:
-        # before twisted 21.2, we have to override the ESMTPSender protocol to disable
-        # TLS
-        factory = build_sender_factory()
-
-        if not enable_tls:
-            factory.protocol = _NoTLSESMTPSender
-    else:
-        # for twisted 21.2 and later, there is a 'hostname' parameter which we should
-        # set to enable TLS.
-        factory = build_sender_factory(hostname=smtphost if enable_tls else None)
-
     if force_tls:
-        factory = TLSMemoryBIOFactory(optionsForClientTLS(smtphost), True, factory)
+        factory = TLSMemoryBIOFactory(optionsForClientTLS(tlsname), True, factory)
 
     endpoint = HostnameEndpoint(
         reactor, smtphost, smtpport, timeout=30, bindAddress=None
@@ -148,6 +173,7 @@ class SendEmailHandler:
         self._require_transport_security = hs.config.email.require_transport_security
         self._enable_tls = hs.config.email.enable_smtp_tls
         self._force_tls = hs.config.email.force_tls
+        self._tlsname = hs.config.email.email_tlsname
 
         self._sendmail = _sendmail
 
@@ -227,4 +253,5 @@ class SendEmailHandler:
             require_tls=self._require_transport_security,
             enable_tls=self._enable_tls,
             force_tls=self._force_tls,
+            tlsname=self._tlsname,
         )

tests/handlers/test_send_email.py

@@ -163,6 +163,7 @@ class SendEmailHandlerTestCaseIPv4(HomeserverTestCase):
             "email": {
                 "notif_from": "noreply@test",
                 "force_tls": True,
+                "tlsname": "example.org",
             },
         }
     )
@@ -186,10 +187,9 @@ class SendEmailHandlerTestCaseIPv4(HomeserverTestCase):
         self.assertEqual(host, self.reactor.lookups["localhost"])
         self.assertEqual(port, 465)
         # We need to make sure that TLS is happenning
-        self.assertIsInstance(
-            client_factory._wrappedFactory._testingContextFactory,
-            ClientTLSOptions,
-        )
+        context_factory = client_factory._wrappedFactory._testingContextFactory
+        self.assertIsInstance(context_factory, ClientTLSOptions)
+        self.assertEqual(context_factory._hostname, "example.org")  # tlsname
         # And since we use endpoints, they go through reactor.connectTCP
         # which works differently to connectSSL on the testing reactor