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

Extendable ca certs v6 #9770

Closed

Conversation

sam-github
Copy link
Contributor

@sam-github sam-github commented Nov 23, 2016

Backport of #9139 to v6 to resolve conflicts.

Checklist
  • make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

tls,https

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v6.x labels Nov 23, 2016
@MylesBorins MylesBorins added lts-agenda semver-minor PRs that contain new features and should be released in the next minor version. labels Nov 28, 2016
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

@sam-github
Copy link
Contributor Author

Thanks for the reviews.

For the info of any watchers - we are waiting on LTS agenda before this can be landed. If it impacts you with v4 or v6, comment on #9139 .

@MylesBorins
Copy link
Contributor

@jasnell
Copy link
Member

jasnell commented Jan 6, 2017

@nodejs/lts ... any objections to getting this landed in v6.x-staging?

@mhdawson
Copy link
Member

+1 from me and I think it was agreed in the latest LTS meeting that it will go out in the next semver minor release.

@mhdawson
Copy link
Member

PR-URL: nodejs#9139
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
In closed environments, self-signed or privately signed certificates are
commonly used, and rejected by Node.js since their root CAs are not
well-known. Allow extending the set of well-known compiled-in CAs via
environment, so they can be set as a matter of policy.

PR-URL: nodejs#9139
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
@sam-github
Copy link
Contributor Author

@nodejs/build Any idea why tes/arm shows up as failed in CI above, but not in jenkins, https://ci.nodejs.org/job/node-test-commit-arm/7007/?

I rebased onto v6.x-staging, anything ele I need to do here?

@gibfahn
Copy link
Member

gibfahn commented Jan 11, 2017

@sam-github not an issue: nodejs/build#572

@italoacasas
Copy link
Contributor

ping @sam-github

@sam-github
Copy link
Contributor Author

pong @italoacasas

@sam-github
Copy link
Contributor Author

@MylesBorins do I push this onto v6.x-staging, or does the 6.x maintainer pull it on?

@MylesBorins
Copy link
Contributor

After the last round of audits on v6.x the original PR can now be cherry-picked onto the v6.x-staging branch with the test suite passing!

closing

@sam-github sam-github deleted the extendable-ca-certs-v6 branch January 24, 2017 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants