Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS certificates not verified #6

Closed
koenrh opened this issue Nov 7, 2016 · 3 comments
Closed

TLS certificates not verified #6

koenrh opened this issue Nov 7, 2016 · 3 comments
Labels
status: help wanted requesting help from the community type: bug bug in the library

Comments

@koenrh
Copy link
Contributor

koenrh commented Nov 7, 2016

Issue Summary

While auditing some of the dependencies that we use in our applications, I noticed that ruby-http-client does not verify TLS certificates. When requesting HTTPS resources using the SendGrid::Client class, presented TLS certificates are not validated at all. It accepts invalid certificates without throwing errors. This makes it trivially easy to launch a man-in-the-middle attack.

Net::HTTP's verify_mode flag should be set to OpenSSL::SSL::VERIFY_PEER (which is the default).

Affected line:

http.verify_mode = OpenSSL::SSL::VERIFY_NONE

def add_ssl(http)
  protocol = host.split(':')[0]
  if protocol == 'https'
    http.use_ssl = true
    http.verify_mode = OpenSSL::SSL::VERIFY_NONE
  end
  http
end

Steps to Reproduce

  1. Launch a ruby shell: irb
  2. Require this gem: require 'ruby_http_client'
  3. Request a resource that serves an invalid certificate: SendGrid::Client.new(host: 'https://wrong.host.badssl.com/').get.

You will see that the request is executed without verifying whether the presented certificate matches the DNS name of the requested resource.

Technical details:

  • ruby-http-client version: master (latest commit: bcc23e2)
@thinkingserious thinkingserious added type: bug bug in the library status: help wanted requesting help from the community labels Nov 7, 2016
@thinkingserious
Copy link
Contributor

Great catch, thanks @koenrh!

I've added this to our backlog for review.

If you would like to submit a PR, please take a moment to sign our CLA.

Otherwise, we still appreciate your submitted issue and would like to send you some swag. Just fill out this form for us, thanks!

@koenrh
Copy link
Contributor Author

koenrh commented Nov 9, 2016

It's only a one-line change, unless you want to allow users to override this secure default setting (OpenSSL::SSL::VERIFY_PEER). Nevertheless, I'm happy to send over the signed CLA, and submit a PR.

@thinkingserious
Copy link
Contributor

Thanks @koenrh,

The change is now in our backlog for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: help wanted requesting help from the community type: bug bug in the library
Projects
None yet
Development

No branches or pull requests

2 participants