crypto: use names for asymmetric key algorithms - #65966
Conversation
Provider-backed keys can have no numeric OpenSSL ID. Identify and construct asymmetric keys by algorithm name, replacing the custom PQC name-to-NID substitution with provider-aware matching in ncrypto. Centralize known algorithm names, public key-type names, capabilities, and backend compatibility in ncrypto. Use named key generation jobs and remove asymmetric EVP_PKEY constants from the internal JavaScript binding. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex
|
Review requested:
|
|
Benchmark GHA (crypto / keyobject): https://github.com/nodejs/node/actions/runs/34348821454 Results
Benchmark results:
Benchmark GHA (crypto / oneshot-sign.js): https://github.com/nodejs/node/actions/runs/34348824410 Results
Benchmark results:
Benchmark GHA (crypto / oneshot-verify.js): https://github.com/nodejs/node/actions/runs/34348827543 Results
Benchmark results:
Benchmark GHA (crypto / webcrypto-sign.js): https://github.com/nodejs/node/actions/runs/34348830291 Results
Benchmark results:
Benchmark GHA (crypto / webcrypto-verify.js): https://github.com/nodejs/node/actions/runs/34348833659 Results
Benchmark results:
Benchmark GHA (crypto / keygen.js): https://github.com/nodejs/node/actions/runs/34348840314 Results
Benchmark results:
Benchmark GHA (crypto / rsa-encrypt-decrypt-throughput.js): https://github.com/nodejs/node/actions/runs/34348843444 Results
Benchmark results:
Benchmark GHA (tls / tls-connect.js): https://github.com/nodejs/node/actions/runs/34348846429 Results
Benchmark results:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #65966 +/- ##
==========================================
- Coverage 92.41% 90.21% -2.21%
==========================================
Files 417 771 +354
Lines 189029 265161 +76132
Branches 28953 50392 +21439
==========================================
+ Hits 174685 239203 +64518
- Misses 14011 16977 +2966
- Partials 333 8981 +8648
🚀 New features to boost your workflow:
|
|
@jasnell please take a look, this is a step towards moving more things to the openssl provider model |
| static EVPKeyCtxPointer New(const EVPKeyPointer& key); | ||
| static EVPKeyCtxPointer NewFromID(int id); | ||
| static EVPKeyCtxPointer NewFromName(const char* name); | ||
| static EVPKeyCtxPointer NewFromAlgorithm(const KeyAlgorithm& algorithm); |
There was a problem hiding this comment.
While I'm +1 on the change overall, removing the current NID apis means anyone else using ncrypto will be forced to transition immediately to the new api. I'd prefer to deprecate first for a short time before removing them to give some flexibility in the transition if possible.
There was a problem hiding this comment.
IIRC all ncrypto consumers use BoringSSL which has its own compatibility branch here. We have not established any intent to maintain ncrypto API compatibility, right now it is merely a snapshot of node's own ncrypto state.
There was a problem hiding this comment.
+ restoring the APIs in nodejs/ncrypto can be done as part of resolving the sync workflow, I don't see why we should subject maintainers of node's crypto with the burden of maintaining compatibility modes for other runtimes.
jasnell
left a comment
There was a problem hiding this comment.
LGTM So long as the update into ncrypto either gates these or waits on downstream consumers to be able to update. I'd still prefer the parallel api approach (either with deprecated apis or IFDEF gates)
|
Landed in 1a3712d |
Provider-backed keys can have no numeric OpenSSL ID. Identify and construct asymmetric keys by algorithm name, replacing the custom PQC name-to-NID substitution with provider-aware matching in ncrypto. Centralize known algorithm names, public key-type names, capabilities, and backend compatibility in ncrypto. Use named key generation jobs and remove asymmetric EVP_PKEY constants from the internal JavaScript binding. Signed-off-by: Filip Skokan <panva.ip@gmail.com> Assisted-by: Codex PR-URL: #65966 Reviewed-By: James M Snell <jasnell@gmail.com>
Provider-backed keys can have no numeric OpenSSL ID. Identify and construct asymmetric keys by algorithm name, replacing the custom PQC name-to-NID substitution with provider-aware matching in ncrypto.
Centralize known algorithm names, public key-type names, capabilities, and backend compatibility in ncrypto. Use named key generation jobs and remove asymmetric EVP_PKEY constants from the internal JavaScript binding.
Assisted-by: Codex