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.

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

Investigate flaky parallel/test-crypto-keygen #22978

Closed
Trott opened this issue Sep 20, 2018 · 6 comments
Closed

Investigate flaky parallel/test-crypto-keygen #22978

Trott opened this issue Sep 20, 2018 · 6 comments
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI.

Comments

@Trott
Copy link
Member

Trott commented Sep 20, 2018

  • Version: 11.0.0-pre (master)
  • Platform: linuxone rhel72-s390x
  • Subsystem: test crypto

https://ci.nodejs.org/job/node-test-commit-linuxone/5311/nodes=rhel72-s390x/console

09:59:09 not ok 2221 parallel/test-crypto-keygen
09:59:09   ---
09:59:09   duration_ms: 120.14
09:59:09   severity: fail
09:59:09   exitcode: -15
09:59:09   stack: |-
09:59:09     timeout
09:59:09     assert.js:662
09:59:09         throw actual;
09:59:09         ^
09:59:09     
09:59:09     Error: error:0D07207B:asn1 encoding routines:ASN1_get_object:header too long
09:59:09         at Sign.sign (internal/crypto/sig.js:83:27)
09:59:09         at testSignVerify (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-crypto-keygen.js:43:42)
09:59:09         at assert.throws (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-crypto-keygen.js:234:7)
09:59:09         at getActual (assert.js:574:5)
09:59:09         at Function.throws (assert.js:692:24)
09:59:09         at AsyncWrap.common.mustCall (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/parallel/test-crypto-keygen.js:233:12)
09:59:09         at AsyncWrap.<anonymous> (/data/iojs/build/workspace/node-test-commit-linuxone/nodes/rhel72-s390x/test/common/index.js:350:15)
09:59:09         at AsyncWrap.wrap.ondone (internal/crypto/keygen.js:42:14)
09:59:09   ...
@Trott Trott added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Sep 20, 2018
@Trott
Copy link
Member Author

Trott commented Sep 20, 2018

Comment from @tniessen in #22660 (comment):

@Trott I fear there might be a small chance (0.392156862745098% if my calculation is correct) of OpenSSL successfully decrypting a key it is not supposed to decrypt, but leading to an invalid key and thus causing this error message. Let me try to confirm that before fixing the test.

@Trott
Copy link
Member Author

Trott commented Sep 20, 2018

While stress-testing locally with -j 96 --repeat 192, I got a different error:

=== release test-crypto-keygen ===                    
Path: parallel/test-crypto-keygen
assert.js:662
    throw actual;
    ^

Error: error:0D0680A8:asn1 encoding routines:asn1_check_tlen:wrong tag
    at Sign.sign (internal/crypto/sig.js:83:27)
    at testSignVerify (/Users/trott/io.js/test/parallel/test-crypto-keygen.js:43:42)
    at assert.throws (/Users/trott/io.js/test/parallel/test-crypto-keygen.js:234:7)
    at getActual (assert.js:574:5)
    at Function.throws (assert.js:692:24)
    at AsyncWrap.common.mustCall (/Users/trott/io.js/test/parallel/test-crypto-keygen.js:233:12)
    at AsyncWrap.<anonymous> (/Users/trott/io.js/test/common/index.js:350:15)
    at AsyncWrap.wrap.ondone (internal/crypto/keygen.js:42:14)
Command: out/Release/node /Users/trott/io.js/test/parallel/test-crypto-keygen.js
[02:08|% 100|+ 191|-   1]: Done 

@Trott
Copy link
Member Author

Trott commented Sep 20, 2018

Failed on a different platform in CI:

https://ci.nodejs.org/job/node-test-commit-arm/18620/nodes=centos7-arm64-gcc6/console

10:06:22 not ok 2388 parallel/test-crypto-keygen
10:06:22   ---
10:06:22   duration_ms: 240.71
10:06:22   severity: fail
10:06:22   exitcode: -15
10:06:22   stack: |-
10:06:22     timeout
10:06:22     assert.js:662
10:06:22         throw actual;
10:06:22         ^
10:06:22     
10:06:22     Error: error:0D07207B:asn1 encoding routines:ASN1_get_object:header too long
10:06:22         at Sign.sign (internal/crypto/sig.js:83:27)
10:06:22         at testSignVerify (/home/iojs/build/workspace/node-test-commit-arm/nodes/centos7-arm64-gcc6/test/parallel/test-crypto-keygen.js:43:42)
10:06:22         at assert.throws (/home/iojs/build/workspace/node-test-commit-arm/nodes/centos7-arm64-gcc6/test/parallel/test-crypto-keygen.js:171:7)
10:06:22         at getActual (assert.js:574:5)
10:06:22         at Function.throws (assert.js:692:24)
10:06:22         at AsyncWrap.common.mustCall (/home/iojs/build/workspace/node-test-commit-arm/nodes/centos7-arm64-gcc6/test/parallel/test-crypto-keygen.js:170:12)
10:06:22         at AsyncWrap.<anonymous> (/home/iojs/build/workspace/node-test-commit-arm/nodes/centos7-arm64-gcc6/test/common/index.js:350:15)
10:06:22         at AsyncWrap.wrap.ondone (internal/crypto/keygen.js:42:14)
10:06:22   ...

@tniessen
Copy link
Member

@Trott This is most likely a randomly occuring problem with low probability (which obviously increases massively by being tested multiple times).

Changing /bad decrypt/ to /bad decrypt|asn1 encoding routines/ should be enough if my assumption is correct, I'll try to confirm that.

@mscdex
Copy link
Contributor

mscdex commented Sep 20, 2018

There should be an opensslStack (or something similarly named) on the error message object that may help as well?

tniessen added a commit to tniessen/node that referenced this issue Sep 20, 2018
@tniessen
Copy link
Member

Fixed in #22980, thanks for letting me know, @Trott. (Out of 25000 generated RSA keys, the test failed 98, which is close enough to the theoretical value to confirm my assumption about the decryption padding.)

targos pushed a commit that referenced this issue Sep 21, 2018
There is a very small chance (about 0.4%) that OpenSSL will
successfully decrypt a key without the correct passphrase and will
then fail while parsing its ASN.1 structure. In those rare cases,
the error message will be different.

PR-URL: #22980
Fixes: #22978
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flaky-test Issues and PRs related to the tests with unstable failures on the CI.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants