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

[v8.x backport] src: add openssl-system-ca-path configure option #18174

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jan 16, 2018

The motivation for this commit is that we need to specify system CA
certificates when building node. While we are aware of the environment
variable NODE_EXTRA_CA_CERTS this is not a great solution as we build
an RPM and we also don't want users to be able to unset them.

The suggestion is to add a configure time property like this:

--openssl-system-ca-path=OPENSSL_SYSTEM_CA_PATH
             Use the specified path to system CA (PEM format) in
             addition to the OpenSSL supplied CA store or compiled-
             in Mozilla CA copy.

Usage example:

$ ./configure --openssl-system-ca-path=/etc/pki/tls/certs/ca-bundle.crt

This would add the specified CA certificates in addition to the ones
already being used.

PR-URL: #16790
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com
Reviewed-By: Ben Noordhuis info@bnoordhuis.nl
Reviewed-By: Tobias Nießen tniessen@tnie.de

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

src

The motivation for this commit is that we need to specify system CA
certificates when building node. While we are aware of the environment
variable NODE_EXTRA_CA_CERTS this is not a great solution as we build
an RPM and we also don't want users to be able to unset them.

The suggestion is to add a configure time property like this:

--openssl-system-ca-path=OPENSSL_SYSTEM_CA_PATH
             Use the specified path to system CA (PEM format) in
             addition to the OpenSSL supplied CA store or compiled-
             in Mozilla CA copy.

Usage example:
$ ./configure --openssl-system-ca-path=/etc/pki/tls/certs/ca-bundle.crt

This would add the specified CA certificates in addition to the ones
already being used.

PR-URL: nodejs#16790
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem. v8.x labels Jan 16, 2018
@danbev
Copy link
Contributor Author

danbev commented Jan 16, 2018

@gibfahn
Copy link
Member

gibfahn commented Jan 26, 2018

@gibfahn
Copy link
Member

gibfahn commented Jan 26, 2018

Everything is green (or known flaky) except this https://ci.nodejs.org/job/node-test-commit-linux/15866/nodes=debian8-64/consoleFull

Rebuild to see whether it's a flake: https://ci.nodejs.org/job/node-test-commit-linux/15871/

not ok 1990 sequential/test-debugger-debug-brk
  ---
  duration_ms: 0.172
  severity: fail
  stack: |-
    assert.js:42
      throw new errors.AssertionError({
      ^
    
    AssertionError [ERR_ASSERTION]: '/home/iojs/build/workspace/node-test-commit-linux/nodes/debian8-64/out/Release/node --inspect --debug-brk /home/iojs/build/workspace/node-test-commit-linux/nodes/debian8-64/test/fixtures/empty.js' should not quit
        at ChildProcess.fail (/home/iojs/build/workspace/node-test-commit-linux/nodes/debian8-64/test/sequential/test-debugger-debug-brk.js:19:29)
        at emitTwo (events.js:126:13)
        at ChildProcess.emit (events.js:214:7)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:198:12)
  ...

not ok 2014 sequential/test-inspector-async-hook-setup-at-signal
  ---
  duration_ms: 120.12
  severity: fail
  stack: |-
    timeout
  ...

not ok 2023 sequential/test-inspector-enabled
  ---
  duration_ms: 0.138
  severity: fail
  stack: |-
    Starting inspector on 127.0.0.1:9229 failed: address already in use
  ...

not ok 2030 sequential/test-inspector-port-cluster
  ---
  duration_ms: 0.411
  severity: fail
  stack: |-
    Starting inspector on 127.0.0.1:9229 failed: address already in use
    Debugger listening on ws://127.0.0.1:12351/5eef0ebc-371a-44f4-8942-160726a5098c
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12346/c9f618ea-5a98-4ed9-8f25-537cd224c77f
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12356/d2542f21-112e-4395-bb06-8ba787a4a81b
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://0.0.0.0:12361/27d2d955-10ed-403e-96a0-ab8a70e37854
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12366/9e04f778-e93b-485d-8f60-bb8ca27293f3
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://[::]:12371/36d6fbd9-6303-4cd8-b69a-e257f3107427
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://[::1]:12376/735f05e2-fc2f-410d-968d-1081e6c9ab80
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12381/95f1ea98-4e0e-46f3-923a-bf38df96e155
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12386/32c41258-eeee-487a-89b0-467a1281f39c
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12391/c34c5dbe-3543-49f7-8d73-d217fa8cf7fe
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12396/65309967-e8de-4129-a32f-7f646c5895c4
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12401/1a5967f1-cade-4b4f-acd5-3e6b84bcafcd
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12406/bd170864-b6fb-424f-9a27-589bf939d490
    For help see https://nodejs.org/en/docs/inspector
    assert.js:42
      throw new errors.AssertionError({
      ^
    
    AssertionError [ERR_ASSERTION]: 12 === 0
        at checkExitCode (/home/iojs/build/workspace/node-test-commit-linux/nodes/debian8-64/test/sequential/test-inspector-port-cluster.js:329:10)
        at ChildProcess.childProcess.fork.on.common.mustCall (/home/iojs/build/workspace/node-test-commit-linux/nodes/debian8-64/test/sequential/test-inspector-port-cluster.js:322:7)
        at ChildProcess.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux/nodes/debian8-64/test/common/index.js:533:15)
        at emitTwo (events.js:126:13)
        at ChildProcess.emit (events.js:214:7)
        at Process.ChildProcess._handle.onexit (internal/child_process.js:198:12)
    Debugger listening on ws://127.0.0.1:12411/65c92ec5-9cfe-441c-96af-3f8f76338aa1
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12416/ee1d8bd3-f92b-4db1-93a1-6e57d487520a
    For help see https://nodejs.org/en/docs/inspector
    Debugger listening on ws://127.0.0.1:12426/8b40b2bf-b727-4d29-ba1f-6a271c66ab15
    For help see https://nodejs.org/en/docs/inspector
  ...

@BridgeAR
Copy link
Member

BridgeAR commented Feb 1, 2018

@gibfahn it is a flake. It comes up often at the moment.

@gibfahn
Copy link
Member

gibfahn commented Feb 18, 2018

gibfahn pushed a commit that referenced this pull request Feb 19, 2018
The motivation for this commit is that we need to specify system CA
certificates when building node. While we are aware of the environment
variable NODE_EXTRA_CA_CERTS this is not a great solution as we build
an RPM and we also don't want users to be able to unset them.

The suggestion is to add a configure time property like this:

--openssl-system-ca-path=OPENSSL_SYSTEM_CA_PATH
             Use the specified path to system CA (PEM format) in
             addition to the OpenSSL supplied CA store or compiled-
             in Mozilla CA copy.

Usage example:
$ ./configure --openssl-system-ca-path=/etc/pki/tls/certs/ca-bundle.crt

This would add the specified CA certificates in addition to the ones
already being used.

PR-URL: #16790
Backport-PR-URL: #18174
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@gibfahn
Copy link
Member

gibfahn commented Feb 19, 2018

Landed in 8a000e8

@gibfahn gibfahn closed this Feb 19, 2018
@danbev danbev deleted the backport-16790-to-v8.x branch March 27, 2018 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. c++ Issues and PRs that require attention from people who are familiar with C++. crypto Issues and PRs related to the crypto subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants