Skip to content
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

Add an option to avoid tls certificate validation #112

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

dampee
Copy link

@dampee dampee commented Aug 23, 2023

This fixes #89

@nblumhardt
Copy link
Member

Hi Damiaan, thanks for giving this a nudge! 👍

Your timing is great, we're rewriting the email app with a view to addressing some of these longstanding issues. Because a lot of deployments already use this package, I'm keen to keep it stable and make changes in the new version, for now.

The source is at: https://github.com/datalust/seq-app-mail - there isn't a published package but that should change today or tomorrow.

The type that lays out TLS options is called ProtocolSecurity: https://github.com/datalust/seq-app-mail/blob/dev/src/Seq.App.Mail.Smtp/ProtocolSecurity.cs

I think that it would be fair game to incorporate the behavior from this PR into the None option, as long as the help text spelled out the caveats (e.g. "Certificate validation will be skipped in this mode. Do not use this option in production environments.").

What do you think?

@dampee
Copy link
Author

dampee commented Aug 24, 2023

I thought there was difference between using the correct protocol and whether the certificate is valid. A mailserver can enforce you to use the StartTLS protocol extension, but its certificate can be expired or be for another hostname. In that case "none" from the suggested ProtocolSecurity would not work, I guess. I might be wrong.

However, there is one remark I would like to add. I don't know if that is an option for SEQ apps. But I would hide this setting in the normal UI and only add this (e.g.) using the JSON configuration. The setting which I proposed to add should be very uncommon and adding in the documentation is enough.

In the meantime, I do understand our use case a little bit better. In our case, the application needs to use an internal mail server (which does not send external e-mail). The server is on another VLAN and my machine is in a cloud. I am not able to use its DNS name, and I need to access it using the IP address (still using TLS). This is the reason MailKit complains, because the certificate is for a different host name and thus rejected.

@vlm---
Copy link

vlm--- commented Aug 24, 2023

Our internal mail server is in failover cluster, but when TLS is starting on a node it responds with its own valid certificate. Then MailKit complains that its not valid for the cluster address and ends with error. Currently, I have to use separate mail app instance for every node of the cluster so at least one of them sends any messages.
I agree that setting for skipping certificate validation should be separate to the TLS one.

@nblumhardt
Copy link
Member

Thanks for your replies. I can see on the surface how separate settings seem to make sense, but as soon as you've selected the "ignore certificate errors" option, the other TLS options are essentially redundant - it's trivial to MITM the traffic (as far as I understand it).

In your case @vlm---, setting up the mail servers with valid certificates for the cluster load balancer's DNS name would be the way to go.

@nblumhardt
Copy link
Member

Howdy! We've pushed forward with Seq.App.Mail.Smtp, it might not perfectly line up with what you had in mind but if I've understood everything correctly it should be usable in the scenario you're targeting 🤞 - all the details are at https://github.com/datalust/seq-app-mail, in case you're able to take a look. HTH!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Untrusted Certificates
3 participants