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

src: add openssl-system-ca-path configure option #16790

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Nov 6, 2017

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.

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

src, build

@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. labels Nov 6, 2017
@danbev
Copy link
Contributor Author

danbev commented Nov 6, 2017

@danbev
Copy link
Contributor Author

danbev commented Nov 6, 2017

cc @nodejs/crypto

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM modulo comments.

I'm not thrilled by yet another way to override the CA store but that in itself is not a reason to reject it.

I had similar misgivings about the original override because I knew it would only be a matter of time before people would request alternatives - and when you say yes to one, you cannot reasonably say no to the next one.

configure Outdated
@@ -1013,6 +1019,9 @@ def configure_openssl(o):
o['variables']['openssl_no_asm'] = 1 if options.openssl_no_asm else 0
if options.use_openssl_ca_store:
o['defines'] += ['NODE_OPENSSL_CERT_STORE']
if options.openssl_system_ca_path:
o['defines'] += ['NODE_OPENSSL_SYSTEM_CERT_PATH '
+ str(options.openssl_system_ca_path)]
Copy link
Member

Choose a reason for hiding this comment

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

NODE_OPENSSL_SYSTEM_CERT_PATH=... instead of NODE_OPENSSL_SYSTEM_CERT_PATH ...?

Operator should go on the first line.

Isn't options.openssl_system_ca_path already a string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry it is, this was a left over from when I use the append action instead of store. I'll fix this.

NODE_STRINGIFY(NODE_OPENSSL_SYSTEM_CERT_PATH);
#else
nullptr;
#endif
Copy link
Member

Choose a reason for hiding this comment

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

static const char system_cert_path[] = ...; ("" when undefined) and I'd prefer defining it in both arms, easier to read.

NODE_STRINGIFY() isn't necessary, or is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll define the in both cases and use "" when undefined.

I do think the NODE_STRINGIFY call is needed to get the value (the path) but if there are better ways of doing this please let me know. Thanks

@@ -792,6 +799,9 @@ static X509_STORE* NewRootCertStore() {
}

X509_STORE* store = X509_STORE_new();
if (system_cert_path != nullptr) {
X509_STORE_load_locations(store, system_cert_path, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

nullptr

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix this

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM once @bnoordhuis' comments are addressed...

@danbev
Copy link
Contributor Author

danbev commented Nov 6, 2017

@danbev
Copy link
Contributor Author

danbev commented Nov 6, 2017

I'm not thrilled by yet another way to override the CA store but that in itself is not a reason to reject it.

I can understand this and just wanted to say that I appreciate this getting in as it does make things easier for us. Until now patches were applied to do this and maintaining them is something I'd rather avoid.

configure Outdated
@@ -1013,6 +1019,9 @@ def configure_openssl(o):
o['variables']['openssl_no_asm'] = 1 if options.openssl_no_asm else 0
if options.use_openssl_ca_store:
o['defines'] += ['NODE_OPENSSL_CERT_STORE']
if options.openssl_system_ca_path:
o['defines'] += ['NODE_OPENSSL_SYSTEM_CERT_PATH='
+ options.openssl_system_ca_path]
Copy link
Member

Choose a reason for hiding this comment

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

Operator on first line. :-)

I think you need the NODE_STRINGIFY macro because you pass a plain string but it should work without it if you wrap the string in quotes:

o['defines'] += ['NODE_OPENSSL_SYSTEM_CERT_PATH="{}"'.format(options.openssl_system_ca_path)]

If you set it unconditionally, you don't need the #if defined(...) in node_crypto.cc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the suggestion and it works perfectly with for node_crypto.cc. I do run into an issue with test-process-config.js though:

$ out/Release/node test/parallel/test-process-config.js
undefined:3
                       "defines": [ "NODE_OPENSSL_SYSTEM_CERT_PATH="/Users/danielbevenius/work/bucharest-gold/node-rpm/rpms/ca-bundle.crt""],
                                                                    ^

SyntaxError: Unexpected token / in JSON at position 164
    at JSON.parse (<anonymous>)
    at Object.<anonymous> (/Users/danielbevenius/work/nodejs/node/test/parallel/test-process-config.js:49:15)
    at Module._compile (module.js:641:30)
    at Object.Module._extensions..js (module.js:652:10)
    at Module.load (module.js:560:32)
    at tryModuleLoad (module.js:503:12)
    at Function.Module._load (module.js:495:3)
    at Function.Module.runMain (module.js:682:10)
    at startup (bootstrap_node.js:191:16)
    at bootstrap_node.js:613:3

I've tried all possible escaping combinations that I can think of in configure but they all seem to break the C++ code.

I'll dig a little further but in the meantime I'll update with the operator on the correct line and also unconditionally set NODE_OPENSSL_SYSTEM_CERT_PATH and remove the #if defined in node_crypto.cc

Copy link
Member

Choose a reason for hiding this comment

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

That's just a bug in the test. It pretends that config.gyp is JSON (and tries to parse it as such) when it's not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh great, I'll see if I can come up with a fix for it. Thanks again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've not found a good way of working/fixing the test. What I've suggested in f8980cd is to skip the test if it cannot be parsed into valid JSON. Is this acceptable?

Copy link
Member

Choose a reason for hiding this comment

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

Would this work?

diff --git a/test/parallel/test-process-config.js b/test/parallel/test-process-config.js
index 05616480e0..05f4ca915a 100644
--- a/test/parallel/test-process-config.js
+++ b/test/parallel/test-process-config.js
@@ -45,7 +45,9 @@ if (!fs.existsSync(configPath)) {
 let config = fs.readFileSync(configPath, 'utf8');
 
 // Clean up comment at the first line.
-config = config.split('\n').slice(1).join('\n').replace(/'/g, '"');
+config = config.split('\n').slice(1).join('\n');
+config = config.replace(/"/g, '\\"');
+config = config.replace(/'/g, '"');
 config = JSON.parse(config, function(key, value) {
   if (value === 'true') return true;
   if (value === 'false') return false;

Aside: looking at this again, I realize this sets the define build-wide, not just for files in src/.

NODE_OPENSSL_CERT_STORE does that too so it's okay for now but it's something we should fix sometime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that worked like a charm.

Aside: looking at this again, I realize this sets the define build-wide, not just for files in src/.

Could you tell me or point me to an example of how I can limit them to only files in src?

Copy link
Member

Choose a reason for hiding this comment

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

I couldn't find a proper example but the basic idea is that you restrict the define to a specific target:

diff --git a/node.gyp b/node.gyp
index 1c3e291471..6ca77a0c19 100644
--- a/node.gyp
+++ b/node.gyp
@@ -304,6 +304,14 @@
         # Warn when using deprecated V8 APIs.
         'V8_DEPRECATION_WARNINGS=1',
       ],
+
+      'conditions': [
+        [ 'openssl_system_ca_path!=""', {
+          'defines': [
+            'NODE_OPENSSL_SYSTEM_CERT_PATH="<(openssl_system_ca_path)"',
+          ],
+        }],
+      ],
     },
     {
       'target_name': 'mkssldef',

You can't see it but that's the 'target_name': '<(node_core_target_name)' target.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah I see, great. Thanks!

I've updated inspired by your patch above. One thing I've done is introduced a variable to the above target and setting it to an empty string if not set so that we always set NODE_OPENSSL_SYSTEM_CERT_PATH. Let me know what you think?

@@ -792,6 +799,9 @@ static X509_STORE* NewRootCertStore() {
}

X509_STORE* store = X509_STORE_new();
if (strlen(system_cert_path) != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Not wrong but I'd write this as if (*system_cert_path != '\0') {.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change this. Thanks

],

'direct_dependent_settings': {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section should really not be required but is because the moment as src/node_crypto.cc is included in the sources for the cctest target (though not explicitly but through node.gypi). I'm working on a PR for this so that no sources other than the ones listed in cctest are compiled.

Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

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.
@danbev
Copy link
Contributor Author

danbev commented Nov 9, 2017

Rebased and squashed CI: https://ci.nodejs.org/job/node-test-pull-request/11319/

configure Outdated
@@ -172,6 +172,12 @@ parser.add_option('--openssl-use-def-ca-store',
dest='use_openssl_ca_store',
help='Use OpenSSL supplied CA store instead of compiled-in Mozilla CA copy.')

parser.add_option('--openssl-system-ca-path',
action="store",
Copy link
Member

Choose a reason for hiding this comment

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

Could you use action='store' (single quotes) here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problems, I'll update that. Thanks

@danbev
Copy link
Contributor Author

danbev commented Nov 10, 2017

Landed in cad1d1f

@danbev danbev closed this Nov 10, 2017
@danbev danbev deleted the system-certs branch November 10, 2017 05:26
danbev added a commit to danbev/node that referenced this pull request Nov 10, 2017
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>
evanlucas pushed a commit that referenced this pull request Nov 13, 2017
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>
@evanlucas evanlucas mentioned this pull request Nov 13, 2017
@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 17, 2017

This lands cleanly on both v6.x and v8.x but I'd like to see it in a release a bit longer before backporting

Is there a reason this landed as semver-patch rather than semver-minor?

edit: also if there is no reason to hold off before LTS please lmk

@danbev
Copy link
Contributor Author

danbev commented Nov 21, 2017

Is there a reason this landed as semver-patch rather than semver-minor?

I forgot to consider the semver of this before landing I'm afraid, and this should have been marked semver-minor I think.

@MylesBorins MylesBorins added the semver-minor PRs that contain new features and should be released in the next minor version. label Nov 21, 2017
@MylesBorins
Copy link
Contributor

@danbev we'd like to land this on v6.x and 8.x but it is no longer landing cleanly can you please backport

danbev added a commit to danbev/node that referenced this pull request 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: 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>
MylesBorins pushed a commit that referenced this pull request Jan 18, 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.

Backport-PR-URL: #18173
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>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
MylesBorins pushed a commit that referenced this pull request Feb 11, 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.

Backport-PR-URL: #18173
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>
MylesBorins added a commit that referenced this pull request Feb 11, 2018
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins pushed a commit that referenced this pull request Feb 12, 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.

Backport-PR-URL: #18173
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>
MylesBorins added a commit that referenced this pull request Feb 12, 2018
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins pushed a commit that referenced this pull request Feb 13, 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.

Backport-PR-URL: #18173
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>
MylesBorins added a commit that referenced this pull request Feb 13, 2018
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins added a commit that referenced this pull request Feb 13, 2018
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins added a commit that referenced this pull request Feb 13, 2018
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
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>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    nodejs#12678
* crypto:
  - expose ECDH class (Bryan English)
    nodejs#8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    nodejs#10209
  - warn on invalid authentication tag length (Tobias Nießen)
    nodejs#17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    nodejs#16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    nodejs#7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    nodejs#13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    nodejs#13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    nodejs#16386
* net:
  - return this from getConnections() (Sam Roberts)
    nodejs#13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    nodejs#13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    nodejs#14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    nodejs#16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    nodejs#12087
  - add process.ppid (cjihrig)
    nodejs#16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    nodejs#12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    nodejs#15179
* url:
  - WHATWG URL api support (James M Snell)
    nodejs#7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    nodejs#10308

PR-URL: nodejs#18342
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. 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.

9 participants