MailKit shouldn't use insecure SSL connections by default

[kpreisser]

Describe the bug
Since MailKit is recommended by Microsoft as alternative to the System.Net.SmtpClient, I started looking into using it. However, I was surprised that it seems to facilitate using insecure SSL connections (which may expose user credentials and mail contens to attackers), doe to the following issues (these are not limited to the SmtpClient):

• The README.md contains sample code that uses a certification validation callback that accepts all certificates:
  

  MailKit//README.md

  Lines 269 to 273 in cb29805

  	using (var client = new SmtpClient ()) {
  	// For demo-purposes, accept all SSL certificates (in case the server supports STARTTLS)
  	client.ServerCertificateValidationCallback = (s,c,h,e) => true;
  	client.Connect ("smtp.friends.com", 587, false);

  
  Basically, when using this code, it means the connection is not secure, because a MITM attacker can just create and inject a self-signed certificate (or a trusted certificate that is valid only for a different domain, etc.), and the SmtpClient will happily connect to it and expose the user's credentials and mail contents.
  Since many people will just copy-paste example code into their project, it is likely people will also copy the code to accept all certificates without understanding or changing it, and then their credentials (and integrity/confidentiality of the mail) are at risk.
  
  I think example code should not promote insecure configuration, and therefore should not use a validation callback that will just accept all certificates. I think it should show how to make a secure connection with SecureSocketOptions.SslOnConnect or StartTls.
  

• I would expect that if I don't specify the ServerCertificateValidationCallback, the SmtpClient will just use the default validation mechanism of the SslStream, just like a browser would do when loading a https URL, or a program like Thunderbird would to when connecting to a server using SSL or STARTTLS, which is to reject non-trusted certificates.
  
  However, actually when the callback is not specified (and ServicePointManager.ServerCertificateValidationCallback is also not set), SmtpClient will fall back to DefaultServerCertificateValidationCallback, which accepts self-signed certificates:
  

  MailKit/MailKit/MailService.cs

  Lines 339 to 358 in cb29805

  	var selfSigned = !string.IsNullOrEmpty (element.Certificate.Subject) && element.Certificate.Subject == certificate.Issuer;
  	foreach (var status in element.ChainElementStatus) {
  	if (selfSigned && status.Status == X509ChainStatusFlags.UntrustedRoot) {
  	// treat self-signed certificates with an untrusted root as valid since they are so
  	// common among mail server installations
  	continue;
  	}
  	if (status.Status != X509ChainStatusFlags.NoError) {
  	// if there are any other errors in the certificate chain, the certificate is invalid,
  	// so return false
  	return false;
  	}
  	}
  	// Note: If we get this far, then the only errors in the certificate chain are untrusted root errors for
  	// self-signed certificates. Since self-signed certificates are so common for mail server installations,
  	// treat the certificate as valid.
  	return true;

  
  Honestly, I was shocked when I saw that code, since that means it also doesn't provide any security (since an MITM attacker can just use a self-signed certificate like with the callback mentioned before), and so the user's credentials and mail contents are also put at risk, even if users don't specify a certificate validation callback and specify SecureSocketOptions.SslOnConnect or SecureSocketOptions.StartTls, where I would expect MailKit to create a secure connection.
  
  I think when the user doesn't specify a certificate validation callback, the SmtpClient should just validate the certificate like other programs do. This can be done by changing DefaultServerCertificateValidationCallback to the following:

        public static bool DefaultServerCertificateValidationCallback(object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
        {
            return sslPolicyErrors == SslPolicyErrors.None;
        }

  For example, this is what SslStream does when you don't specify a certificate validation callback.

  If the user still wants to accept an self-signed certificate, they can still set the ServerCertificateValidationCallback and implement the corresponding checks to allow that certificate.
  However, I would expect that in most cases users will use an external server that has a publicly trusted certificate (like smtp.gmail.com), and therefore can use a secure connection without needing to specifically allow a self-signed certificate.

• SecureSocketOptions has members StartTlsWhenAvailable and Auto, which attempt to establish a secure connection, but continue with an unencrypted connection if the server doesn't support a secure connection. From a security perspective, this means these options don't provide security (like None), as a MITM attacker could just inject a mail server that doesn't support SSL/STARTTLS, and then MailKit will continue with an unencrypted connection.
  In order to establish a secure connection, you will need to use SslOnConnect or StartTls, which fail if a secure connection cannot be established.
  
  So while it's possible to force a secure connection, my concern with Auto is that it may lead people into thinking that it will automatically be secure, which is not the case. Also, StartTlsWhenAvailable looks like it will be secure as it says Tls, whereas actually it may use an unencrypted connection.
  
  Maybe these enum values can be removed (or be deprecated)?
  For example, when I set the SMTP settings in Thunderbird, it shows only the following options (and both SSL/TLS and STARTTLS will fail if no secure connection can be established):
  

Expected behavior

• The README.md should not contain example code that uses a certification validation callback that accepts all certificates (which is insecure).
• DefaultServerCertificateValidationCallback should not accept self-signed certificates (which is insecure); it should use the default certificate validation mechanism.
• Maybe SecureSocketOptions members Auto and StartTlsWhenAvailable can be reworked or removed/deprecated, as they may lead to creating an insecure connection.

Thank you!

[jstedfast]

The README.md contains sample code that uses a certification validation callback that accepts all certificates:

Yea, because I got tired of people filing bug reports about MailKit not connecting to their IMAP, POP3 and/or SMTP servers.

There's a reason I had to add that snippet to the samples in the README.md. And there's a reason I had to implement DefaultServerCertificateValidationCallback.

I can assure you, those methods weren't for me.

I would expect that if I don't specify the ServerCertificateValidationCallback, the SmtpClient will just use the default validation mechanism of the SslStream

A fair expectation, for sure, but anyone with any real security aficionado would implement a custom callback, anyway.

The average newbie writing their first program that they just want to point at their IMAP server and download their messages w/o having to understand anything about how SSL certificate validation works, however, won't have the knowledge to do so.

Life comes with compromises:

I can either tailor MailKit toward the security aficionados who know how to write their own validation callback...

Or I can tailor MailKit toward the newbs.

I chose the newbs because that's mostly who is using MailKit (or, at least, the loudest voices that I frequently hear from).

If the user still wants to accept an self-signed certificate, they can still set the ServerCertificateValidationCallback and implement the corresponding checks to allow that certificate.
However, I would expect that in most cases users will use an external server that has a publicly trusted certificate (like smtp.gmail.com), and therefore can use a secure connection without needing to specifically allow a self-signed certificate.

Do you know which server it was that I was getting the most complaints about MailKit having problems connecting to over SSL?

Hint: GMail.

You might have the Root CA certificate for GMail's SSL certificate stored and trusted on your machine, but a surprisingly large number of developers are using MailKit on Linux (and Mac?) which do not have the Root CA certificates that you expect to exist, and so SSL/STARTTLS connections will always fail.

So while it's possible to force a secure connection, my concern with Auto is that it may lead people into thinking that it will automatically be secure, which is not the case.

Just because an option exists doesn't mean you have to use it. If you don't want to use it, then the option isn't for people like you.

That doesn't mean the option isn't useful for other people.

The Thunderbird options are the equivalent of SslOnConnect, StartTls and None. You can limit yourself to those options. It'll be fine. Really.

[jstedfast]

I'm open to having a real conversation that could lead to improvements in this area, but I need real proposals that allow the average newbie programmer to get something working.

If you'd like to write up some documentation on how to properly configure the SSL certificate validation callback such that it will work and explain it in such a way that even the average newbie programmer will understand and be able to accomplish the task, then it would allow me to restore MailKit's defaults (and README.md docs) to being more secure which I would love to be able to do.

In the meantime, perhaps something like this would be a start toward a compromise?

const string AppleCertificateIssuer = "C=US, O=Apple Inc., OU=Certification Authority, CN=Apple IST CA 2 - G1";
const string GMailCertificateIssuer = "CN=GTS CA 1O1, O=Google Trust Services, C=US";
const string OutlookCertificateIssuer = "CN=DigiCert Cloud Services CA-1, O=DigiCert Inc, C=US";
const string YahooCertificateIssuer = "CN=DigiCert SHA2 High Assurance Server CA, OU=www.digicert.com, O=DigiCert Inc, C=US";
const string GmxCertificateIssuer = "CN=TeleSec ServerPass Extended Validation Class 3 CA, STREET=Untere Industriestr. 20, L=Netphen, OID.2.5.4.17=57250, S=Nordrhein Westfalen, OU=T-Systems Trust Center, O=T-Systems International GmbH, C=DE";

/// <summary>
/// The default server certificate validation callback used when connecting via SSL or TLS.
/// </summary>
/// <remarks>
/// <para>The default server certificate validation callback recognizes and accepts the certificates
/// for a list of commonly used mail servers such as gmail.com, outlook.com, mail.me.com, yahoo.com,
/// and gmx.net.
/// </remarks>
/// <returns><c>true</c> if the certificate is deemed valid; otherwise, <c>false</c>.</returns>
/// <param name="sender">The object that is connecting via SSL or TLS.</param>
/// <param name="certificate">The server's SSL certificate.</param>
/// <param name="chain">The server's SSL certificate chain.</param>
/// <param name="sslPolicyErrors">The SSL policy errors.</param>
public static bool DefaultServerCertificateValidationCallback (object sender, X509Certificate certificate, X509Chain chain, SslPolicyErrors sslPolicyErrors)
{
	if (sslPolicyErrors == SslPolicyErrors.None)
		return true;

	if (certificate is X509Certificate2 certificate2) {
		var cn = certificate2.GetNameInfo (X509NameType.SimpleName, false);
		var fingerprint = certificate2.GetCertHashString ();
		var serial = certificate2.SerialNumber;
		var issuer = certificate2.Issuer;

		switch (cn) {
		case "imap.gmail.com":
			return issuer == GMailCertificateIssuer && serial == "0096768414983DDE9C0800000000320A68" && fingerprint == "A53BA86C137D828618540738014F7C3D52F699C7";
		case "pop.gmail.com":
			return issuer == GMailCertificateIssuer && serial == "00D80446EA4406BA970800000000320A6A" && fingerprint == "379A18659C855AE5CD00E24CEBE2C6552235B701";
		case "smtp.gmail.com":
			return issuer == GMailCertificateIssuer && serial == "00A2683EEFC8500CA20800000000320A71" && fingerprint == "8F0A0B43DE223D360C4BBC41725C202B806CED32";
		case "outlook.com":
			return issuer == OutlookCertificateIssuer && serial == "0654F84B6325595A20BC68A6A5851CBB" && fingerprint == "7F0804B4D0A6C83E46A3A00EC98F8343D7308566";
		case "imap.mail.me.com":
			return issuer == AppleCertificateIssuer && serial == "62CBBFC566127C4758E96BDBC38EC9E6" && fingerprint == "E1A5F9D22A810979CACDFC0B4151F561E8D02976";
		case "smtp.mail.me.com":
			return issuer == AppleCertificateIssuer && serial == "3460D64A763D9ACA4B460C25021653C7" && fingerprint == "C262F01E83D6CE0C361E8B049E5BE8FE6E55806B";
		case "*.imap.mail.yahoo.com":
			return issuer == YahooCertificateIssuer && serial == "0B2804C9ED82D14FEFEF111E54A0551C" && fingerprint == "F8047F0F60C4641F718353BE7DDC31665B96B5C0";
		case "legacy.pop.mail.yahoo.com":
			return issuer == YahooCertificateIssuer && serial == "05179AA3E07FA5B4D0FC55A7A950B8D8" && fingerprint == "08E010CBAEFAADD20DB0B222C8B6812E762F28EC";
		case "smtp.mail.yahoo.com":
			return issuer == YahooCertificateIssuer && serial == "0F962C48837807B6556C5B6961FC4671" && fingerprint == "E53995EBA816FB73FD4F4BD55ABED04981DA0F18";
		case "mail.gmx.net":
			return issuer == GmxCertificateIssuer && serial == "218296213149726650EB233346353EEA" && fingerprint == "67DED57393303E005937D5EDECB6A29C136024CA";
		}
	}

	return false;
}

[kpreisser]

Thanks for your reply. I'm sorry for sounding so critisizing, but for me security is an important criterion when deciding what library to use. Nevertheless, I want to thank you for implementing and maintaining such a great .NET mail library!

Regarding your previous comment:

There's a reason I had to add that snippet to the samples in the README.md. And there's a reason I had to implement DefaultServerCertificateValidationCallback.

I can assure you, those methods weren't for me.

OK, I can understand the point about avoiding complaints by users, by providing example code that accepts all certificates. (Still, I think it can be stated more explicitly in the comments of that code that accepting all certificates whichout validating them is not secure).

However, the current DefaultServerCertificateValidationCallback implementation which acceps any self-singed certificate can be viewed as a security vulnerability, as it allows insecure connections even if the user doesn't specify a certificate validation callback and uses SslOnConnect or StartTls (which normally implies the connection will be secure, as that's also the behavior of SslStream). (And if users use the example code from the README, they will already be using the callback that accepts all certificates anyway).

For example, if I hadn't inspected the source code of SmtpClient and I hadn't checked what SslStream does when not specifying a validation callback, I wouldn't know that I specifically have to write client.ServerCertificateValidationCallback = (s, c, h, e) => e == SslPolicyErrors.None;, just to use the regular process of certificate validation against known root CAs that other clients also do, rather than accepting any self-signed certificate and therefore possibly exposing my credentials or other content to unauthenticated servers.

Hint: GMail.

You might have the Root CA certificate for GMail's SSL certificate stored and trusted on your machine, but a surprisingly large number of developers are using MailKit on Linux (and Mac?) which do not have the Root CA certificates that you expect to exist, and so SSL/STARTTLS connections will always fail.

Do you know a specific OS/runtime configuration where validating the GMail certificate fails?

I just created a .NET Core 2.1 app that creates a TcpClient to connect to smtp.gmail.com:445 (and to untrusted-root.badssl.com:443 to check that certificate validation actually works), and uses a SslStream to call AuthenticateAsClient(hostname), without specifying a certificate validation callback.

I then run it on Ubuntu 18.04, Debian 9.12, macOS 10.14.6, and on Windows 10 Version 1909, and on all these OSes the Gmail certificate was trusted, whereas the "untrusted-root.badssl-com" was rejected as expected ("The remote certificate is invalid according to the validation procedure."),

Additionally, I compiled the program for net47 and installed the current Mono version (6.8.0) on Debian 9.12, but with the same result.

I could imagine that this might happen with some (older?) Mono versions, that might use a different/outdated root certificate store. If that's the case, the problem will hopefully vanish in the future with the unified .NET runtime. (I'd think the problem of missing up-to-date root certificates will not be limited to MailKit, but would also affect components like HttpClient, which also do a certificate validation for https URLs.)

In the meantime, perhaps something like this would be a start toward a compromise?

If there are still runtime configurations where validating one of these certificates fails, then yes, I think it can be a good starting point to hard-code certificates of popular mail servers, to serve as a fallback if the user's runtime wouldn't consider these certificates to be trusted.

(However, note that I'm not a cryptographer, so I don't know the "right" way to validate the certificate, which is why I normally want to delegate it to the .NET runtime which knows how to do it correctly.)

I think as in your sample code, comparing the serial and fingerprint will work (maybe also the public key needs to be compared). However, I think rather than comparing the hostname from the CN field, we should use the hostname that the SslStream wants to authenticate against, to select the certificate that we use for the checks. Otherwise, one of the mail servers could present his (trusted) certificate when acting as MITM, when the user actually wants to connect to a different mail server.

Thank you!

[jstedfast]

Awesome, I think this is shaping up to be a more productive conversation. Thanks.

the current DefaultServerCertificateValidationCallback implementation which acceps any self-singed certificate can be viewed as a security vulnerability

Yes, I agree. Let's see if we can figure out a better way to solve the problem that the DefaultServerCertificateValidationCallback is addressing.

Here are some ideas I've come up with (although I am open to other ideas as well):

1. Rewriting DefaultServerCertificateValidationCallback such that it contains some way of diagnosing this common issue and providing that information to the developer in an Exception that will inevitably get thrown when the callback returns false (or maybe we just throw inside of DefaultServerCertificateValidationCallback? Not sure what the best practice for this is).
2. Writing documentation in the FAQ.md that explains how to figure out if this is the problem and how to fix it. That can either be instructions per-platform (Windows/Linux/Mac) on how to download/install said certificate into their X509 Certificate Store so that SslStream will be satisfied *-or- * by writing their own custom callback method that will accept it.
3. I had a third one a second ago, but now I've forgotten it...

Do you know a specific OS/runtime configuration where validating the GMail certificate fails?

I do not.

For many years, Mono did not, by default, have anything in its trusted SSL certificate store until the user manually ran a command to import the root certificates from Mozilla (I think).

It may be that newer Mono packages for the various Linux distros no longer require users to manually run these commands, so it's possible that this is no longer an issue which is good news!

If there were reports about this not working for .NET Core, it was probably during the early days of .NET Core being ported to Linux/Mac (as in ~2-3 years ago).

(Thanks for testing this on various platforms, btw, that is super helpful)

However, I think rather than comparing the hostname from the CN field, we should use the hostname that the SslStream wants to authenticate against, to select the certificate that we use for the checks. Otherwise, one of the mail servers could present his (trusted) certificate when acting as MITM, when the user actually wants to connect to a different mail server.

Right, yes, you are correct. The hostname from the CN field should be matched against the hostname being connected to. I'll be sure to add that if we end up keeping that as a fallback.

[jstedfast]

I've made it such that the default SSL validation logic does 2 things:

1. If the only error is that the root CA certificate is untrusted and it matches a known mail server certificate, then it will return succeed (otherwise it will fail).
2. It now captures the validation callback arguments for use with the SshHandshakeException such that MailKit can throw an SslHandshakeException with a more informative message as well as including the Root CA and server certificates for diagnostic purposes.

I'm going to close this as fixed, but if you are not satisfied, we can continue discussion.

[jstedfast]

All docs have been updated to not use the client.ServerCertificateValidationCallback = (s, c, ch, e) => true; snippet as well, now.

[kpreisser]

Great, thanks a lot!