-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
crypto: use EVP_MD_fetch and cache EVP_MD for hashes #51034
Conversation
Review requested:
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
From CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1479/console See benchmark results
|
Updated to cache all the explicitly fetched implementations from See benchmark results
|
cff9d1d
to
145c5c6
Compare
Will caching the fetches run into issues given that OpenSSL 3 providers are dynamic (although we don't have an API in Node.js for this but one could feasibly write an addon to unload/load providers)? |
I think for our built-in APIs we always do the implicit fetching with no additional library context or provider property configured (because we use |
This comment was marked as outdated.
This comment was marked as outdated.
Landed in 57c22e4 |
On OpenSSL 3, migrate from EVP_get_digestbyname() to EVP_MD_fetch() to get the implementation and use a per-Environment cache for it. The EVP_MDs are freed during Environment cleanup. Drive-by: declare the smart pointer for EVP_MD_CTX as EVPMDCtxPointer instead of EVPMDPointer to avoid confusion with EVP_MD pointers. PR-URL: nodejs#51034 Refs: https://www.openssl.org/docs/man3.0/man7/crypto.html#Explicit-fetching Refs: nodejs/performance#136 Reviewed-By: James M Snell <[email protected]>
On OpenSSL 3, migrate from EVP_get_digestbyname() to EVP_MD_fetch() to get the implementation and use a per-Environment cache for it. The EVP_MDs are freed during Environment cleanup. Drive-by: declare the smart pointer for EVP_MD_CTX as EVPMDCtxPointer instead of EVPMDPointer to avoid confusion with EVP_MD pointers. PR-URL: nodejs#51034 Refs: https://www.openssl.org/docs/man3.0/man7/crypto.html#Explicit-fetching Refs: nodejs/performance#136 Reviewed-By: James M Snell <[email protected]>
On OpenSSL 3, migrate from EVP_get_digestbyname() to EVP_MD_fetch() to get the implementation and use a per-Environment cache for it. The EVP_MDs are freed during Environment cleanup. Drive-by: declare the smart pointer for EVP_MD_CTX as EVPMDCtxPointer instead of EVPMDPointer to avoid confusion with EVP_MD pointers. PR-URL: #51034 Refs: https://www.openssl.org/docs/man3.0/man7/crypto.html#Explicit-fetching Refs: nodejs/performance#136 Reviewed-By: James M Snell <[email protected]>
On OpenSSL 3, migrate from EVP_get_digestbyname() to EVP_MD_fetch() to get the implementation and use a per-Environment cache for it. The EVP_MDs are freed during Environment cleanup. Drive-by: declare the smart pointer for EVP_MD_CTX as EVPMDCtxPointer instead of EVPMDPointer to avoid confusion with EVP_MD pointers. PR-URL: #51034 Refs: https://www.openssl.org/docs/man3.0/man7/crypto.html#Explicit-fetching Refs: nodejs/performance#136 Reviewed-By: James M Snell <[email protected]>
On OpenSSL 3, migrate from EVP_get_digestbyname() to EVP_MD_fetch() to get the implementation and use a per-Environment cache for it. The EVP_MDs are freed during Environment cleanup. Drive-by: declare the smart pointer for EVP_MD_CTX as EVPMDCtxPointer instead of EVPMDPointer to avoid confusion with EVP_MD pointers. PR-URL: #51034 Refs: https://www.openssl.org/docs/man3.0/man7/crypto.html#Explicit-fetching Refs: nodejs/performance#136 Reviewed-By: James M Snell <[email protected]>
* chore: bump node in DEPS to v20.12.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: bump node in DEPS to v20.12.1 * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: bump node in DEPS to v20.12.2 * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: rebase on main * chore: remove stray log --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <[email protected]> Co-authored-by: Shelley Vohr <[email protected]> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
* chore: bump node in DEPS to v20.13.1 * chore: bump node in DEPS to v20.14.0 * chore: update build_add_gn_build_files.patch * chore: update patches * chore: update patches * build: encode non-ASCII Latin1 characters as one byte in JS2C nodejs/node#51605 * crypto: use EVP_MD_fetch and cache EVP_MD for hashes nodejs/node#51034 * chore: update filenames.json * chore: update patches * src: support configurable snapshot nodejs/node#50453 * test: remove test-domain-error-types flaky designation nodejs/node#51717 * src: avoid draining platform tasks at FreeEnvironment nodejs/node#51290 * chore: fix accidentally deleted v8 dep * lib: define FormData and fetch etc. in the built-in snapshot nodejs/node#51598 * chore: remove stray log * crypto: enable NODE_EXTRA_CA_CERTS with BoringSSL nodejs/node#52217 * test: skip test for dynamically linked OpenSSL nodejs/node#52542 * lib, url: add a `windows` option to path parsing nodejs/node#52509 * src: use dedicated routine to compile function for builtin CJS loader nodejs/node#52016 * test: mark test as flaky nodejs/node#52671 * build,tools: add test-ubsan ci nodejs/node#46297 * src: preload function for Environment nodejs/node#51539 * deps: update c-ares to 1.28.1 nodejs/node#52285 * chore: fixup * events: extract addAbortListener for safe internal use nodejs/node#52081 * module: print location of unsettled top-level await in entry points nodejs/node#51999 * fs: add stacktrace to fs/promises nodejs/node#49849 * chore: fixup indices --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Cheng <[email protected]> Co-authored-by: Shelley Vohr <[email protected]> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
On OpenSSL 3, migrate from EVP_get_digestbyname() to EVP_MD_fetch() to get the implementation and use a per-Environment cache for it. The EVP_MDs are freed during Environment cleanup.
Drive-by: declare the smart pointer for EVP_MD_CTX as EVPMDCtxPointer instead of EVPMDPointer to avoid confusion with EVP_MD pointers.
Refs: https://www.openssl.org/docs/man3.0/man7/crypto.html#Explicit-fetching
Refs: nodejs/performance#136
On MacOS + M2
On Ubuntu + Intel(R) Xeon(R) Platinum 8280 CPU @ 2.70GHz