Proper support for SSL certificate verification

Issue #314 new
Guillaume DUPUY
created an issue

Currently, the client is perfectly capable of managing HTTPS connection, however certificate verification is system-dependent. On linux, everything works fine, openSSL check the OS certificate database, and has access to registered CA/Root to verify certs.

On windows, openSSL cannot do it directly, and you have the following error message : "Peer certificate cannot be authenticated with given CA certificates" To solve this, there is two solutions : Use Schannels, which is Microsoft re-implementation of openSSL. It's a drop in (curl can compile fine with it). Problem : it's not open source Open windows cert store, and load them into openSSL cert store (see https://stackoverflow.com/questions/9507184/can-openssl-on-windows-use-the-system-certificate-store for an example code)

On Mac OS, I'm not entirely sure, it seems like the steam client (& the app store, not sure about official one) cannot do it, but with a different error message : "Problem with the SSL CA cert (path? access right?)". To solve this, you need to use Secure transport (Apple re-implementation of openSSL), which is also a drop-in that curl can use, and it's open source.

I think the best fix is do nothing on linux (it works fine atm), add windows-specific code to load windows cert store into openSSL cert store, and add some documentation about mac OS & howto use secure transport for curl.

Comments (16)

  1. Rodolphe Breard

    By not supporting HTTPS for some platforms, app developers are forced to use plain HTTP. Right now it is still acceptable, but considering major web browsers vendors are going to deprecate plain HTTP and mark it as insecure (see announcement from Chromium security and Mozilla), we will soon be in a situation where app will work either in or out-game but not both.

    Well, I'm exaggerating a little bit, it is still possible to support both and redirect on HTTPS only if the user agent isn't Ryzom's one. But such configuration is out of the knowledge and/or control of most app developers. The only real solution is to quickly have a decent cross-platform support for HTTPS.

  2. Guillaume DUPUY reporter

    My main objection is that ryzom installer is using openSSL libraries (see CMakeLists.txt:365 " SET(QT_LIBRARIES ${QT_LIBRARIES} ${OPENSSL_LIBRARIES} ${ZLIB_LIBRARIES})"), and I don't think it's a good idea to use both openSSL and winSSL on the same OS (could lead to confusion and multiples bugs)

  3. Cédric OCHS

    Ryzom Installer is using Qt in static, that needs to be statically linked to OpenSSL :) I don't think WinSSL supports all features of OpenSSL. And great job Nimetu :) Could you apply your patch please ? I also prefer to keep using OpenSSL with custom code to read certificates from native OS :)

    I think using libcurl SSPI will remove the option to read certificates from BNP, but if we're using Windows native CA certificates, will we still need to read the one from BNP since it's a CA certificate (if I'm right) ? I think we need to make some tests :)

  4. Meelis Mägi

    I pushed it to develop 00d22baccc91 (with small fix for CertFree/CertClose).

    If you can verify that it compiles with the tools used, then please merge it to compatibilty-develop. I can only do limited testing with vs2015/win10

  5. Guillaume DUPUY reporter

    It compiles fine with visual studio 13 and wine, exact version is : Compilateur d'optimisation Microsoft (R) C/C++ version 18.00.40629 pour x64

    Can someone on Mac check if it works well with Secure Transport, or submit a similar patch ?

  6. Guillaume DUPUY reporter

    also, if we have a fully TLS-capable client, this mean we can use TLS for auth - as long as we patch in the same way http_client_curl. This would leave only the connection to FSHost in clear, and one can only get the Cookie from it - which is a one-time token (so you can't use it to log in - either the user already consumed it, or you consume it before him, and he won't be able to login, so he will try again and disconnect you), with only the user IP (which you already have if you are sniffing his packets) and user ID (which is a worthless information, I think) beeing available in the cookie.

    Obviously, connecting to FSHost over TLS would be even better, but wrapping ryzom protocol in TLS is a lot more dev work than just making sure curl works fine with TLS, and could be quite costly in terms of bandwidth (not an expert in TLS, but the overhead can be quite big on very little packets as far as I know)

  7. Guillaume DUPUY reporter

    Can anyone on mac confirm that using a curl compiled with Apple's Secure Transport solves the HTTPS issue ? One way to get it is by installing macport's curl with darwinssl variant :

    macports install curl +darwinssl

  8. Guillaume DUPUY reporter

    Is it a needed feature tho ? If we have proper TLS support on all platforms, the only situation where we would need to manually load a cert is if we have an invalid (self signed, out of date ...) Cert you want to be trusted. With let's encrypt, I don't see a situation where that were would happen. So I don't think it will be a missed feature :-)

  9. Cédric OCHS

    I have no idea :p I wondered about that previously :)

    I think using libcurl SSPI will remove the option to read certificates from BNP, but if we're using Windows native CA certificates, will we still need to read the one from BNP since it's a CA certificate (if I'm right) ? I think we need to make some tests :)

  10. Cédric OCHS

    In fact, I implemented reading certificates from BNP to check and patch certificates more easily. Before that they could only be read from a file and needed to be extracted :( curl was always able to process HTTPS, but I'm not sure if it was able to access system CA roots certificates.

  11. Cédric OCHS

    Apparently, compiling curl with native certificates support is enough to fix this issue, but I didn't yet test under Linux. It works under Windows and OS X. When all tests will be made and new external will be uploaded, I'll close this issue. I think perhaps we'll need to remove the NULL value for system certificate path under Linux.

  12. Log in to comment