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

test: more tests for PKCS#8 private keys #24928

Closed
bnoordhuis opened this issue Dec 9, 2018 · 7 comments
Closed

test: more tests for PKCS#8 private keys #24928

bnoordhuis opened this issue Dec 9, 2018 · 7 comments
Labels
crypto Issues and PRs related to the crypto subsystem. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. test Issues and PRs related to the tests.

Comments

@bnoordhuis
Copy link
Member

We currently lack coverage for unencrypted PKCS#8 private keys (-----BEGIN PRIVATE KEY-----) with the sole exception of test/fixtures/foafssl.key used in test/parallel/test-https-foafssl.js.

Compare PKCS#1 private keys (-----BEGIN RSA PRIVATE KEY-----) for which we have ample coverage.

I count three tests that cover encrypted PKCS#8 keys (-----BEGIN ENCRYPTED PRIVATE KEY-----):

test/parallel/test-tls-passphrase.js
test/parallel/test-crypto-rsa-dsa.js
test/parallel/test-crypto-rsa-dsa.js

For unencrypted PKCS#8, it would be nice to have tests for at least RSA and DSA, with and without PEM attributes for bonus points. See test/fixtures/foafssl.key for an example of attributes.

cc @sam-github - possibly relevant to your interests

@bnoordhuis bnoordhuis added help wanted Issues that need assistance from volunteers or PRs that need help to proceed. crypto Issues and PRs related to the crypto subsystem. test Issues and PRs related to the tests. labels Dec 9, 2018
@uttampawar
Copy link
Contributor

@bnoordhuis If you can be a mentor and guide me, I would like to help out on this issue.

@bnoordhuis
Copy link
Member Author

@uttampawar Thanks. Just ask questions and I'll try to answer them.

@bmsdave
Copy link
Contributor

bmsdave commented Feb 7, 2019

@uttampawar
Hi! Do you need help with this task?

@sasurau4
Copy link
Contributor

Can I work on this issue?

danbev pushed a commit that referenced this issue Mar 28, 2019
PR-URL: #26898
Refs: #24928
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this issue Mar 28, 2019
PR-URL: #26898
Refs: #24928
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this issue Mar 29, 2019
PR-URL: #26898
Refs: #24928
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this issue Mar 30, 2019
PR-URL: #26898
Refs: #24928
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
BethGriggs pushed a commit that referenced this issue Apr 5, 2019
PR-URL: #26898
Refs: #24928
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this issue May 16, 2019
PR-URL: #26898
Refs: #24928
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
MylesBorins pushed a commit that referenced this issue May 16, 2019
PR-URL: #26898
Refs: #24928
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@dhuang612
Copy link

Hi,

Is this available to work on?

@tniessen
Copy link
Member

@bnoordhuis Is this still pending? The C++ coverage seems to be pretty good around key parsing, but it does not necessarily reflect all possible inputs.

@bnoordhuis
Copy link
Member Author

Yes, 85546c2 resolves this - it should have had a Fixes: tag, not a Refs: tag. I'll close this out, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests

8 participants