Skip to content

Respect the SSL context's verify_hostname value #632

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

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

colemannugent
Copy link
Contributor

I stumbled upon an issue where http.rb wouldn't respect the verify_hostname value of the current OpenSSL::SSL::SSLContext.

Previously we would unconditionally call post_connection_check, whose only purpose is to verify hostnames, regardless of what verify_hostname was set to. This results in quite a bit of user confusion since one expects hostname verification to be handled by OpenSSL itself rather than by the HTTP library.

Digging through the blames, it looks like this was never implemented properly. Ruby's Net::HTTP fixed the same issue with this pull in January.

This simple change fixes the issue for me and brings http.rb in line with the other Ruby HTTP libraries.

@tarcieri
Copy link
Member

This results in quite a bit of user confusion since one expects hostname verification to be handled by OpenSSL itself rather than by the HTTP library.

Yeah, it's a big mess. OpenSSL itself didn't implement hostname verification at all until quite recently, leaving all users of it to reimplement it poorly themselves (and as it were, I ended up rewriting the hostname verification code used by Ruby itself due to numerous problems).

Anyway, approved! Thanks for the fix.

@tarcieri
Copy link
Member

Per #633 CI is broken (thanks Travis), but I'm going to go ahead and merge this.

@tarcieri tarcieri merged commit ce6b301 into httprb:master Dec 17, 2020
@ixti
Copy link
Member

ixti commented Dec 27, 2020

Seems like this PR broke jRuby:


image

@tarcieri
Copy link
Member

@ixti aah, unfortunate. Sorry about that.

Seems like it can at least be worked around with reflection.

@ixti
Copy link
Member

ixti commented Dec 27, 2020

jRuby has older version of OpenSSL gem bundled in.

@ixti
Copy link
Member

ixti commented Dec 27, 2020

f8dfb38

@tarcieri
Copy link
Member

I was suggesting something like:

if ssl_context.respond_to?(:verify_hostname) && ssl_context.verify_hostname
  @socket.post_connection_check(host)
end

@ixti
Copy link
Member

ixti commented Dec 28, 2020

@tarcieri 👍 Will do so!

ixti added a commit that referenced this pull request Dec 28, 2020
jRuby is using outdated openssl gem bundled in, which don't have
validate_hostname getter on SSLContext.
ixti added a commit that referenced this pull request Dec 28, 2020
* Switch to GitHub Actions

* Use GitHub Actions for CI
* Update Coveralls integration: generate lcov report with SimpleCov and
  send it after the test suite using coveralls GitHub Actions plugin
* Update and cleanup RSpec config
* Cleanup Rakefile
* Remove active_model dependency (certificate_authority was fixed)

PS: GH Actions syntax is ugly.
  Should we switch to Cirlce CI or GitLab CI? XD

Resolves: #633

* Disable SSL related specs

Specs are failing due to some misconfiguration caused by new OpenSSL.
TODO: #627

* Remove windows and macos from test matrix

We were not testing those on Travis-CI, thus to simplify migration I've
deicded to disable those. Once everything is fixed and stabilized we
will add those too.

* Bump min version of CA and rspec

Just to make sure we're using expected ones

* Make stubbed client in spec more predictable

- use stub_const
- consistently normalize URIs

* Add OpenSSL requirement details for jRuby

/cc @tarcieri Probably we should just add dependency on gem itself?

* Revert "Add OpenSSL requirement details for jRuby"

This reverts commit f8dfb38.

* Fix jRuby regression introduced in #632

jRuby is using outdated openssl gem bundled in, which don't have
validate_hostname getter on SSLContext.

* Fix jRuby coverage abilities
@tarcieri tarcieri mentioned this pull request May 13, 2021
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 11, 2025
Changelog:
## [5.3.1] - 2025-06-09

### Changed

- Revert switch to the native llhttp on MRI, as it's not compatible with
  standalone bundles
  ([#802](httprb/http#802))


## [5.3.0] - 2025-06-09

### Added

- (backported) Add .retriable feature to Http
- (backported) Add more specific ConnectionError classes
- (backported) New feature: RaiseError

### Changed

- (backported) Drop depenency on base64
- (backported) Cache header normalization to reduce object allocation
- (backported) Use native llhttp on MRI


## [5.2.0] - 2024-02-05

### Added

- Add `Connection#finished_request?`
  ([#743](httprb/http#743))
- Add `Instrumentation#on_error`
  ([#746](httprb/http#746))
- Add `base64` dependency (suppresses warnings on Ruby 3.0)
  ([#759](httprb/http#759))
- Add `PURGE` HTTP verb
  ([#757](httprb/http#757))
- Add Ruby-3.3 support

### Changed

- **BREAKING** Process features in reverse order
  ([#766](httprb/http#766))
- **BREAKING** Downcase Content-Type charset name
  ([#753](httprb/http#753))
- **BREAKING** Make URI normalization more conservative
  ([#758](httprb/http#758))

### Fixed

- Close sockets on initialize failure
  ([#762](httprb/http#762))
- Prevent CRLF injection due to broken URL normalizer
  ([#765](httprb/http#765))

[unreleased]: httprb/http@v5.3.0...5-x-stable
[5.3.0]: httprb/http@v5.2.0...v5.3.0
[5.2.0]: httprb/http@v5.1.1...v5.2.0

## 5.1.1 (2022-12-17)

* [#731](httprb/http#731)
  Strip brackets from IPv6 addresses in `HTTP::URI`.
  ([@jeraki])

* [#722](httprb/http#722)
  Add `on_redirect` callback.
  ([@benubois])

## 5.1.0 (2022-06-17)

* Drop ruby-2.5 support.

* [#715](httprb/http#715)
  Set default encoding to UTF-8 for `application/json`.
  ([@drwl])

* [#712](httprb/http#712)
  Recognize cookies set by redirect.
  ([@tkellogg])

* [#707](httprb/http#707)
  Distinguish connection timeouts.
  ([@YuLeven])

## 5.0.4 (2021-10-07)

* [#698](httprb/http#698)
  Fix `HTTP::Timeout::Global#connect_ssl`.
  ([@tarcieri])

## 5.0.3 (2021-10-06)

* [#695](httprb/http#695)
  Revert DNS resolving feature.
  ([@PhilCoggins])

* [#694](httprb/http#694)
  Fix cookies extraction.
  ([@flosacca])

## 5.0.2 (2021-09-10)

* [#686](httprb/http#686)
  Correctly reset the parser.
  ([@bryanp])

* [#684](httprb/http#684)
  Don't set Content-Length for GET, HEAD, DELETE, or CONNECT requests without a BODY.
  ([@jyn514])

* [#679](httprb/http#679)
  Use features on redirected requests.
  ([@nomis])

* [#678](httprb/http#678)
  Restore `HTTP::Response` `:uri` option for backwards compatibility.
  ([@schwern])

* [#676](httprb/http#676)
  Update addressable because of CVE-2021-32740.
  ([@matheussilvasantos])

* [#653](httprb/http#653)
  Avoid force encodings on frozen strings.
  ([@bvicenzo])

* [#638](httprb/http#638)
  DNS failover handling.
  ([@midnight-wonderer])


## 5.0.1 (2021-06-26)

* [#670](httprb/http#670)
  Revert `Response#parse` behavior introduced in [#540].
  ([@DannyBen])

* [#669](httprb/http#669)
  Prevent bodies from being resubmitted when following unsafe redirects.
  ([@odinhb])

* [#664](httprb/http#664)
  Bump llhttp-ffi to 0.3.0.
  ([@bryanp])


## 5.0.0 (2021-05-12)

* [#656](httprb/http#656)
  Handle connection timeouts in `Features`
  ([@semenyukdmitry])

* [#651](httprb/http#651)
  Replace `http-parser` with `llhttp`
  ([@bryanp])

* [#647](httprb/http#647)
  Add support for `MKCALENDAR` HTTP verb
  ([@meanphil])

* [#632](httprb/http#632)
  Respect the SSL context's `verify_hostname` value
  ([@colemannugent])

* [#625](httprb/http#625)
  Fix inflator with empty responses
  ([@LukaszMaslej])

* [#599](httprb/http#599)
  Allow passing `HTTP::FormData::{Multipart,UrlEncoded}` object directly.
  ([@ixti])

* [#593](httprb/http#593)
  [#592](httprb/http#592)
  Support informational (1XX) responses.
  ([@ixti])

* [#590](httprb/http#590)
  [#589](httprb/http#589)
  Fix response headers paring.
  ([@Bonias])

* [#587](httprb/http#587)
  [#585](httprb/http#585)
  Fix redirections when server responds with multiple Location headers.
  ([@ixti])

* [#581](httprb/http#581)
  [#582](httprb/http#582)
  Add Ruby 2.7.x support.
  ([@janko])

* [#577](httprb/http#577)
  Fix `Chainable#timeout` with frozen Hash.
  ([@antonvolkoff])

* [#576](httprb/http#576)
  [#524](httprb/http#524)
  **BREAKING CHANGE**
  Preserve header names casing.
  ([@joshuaflanagan])

* [#540](httprb/http#540)
  [#538](httprb/http#538)
  **BREAKING CHANGE**
  Require explicit MIME type for Response#parse
  ([@ixti])

* [#532](httprb/http#532)
  Fix pipes support in request bodies.
  ([@ixti])

* [#530](httprb/http#530)
  Improve header fields name/value validation.
  ([@Bonias])

* [#506](httprb/http#506)
  [#521](httprb/http#521)
  Skip auto-deflate when there is no body.
  ([@Bonias])

* [#489](httprb/http#489)
  Fix HTTP parser.
  ([@ixti], [@fxposter])

* [#546](httprb/http#546)
  **BREAKING CHANGE**
  Provide initiating `HTTP::Request` object on `HTTP::Response`.
  ([@joshuaflanagan])

* [#571](httprb/http#571)
  Drop Ruby 2.3.x support.
  ([@ixti])

* [3ed0c31](httprb/http@3ed0c31)
  Drop Ruby 2.4.x support.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants