TLS hostname check

[jhass]

Based on #2707

With 1.0.2 OpenSSL gained support for doing hostname checks, see https://wiki.openssl.org/index.php/Hostname_validation

However is it okay to depend on 1.0.2? Should we implement our own hostname checking like Ruby does? Or given we find a way to detect the OpenSSL version (how?), should we use this only for OpenSSL 1.0.2+ or either this or a manual check?

I included a network dependent integration spec that should only manually be run, as well as API to enable CRL checks fairly easily, see #2707 and #2709.

[bcardiff]

I might be wrong, but I think trusty does not come with openssl 1.0.2+.
trusty is the the latest LTS. (Xenial was released in april, but trusty is used a lot).

I don't mind using either this or a manual check.
It's security, low level stuff, we will need to deal with some outdated libs IMO.

[kostya]

is there option to disable check?

[fernandes]

@bcardiff is right
Package: openssl (1.0.1f-1ubuntu2.19 and others) source

[jhass]

@kostya No checks are actually performed unless the verify mode is peer.

[kostya]

this branch not compiled on ubuntu 14.04:

OpenSSL5858SSL5858Socket5858Client.o: In function `*OpenSSL::SSL::Socket::Client#initialize:context:hostname<OpenSSL::SSL::Socket::Client, TCPSocket, OpenSSL::SSL::Context::Client, String>:Nil':
OpenSSL::SSL::Socket::Client:(.text+0x155): undefined reference to `SSL_get0_param'
OpenSSL::SSL::Socket::Client:(.text+0x19c): undefined reference to `X509_VERIFY_PARAM_set1_ip_asc'
OpenSSL::SSL::Socket::Client:(.text+0x1d4): undefined reference to `X509_VERIFY_PARAM_set1_host'

openssl1.0.1f

[jhass]

I'm aware, CI is failing for the same reason. Still working on it.

[jhass]

Well, that would have been too easy I guess. In the latter {% ifs this is still a MacroExpression. @asterite how do I run something at compile time once and use its result in different places at compile time without running it again and again? Alternatively I'd be already happy if I could run the macro expression to get its value and don't have to repeat that thing here all over the place.

[ysbaddaden]

Note: darwin may have OpenSSL 1.0.2 installed (brew install openssl) and it's perfectly possible to link against it --link-flags="-L/usr/local/opt/openssl/lib".

[jhass]

Hence my question below as to whether we could detect that with pkg-config on OS X too.

[ysbaddaden]

There is a single reason it's hard to enforce OpenSSL 1.0.2: it's barely available on stable distributions:

• OSX ships with OpenSSL 0.9.8, and despite the homebrew package installing the latest OpenSSL, Crystal still links against the system OpenSSL by default;
• Ubuntu Xenial ships with 1.0.2, but its barely used, and since it comes with systemd... I'm afraid it will take a while before it supersedes Trusty... Anyway Precise is still used, despite Trusty being available for 2+ years (both ship 1.0.1).

It would be very nice to have 1.0.2. Any older version isn't capable to negotiate HTTP/2 (ALPN is required) plus your case, the hostname verification, has become really simple.

[jhass]

So let's find a way to optionally use 1.0.2 features. There's another question/comment I've already written, I wanted to wait for @asterite whether he sees a way to make something like the above possible, but since you brought OS X up, here it goes:

How common is pkg-config on OS X, especially if there's a recent OpenSSL installed from homebrew? And would pkg-config --libs libssl link against the homebrew OpenSSL then? On Linux it seems to provide a good way to check which OpenSSL version we got, so this becomes an easy low hanging fruit until we reimplement hostname verification for older versions.

Whether we include this or not, implementing manual hostname verification will take the same amount of time, so this does provide better security for people running OpenSSL 1.0.2 at least, compared to doing it for nobody.

[ysbaddaden]

OpenSSL is a great showcase for bundling CrystalLib into Crystal itself —as long as it doesn't fail when a symbol isn't found (this is expected). Generate the LibSSL bindings on-the-fly, then run feature detection:

{% if LibSSL.methods.includes?("x509_verify_param_set1_host".id) %}
  param = param = LibSSL.get0_param(@handle)
  LibSSL.x509_verify_param_set1_host(param, hostname, 0)
{% else %}
  # the hard way
{% end %}

Another, harder way, would be to use DL and verify the presence of functions at runtime...

[asterite]

@ysbaddaden In fact we thought about something like that with @waj, well, actually, giving more power to ifdef, but I guess the solution is #2710, and of course some more macro methods.

[asterite]

Maybe even having if defined?(...) inside a macro (could be used for other things too)

[jhass]

Could we not get too sidetracked here please? Any solution to this problem we can apply now?

[jhass]

Looks like I found a workaround. So the last part to figure out is how to link to the homebrew OpenSSL on OS X automatically if available. I had a friend check and sadly even if pkg-config is available, pkg-config --libs libssl returns just link flags for the system OpenSSL.

[ysbaddaden]

Aside: I'm trying the manual validation, to enable it on previous OpenSSL releases, but it's tedious, thanks to the numerous nested macro defines of OpenSSL... so it will take some time.

[fernandes]

@jhass do you need help testing on OS X?

[jhass]

I had a friend do a few more checks and it looks like that if crystal was installed through homebrew and there's a homebrew openssl available, pkg-config --libs libssl run through crystal will return the right thing. Double verification is always good, so if you want to you can check what

crystal eval "p \`pkg-config --libs libssl\`"

returns at various situations/places.

@ysbaddaden since this now does degrade gracefully, I'd vote to merge this so you can base your work upon it.