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

Customize hostname check to allow also wildcard certificates #798

Merged
merged 6 commits into from
Aug 10, 2020

Conversation

imsoulfly
Copy link
Contributor

Hello everyone,

I already wrote one solution that I took over from the mint library to activate validation of wildcard certificates with the now stricter Erlang TLS handling in the issue here: #797

But I actually was able to find a better solution of using the wildcard hostname verification directly from the
public_key Erlang module that does to knowledge exactly the same.

Disclaimer: I did manual testing of this but wasn't really able to come up with how to actually test this. Speaking of having a proper test setup that provides a server with wildcard certificate for its domain. If you know a way let me know. For now I can only deliver this 😄

Have a nice day!

@imsoulfly
Copy link
Contributor Author

Ok I already see that I should provide a fallback for older Erlang versions.

lib/hex/http/ssl.ex Outdated Show resolved Hide resolved
lib/hex/http/ssl.ex Outdated Show resolved Hide resolved
lib/hex/http/ssl.ex Outdated Show resolved Hide resolved
@ericmj
Copy link
Member

ericmj commented Jul 31, 2020

Can you take a look at this @voltone since you created the original PR for mint?

lib/hex/http/ssl.ex Outdated Show resolved Hide resolved
lib/hex/http/ssl.ex Outdated Show resolved Hide resolved
lib/hex/http/ssl.ex Outdated Show resolved Hide resolved
@imsoulfly
Copy link
Contributor Author

Hi @voltone, just had time to apply your change requests. Hope that fits and I understood you correctly with regards to the reuse of the VerifyHostname implementation? I now added the a slightly changed match_fun/2 function directly in that module which just makes use of the already existing function for matching the hostnames.

Copy link

@voltone voltone left a comment

Choose a reason for hiding this comment

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

Yes, I think that cleans things up nicely. Thanks!

@ericmj
Copy link
Member

ericmj commented Aug 10, 2020

Thank you @imsoulfly and @voltone! 💜

@ericmj ericmj merged commit 6d7ff12 into hexpm:main Aug 10, 2020
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