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

OpenSSLv3 - loading certificates is much slower than previous versions #16871

Open
mhdawson opened this issue Oct 20, 2021 · 9 comments
Open
Labels
branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature

Comments

@mhdawson
Copy link

mhdawson commented Oct 20, 2021

Recreated on Linux x64 and Windows x64 with Node.js 17 that uses OpenSSL v3

Node.js provides the option to add extra certificates. When this is done the certificates are loaded using PEM_read_bio_X509 as follows:

if (root_certs_vector.empty() &&
      per_process::cli_options->ssl_openssl_cert_store == false) {
    for (size_t i = 0; i < arraysize(root_certs); i++) {
      X509* x509 =
          PEM_read_bio_X509(NodeBIO::NewFixed(root_certs[i],
                                              strlen(root_certs[i])).get(),
                            nullptr,   // no re-use of X509 structure
                            NoPasswordCallback,
                            nullptr);  // no callback data

      // Parse errors from the built-in roots are fatal.
      CHECK_NOT_NULL(x509);

      root_certs_vector.push_back(x509);
    }
  }

This seems to be much slower in OpenSSL v3 (which is what was pulled into Node.js 17) versus earlier versions. Loading extra certificates seems to add 60 or more milliseconds (will vary by machine) than it did before. Since startup time is only 40ms on the same machine without extra certificates going from 40 to over 100ms is pretty significant.

Is this a known issue or expected? I searched through github and the release notes but I've not been able to find anything that might explain or suggest this would be expected.

@mhdawson
Copy link
Author

Just to add that I did also cut down the code above to confirm it was the call to PEM_read_bio_X509 versus anything else. In both Node.js 17 and Node.js 16 there are 131 entries in the array to be read in.

@mhdawson
Copy link
Author

And to answer one of the questions from the report template. Node.js compiles in openssl.

@t8m t8m added branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature and removed issue: bug report The issue was opened to report a bug labels Oct 21, 2021
@t8m
Copy link
Member

t8m commented Oct 21, 2021

This is a known deficiency of the OpenSSL 3.0. Unfortunately the way how the decoding works is much more complicated than in 1.1.1 and there is probably a lot of space for potential optimizations. We would like to tackle this in a future release.

@t8m t8m changed the title OpenSSLv3 - PEM_read_bio_X509 much slower than previous versions OpenSSLv3 - loading certificates is much slower than previous versions Oct 21, 2021
@mhdawson
Copy link
Author

@t8m thanks for the update. Good to know that it's not something we are doing wrong on our side.

@slontis
Copy link
Member

slontis commented Oct 28, 2021

Any idea how many root_certs are loaded here?

@mhdawson
Copy link
Author

@slontis the array that is iterated through has 131 entries, but don't know if that matches your question about root_certs?

@mhdawson
Copy link
Author

Looking back at the code, since the array is called root_certs it should match the question and the answer is 131.

@dzuelke
Copy link

dzuelke commented Mar 9, 2023

I can confirm a hit of about 60 ms, measured on different architectures and CPU sizes, with e.g. the default Ubuntu CA certs, compared to 1.1.1.

@birojnayak
Copy link

we also see the performance degrade in Amazon Linux 2023 (at least in the .NET runtime).. would be good to know once fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

No branches or pull requests

5 participants