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

Verify hostname shim #29

Merged
merged 3 commits into from
Apr 26, 2018
Merged

Verify hostname shim #29

merged 3 commits into from
Apr 26, 2018

Conversation

voltone
Copy link
Contributor

@voltone voltone commented Apr 20, 2018

This enables HTTPS hostname verification on OTP versions that do not automatically perform the check in the :ssl application, by bringing a copy of the OTP 20.3 implementation of pkix_verify_hostname and enabling it when:

  • verify option is set to :verify_peer, and
  • no custom verify_fun is set, and
  • the loaded :ssl module version does not perform hostname verification

It inherits the bugs and limitations of the OTP 20 implementation, most notably ERL-542

%% Shims for functions introduced in recent Erlang/OTP releases,
%% to enable use of XHTTP on older releases. The code in this module
%% was taken directly from the Erlang/OTP project. The original license
%% follows:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Let's include the commit and file we took this from to make it easier to track bugs and knowing when to update.

@ericmj
Copy link
Member

ericmj commented Apr 22, 2018

I pushed a fix for the failing tests: b682c18.

with :verify_peer <- Keyword.get(opts, :verify),
nil <- Keyword.get(opts, :verify_fun),
true <- use_pkix_verify_hostname_shim?() do
Logger.debug("ssl application does not perform hostname verifaction; activating shim")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should remove this logger message, but not important right now.

@ericmj ericmj requested a review from whatyouhide April 22, 2018 14:41
@whatyouhide
Copy link
Contributor

I have a bunch of refactoring/stylistic comments but I'll just merge the PR and do the changes after that. Thanks so much @voltone! ❤️

@whatyouhide whatyouhide merged commit c245036 into elixir-mint:master Apr 26, 2018
@voltone voltone deleted the verify_hostname_shim branch May 18, 2018 06:40
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