Skip to content

Conversation

@sasurau4
Copy link
Contributor

@sasurau4sasurau4 commented Mar 25, 2019

I added tests for unencrypted PKCS#8 private keys.
I generated the test PKCS#8 private keys by converting test_rsa_privkey,pem and test_dsa_privkey.pem by openssl command.

Refs: #24928

I ran make -j4 test command, but failed.

detailed error log
 ... make[2]: Leaving directory '/home/ihara/hobby/node/test/addons/not-a-binding/build' Building addon in /home/ihara/hobby/node/test/addons/openssl-client-cert-engine /home/ihara/hobby/node/tools/build-addons.js:58 main(process.argv[3]).catch((err) => setImmediate(() =>{throw err})); ^ 

Error: spawn /home/ihara/hobby/node/out/Release/node EACCES
at Process.ChildProcess._handle.onexit (internal/child_process.js:246:19)
at onErrorNT (internal/child_process.js:431:16)
at processTicksAndRejections (internal/process/task_queues.js:81:17)
Makefile:385: recipe for target 'test/addons/.buildstamp' failed
make[1]: *** [test/addons/.buildstamp] Error 1
make[1]: *** Waiting for unfinished jobs....
touch /home/ihara/hobby/node/out/Release/obj.target/rename_node_bin_win.stamp
g++ -o /home/ihara/hobby/node/out/Release/cctest -pthread -rdynamic -m64 -Wl,--whole-archive,/home/ihara/hobby/node/out/Release/obj.target/deps/zlib/libzlib.a -Wl,--no-whole-archive -Wl,--whole-archive,/home/ihara/hobby/node/out/Release/obj.target/deps/uv/libuv.a -Wl,--no-whole-archive -Wl,-z,noexecstack -Wl,--whole-archive /home/ihara/hobby/node/out/Release/obj.target/deps/v8/gypfiles/libv8_base.a -Wl,--no-whole-archive -Wl,-z,relro -Wl,-z,now -Wl,--whole-archive,/home/ihara/hobby/node/out/Release/obj.target/deps/openssl/libopenssl.a -Wl,--no-whole-archive -pthread -Wl,--start-group /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/node_test_fixture.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_aliased_buffer.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_base64.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_node_postmortem_metadata.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_environment.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_linked_binding.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_platform.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_report_util.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_traced_value.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_util.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_url.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_inspector_socket.o /home/ihara/hobby/node/out/Release/obj.target/cctest/test/cctest/test_inspector_socket_server.o /home/ihara/hobby/node/out/Release/obj.target/libnode.a /home/ihara/hobby/node/out/Release/obj.target/deps/gtest/libgtest.a /home/ihara/hobby/node/out/Release/obj.target/deps/histogram/libhistogram.a /home/ihara/hobby/node/out/Release/obj.target/deps/v8/gypfiles/libv8_libplatform.a /home/ihara/hobby/node/out/Release/obj.target/tools/icu/libicui18n.a /home/ihara/hobby/node/out/Release/obj.target/deps/zlib/libzlib.a /home/ihara/hobby/node/out/Release/obj.target/deps/http_parser/libhttp_parser.a /home/ihara/hobby/node/out/Release/obj.target/deps/llhttp/libllhttp.a /home/ihara/hobby/node/out/Release/obj.target/deps/cares/libcares.a /home/ihara/hobby/node/out/Release/obj.target/deps/uv/libuv.a /home/ihara/hobby/node/out/Release/obj.target/deps/nghttp2/libnghttp2.a /home/ihara/hobby/node/out/Release/obj.target/deps/brotli/libbrotli.a /home/ihara/hobby/node/out/Release/obj.target/deps/openssl/libopenssl.a /home/ihara/hobby/node/out/Release/obj.target/deps/v8/gypfiles/libv8_base.a /home/ihara/hobby/node/out/Release/obj.target/deps/v8/gypfiles/libv8_libbase.a /home/ihara/hobby/node/out/Release/obj.target/deps/v8/gypfiles/libv8_libsampler.a /home/ihara/hobby/node/out/Release/obj.target/tools/icu/libicuucx.a /home/ihara/hobby/node/out/Release/obj.target/tools/icu/libicudata.a /home/ihara/hobby/node/out/Release/obj.target/tools/icu/libicustubdata.a /home/ihara/hobby/node/out/Release/obj.target/deps/v8/gypfiles/libv8_snapshot.a -ldl -lrt -lm -Wl,--end-group
Makefile:291: recipe for target 'test' failed
make: *** [test] Error 2

My environment is Ubuntu 18.04.2 LTS and succeeded when I ran make test-only command.

Please help me 🙏

Updated: solved the above problem.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-botnodejs-github-bot added the test Issues and PRs related to the tests. label Mar 25, 2019
@sasurau4sasurau4 changed the title Add tests for pkcs8 private keysAdd tests for #PKCS#8 private keysMar 25, 2019
@sasurau4sasurau4force-pushed the add-tests-for-pkcs8-private-keys branch from 66f3032 to 57fd106CompareMarch 25, 2019 04:38
@TrottTrott added the crypto Issues and PRs related to the crypto subsystem. label Mar 25, 2019
@sasurau4sasurau4 marked this pull request as ready for review March 25, 2019 05:01
Copy link
Member

@bnoordhuisbnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for picking this up!


assert.strictEqual(verify.verify(dsaPubPem,signature,'hex'),true);

// Test the legacy 'DSS1' name.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be okay with dropping this, that's already checked around line 250.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my curious, is DSS1 same as SHA-1? 🤔
I found following line in Node.js Project but I couldn't find what happened historically about the relationship between DSS1 and SHA-1 from Google search.

// Historically, "dss1" and "DSS1" were DSA aliases for SHA-1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DSS1 stands for DSA (Digital Signature Algorithm) with SHA-1 as the hash function. It's a long-deprecated (and now removed) openssl synonym from when openssl conflated public key algorithms with their hash functions.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Thanks 👍 👍 👍

@BridgeAR
Copy link
Member

@sasurau4 the problem about the test suite can be circumvented by running the tests directly with: python tools/test.py -j4. This is a problem with GYP right now and AFAIK there is no solution so far.

@BridgeARBridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Mar 25, 2019
@nodejs-github-bot
Copy link
Collaborator

@sasurau4
Copy link
ContributorAuthor

@BridgeAR The tests succeeded when I ran python tools/test.py -j4. Thanks for your response 👍

@sasurau4
Copy link
ContributorAuthor

@tniessen I fixed pointed out. Thanks for your review 👍

@nodejs-github-bot
Copy link
Collaborator

@tniessentniessen changed the title Add tests for #PKCS#8 private keysAdd tests for PKCS#8 private keysMar 27, 2019
@danbev
Copy link
Contributor

Landed in 85546c2.

@danbevdanbev closed this Mar 28, 2019
danbev pushed a commit that referenced this pull request Mar 28, 2019
PR-URL: #26898 Refs: #24928 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
targos pushed a commit that referenced this pull request Mar 28, 2019
PR-URL: #26898 Refs: #24928 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
targos pushed a commit that referenced this pull request Mar 29, 2019
PR-URL: #26898 Refs: #24928 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
targos pushed a commit that referenced this pull request Mar 30, 2019
PR-URL: #26898 Refs: #24928 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
BethGriggs pushed a commit that referenced this pull request Apr 5, 2019
PR-URL: #26898 Refs: #24928 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
@BethGriggsBethGriggs mentioned this pull request Apr 9, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.cryptoIssues and PRs related to the crypto subsystem.testIssues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@sasurau4@BridgeAR@nodejs-github-bot@danbev@bnoordhuis@jasnell@tniessen@MylesBorins@Trott