From f2b0002798370665dfd2a16b48793d5bf7ea21f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sch=C3=A4r?= Date: Mon, 18 Jul 2022 18:43:29 +0200 Subject: [PATCH 1/5] Support Implicit TLS for sending emails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, TLS could only be used with STARTTLS. Add a new option `implicit_tls`, where TLS is used from the start. Implicit TLS is recommended over STARTLS, see https://datatracker.ietf.org/doc/html/rfc8314 Fixes #8046. Signed-off-by: Jan Schär --- changelog.d/13317.feature | 1 + .../configuration/config_documentation.md | 7 +++- synapse/config/emailconfig.py | 5 ++- synapse/handlers/send_email.py | 36 +++++++++++++------ 4 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 changelog.d/13317.feature diff --git a/changelog.d/13317.feature b/changelog.d/13317.feature new file mode 100644 index 000000000000..854d7f548fb8 --- /dev/null +++ b/changelog.d/13317.feature @@ -0,0 +1 @@ +Support Implicit TLS for sending emails, enabled by the new option `implicit_tls`. Contributed by Jan Schär. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 5fe502e33ae3..93283712d1d0 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3150,9 +3150,13 @@ Server admins can configure custom templates for email content. See This setting has the following sub-options: * `smtp_host`: The hostname of the outgoing SMTP server to use. Defaults to 'localhost'. -* `smtp_port`: The port on the mail server for outgoing SMTP. Defaults to 25. +* `smtp_port`: The port on the mail server for outgoing SMTP. Defaults to 465 if `implicit_tls` is true, else 25. * `smtp_user` and `smtp_pass`: Username/password for authentication to the SMTP server. By default, no authentication is attempted. +* `implicit_tls`: By default, Synapse connects over plain text and then optionally upgrades + to TLS via STARTTLS. If this option is set to true, TLS is used from the start, + and the options `require_transport_security` and `enable_tls` are ignored. + It is recommended to enable this if supported by your mail server. * `require_transport_security`: Set to true to require TLS transport security for SMTP. By default, Synapse will connect over plain text, and will then switch to TLS via STARTTLS *if the SMTP server supports it*. If this option is set, @@ -3217,6 +3221,7 @@ email: smtp_port: 587 smtp_user: "exampleusername" smtp_pass: "examplepassword" + implicit_tls: true require_transport_security: true enable_tls: false notif_from: "Your Friendly %(app)s homeserver " diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 3ead80d98522..5fc32e4b6b94 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -85,8 +85,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: if email_config is None: email_config = {} + self.implicit_tls = email_config.get("implicit_tls", False) self.email_smtp_host = email_config.get("smtp_host", "localhost") - self.email_smtp_port = email_config.get("smtp_port", 25) + self.email_smtp_port = email_config.get( + "smtp_port", 465 if self.implicit_tls else 25 + ) self.email_smtp_user = email_config.get("smtp_user", None) self.email_smtp_pass = email_config.get("smtp_pass", None) self.require_transport_security = email_config.get( diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py index a305a66860a5..48adbd89c8d1 100644 --- a/synapse/handlers/send_email.py +++ b/synapse/handlers/send_email.py @@ -23,10 +23,12 @@ import twisted from twisted.internet.defer import Deferred -from twisted.internet.interfaces import IOpenSSLContextFactory, IReactorTCP +from twisted.internet.interfaces import IOpenSSLContextFactory +from twisted.internet.ssl import optionsForClientTLS from twisted.mail.smtp import ESMTPSender, ESMTPSenderFactory from synapse.logging.context import make_deferred_yieldable +from synapse.types import ISynapseReactor if TYPE_CHECKING: from synapse.server import HomeServer @@ -48,7 +50,7 @@ def _getContextFactory(self) -> Optional[IOpenSSLContextFactory]: async def _sendmail( - reactor: IReactorTCP, + reactor: ISynapseReactor, smtphost: str, smtpport: int, from_addr: str, @@ -59,6 +61,7 @@ async def _sendmail( require_auth: bool = False, require_tls: bool = False, enable_tls: bool = True, + implicit_tls: bool = False, ) -> None: """A simple wrapper around ESMTPSenderFactory, to allow substitution in tests @@ -73,8 +76,9 @@ async def _sendmail( 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 - enable_tls: True to enable TLS. If this is False and require_tls is True, + enable_tls: True to enable STARTTLS. If this is False and require_tls is True, the request will fail. + implicit_tls: True to enable Implicit TLS. """ msg = BytesIO(msg_bytes) d: "Deferred[object]" = Deferred() @@ -105,13 +109,23 @@ def build_sender_factory(**kwargs: Any) -> ESMTPSenderFactory: # set to enable TLS. factory = build_sender_factory(hostname=smtphost if enable_tls else None) - reactor.connectTCP( - smtphost, - smtpport, - factory, - timeout=30, - bindAddress=None, - ) + if implicit_tls: + reactor.connectSSL( + smtphost, + smtpport, + factory, + optionsForClientTLS(smtphost), + timeout=30, + bindAddress=None, + ) + else: + reactor.connectTCP( + smtphost, + smtpport, + factory, + timeout=30, + bindAddress=None, + ) await make_deferred_yieldable(d) @@ -132,6 +146,7 @@ def __init__(self, hs: "HomeServer"): 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._implicit_tls = hs.config.email.implicit_tls self._sendmail = _sendmail @@ -189,4 +204,5 @@ async def send_email( require_auth=self._smtp_user is not None, require_tls=self._require_transport_security, enable_tls=self._enable_tls, + implicit_tls=self._implicit_tls, ) From 8ec2339679a93a611cb75ee4d2345ddd3c49157d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sch=C3=A4r?= Date: Tue, 19 Jul 2022 14:06:14 +0200 Subject: [PATCH 2/5] Apply suggestions from code review Document changes in options Co-authored-by: David Robertson --- docs/usage/configuration/config_documentation.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 93283712d1d0..02bb1b2a0705 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3151,12 +3151,16 @@ Server admins can configure custom templates for email content. See This setting has the following sub-options: * `smtp_host`: The hostname of the outgoing SMTP server to use. Defaults to 'localhost'. * `smtp_port`: The port on the mail server for outgoing SMTP. Defaults to 465 if `implicit_tls` is true, else 25. + + _Changed in Synapse 1.64.0:_ the default port is now aware of `implicit_tls`. * `smtp_user` and `smtp_pass`: Username/password for authentication to the SMTP server. By default, no authentication is attempted. * `implicit_tls`: By default, Synapse connects over plain text and then optionally upgrades to TLS via STARTTLS. If this option is set to true, TLS is used from the start, and the options `require_transport_security` and `enable_tls` are ignored. It is recommended to enable this if supported by your mail server. + + _New in Synapse 1.64.0._ * `require_transport_security`: Set to true to require TLS transport security for SMTP. By default, Synapse will connect over plain text, and will then switch to TLS via STARTTLS *if the SMTP server supports it*. If this option is set, From c785900abd089107da5feb33fdebeba307eec32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sch=C3=A4r?= Date: Tue, 19 Jul 2022 14:10:10 +0200 Subject: [PATCH 3/5] Test sending email with implicit_tls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan Schär --- tests/handlers/test_send_email.py | 57 ++++++++++++++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/tests/handlers/test_send_email.py b/tests/handlers/test_send_email.py index 6f77b1237c97..b090da787fce 100644 --- a/tests/handlers/test_send_email.py +++ b/tests/handlers/test_send_email.py @@ -23,7 +23,7 @@ from twisted.mail import interfaces, smtp from tests.server import FakeTransport -from tests.unittest import HomeserverTestCase +from tests.unittest import HomeserverTestCase, override_config @implementer(interfaces.IMessageDelivery) @@ -110,3 +110,58 @@ def test_send_email(self): user, msg = message_delivery.messages.pop() self.assertEqual(str(user), "foo@bar.com") self.assertIn(b"Subject: test subject", msg) + + @override_config( + { + "email": { + "notif_from": "noreply@test", + "implicit_tls": True, + }, + } + ) + def test_send_email_implicit_tls(self): + """Happy-path test that we can send email to an Implicit TLS server.""" + h = self.hs.get_send_email_handler() + d = ensureDeferred( + h.send_email( + "foo@bar.com", "test subject", "Tests", "HTML content", "Text content" + ) + ) + # there should be an attempt to connect to localhost:465 + self.assertEqual(len(self.reactor.sslClients), 1) + ( + host, + port, + client_factory, + contextFactory, + _timeout, + _bindAddress, + ) = self.reactor.sslClients[0] + self.assertEqual(host, "localhost") + self.assertEqual(port, 465) + + # wire it up to an SMTP server + message_delivery = _DummyMessageDelivery() + server_protocol = smtp.ESMTP() + server_protocol.delivery = message_delivery + # make sure that the server uses the test reactor to set timeouts + server_protocol.callLater = self.reactor.callLater # type: ignore[assignment] + + client_protocol = client_factory.buildProtocol(None) + client_protocol.makeConnection(FakeTransport(server_protocol, self.reactor)) + server_protocol.makeConnection( + FakeTransport( + client_protocol, + self.reactor, + peer_address=IPv4Address("TCP", "127.0.0.1", 1234), + ) + ) + + # the message should now get delivered + self.get_success(d, by=0.1) + + # check it arrived + self.assertEqual(len(message_delivery.messages), 1) + user, msg = message_delivery.messages.pop() + self.assertEqual(str(user), "foo@bar.com") + self.assertIn(b"Subject: test subject", msg) From 862bc5222426c7ca978d2e323abd2ae62c0d7ba4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sch=C3=A4r?= Date: Tue, 19 Jul 2022 14:17:30 +0200 Subject: [PATCH 4/5] Require enable_smtp_tls if implicit_tls is true MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan Schär --- docs/usage/configuration/config_documentation.md | 2 +- synapse/config/emailconfig.py | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 02bb1b2a0705..b140caef040d 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3157,7 +3157,7 @@ This setting has the following sub-options: authentication is attempted. * `implicit_tls`: By default, Synapse connects over plain text and then optionally upgrades to TLS via STARTTLS. If this option is set to true, TLS is used from the start, - and the options `require_transport_security` and `enable_tls` are ignored. + and the option `require_transport_security` is ignored. It is recommended to enable this if supported by your mail server. _New in Synapse 1.64.0._ diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index 5fc32e4b6b94..b426cd36189a 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -96,6 +96,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: "require_transport_security", False ) self.enable_smtp_tls = email_config.get("enable_tls", True) + if self.implicit_tls and not self.enable_smtp_tls: + raise ConfigError("email.implicit_tls requires email.enable_tls to be 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" From f1f1062c7dee6d6f3bc7fed78e6528a1c4091b21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Sch=C3=A4r?= Date: Tue, 19 Jul 2022 15:57:51 +0200 Subject: [PATCH 5/5] Rename option to force_tls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan Schär --- changelog.d/13317.feature | 2 +- docs/usage/configuration/config_documentation.md | 10 +++++----- synapse/config/emailconfig.py | 8 ++++---- synapse/handlers/send_email.py | 10 +++++----- tests/handlers/test_send_email.py | 4 ++-- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/changelog.d/13317.feature b/changelog.d/13317.feature index 854d7f548fb8..e0ebd2b51ff0 100644 --- a/changelog.d/13317.feature +++ b/changelog.d/13317.feature @@ -1 +1 @@ -Support Implicit TLS for sending emails, enabled by the new option `implicit_tls`. Contributed by Jan Schär. +Support Implicit TLS for sending emails, enabled by the new option `force_tls`. Contributed by Jan Schär. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index b140caef040d..e9ea4973e7e4 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3150,13 +3150,13 @@ Server admins can configure custom templates for email content. See This setting has the following sub-options: * `smtp_host`: The hostname of the outgoing SMTP server to use. Defaults to 'localhost'. -* `smtp_port`: The port on the mail server for outgoing SMTP. Defaults to 465 if `implicit_tls` is true, else 25. +* `smtp_port`: The port on the mail server for outgoing SMTP. Defaults to 465 if `force_tls` is true, else 25. - _Changed in Synapse 1.64.0:_ the default port is now aware of `implicit_tls`. + _Changed in Synapse 1.64.0:_ the default port is now aware of `force_tls`. * `smtp_user` and `smtp_pass`: Username/password for authentication to the SMTP server. By default, no authentication is attempted. -* `implicit_tls`: By default, Synapse connects over plain text and then optionally upgrades - to TLS via STARTTLS. If this option is set to true, TLS is used from the start, +* `force_tls`: By default, Synapse connects over plain text and then optionally upgrades + to TLS via STARTTLS. If this option is set to true, TLS is used from the start (Implicit TLS), and the option `require_transport_security` is ignored. It is recommended to enable this if supported by your mail server. @@ -3225,7 +3225,7 @@ email: smtp_port: 587 smtp_user: "exampleusername" smtp_pass: "examplepassword" - implicit_tls: true + force_tls: true require_transport_security: true enable_tls: false notif_from: "Your Friendly %(app)s homeserver " diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index b426cd36189a..73b469f41480 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -85,10 +85,10 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: if email_config is None: email_config = {} - self.implicit_tls = email_config.get("implicit_tls", False) + self.force_tls = email_config.get("force_tls", False) self.email_smtp_host = email_config.get("smtp_host", "localhost") self.email_smtp_port = email_config.get( - "smtp_port", 465 if self.implicit_tls else 25 + "smtp_port", 465 if self.force_tls else 25 ) self.email_smtp_user = email_config.get("smtp_user", None) self.email_smtp_pass = email_config.get("smtp_pass", None) @@ -96,8 +96,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: "require_transport_security", False ) self.enable_smtp_tls = email_config.get("enable_tls", True) - if self.implicit_tls and not self.enable_smtp_tls: - raise ConfigError("email.implicit_tls requires email.enable_tls to be true") + if self.force_tls and not self.enable_smtp_tls: + raise ConfigError("email.force_tls requires email.enable_tls to be 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" diff --git a/synapse/handlers/send_email.py b/synapse/handlers/send_email.py index 48adbd89c8d1..e2844799e886 100644 --- a/synapse/handlers/send_email.py +++ b/synapse/handlers/send_email.py @@ -61,7 +61,7 @@ async def _sendmail( require_auth: bool = False, require_tls: bool = False, enable_tls: bool = True, - implicit_tls: bool = False, + force_tls: bool = False, ) -> None: """A simple wrapper around ESMTPSenderFactory, to allow substitution in tests @@ -78,7 +78,7 @@ async def _sendmail( require_tls: if TLS is not offered, fail the reqest enable_tls: True to enable STARTTLS. If this is False and require_tls is True, the request will fail. - implicit_tls: True to enable Implicit TLS. + force_tls: True to enable Implicit TLS. """ msg = BytesIO(msg_bytes) d: "Deferred[object]" = Deferred() @@ -109,7 +109,7 @@ def build_sender_factory(**kwargs: Any) -> ESMTPSenderFactory: # set to enable TLS. factory = build_sender_factory(hostname=smtphost if enable_tls else None) - if implicit_tls: + if force_tls: reactor.connectSSL( smtphost, smtpport, @@ -146,7 +146,7 @@ def __init__(self, hs: "HomeServer"): 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._implicit_tls = hs.config.email.implicit_tls + self._force_tls = hs.config.email.force_tls self._sendmail = _sendmail @@ -204,5 +204,5 @@ async def send_email( require_auth=self._smtp_user is not None, require_tls=self._require_transport_security, enable_tls=self._enable_tls, - implicit_tls=self._implicit_tls, + force_tls=self._force_tls, ) diff --git a/tests/handlers/test_send_email.py b/tests/handlers/test_send_email.py index b090da787fce..da4bf8b5829a 100644 --- a/tests/handlers/test_send_email.py +++ b/tests/handlers/test_send_email.py @@ -115,11 +115,11 @@ def test_send_email(self): { "email": { "notif_from": "noreply@test", - "implicit_tls": True, + "force_tls": True, }, } ) - def test_send_email_implicit_tls(self): + def test_send_email_force_tls(self): """Happy-path test that we can send email to an Implicit TLS server.""" h = self.hs.get_send_email_handler() d = ensureDeferred(