security: Verify SSL certificates

[Catharz]

Verifying SSL host against the certificate provided.

Certificate host expiry, etc is checked only as a manual step after connection. It is not an automatic action in OpenSSL. This enables that check.

This means that:

• self signed certs will not work without specifying the SSL context
• hosts with a cert for "my-evil-domain.com" will fail to connect if their host name is "my-real-company.com"

Without these checks, all SSL connections are vulnerable to man in the middle attacks.

[Catharz]

Note: Master is failing to build at the moment (due to Ruby 2.3 on Travis).

[dwbutler]

👍

This is a Good Thing, but extremely likely to break existing setups. I think it would make sense to make this opt-in, with a warning that it will become the default in the future.

[Catharz]

Made the SSL context configurable (and defaulting to none).
But if the SSL context is not provided, it will output a deprecation warning.

[glaszig]

can we please have this merged? i just discovered, to my surprise, that the tcp connector doesn't even do anything with the certificate i give it. it just opens a dumb ssl socket.

[dwbutler]

Sorry, there has been a lot going on and I've been out of action for a while. But I'm reviewing the open pull requests and issues and plan to address them soon.

[dwbutler]

@glaszig You're right, the :ssl_certificate option is completely useless. The user should instead be responsible for passing in their own OpenSSL::SSL::SSLContext, as implemented in this PR. I'll deprecate that option.

[franklouwers]

Hi,

Any updates on this open issue? Do you stil plan on merging it?

[dwbutler]

Getting SSL working correctly is of course very important. However, I think there is more work involved to get it working than is contained in this PR. I haven't been able to find the time to investigate this further, and I don't use SSL over TCP myself.

[glaszig]

i took the liberty to extend upon @Catharz's work and

• made #use_ssl? respect whether an SSLContext has been set
• removed the :use_ssl option because it was not documented and is being duplicated by the documented :ssl_enable
• have certificate hostnames be verified if the context has its verify_mode set to anything other than OpenSSL::SSL::VERIFY_NONE.

see this diff.

unless i missed something this should complete this pr.

@Catharz i can push these commits to your branch if you'll give me access.

[dwbutler]

Thanks @glaszig. I'll do some due diligence to make sure there aren't any other steps we're missing. I know there are a few gotchas when using SSL in Ruby.

[glaszig]

oh btw, if you'd just provide an :ssl_certificate, it'll create a simple context with that certificate for you and use that. so this dysfunctional but documented option will just work.

[glaszig]

interesting and related: ruby/openssl#8
i think we should do a similar thing and not hijack VERIFY_PEER mode as i did. will adjust.

[dwbutler]

After some research, the gotchas I was concerned about are mainly in older versions of Ruby. Modern versions of Ruby seem to do the correct thing.

@Catharz @glaszig I'll pull your commits into a new branch and do some merging/cleanup. Thanks for the hard work on this!

[glaszig]

i have one thing left to do. will pr later.

[dwbutler]

@glaszig Let me know when it's ready. I can pull it into #114

[glaszig]

@dwbutler done. refactored the tcp device a little bit and improved hostname verification documentation.

[dwbutler]

@glaszig Thanks, I reviewed your latest changes and they look correct. I've pulled them into #114. This should be ready to go out soon.

[dwbutler]

Fix released in 0.23.0. Sorry again for the long lead time on this.