-
Notifications
You must be signed in to change notification settings - Fork 291
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
Respect OpenSSL default certificate store and SSL_CERT_FILE environment #386
base: master
Are you sure you want to change the base?
Conversation
test/test_ssl.rb
Outdated
@@ -429,6 +458,15 @@ def test_timeout | |||
|
|||
private | |||
|
|||
def set_x509_const(name, value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use a block here and ensure
after yield
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
respect SSL_CERT_DIR and SSL_CERT_FILE environment variables
* do not rely on distributed cacer.pem file * respect SSL_CERT_DIR and SSL_CERT_FILE * working OpenSSL platform has working default paths
* disable coverage tracking unless on CI
👍 |
* the git sha no longer exists * nahi/httpclient#386
Will this be merged soon? |
It would be interesting to be able to the use system or any OpenSSL CA with httpclient, still I don't think it's a good idea to change casually for such a widely used lib. httpclient's policy was to bundle a CA, and that is well-documented. It might be the reason why some people use it. It's the same thing that Java does. It's also different from what net/http and most other ruby http clients do. So adding an option might be fine, changing basic policies for this I'd warn against - at least without a major version bump. |
A few users of GitLab are running into this issue while trying to get OpenID working with a self-signed certificate. Could we get this branch merged and released? |
By default, httpclient (and hence anything that uses rack-oauth2) ignores the system-wide SSL certificate configuration in favor of its own `cacert.pem`. This makes it impossible to use custom certificates without patching that file. Until nahi/httpclient#386 is merged, we work around this limitation by forcing the `HTTPClient` SSL store to use the default system configuration. Closes https://gitlab.com/charts/gitlab/issues/1436
By default, httpclient (and hence anything that uses rack-oauth2) ignores the system-wide SSL certificate configuration in favor of its own `cacert.pem`. This makes it impossible to use custom certificates without patching that file. Until nahi/httpclient#386 is merged, we work around this limitation by forcing the `HTTPClient` SSL store to use the default system configuration. Closes https://gitlab.com/charts/gitlab/issues/1436
OpenSSL provides default certificate file and directory in:
OpenSSL::X509::DEFAULT_CERT_FILE
OpenSSL::X509::DEFAULT_CERT_DIR
Those defaults can be loaded by
OpenSSL::X509::Store#set_default_paths
.On platforms that do not have this set up properly
HTTPClient::SSLContext#working_openssl_platform?
can be made to return false and trigger the old behaviour.Closes #369
Also fixes failing build due to expired certificate in the test suite. New certificate will expire in 10 years.