-
Notifications
You must be signed in to change notification settings - Fork 664
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sendmail: Use actual TLS connection if enabled #2896
base: dev
Are you sure you want to change the base?
Conversation
Currently, enabling TLS actually enables STARTTLS while the initial connection is still done as plain SMTP. This is counter-intuitive, since TLS, the way providers use it in their docs, usually means a TLS-encrypted connection right from the start. STARTTLS is usually named as such, and handled on another port, or enforced automatically on plain connections. This commit changes the behaviour to use an initial TLS-encrypted SMTP connection if this setting is enabled. While most providers allow and redirect all connection types on all ports, for some this may be a breaking change, i.e. it might be required to change the port e.g. from 587 to 465. Signed-off-by: MichaIng <micha@dietpi.com>
I tested GMail app password in current dev (0.43.1b1) and with these changes, using the "Test Email" button in ME. I hope it doesn't function different from the actual email sending (I didn't check from code if it's the same code that is run in both cases. It should, I guess.). Some notes from testing with dev branch: Testing with this PR:
I wonder if https://stackoverflow.com/a/69975850 and the link provided to Python SSL lib docs there helps in figuring this out 🤔 With port 587 + "Use TLS" disabled I get "SMTP AUTH extension not supported by server" which I think is fine as this was not changed. Additional finding: motioneye/motioneye/static/js/main.js Line 3113 in 2862efe
I also noticed that due to the lack of IPv6 support in my LAN and also Internet connection I had to enter the IP instead of the Google SMTP host (log complained about network being not reachable). This however occurs locally also with e.g. |
The preferred options for Gmail, which also match their docs is, after this PR, port 465 with TLS. It makes sense that port 465 with TLS did not work before since motionEye tries a plain connection (and STARTTLS) without this PR. Similarly it makes sense that it worked with port 587 and TLS before, since this is the port usually used for STARTTLS. Port 25 is usually for plain connections without STARTTLS, and I guess Gmail does not support this at all, but probably allows STARTTLS as well on this port. But then this should have worked before with TLS enabled in motionEye. So basically if now TLS works on port 465, all is as intended. Not sure whether we should support STARTTLS at all. |
Ah, yes, port 465 + "Use TLS" enabled works ok with these changes 👍 (and 465 with TLS disabled times out like before). So like you suspected in the PR description, port change would indeed be needed at least for GMail users. I'm wondering about the errors and instructions though, as at least I don't understand at all how the secure SMTP works with these two ports, so the regular user likely is no better. One easy option of course is to continue with the idea that "try 465 and 587 and see what works for you". |
So far so good. As said, port 465 is for TLS while port 587 is for "connect without TLS and then switch to TLS for data transfer via STARTTLS protocol", which we now do not support anymore. So with most providers 465 will be correct, or port 25 for entirely plain connections and plain data transfer. In case of the issue this PR is linked to, it did not work because the user followed Gmail instructions with port 465 + TLS which did not work because motionEye tried a plain connection and STARTTLS instead. Now TLS really means TLS. What we could do for a more graceful migration: Have a drop down with "plain/TLS/STARTTLS", and convert the old boolean config into STARTTLS, so it keeps working for those where it worked before. And we could/should add info to the port tooltip about the most commonly correct ports with Gmail as example. |
That would be pretty nice UX-wise, I think, and then there'd be also support for STARTTLS should some server require that. Some extra work, sure, but might save us from at least a couple of issues reporting upgrade broke email notifications... |
Currently, enabling TLS actually enables STARTTLS while the initial connection is still done as plain SMTP. This is counter-intuitive, since TLS, the way providers use it in their docs, usually means a TLS-encrypted connection right from the start. STARTTLS is usually named as such, and handled on another port, or enforced automatically on plain connections.
This commit changes the behaviour to use an initial TLS-encrypted SMTP connection if this setting is enabled. While most providers allow and redirect all connection types on all ports, for some this may be a breaking change, i.e. it might be required to change the port e.g. from 587 to 465.