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

Loads complete root certificate store per connection. #1110

Closed
kroeckx opened this Issue Nov 6, 2016 · 15 comments

Comments

Projects
None yet
2 participants
@kroeckx

kroeckx commented Nov 6, 2016

Hi,

It seems that somewhere between 7.38 and 7.50 there is a change in behavior when it comes to checking the certificates. In 7.38 I see that it each times open the file for the root CA in /etc/ssl/certs/ after it has received the certificates from the server. In 7.50 instead once the connection is established, before any SSL negotiation is done, it opens /etc/ssl/certs/ca-certificates.crt. This seems to make me use about 5 times more CPU with 7.50 than with 7.38.

Since I use multiple connections with the same application, it would actually make sense to store this once in memory instead of opening the same file each time. I assume that was at least the intention. You should try to keep this certificate store in RAM and only load it once. You might want to consider actually doing a stat on the file to see if it changed and reload it in that case.

@bagder bagder added the SSL/TLS label Nov 6, 2016

@bagder

This comment has been minimized.

Member

bagder commented Nov 6, 2016

I can't recall any such change. What TLS backend are you using when measuring this and how do you measure?

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

I've seen this with both the openssl and gnutls backend. The CPU usage is just measured as what is reported by ps. I looked at the difference in behavior using strace.

Since I actually see this difference with both backends, I don't think it's related to a change in the libraries.

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

It's comparing the version in Debian's stable and testing. It's build against the version in stable, but when run against the version in testing it's using more cpu.

@bagder

This comment has been minimized.

Member

bagder commented Nov 6, 2016

If you're testing Debian's builds then you should take this to the Debian builders first, since we cannot take responsibility for how they build or ship libcurl (I've noted crazy startup times in their curl builds before - we compared curl vs wget on the mailing list a while ago which basically boiled down to the debian curl version spending ages on cert setup, something my own builds certainly don't do).

If you build libcurl 7.51.0 yourself from source against one of these libraries and then repeat the same build setup with 7.38.x, do they show this difference?

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

I just build an unpatched 7.51.0 version with just calling ./configure and it behaves the same as the version in testing.

@bagder

This comment has been minimized.

Member

bagder commented Nov 6, 2016

I spot no significant difference in lib/vtls/openssl.c between 7.38.0 and 7.51.0 when it comes to loading the CA certs so I assume there's something else that has changed.

Based on that, I don't think you're seeing a regression but simply something that isn't really optimized or done in an ideal way. Not a bug, but certainly room for improvement.

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

My guess is that it might depend on whether the ca-certificates packages was installed at ./configure time or not. If it was installed, STRING_SSL_CAFILE is set, otherwise it isn't.

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

And Debian started build-depending on the ca-certificates package in 7.28 already, so it at least like that didn't change.

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

@ghedo: Do you have any idea?

@bagder

This comment has been minimized.

Member

bagder commented Nov 6, 2016

@kroeckx does it also mention the ca cert path: set in the configure summary?

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

No, it only mentions the bundle, not the path.

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

But in the Debian package it does mention both:
ca cert bundle: /etc/ssl/certs/ca-certificates.crt
ca cert path: /etc/ssl/certs

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

But the 7.38 build log shows:
ca cert bundle: no
ca cert path: /etc/ssl/certs

@kroeckx

This comment has been minimized.

kroeckx commented Nov 6, 2016

The logs in Debian change between 7.42.1-2 and 7.42.1-3. From the changelog I understand that configure is called differently since that version.

@bagder

This comment has been minimized.

Member

bagder commented Nov 9, 2016

This issue seems to mostly say what the TODO item Cache OpenSSL contexts does. Room for improvement certainly, but a known limitation. We welcome help and patches but this issue will be closed within shortly.

@bagder bagder added the enhancement label Nov 10, 2016

@bagder bagder closed this in 25543b8 Jan 17, 2017

bagder added a commit that referenced this issue Jan 19, 2017

TODO: share OpenSSL contexts
By supporting this, subsequent connects would load a lot less data from
disk.

Closes #1110

jkralik pushed a commit to jkralik/curl that referenced this issue Jan 23, 2017

TODO: share OpenSSL contexts
By supporting this, subsequent connects would load a lot less data from
disk.

Closes curl#1110

peterpih pushed a commit to railsnewbie257/curl that referenced this issue Jan 24, 2017

TODO: share OpenSSL contexts
By supporting this, subsequent connects would load a lot less data from
disk.

Closes curl#1110

@lock lock bot locked as resolved and limited conversation to collaborators May 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.