nodejs
crypto: return a clearer error when loading an unsupported pkcs12
Currently when a PFX file with an unsupported format is used, it will throw an error because it's not supported (typically because you need the OpenSSL legacy provider). That fails with:
- A message that is literally just
unsupported - No error code
- A stack that's not very useful for many users, e.g.:
Error: unsupported at configSecureContext (node:internal/tls/secure-context:285:15) at Object.createSecureContext (node:_tls_common:116:3) at Object.connect (node:_tls_wrap:1763:48) at Agent.createConnection (node:https:170:22) at Agent.createSocket (node:_http_agent:340:26) at Agent.addRequest (node:_http_agent:288:10) at new ClientRequest (node:_http_client:337:16) at request (node:https:378:10)
Lots of users run into this (e.g. https://github.com/nodejs/node/issues/40672 - and there's plenty of other similar issues) but it's not really clear from this error what's going on, or which of the options they've provided is failing and why.
This PR improves that for the common LoadPKCS12 case, with an explicit error describing what is not supported (you're loading a PFX file that is not supported) and a standard error code so you can recognize and google this more usefully (ERR_CRYPTO_UNSUPPORTED_OPERATION). The code is also useful for people building on Node.js and processing user-provided PFX files (this is me) who would like to be able to recognize failures in processing these automatically & reliably.
I explored trying to get more info from OpenSSL's errors on exactly what was unsupported, but the best available is the data string, which looks something like Global default library context, Algorithm (RC2-CBC : 3), Properties () (as a plain string - I can't see a way to reach the data itself directly). I assume that's not really useful/friendly enough to include here, but happy to add that if people disagree.
The tests here use a PFX I've generated manually with an old OpenSSL version using RC2-40-CBC. You can see the contents with openssl pkcs12 -info -legacy -in ./test/fixtures/keys/legacy.pfx and password legacy (note that without the -legacy OpenSSL flag it will fail to open).
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 87.60%. Comparing base (
821ffab) to head (f4db363). Report is 127 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #54485 +/- ##
==========================================
+ Coverage 87.31% 87.60% +0.28%
==========================================
Files 648 650 +2
Lines 182365 182832 +467
Branches 34988 35382 +394
==========================================
+ Hits 159236 160173 +937
+ Misses 16393 15927 -466
+ Partials 6736 6732 -4
| Files with missing lines | Coverage Δ | |
|---|---|---|
| src/crypto/crypto_context.cc | 68.28% <100.00%> (+0.22%) |
:arrow_up: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
CI errors here is an actual failures, in the sharedlibs_openssl111fips config - should be able to find some time to fix that up and finish this in the next couple of days, watch this space.
OpenSSL 111 test are now passing! It looks like the only remaining failures here are spurious so I'll rerun those soon once CI finishes. A rereview would be nice when you have a minute @lpinca @mertcanaltin