Provide lwt_ssl for encrypted connection

[xapantu]

Just some replace in the modules name to make the client work on ssl. The ssl context could be exposed, but I am not sure it is really worth it.

[coveralls]

Coverage remained the same at 36.667% when pulling 825789d on xapantu:master into 90475b8 on johnelse:master.

[c-cube]

First: I'd be really happy if irc-client had SSL support, so thanks!

I did not read in detail, but it looks like there is no certificate checking (which might be fine) and that it relies on old versions of SSL (Sslv23?). In other words, how robust is this security-wise?

[xapantu]

Sorry for the late answer.
For the certificate checking, it relies on which certificate are accepted at the system level (so that is probably fine, unless someone uses self-signed certificate for a test server, but I think that it is old fashioned nowadays - anyway in that case it can still be added to the system certificate list).

Regarding the SSL version, I think I ran into problems with newer versions. I think we could make this module a functor taking the SSL version as an argument.

[c-cube]

Ok, I suppose it's fine for the certificates then! It would indeed be nice to be able to pass Ssl.TLSv12 or something similar (more recent crypto than SSLv3…), either with a functor or trying again with the best constructor. Another possibility is to add a custom default argument in the signature of Irc_client, that would be unit in most implementations and a tuple of parameters for SSL…

[hannesm]

out of curiosity, where does the certificate checking happen? I cannot find it in the code of this PR, is there some magic in Lwt_ssl (or ocaml-ssl) which loads the trust store? (examples from ocaml-ssl call out to set_verify (see https://github.com/savonet/ocaml-ssl/blob/9bbbda1c5015cc2a8ddaeae4fdc4a19f24448a9e/examples/stelnet.ml#L64)).

regarding versions: you have to decide whether you want to have proper crypto (AEAD ciphers and TLS-1.2) or not (all previous versions).

[xapantu]

Hum, you are right, I misunderstood the libssl library then. According to this page, https://www.openssl.org/docs/manmaster/ssl/SSL_CTX_set_verify.html (and I suppose that the default behavior is SSL_VERIFY_NONE, but that's not clear), it seems that the default behavior is to check the certificate but continue even if it fails.

Then I will change that to be a functor taking the TLS version and a boolean to indicate that the certificate should be checked as arguments.

[xapantu]

I just updated the PR with the discussed changes.

[coveralls]

Coverage remained the same at 36.667% when pulling c30bd04 on xapantu:master into 90475b8 on johnelse:master.

[johnelse]

Apologies for the lack of response on this, my github notifications seem have been disappearing into the void for the last few weeks.

Thanks for the welcome comments @c-cube and @hannesm. I'll try to take a look at this in the next couple of days.

[c-cube]

I gave a try to the test bot (on commit c30bd04), as follows:

./example3.byte -host chat.freenode.net -chan '##testocamlirc' -port 6697

and get an error:

Connecting...
Certificate[2] subject=/C=US/ST=New Jersey/L=Jersey City/O=The USERTRUST Network/CN=USERTrust RSA Certification Authority
Certificate[2] issuer =/C=SE/O=AddTrust AB/OU=AddTrust External TTP Network/CN=AddTrust External CA Root
SSL: rejecting connection - error=20
Fatal error: exception Ssl.Connection_error(1)

Any idea why this fails? Is there some check missing, or is some specific IRC message related to ssl connections missing?

[xapantu]

Humpf, I tried on my macbook at work and it seemed to work, although I have the same issue now on my laptop :(

So, I investigated that, and I have no idea how to make it work, if you take a look at the example (the only one for ocaml ssl I am aware of?) https://github.com/savonet/ocaml-ssl/blob/master/examples/stelnet.ml, and launch it with chat.freenode.net and 6697, it does not work either.

I do not know wether I did something wrong when I tried on the mac, or if it is a linux-specific problem.

[c-cube]

This is quite puzzling, maybe @smimram would have an idea?

[c-cube]

So, should we escalate this to ocaml-tls? Unless we missed something in the documentation...

[c-cube]

@xapantu for me the stelnet.ml examples works with chat.freenode.net -p 6697, but fails if I add -w (paranoid mode). So my guess is that it fails when it checks certificates. I think it would be nice if the option was clearly available (maybe you did it already, then example3 needs to be updated).

[hannesm]

(jumping into this discussion again: using tlsclient from https://github.com/hannesm/tlsclient I see the following (using system wide trust anchors))

# ./tlsclient.native --ca /usr/local/etc/ssl/ chat.freenode.net:6697
protocol : TLS_1_2
timestamp: 2016-09-18T11:30:26-00:00 (raw: 1474198226)
cipher   : TLS_DHE_RSA_WITH_AES_256_CBC_SHA256
master   : c4a10552b0978314b3e17b41715da9a21cd6bd25b8008bee4651eb30bab6d70c07231ee0828724261e947fcab85c3263
keysize  : 2048
chain    : subject=OU=Domain Control Validated/OU=Gandi Standard Wildcard SSL/CN=*.freenode.net
           issuer=C=FR/SP=Paris/L=Paris/O=Gandi/CN=Gandi Standard SSL CA 2
           certificate sha256 fingerprint: cb7607bbd5a49ba59e1fbbfdbd2b853320c0b5ab39a120ce002d8c5eaf6d30df
           public key sha256 fingerprint:  c11188855ca89a61315b27c91d1df6c142d53905613699ef8512b2d9fd085daf

           subject=C=FR/SP=Paris/L=Paris/O=Gandi/CN=Gandi Standard SSL CA 2
           issuer=C=US/SP=New Jersey/L=Jersey City/O=The USERTRUST Network/CN=USERTrust RSA Certification Authority
           certificate sha256 fingerprint: b9f2164323638dce0b92218b43c41c1b2b2696389329db19f5cf7ad49b5cb372
           public key sha256 fingerprint:  586264c988f1d5031d31ed14aa5c8e297b7274f0d5ae4eec9767d5fa7366d6be

server   : subject=OU=Domain Control Validated/OU=Gandi Standard Wildcard SSL/CN=*.freenode.net
           issuer=C=FR/SP=Paris/L=Paris/O=Gandi/CN=Gandi Standard SSL CA 2
           certificate sha256 fingerprint: cb7607bbd5a49ba59e1fbbfdbd2b853320c0b5ab39a120ce002d8c5eaf6d30df
           public key sha256 fingerprint:  c11188855ca89a61315b27c91d1df6c142d53905613699ef8512b2d9fd085daf

anchor   : C=US/SP=New Jersey/L=Jersey City/O=The USERTRUST Network/CN=USERTrust RSA Certification Authority

thus, the certificate chain of chat.freenode.net is pretty ok. on a related node, in jackline (an xmpp client) I provide two ways of checking the validity of the server: either by a chain of trust or by a fingerprint of the server certificate (soon public key). This is nice as well for private servers which do not have a "valid" server certificate (but rather a self-signed one).

[c-cube]

Someone suggested, on #OCaml, that we use ocaml-tls instead of SSL… It might be simpler?

[xapantu]

Well, when I looked for ssl libraries, it did not look very finished. In the readme of ocaml-tls:

Please be aware that this software is beta and is missing external code audits. It is not yet intended for use in any security critical applications.

However, it's unclear wether an irc-client can be considered as a critical application…

[c-cube]

@xapantu indeed, although ocaml-tls seems reasonably robust. Arguably, some TLS is better than no TLS in both cases, and since we cannot seem to make lwt.ssl work reliably…

[hannesm]

That's me (one of the authors of OCaml-TLS), maybe we should rephrase. In a similar project jackline, an XMPP client, I'm using OCaml-TLS already. It is also serving several websites, such as https://mirage.io https://nqsb.io - and works reliable there.

Also, our Piñata is still up and holding the 10 BTC. But code reviews are appreciated, as well as usage experiences.

[xapantu]

Ok, I see. Then I guess that is probably the best option. I may or may not have the time update this branch, so everyone is free to jump on this ;)

[c-cube]

This should be closed, I think, since #29 was merged.

[johnelse]

Thanks to @c-cube this is now in - see #49.