GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [v10.x backport] generate more certs, and more test coverage using them by sam-github · Pull Request #25501 · nodejs/node

Conversation

@sam-github

@sam-github sam-github commented Jan 14, 2019

Copy link
Copy Markdown
Contributor
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nodejs-github-bot

nodejs-github-bot commented Jan 14, 2019

Copy link
Copy Markdown
Collaborator

@sam-github sadly an error occured when I tried to trigger a build :(
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to Node.js core tests and test infrastructure. v10.x labels Jan 14, 2019
@sam-github
sam-github force-pushed the backport-24374-to-v10.x branch from 3dca6d9 to 9510496 Compare January 14, 2019 19:54
@joyeecheung joyeecheung mentioned this pull request Jan 16, 2019
CI failures: 20190110 - 20190116 nodejs/reliability#20
Closed
@MylesBorins
MylesBorins force-pushed the v10.x-staging branch from c1ee936 to c6cffad Compare January 29, 2019 05:06
@codebytere
codebytere force-pushed the v10.x-staging branch 3 times, most recently from 2d6e145 to 7840f71 Compare January 29, 2019 18:12
sam-github added 4 commits February 4, 2019 11:22
@sam-github
test: rename agent1-pfx.pem to agent1.pfx
96fe1e7 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@sam-github
test: add independent multi-alg crypto identities
18593f0 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@sam-github
test: cover tls multi-identity option mixtures
f1ff355 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@sam-github
test: confirm tls server suite default is its own
8365239 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@sam-github
sam-github force-pushed the backport-24374-to-v10.x branch from 9510496 to 8365239 Compare February 4, 2019 19:22
@sam-github

sam-github commented Feb 4, 2019

Copy link
Copy Markdown
Contributor Author

@BethGriggs rebased cleanly
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BethGriggs

BethGriggs commented Feb 5, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

CI: https://ci.nodejs.org/job/node-test-pull-request/20585/
Resume: https://ci.nodejs.org/job/node-test-commit/25623
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
BethGriggs
BethGriggs approved these changes Feb 5, 2019
View reviewed changes
BethGriggs pushed a commit that referenced this pull request Feb 5, 2019
@sam-github @BethGriggs
test: cover tls multi-identity option mixtures
83145ec 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 5, 2019
@sam-github @BethGriggs
test: add independent multi-alg crypto identities
43dcbbe 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 5, 2019
@sam-github @BethGriggs
test: rename agent1-pfx.pem to agent1.pfx
3c2b40b 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 5, 2019
@sam-github @BethGriggs
test: confirm tls server suite default is its own
0e63d0a 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
@BethGriggs

BethGriggs commented Feb 5, 2019

Copy link
Copy Markdown
Member

Landed on v10.x-staging in 26f7edb...0e63d0a
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BethGriggs BethGriggs closed this Feb 5, 2019
@sam-github
sam-github deleted the backport-24374-to-v10.x branch February 5, 2019 16:07
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@sam-github @rvagg
test: cover tls multi-identity option mixtures
0a36fb2 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@sam-github @rvagg
test: add independent multi-alg crypto identities
24d7071 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@sam-github @rvagg
test: rename agent1-pfx.pem to agent1.pfx
3e31344 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@sam-github @rvagg
test: confirm tls server suite default is its own
9293b49 
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 <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Franziska Hinkelmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BethGriggs BethGriggs BethGriggs approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

test Issues and PRs related to Node.js core tests and test infrastructure.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@sam-github @nodejs-github-bot @BethGriggs
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.