search

datalust / seq-app-htmlemail

Plug-in apps that act on event streams in the Seq log server
Apache License 2.0
51 stars 38 forks source link

Add an option to avoid tls certificate validation #112

Open dampee opened 1 year ago

dampee commented 1 year ago

This fixes #89

nblumhardt commented 1 year ago

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 commented 1 year ago

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--- commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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!