Add email.tlsname config option

[cynhr]

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.

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct
  (run the linters)

[MadLittleMods]

Seems reasonable on face value.

Have you tested to make sure this works?

[cynhr]

Have you tested to make sure this works?

I did test it in one configuration (force_tls: true and Twisted v24.7.0, by applying the patch to the .deb-installed venv) and it did work as expected.

The build_sender_factory(hostname) argument is ultimately also passed to optionsForClientTLS, so it seems reasonable to believe that this has a similar effect with force_tls: false, though I did not test that.

I did not touch the branch for Twisted < v21. Should I try to implement this for old Twisted versions as well? That would basically entail backporting the hostname parameter introduced in Twisted v21 into our _NoTLSESMTPSender class, where that class would also need to close over the tlsname parameter, as it's value isn't provided through the v21 ESTMPSenderFactory. I don't have a system with old Twisted readily available to test that such an implementation.
Or maybe just log an error and refuse to send mail if Twisted < v21 and tlsname != smtphost?

[MadLittleMods]

I did not touch the branch for Twisted < v21. Should I try to implement this for old Twisted versions as well? That would basically entail backporting the hostname parameter introduced in Twisted v21 into our _NoTLSESMTPSender class, where that class would also need to close over the tlsname parameter, as it's value isn't provided through the v21 ESTMPSenderFactory. I don't have a system with old Twisted readily available to test that such an implementation.
Or maybe just log an error and refuse to send mail if Twisted < v21 and tlsname != smtphost?

Thanks for bringing this up!

If we're not going to support this option for Twisted < v21, it would be ideal fail early and not start up at all (error when parsing the config). Along with updating the documentation to state the version requirement. But I'm not sure of another spot where we make a similar constraint. Perhaps it's important that we support Twisted < v21 but I don't have that context.

I also don't have a sense for how big or complex the hostname backport would be but seems like it's do-able to you.

I don't have a system with old Twisted readily available to test that such an implementation.

It looks like the trial-olddeps CI job uses the minimum versions from our pyproject.toml which will be Twisted 18.9.0. So as long as we have some tests for this new functionality in tests/handlers/test_send_email.py, we should be covered.

I can run the CI when you want but should be also possible emulate the same thing that the CI job is doing to test it locally as well.