karastojko / mailio

mailio is a cross platform C++ library for MIME format and SMTP, POP3 and IMAP protocols. It is based on standard C++ 17 and Boost library.
Other
372 stars 98 forks source link

SSL errors #173

Open FillipMatthew opened 1 month ago

FillipMatthew commented 1 month ago

Regarding the SSL issues as mentioned: https://github.com/karastojko/mailio/issues/163 https://github.com/karastojko/mailio/issues/148#issuecomment-1850034861

The suggested solution of disabling peer verification is unsafe.

Would it be possible to add an overload to the smtp::authenticate() method and for other classes that takes an externally created ssl::context? std::string authenticate(const std::string& username, const std::string& password, auth_method_t method, std::shared_ptr<ssl::context> ctx); This would allow users to setup certificate stores etc. before the connection is made.

Another option could be a callback after the ssl context is initialised that allows the user to modify the context before the socket gets initialised.

karastojko commented 4 weeks ago

Did you take a look of smtps::ssl_options(), pop3s::ssl_options() and imaps::ssl_options()? I believe they should provide you the way to control the SSL options. The reason for disabling verifying peer verification is that many email servers returned an error in this case.

FillipMatthew commented 4 weeks ago

ssl_options() does not solve the security issue and is also not flexible.

By disabling peer verification, it means that a hacker can make the Mailio client connect to a different server than intended and you won't know about it. The error you are seeing is not from the email servers, it is your client saying it cannot verify that the server is who it says it is. For SSL/TLS to work correctly you need to give the ssl context the certificate chain to use so it can verify the server it is connecting to. You are receiving this error because you didn't setup certificates.

Here are some ways to correctly do this:

  1. add_certificate_authority() can be used to authenticate specific servers that the user wants. For example maybe I only need my mail client to support a private email server, or I need to connect to a server that is using a self-signed key, I can add the certificate chain for that server, then my mail client will only be able to connect to that server. This can be called more than once to support a few specific servers.
    m_ctx.add_certificate_authority(const_buffer(caCertificate.c_str(), caCertificate.size()));
  2. On linux, or if using the OpenSSL certificate store you can use set_default_verify_paths(). This will allow you to use public servers that are your OpenSSL key store can authenticate. This doesn't work well on Windows. Self-signed or not very well-known servers may need to have their certificate chain manually installed to work.
  3. On Windows you need to manually import the certificates from the Windows keystore. https://stackoverflow.com/a/40710806 This will make the mail client able to connect to servers that your Windows machine can verify.

The Stack Overflow example shows using 1 and 2 together. This would be similar to how browsers can know if a website's server is safe. Since this in a library you would also want to support option one since someone may use it to make a tool for sending mail just through their own server, such as for notifications from their app. An example of context setup may look like this:

ssl::context ctx(ssl::context::sslv23);
ctx.set_options(ssl::context::default_workarounds | ssl::context::no_sslv2 | ssl::context::no_sslv3 | ssl::context::sslv23 | ssl::context::single_dh_use);
ctx.set_verify_mode(ssl::context::verify_peer);

#ifdef BOOST_OS_WINDOWS
add_windows_root_certs(ctx);
#else
ctx.set_default_verify_paths();
#endif

Like the example I also set extra options with set_options(). I don't remember why but I think it didn't work properly in the constructor.

Disabling peer verification is very bad, instead using those options should solve the error. If the user has a way to work with the ssl context before the socket creation then you won't need to write much support for all these things since the library user can then choose what method they need for themselves.

More advanced use also allows servers to require the client to verify itself with a certificate and key, then the client would need to register a certificate and private key or even a password callback to be able to use that key. This may look like this:

m_ctx.use_certificate_chain(const_buffer(caCertificateChain.c_str(), caCertificateChain.size()));
m_ctx.use_private_key(const_buffer(key.c_str(), key.size()), ssl::context::file_format::pem);
m_ctx.set_password_callback(&my_password_callback);

Or limiting the ciphers to only allow the most secure ones:

SSL_CTX_set_cipher_list(ctx.native_handle(), "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384");
SSL_CTX_set_ciphersuites(ctx.native_handle(), "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384");

I hope this helps, I didn't explain deeply how certificates work, read about SSL/TLS and x509 certificates if you didn't already understand it well.

FillipMatthew commented 3 weeks ago

mailio.zip If you're interested these are the changes I made for smtp. These changes are based on version 0.23.0.

My usage looks something like this:

m_mailClient.reset(new mailio::smtps(m_mailServer, m_mailServerPort));

// Setup the SSL context
auto ctx = make_shared<mailio::dialog_ssl::ssl_context>(ssl::context::sslv23);
ctx->set_options(ssl::context::default_workarounds | ssl::context::no_sslv2 | ssl::context::no_sslv3 | ssl::context::single_dh_use);
ctx->set_verify_mode(ssl::verify_peer);

ctx->set_default_verify_paths();
boost::certify::enable_native_https_server_verification(*ctx);

m_mailClient->authenticate(m_mailAddress, m_password, mailio::smtps::auth_method_t::START_TLS, std::move(ctx));
karastojko commented 3 weeks ago

Hey, I agree with your remarks,. This is a debt from my side. Please make a PR with your changes, so I could take a look. Since I am doing refactoring regarding the line policy and folding, I cannot immediately test your changes, but I will do it for sure and get back to you.

FillipMatthew commented 3 weeks ago

I had to manually copy my changes, I don't think I missed anything. I had tested it but only made the changes for smtps. Hope that helps