ssl: add verify_hostname option to SSLContext #60
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There is a function ossl_verify_cb() that fetches the custom callback Proc from X509_STORE/X509_STORE_CTX and calls it, but it was not very useful for SSL code. It's only used in ossl_x509store.c and ossl_ssl.c so move X509::Store specific code to ossl_x509store.c. Also make struct ossl_verify_cb_args and ossl_call_verify_cb_proc() local to ossl.c.
|
Just wanted to say thanks for doing this and that it addresses my concerns in #8, which I think can be closed when this is merged. |
Set verify_mode to OpenSSL::SSL::VERIFY_PEER directly. They are tests for verify_callback so they don't need to use SSLContext#set_params.
If a client sets this to true and enables SNI with SSLSocket#hostname=, the hostname verification on the server certificate is performed automatically during the handshake using OpenSSL::SSL.verify_certificate_identity(). Currently an user who wants to do the hostname verification needs to call SSLSocket#post_connection_check explicitly after the TLS connection is established. This commit also enables the option in SSLContext::DEFAULT_PARAMS. Applications using SSLContext#set_params may be affected by this. [GH ruby#8]
rhenium
force-pushed
the
topic/ssl-verify-hostname
branch
from
f584b51
to
028e495
Compare
glaszig
added a commit
to glaszig/logstash-logger
that referenced
this pull request
Feb 14, 2017
with ruby < 2.4 support see ruby/openssl#60
joshcooper
added a commit
to joshcooper/puppet
that referenced
this pull request
Feb 21, 2019
Previously, if the server's cert did not match the hostname we tried to connect
to, puppet would output a confusing message:
certificate verify failed: [ok for /CN=XXX]
This occurs because ruby 2.4 introduced a new security feature whereby the cert
is automatically verified during the call to `SSLSocket#connect`[1]. In earlier
ruby versions, the application had to call `SSLSocket#post_connection_check`,
but of course, many people forgot to, or didn't know they had to, leading to
MITM vulnerabilities.
However, when a mismatch occurs, ruby 2.4 invokes our `verify_callback` with
`preverify_ok=false`, but `store_context.error=0` which is `OpenSSL::SSL::V_OK`.
Ruby then raises an `SSLError` whose message is 'certificate verify failed',
which matches the first "if" statement in our error handler.
This commit changes the order so that if an SSLError is rescued, we check to see
if there's a host mismatch first. If not, we check if there was a certificate
verify error, or raise the original error.
This change is compatible with ruby versions prior to 2.4, because both
`SSLSocket#post_connection_check` and our error handler use
`OpenSSL::SSL.verify_certificate_identity` to detect the certname mismatch.
[1] ruby/openssl#60
joshcooper
added a commit
to joshcooper/puppet
that referenced
this pull request
Feb 21, 2019
Previously, if the server's cert did not match the hostname we tried to connect
to, puppet would output a confusing message:
certificate verify failed: [ok for /CN=XXX]
This occurs because ruby 2.4 introduced a new security feature whereby the cert
is automatically verified during the call to `SSLSocket#connect`[1]. In earlier
ruby versions, the application had to call `SSLSocket#post_connection_check`,
but of course, many people forgot to, or didn't know they had to, leading to
MITM vulnerabilities.
However, when a mismatch occurs, ruby 2.4 invokes our `verify_callback` with
`preverify_ok=false`, but `store_context.error=0` which is `OpenSSL::SSL::V_OK`.
Ruby then raises an `SSLError` whose message is 'certificate verify failed',
which matches the first "if" statement in our error handler.
This commit changes the order so that if an SSLError is rescued, we
check to see if there's a host mismatch first. If not, we check if there
were *any* verify errors, or raise the original error.
This change is compatible with ruby versions prior to 2.4, because both
`SSLSocket#post_connection_check` and our error handler use
`OpenSSL::SSL.verify_certificate_identity` to detect the certname mismatch.
[1] ruby/openssl#60
joshcooper
added a commit
to joshcooper/puppet
that referenced
this pull request
Feb 21, 2019
Previously, if the server's cert did not match the hostname we tried to connect
to, puppet would output a confusing message:
certificate verify failed: [ok for /CN=XXX]
This occurs because ruby 2.4 introduced a new security feature whereby the cert
is automatically verified during the call to `SSLSocket#connect`[1]. In earlier
ruby versions, the application had to call `SSLSocket#post_connection_check`,
but of course, many people forgot to, or didn't know they had to, leading to
MITM vulnerabilities.
However, when a mismatch occurs, ruby 2.4 invokes our `verify_callback` with
`preverify_ok=false`, but `store_context.error=0` which is `OpenSSL::SSL::V_OK`.
Ruby then raises an `SSLError` whose message is 'certificate verify failed',
which matches the first "if" statement in our error handler.
This commit changes the order so that if an SSLError is rescued, we
check to see if there's a host mismatch first. If not, we check if there
were *any* verify errors, or raise the original error.
This change is compatible with ruby versions prior to 2.4, because both
`SSLSocket#post_connection_check` and our error handler use
`OpenSSL::SSL.verify_certificate_identity` to detect the certname mismatch.
[1] ruby/openssl#60
ganmacs
added a commit
to ganmacs/ruby
that referenced
this pull request
Jan 23, 2020
According to ruby/openssl#60, > Currently an user who wants to do the hostname verification needs to call SSLSocket#post_connection_check explicitly after the TLS connection is established. if an user who wants to skip the hostname verification, SSLSocket#post_connection_check doesn't need to be called
ganmacs
added a commit
to ganmacs/ruby
that referenced
this pull request
Jan 23, 2020
According to ruby/openssl#60, > Currently an user who wants to do the hostname verification needs to call SSLSocket#post_connection_check explicitly after the TLS connection is established. if an user who wants to skip the hostname verification, SSLSocket#post_connection_check doesn't need to be called
ganmacs
added a commit
to ganmacs/ruby
that referenced
this pull request
Jan 23, 2020
According to ruby/openssl#60, > Currently an user who wants to do the hostname verification needs to call SSLSocket#post_connection_check explicitly after the TLS connection is established. if an user who wants to skip the hostname verification, SSLSocket#post_connection_check doesn't need to be called
nurse
pushed a commit
to ruby/ruby
that referenced
this pull request
Jan 23, 2020
…ion (#2858) According to ruby/openssl#60, > Currently an user who wants to do the hostname verification needs to call SSLSocket#post_connection_check explicitly after the TLS connection is established. if an user who wants to skip the hostname verification, SSLSocket#post_connection_check doesn't need to be called https://bugs.ruby-lang.org/issues/16555
hsbt
pushed a commit
to ruby/net-http
that referenced
this pull request
Feb 21, 2020
…ion (#2858) According to ruby/openssl#60, > Currently an user who wants to do the hostname verification needs to call SSLSocket#post_connection_check explicitly after the TLS connection is established. if an user who wants to skip the hostname verification, SSLSocket#post_connection_check doesn't need to be called https://bugs.ruby-lang.org/issues/16555
This was referenced Sep 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a client sets this to true and enables SNI with SSLSocket#hostname=,
the hostname verification on the server certificate is performed
automatically during the handshake using
OpenSSL::SSL.verify_certificate_identity().
Currently an user who wants to do the hostname verification needs to
call SSLSocket#post_connection_check explicitly after the TLS connection
is established.
This commit also enables the option in SSLContext::DEFAULT_PARAMS.
Applications using SSLContext#set_params may be affected by this.
[GH #8]