#41935 smtp: allow self-signed certificates on localhost

[Cybso]

• Resolves:
  • Automatically set allow_self_signed=true if mail_smtphost = localhost #41935
  • [Bug]: NEXTCLOUD 26 Email Setup no Test successfully #37329
  • [Bug]: No E-Mails sent in Nextcloud 26.0.0 #37694
  • [Bug]: Nextcloud 26.0.2 email notification error #38957
  • [Bug]: Mail Component ignoring encryption setting (NC 26) #39452
  • [Bug]: Cant sending mail plain text to 127.0.0.1 after update to 26.0.4 or cant use STARTTLS with TLS v1. The mail system require min TLS v1.2! #39538
  • [Bug]: Email could not be sent #40073
  • and maybe more

Summary

A mailserver listening on localhost can never have a valid TLS certificate and it does not require one. This patch enables allow_self_signed and disables verify_peer and verify_peer_name when mail_smtphost equals localhost or 127.0.0.1.

Pull request as suggested by joshtrichards in #41935

[szaimen]

In theory we could even do this if no valid host was provided, e.g. by using such a filter: https://github.com/nextcloud/all-in-one/blob/c9b97220d0175914c3ee8c7199949376f7bfe3b0/php/src/Data/ConfigurationManager.php#L274-L295

However this sounds like magic and not sure if we want to actually do this automatically.

WDYT @nextcloud/server-backend @miaulalala

[kesselb]

1) Test\Mail\MailerTest::testStreamingOptionsWrongType
Failed asserting that 1 matches expected 0.

/home/runner/actions-runner/_work/server/server/tests/lib/Mail/MailerTest.php:291

[Cybso]

1) Test\Mail\MailerTest::testStreamingOptionsWrongType
Failed asserting that 1 matches expected 0.

/home/runner/actions-runner/_work/server/server/tests/lib/Mail/MailerTest.php:291

I will adapt the test as soon as it is clear whether this change has a chance of being accepted or not.

Personally I'm totally fine if the PR is being rejected. #41935 was just an idea to reduce confusion for users using a local mail server, resulting in many false bug reports.

[miaulalala]

I'm of two minds here - one is that it indeed makes localhosts work without any setup, which, as you stated has confused many users due to how the Symfony Mailer works. OTOH I'm not entirely happy with overwriting config options per default.

I think there's some middle ground here: check if the ssl option is set and only overwrite it if a) the host is localhost || 127.0.0.1 and b) if the option doesn't already exist in the $currentStreamingOptions

I also think it doesn't need to be an elseif but can be an if condition on it's own. Something like:

Suggested change

	} else if ($mailSmtphost === 'localhost' || $mailSmtphost === '127.0.0.1') {
	$currentStreamingOptions = $stream->getStreamOptions();
	$currentStreamingOptions = array_merge_recursive($currentStreamingOptions, array(
	'ssl' => array(
	'allow_self_signed' => true,
	'verify_peer' => false,
	'verify_peer_name' => false
	)
	));
	}
	$local = ['localhost', '127.0.0.1']; // You could make this a constant
	if (in_array($mailSmtphost, $local) && !isset($currentStreamingOptions['ssl'])) {
	$currentStreamingOptions = $stream->getStreamOptions();
	$currentStreamingOptions = array_merge_recursive($currentStreamingOptions, array(
	'ssl' => array(
	'allow_self_signed' => true,
	'verify_peer' => false,
	'verify_peer_name' => false
	)
	));

[kesselb]

symfony/symfony#53621 (but it will take a while until we are on mailer 7.1)

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[szaimen]

Hi, we have a checkbox now to allow insecure certificates: https://github.com/nextcloud/server/pull/57544/changes#diff-d611085ec819ab74b9abec857410a14d726ea1b1c0b84dc0d99bd88be39f54d0R165-R167 so this can be closed imho

[miaulalala]

Thanks so much for your time and effort @Cybso ! #57544 will hopefully solve the issues with self signed certificates (and servers that advertise TLS but don't actually support it).