GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||
GeezerVison is showing github.com — src: fix crypto.privateEncrypt fails first time by liuxingbaoyu · Pull Request #42793 · nodejs/node Conversation
| |||||||||||||||||||||
|
Review requested:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
crypto.privateEncrypt fails for the first time after crypto.generateKeyPairSync with certain parameters Because the error stack is not cleaned up when crypto.generateKeyPairSync exits. Fixes: nodejs#40814
686d32a to
b306018
Compare
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
It would be interesting to know why some ciphers populate the OpenSSL error queue even though WritePrivateKey appears to succeed.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| @@ -0,0 +1,35 @@ | |||
| 'use strict'; | |||
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Please either rename this file to better match what it tests, or add the test case to an existing test file that tests key objects.
Also, please add a comment explaining that this is a regression test for #40814.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
indeed! I'm also interested in why the exception is populated. But I don't know about openssl and I didn't find relevant information on the documentation. I'll show the stack for anyone with experience to investigate in depth. Callstack: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Also, do we need to add an assertion or something? Once any method is called and the error is left, it is easy to affect other methods. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
96487d5 to
9a7c063
Compare
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@liuxingbaoyu I believe there's a consistent failure when using linked OpenSSL 1.1.1 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
This looks to be another problem with In openssl3, the error is cleared inside openssl, but not in openssl1.1.1. I don't know if this is a bug or a behavior change. Exception source: ERR_set_mark(); ret = pem_read_bio_key_decoder(bp, x, ossl_pw_pem_password, &pwdata, libctx, propq, selection); if (ret == NULL && (BIO_seek(bp, pos) < 0 || (ret = pem_read_bio_key_legacy(bp, x, ossl_pw_pem_password, &pwdata, libctx, propq, selection)) == NULL)) ERR_clear_last_mark(); else ERR_pop_to_mark(); node/src/crypto/crypto_keys.cc Lines 257 to 258 in 6b004f1
Also, does anyone know what's the reason here? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ping @nodejs/crypto for (re)reviews. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Commit Queue failed- Loading data for nodejs/node/pull/42793
FetchError: Invalid response body while trying to fetch https://api.github.com/graphql: Premature close
at consumeBody (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:234:60)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Response.text (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/node_modules/node-fetch/src/body.js:158:18)
at async Request.json (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:51:18)
at async Request.query (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:109:20)
at async Request.queryAll (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:136:20)
at async Request.gql (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/request.js:66:22)
at async PRData.getComments (file:///opt/hostedtoolcache/node/18.12.0/x64/lib/node_modules/node-core-utils/lib/pr_data.js:97:21)
at async Promise.all (index 2)
at async Promise.all (index 1) {
type: 'system',
errno: 'ERR_STREAM_PREMATURE_CLOSE',
code: 'ERR_STREAM_PREMATURE_CLOSE',
erroredSysCall: undefined
}https://github.com/nodejs/node/actions/runs/3352810353 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Landed in e512786 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
`crypto.privateEncrypt` fails for the first time after `crypto.generateKeyPairSync` with certain parameters because the error stack is not cleaned up when `crypto.generateKeyPairSync` exits. Fixes: #40814 PR-URL: #42793 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
`crypto.privateEncrypt` fails for the first time after `crypto.generateKeyPairSync` with certain parameters because the error stack is not cleaned up when `crypto.generateKeyPairSync` exits. Fixes: #40814 PR-URL: #42793 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
`crypto.privateEncrypt` fails for the first time after `crypto.generateKeyPairSync` with certain parameters because the error stack is not cleaned up when `crypto.generateKeyPairSync` exits. Fixes: #40814 PR-URL: #42793 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
`crypto.privateEncrypt` fails for the first time after `crypto.generateKeyPairSync` with certain parameters because the error stack is not cleaned up when `crypto.generateKeyPairSync` exits. Fixes: #40814 PR-URL: #42793 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Filip Skokan <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Exception when usingprivateEncrypt/privateDecrypt with certain encrypted keys
There was an error while loading. Please reload this page.