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

Don't verify hostname when verify_hostname is false in tls_options #349

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jurta
Copy link

@jurta jurta commented Feb 14, 2020

https://ruby.github.io/openssl/OpenSSL/SSL/SSLContext.html documents the option verify_hostname as whether to check the server certificate is valid for the hostname.

But when I set verify_hostname to false in tls_options, it has no effect, it still reports the error "hostname does not match the server certificate".

Then I found this is the result of #259 but the assumption is wrong that users who don't care about hostname validation should set verify_mode to OpenSSL::SSL::VERIFY_NONE. This disables the certificate validation completely. Maybe the reason why #259 didn't add a check for verify_hostname is because verify_hostname was added later in ruby/openssl#60 (i.e. the same year but a few months later).

So for more fine-grained configuration to disable only hostname verification without disabling certificate validation, here is this pull request.

@secos
Copy link

secos commented Apr 22, 2020

Any updates on this?

@zeroSteiner zeroSteiner self-assigned this Dec 7, 2024
@zeroSteiner
Copy link
Collaborator

I merged the changes from master into this branch so the tests can be run and it looks like there's some failures. I think one is due to 127.0.0.1 not being the IP address of the LDAP server in the CI environment. I'd recommend changing it to @ldap.host = Resolv.getaddress(INTEGRATION_HOSTNAME). I ran another test with that in place and got a new error though which is when I stopped investigating.

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