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

Add tests for PKCS#8 private keys #26898

Closed

Conversation

sasurau4
Copy link
Contributor

@sasurau4 sasurau4 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-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 25, 2019
@sasurau4 sasurau4 changed the title Add tests for pkcs8 private keys Add tests for #PKCS#8 private keys Mar 25, 2019
@sasurau4 sasurau4 force-pushed the add-tests-for-pkcs8-private-keys branch from 66f3032 to 57fd106 Compare March 25, 2019 04:38
@Trott Trott added the crypto Issues and PRs related to the crypto subsystem. label Mar 25, 2019
@sasurau4 sasurau4 marked this pull request as ready for review March 25, 2019 05:01
Copy link
Member

@bnoordhuis bnoordhuis 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
Contributor Author

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
Contributor Author

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.

@BridgeAR BridgeAR 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
Contributor Author

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

test/parallel/test-crypto-rsa-dsa.js Outdated Show resolved Hide resolved
test/parallel/test-crypto-rsa-dsa.js Outdated Show resolved Hide resolved
@sasurau4
Copy link
Contributor Author

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

@nodejs-github-bot
Copy link
Collaborator

@tniessen tniessen changed the title Add tests for #PKCS#8 private keys Add tests for PKCS#8 private keys Mar 27, 2019
@danbev
Copy link
Contributor

danbev commented Mar 28, 2019

Landed in 85546c2.

@danbev danbev 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]>
@BethGriggs BethGriggs mentioned this pull request Apr 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants