-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
generate more certs, and more test coverage using them #24374
Conversation
Update of #10747, which was approved by @bnoordhuis and @mhdawson Tests pass locally, lets see what ci thinks: https://ci.nodejs.org/job/node-test-pull-request/18621/ |
Mostly passed, some problems with with windows-binary on vs2017... shouldn't be related, but I'm rebuilding. |
There was a single problematic Windows host on CI that I just took offline several minutes ago, so you may need to do one more rebuild to get to green. Sorry. |
Resume Build: https://ci.nodejs.org/job/node-test-pull-request/18641/ ✅ |
401cb33
to
7e5c877
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I didn't do a bit-for-bit check of the generated certs though. :-)
PFX is not PEM, its binary DER. Use the same .pfx extension as test/fixtures/test_cert.pfx does.
7e5c877
to
dcfdfba
Compare
agent6 was the only cert that had a chain (an intermediate certificate), and there were no non-RSA certs other than a single self-signed one. This makes it impossible to test cert-chain scenarios with multiple identities which require chains to prove chain completion, and multi-algorithm because OpenSSL doesn't support multiple identities unless they are multi-algorithm. PFX files were also missing for most identities, making it difficult to test multi-PFX and PFX interactions with cert-chain+key and CA options. New server cert chains: - ECC: ca5 signs ca6 signs ec10, CN=agent10.example.com - RSA: ca2 signs ca4 signs agent10, CN=agent10.example.com PFX added for: - agent6 - agent10 - ec10 All pem and pfx regenerated from scratch to test that the Makefile is actually working as intended.
Prove that cert and key options do not have to be ordered, and that the pfx option can be used at the same time as the cert/key option (which was claimed to be impossible by some pre-existing documentation).
When honorCipherOrder is not explicitly set, it defaults to true, cover this condition in the test. Also, run all tests in parallel, instead of sequentially.
dcfdfba
to
27af1b0
Compare
Regenerated certs after |
Landed in a569686...13a6798 |
PFX is not PEM, its binary DER. Use the same .pfx extension as test/fixtures/test_cert.pfx does. PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
agent6 was the only cert that had a chain (an intermediate certificate), and there were no non-RSA certs other than a single self-signed one. This makes it impossible to test cert-chain scenarios with multiple identities which require chains to prove chain completion, and multi-algorithm because OpenSSL doesn't support multiple identities unless they are multi-algorithm. PFX files were also missing for most identities, making it difficult to test multi-PFX and PFX interactions with cert-chain+key and CA options. New server cert chains: - ECC: ca5 signs ca6 signs ec10, CN=agent10.example.com - RSA: ca2 signs ca4 signs agent10, CN=agent10.example.com PFX added for: - agent6 - agent10 - ec10 All pem and pfx regenerated from scratch to test that the Makefile is actually working as intended. PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Prove that cert and key options do not have to be ordered, and that the pfx option can be used at the same time as the cert/key option (which was claimed to be impossible by some pre-existing documentation). PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
When honorCipherOrder is not explicitly set, it defaults to true, cover this condition in the test. Also, run all tests in parallel, instead of sequentially. PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PFX is not PEM, its binary DER. Use the same .pfx extension as test/fixtures/test_cert.pfx does. PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
agent6 was the only cert that had a chain (an intermediate certificate), and there were no non-RSA certs other than a single self-signed one. This makes it impossible to test cert-chain scenarios with multiple identities which require chains to prove chain completion, and multi-algorithm because OpenSSL doesn't support multiple identities unless they are multi-algorithm. PFX files were also missing for most identities, making it difficult to test multi-PFX and PFX interactions with cert-chain+key and CA options. New server cert chains: - ECC: ca5 signs ca6 signs ec10, CN=agent10.example.com - RSA: ca2 signs ca4 signs agent10, CN=agent10.example.com PFX added for: - agent6 - agent10 - ec10 All pem and pfx regenerated from scratch to test that the Makefile is actually working as intended. PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Prove that cert and key options do not have to be ordered, and that the pfx option can be used at the same time as the cert/key option (which was claimed to be impossible by some pre-existing documentation). PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
@sam-github do you think this is worth backporting to |
PFX is not PEM, its binary DER. Use the same .pfx extension as test/fixtures/test_cert.pfx does. Backport-PR-URL: nodejs#25501 PR-URL: nodejs#24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
agent6 was the only cert that had a chain (an intermediate certificate), and there were no non-RSA certs other than a single self-signed one. This makes it impossible to test cert-chain scenarios with multiple identities which require chains to prove chain completion, and multi-algorithm because OpenSSL doesn't support multiple identities unless they are multi-algorithm. PFX files were also missing for most identities, making it difficult to test multi-PFX and PFX interactions with cert-chain+key and CA options. New server cert chains: - ECC: ca5 signs ca6 signs ec10, CN=agent10.example.com - RSA: ca2 signs ca4 signs agent10, CN=agent10.example.com PFX added for: - agent6 - agent10 - ec10 All pem and pfx regenerated from scratch to test that the Makefile is actually working as intended. Backport-PR-URL: nodejs#25501 PR-URL: nodejs#24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Prove that cert and key options do not have to be ordered, and that the pfx option can be used at the same time as the cert/key option (which was claimed to be impossible by some pre-existing documentation). Backport-PR-URL: nodejs#25501 PR-URL: nodejs#24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
When honorCipherOrder is not explicitly set, it defaults to true, cover this condition in the test. Also, run all tests in parallel, instead of sequentially. Backport-PR-URL: nodejs#25501 PR-URL: nodejs#24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Prove that cert and key options do not have to be ordered, and that the pfx option can be used at the same time as the cert/key option (which was claimed to be impossible by some pre-existing documentation). Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
agent6 was the only cert that had a chain (an intermediate certificate), and there were no non-RSA certs other than a single self-signed one. This makes it impossible to test cert-chain scenarios with multiple identities which require chains to prove chain completion, and multi-algorithm because OpenSSL doesn't support multiple identities unless they are multi-algorithm. PFX files were also missing for most identities, making it difficult to test multi-PFX and PFX interactions with cert-chain+key and CA options. New server cert chains: - ECC: ca5 signs ca6 signs ec10, CN=agent10.example.com - RSA: ca2 signs ca4 signs agent10, CN=agent10.example.com PFX added for: - agent6 - agent10 - ec10 All pem and pfx regenerated from scratch to test that the Makefile is actually working as intended. Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PFX is not PEM, its binary DER. Use the same .pfx extension as test/fixtures/test_cert.pfx does. Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
When honorCipherOrder is not explicitly set, it defaults to true, cover this condition in the test. Also, run all tests in parallel, instead of sequentially. Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Prove that cert and key options do not have to be ordered, and that the pfx option can be used at the same time as the cert/key option (which was claimed to be impossible by some pre-existing documentation). Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
agent6 was the only cert that had a chain (an intermediate certificate), and there were no non-RSA certs other than a single self-signed one. This makes it impossible to test cert-chain scenarios with multiple identities which require chains to prove chain completion, and multi-algorithm because OpenSSL doesn't support multiple identities unless they are multi-algorithm. PFX files were also missing for most identities, making it difficult to test multi-PFX and PFX interactions with cert-chain+key and CA options. New server cert chains: - ECC: ca5 signs ca6 signs ec10, CN=agent10.example.com - RSA: ca2 signs ca4 signs agent10, CN=agent10.example.com PFX added for: - agent6 - agent10 - ec10 All pem and pfx regenerated from scratch to test that the Makefile is actually working as intended. Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
PFX is not PEM, its binary DER. Use the same .pfx extension as test/fixtures/test_cert.pfx does. Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
When honorCipherOrder is not explicitly set, it defaults to true, cover this condition in the test. Also, run all tests in parallel, instead of sequentially. Backport-PR-URL: #25501 PR-URL: #24374 Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes