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.

Sign up for GitHub

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

Jump to bottom

parallel.test-crypto-rsa-dsa fails with Missing expected exception #52537

Closed
richardlau opened this issue Apr 15, 2024 · 7 comments
Closed

parallel.test-crypto-rsa-dsa fails with Missing expected exception #52537

richardlau opened this issue Apr 15, 2024 · 7 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. linux Issues and PRs related to the Linux platform.

Comments

@richardlau
Copy link
Member

Test

test-crypto-rsa-dsa

Platform

Linux x64

Console output

08:08:54 not ok 758 parallel/test-crypto-rsa-dsa
08:08:54   ---
08:08:54   duration_ms: 245.73400
08:08:54   severity: fail
08:08:54   exitcode: 1
08:08:54   stack: |-
08:08:54     node:assert:126
08:08:54       throw new AssertionError(obj);
08:08:54       ^
08:08:54     
08:08:54     AssertionError [ERR_ASSERTION]: Missing expected exception.
08:08:54         at test_rsa (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-crypto-rsa-dsa.js:226:12)
08:08:54         at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/test/parallel/test-crypto-rsa-dsa.js:258:1)
08:08:54         at Module._compile (node:internal/modules/cjs/loader:1476:14)
08:08:54         at Module._extensions..js (node:internal/modules/cjs/loader:1555:10)
08:08:54         at Module.load (node:internal/modules/cjs/loader:1288:32)
08:08:54         at Module._load (node:internal/modules/cjs/loader:1104:12)
08:08:54         at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:187:14)
08:08:54         at node:internal/main/run_main_module:28:49 {
08:08:54       generatedMessage: false,
08:08:54       code: 'ERR_ASSERTION',
08:08:54       actual: undefined,
08:08:54       expected: { code: 'ERR_INVALID_ARG_VALUE' },
08:08:54       operator: 'throws'
08:08:54     }
08:08:54     
08:08:54     Node.js v22.0.0-pre
08:08:54   ...

Build links

Additional information

Looks like this test is constantly failing on test-ibm-ubi81_container-x64-1 since yesterday.
@richardlau richardlau added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Apr 15, 2024
@github-actions github-actions bot added the linux Issues and PRs related to the Linux platform. label Apr 15, 2024
@richardlau
Copy link
Member Author

richardlau commented Apr 15, 2024 •
edited

The stack is pointing at the test added in 54cd268 for https://hackerone.com/reports/2269177.

IIRC ubi81_sharedlibs_openssl111fips_x64 builds against the system OpenSSL (i.e. the one in UBI 8). The container was recently redeployed -- I'm wondering if that has picked up a patched system OpenSSL that the test case doesn't pass with.

cc @mhdawson

@richardlau
Copy link
Member Author

The code does a runtime check for whether the OpenSSL implementation supports implicit rejection and conditionally throws an exception based on the check:

node/src/crypto/crypto_cipher.cc

Lines 1081 to 1095 in f098b7a

int rsa_pkcs1_implicit_rejection =
EVP_PKEY_CTX_ctrl_str(ctx.get(), "rsa_pkcs1_implicit_rejection", "1");
// From the doc -2 means that the option is not supported.
// The default for the option is enabled and if it has been
// specifically disabled we want to respect that so we will
// not throw an error if the option is supported regardless
// of how it is set. The call to set the value
// will not affect what is used since a different context is
// used in the call if the option is supported
if (rsa_pkcs1_implicit_rejection <= 0) {
return THROW_ERR_INVALID_ARG_VALUE(
env,
"RSA_PKCS1_PADDING is no longer supported for private decryption,"
" this can be reverted with --security-revert=CVE-2024-PEND");
}

However the test looks to be always expecting the exception. It may not be easy for the test to do the feature detection without native code.

@richardlau
Copy link
Member Author

@mhdawson Thoughts? Perhaps as a short term fix we can skip the test if built with a dynamically linked OpenSSL?

I presume we'd expect to have to update the test in the future when we update the statically linked OpenSSL to a version that supports the implicit rejection.

@targos
Copy link
Member

targos commented Apr 15, 2024

This is a similar problem to #52196

@richardlau

This comment was marked as outdated.

Sign in to view
@targos
Copy link
Member

targos commented Apr 15, 2024 •
edited

I also rebuilt test-digitalocean-ubi81_container-x64-1 (and all other containers on that host) today.

@richardlau richardlau mentioned this issue Apr 15, 2024
Merged
@richardlau
Copy link
Member Author

I had a quick discussion with @mhdawson who is going to look at if there's a way for the test to detect if the underlying OpenSSL implementation supports implicit rejections.

As a temporary fix to unblock the CI, I've opened #52542 to skip that part of the test if Node.js has been dynamically linked against OpenSSL.

nodejs-github-bot pushed a commit that referenced this issue Apr 15, 2024
@richardlau
test: skip test for dynamically linked OpenSSL
4ec049b 
As a temporary measure to unblock the CI, skip the RSA implicit
rejection test when Node.js is built against a dynamically linked
OpenSSL.

PR-URL: #52542
Refs: #52537
Refs: nodejs-private/node-private#525
Refs: https://hackerone.com/reports/2269177
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
richardlau added a commit that referenced this issue Apr 17, 2024
@richardlau
test: skip test for dynamically linked OpenSSL
b9eb003 
As a temporary measure to unblock the CI, skip the RSA implicit
rejection test when Node.js is built against a dynamically linked
OpenSSL.

PR-URL: #52542
Refs: #52537
Refs: nodejs-private/node-private#525
Refs: https://hackerone.com/reports/2269177
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
richardlau added a commit that referenced this issue Apr 17, 2024
@richardlau
test: skip test for dynamically linked OpenSSL
c60cd67 
As a temporary measure to unblock the CI, skip the RSA implicit
rejection test when Node.js is built against a dynamically linked
OpenSSL.

PR-URL: #52542
Refs: #52537
Refs: nodejs-private/node-private#525
Refs: https://hackerone.com/reports/2269177
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
marco-ippolito pushed a commit that referenced this issue Apr 23, 2024
@richardlau @marco-ippolito
test: skip test for dynamically linked OpenSSL
afaf889 
As a temporary measure to unblock the CI, skip the RSA implicit
rejection test when Node.js is built against a dynamically linked
OpenSSL.

PR-URL: #52542
Refs: #52537
Refs: nodejs-private/node-private#525
Refs: https://hackerone.com/reports/2269177
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
mhdawson added a commit to mhdawson/io.js that referenced this issue May 1, 2024
@mhdawson
test: crypto-rsa-dsa testing for dynamic openssl
fec92e8 
Fixes: nodejs#52537

Signed-off-by: Michael Dawson <[email protected]>
@mhdawson mhdawson mentioned this issue May 1, 2024
Closed
Ch3nYuY pushed a commit to Ch3nYuY/node that referenced this issue May 8, 2024
@mhdawson @Ch3nYuY
test: crypto-rsa-dsa testing for dynamic openssl
8327822 
Fixes: nodejs#52537

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: nodejs#52781
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue May 8, 2024
@mhdawson @targos
test: crypto-rsa-dsa testing for dynamic openssl
30a35ae 
Fixes: #52537

Signed-off-by: Michael Dawson <[email protected]>
PR-URL: #52781
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI. linux Issues and PRs related to the Linux platform.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: crypto-rsa-dsa testing for dynamic openssl mhdawson/io.js
2 participants
@targos @richardlau