From cc1d02d2f7ce4f295bfe3b835d161a363f09d9db Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Wed, 21 Dec 2016 10:16:19 -0800 Subject: [PATCH 1/6] test: getgroups() may contain duplicate GIDs Some systems may have multiple group names with the same group ID, in which case getgroups() returns duplicate values, where `id -G` will filter the duplicates. Unique and sort the arrays so they can be compared. PR-URL: https://github.com/nodejs/node/pull/10389 Reviewed-By: James M Snell Reviewed-By: Gibson Fahnestock Reviewed-By: Michael Dawson --- test/parallel/test-process-getgroups.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-process-getgroups.js b/test/parallel/test-process-getgroups.js index 497da2be1235f8..390f188cf5bf4d 100644 --- a/test/parallel/test-process-getgroups.js +++ b/test/parallel/test-process-getgroups.js @@ -1,21 +1,23 @@ 'use strict'; const common = require('../common'); -const assert = require('assert'); -const exec = require('child_process').exec; + +// Check `id -G` and `process.getgroups()` return same groups. if (common.isOSX) { common.skip('Output of `id -G` is unreliable on Darwin.'); return; } +const assert = require('assert'); +const exec = require('child_process').exec; if (typeof process.getgroups === 'function') { - const groups = process.getgroups(); + const groups = unique(process.getgroups()); assert(Array.isArray(groups)); assert(groups.length > 0); exec('id -G', function(err, stdout) { - if (err) throw err; - const real_groups = stdout.match(/\d+/g).map(Number); - assert.strictEqual(groups.length, real_groups.length); + assert.ifError(err); + const real_groups = unique(stdout.match(/\d+/g).map(Number)); + assert.deepStrictEqual(groups, real_groups); check(groups, real_groups); check(real_groups, groups); }); @@ -24,3 +26,7 @@ if (typeof process.getgroups === 'function') { function check(a, b) { for (let i = 0; i < a.length; ++i) assert.notStrictEqual(b.indexOf(a[i]), -1); } + +function unique(groups) { + return [...new Set(groups)].sort(); +} From 05039c331f93ba33796516aa92b5dc4e412a0479 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Wed, 21 Dec 2016 09:33:13 -0800 Subject: [PATCH 2/6] doc,test: tls .ca option supports multi-PEM files PR-URL: https://github.com/nodejs/node/pull/10389 Reviewed-By: James M Snell Reviewed-By: Gibson Fahnestock Reviewed-By: Michael Dawson --- doc/api/tls.md | 19 +++++++++++++++---- test/parallel/test-tls-ca-concat.js | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-tls-ca-concat.js diff --git a/doc/api/tls.md b/doc/api/tls.md index 5b0cef22f02641..ee3d37d6a43df2 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -906,10 +906,21 @@ added: v0.11.13 the same order as their private keys in `key`. If the intermediate certificates are not provided, the peer will not be able to validate the certificate, and the handshake will fail. - * `ca`{string|string[]|Buffer|Buffer[]} Optional CA certificates to trust. - Default is the well-known CAs from Mozilla. When connecting to peers that - use certificates issued privately, or self-signed, the private root CA or - self-signed certificate must be provided to verify the peer. + * `ca` {string|string[]|Buffer|Buffer[]} Optionally override the trusted CA + certificates. Default is to trust the well-known CAs curated by Mozilla. + Mozilla's CAs are completely replaced when CAs are explicitly specified + using this option. The value can be a string or Buffer, or an Array of + strings and/or Buffers. Any string or Buffer can contain multiple PEM CAs + concatenated together. The peer's certificate must be chainable to a CA + trusted by the server for the connection to be authenticated. When using + certificates that are not chainable to a well-known CA, the certificate's CA + must be explicitly specified as a trusted or the connection will fail to + authenticate. + If the peer uses a certificate that doesn't match or chain to one of the + default CAs, use the `ca` option to provide a CA certificate that the peer's + certificate can match or chain to. + For self-signed certificates, the certificate is its own CA, and must be + provided. * `crl` {string|string[]|Buffer|Buffer[]} Optional PEM formatted CRLs (Certificate Revocation Lists). * `ciphers` {string} Optional cipher suite specification, replacing the diff --git a/test/parallel/test-tls-ca-concat.js b/test/parallel/test-tls-ca-concat.js new file mode 100644 index 00000000000000..65c837bed9dc37 --- /dev/null +++ b/test/parallel/test-tls-ca-concat.js @@ -0,0 +1,24 @@ +'use strict'; +const common = require('../common'); + +// Check ca option can contain concatenated certs by prepending an unrelated +// non-CA cert and showing that agent6's CA root is still found. + +const join = require('path').join; +const { + assert, connect, keys +} = require(join(common.fixturesDir, 'tls-connect'))(); + +connect({ + client: { + checkServerIdentity: (servername, cert) => { }, + ca: keys.agent1.cert + '\n' + keys.agent6.ca, + }, + server: { + cert: keys.agent6.cert, + key: keys.agent6.key, + }, +}, function(err, pair, cleanup) { + assert.ifError(err); + return cleanup(); +}); From d59c9f5dc3b4bf96f7886f531309136af20618c5 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Tue, 20 Dec 2016 14:16:29 -0800 Subject: [PATCH 3/6] test: tls cert chain completion scenarios PR-URL: https://github.com/nodejs/node/pull/10389 Reviewed-By: James M Snell Reviewed-By: Gibson Fahnestock Reviewed-By: Michael Dawson --- test/parallel/test-tls-cert-chains-concat.js | 50 ++++++++++++++++++++ test/parallel/test-tls-cert-chains-in-ca.js | 46 ++++++++++++++++++ 2 files changed, 96 insertions(+) create mode 100644 test/parallel/test-tls-cert-chains-concat.js create mode 100644 test/parallel/test-tls-cert-chains-in-ca.js diff --git a/test/parallel/test-tls-cert-chains-concat.js b/test/parallel/test-tls-cert-chains-concat.js new file mode 100644 index 00000000000000..d53edef89842b9 --- /dev/null +++ b/test/parallel/test-tls-cert-chains-concat.js @@ -0,0 +1,50 @@ +'use strict'; +const common = require('../common'); + +// Check cert chain is received by client, and is completed with the ca cert +// known to the client. + +const join = require('path').join; +const { + assert, connect, debug, keys +} = require(join(common.fixturesDir, 'tls-connect'))(); + +// agent6-cert.pem includes cert for agent6 and ca3 +connect({ + client: { + checkServerIdentity: (servername, cert) => { }, + ca: keys.agent6.ca, + }, + server: { + cert: keys.agent6.cert, + key: keys.agent6.key, + }, +}, function(err, pair, cleanup) { + assert.ifError(err); + + const peer = pair.client.conn.getPeerCertificate(); + debug('peer:\n', peer); + assert.strictEqual(peer.subject.emailAddress, 'adam.lippai@tresorit.com'); + assert.strictEqual(peer.subject.CN, 'Ádám Lippai'), + assert.strictEqual(peer.issuer.CN, 'ca3'); + assert.strictEqual(peer.serialNumber, 'C4CD893EF9A75DCC'); + + const next = pair.client.conn.getPeerCertificate(true).issuerCertificate; + const root = next.issuerCertificate; + delete next.issuerCertificate; + debug('next:\n', next); + assert.strictEqual(next.subject.CN, 'ca3'); + assert.strictEqual(next.issuer.CN, 'ca1'); + assert.strictEqual(next.serialNumber, '9A84ABCFB8A72ABF'); + + debug('root:\n', root); + assert.strictEqual(root.subject.CN, 'ca1'); + assert.strictEqual(root.issuer.CN, 'ca1'); + assert.strictEqual(root.serialNumber, '8DF21C01468AF393'); + + // No client cert, so empty object returned. + assert.deepStrictEqual(pair.server.conn.getPeerCertificate(), {}); + assert.deepStrictEqual(pair.server.conn.getPeerCertificate(true), {}); + + return cleanup(); +}); diff --git a/test/parallel/test-tls-cert-chains-in-ca.js b/test/parallel/test-tls-cert-chains-in-ca.js new file mode 100644 index 00000000000000..69f62c3f72d859 --- /dev/null +++ b/test/parallel/test-tls-cert-chains-in-ca.js @@ -0,0 +1,46 @@ +'use strict'; +const common = require('../common'); + +// Check cert chain is received by client, and is completed with the ca cert +// known to the client. + +const join = require('path').join; +const { + assert, connect, debug, keys +} = require(join(common.fixturesDir, 'tls-connect'))(); + + +// agent6-cert.pem includes cert for agent6 and ca3, split it apart and +// provide ca3 in the .ca property. +const agent6Chain = keys.agent6.cert.split('-----END CERTIFICATE-----') + .map((c) => { return c + '-----END CERTIFICATE-----'; }); +const agent6End = agent6Chain[0]; +const agent6Middle = agent6Chain[1]; +connect({ + client: { + checkServerIdentity: (servername, cert) => { }, + ca: keys.agent6.ca, + }, + server: { + cert: agent6End, + key: keys.agent6.key, + ca: agent6Middle, + }, +}, function(err, pair, cleanup) { + assert.ifError(err); + + const peer = pair.client.conn.getPeerCertificate(); + debug('peer:\n', peer); + assert.strictEqual(peer.serialNumber, 'C4CD893EF9A75DCC'); + + const next = pair.client.conn.getPeerCertificate(true).issuerCertificate; + const root = next.issuerCertificate; + delete next.issuerCertificate; + debug('next:\n', next); + assert.strictEqual(next.serialNumber, '9A84ABCFB8A72ABF'); + + debug('root:\n', root); + assert.strictEqual(root.serialNumber, '8DF21C01468AF393'); + + return cleanup(); +}); From 7c6aec677d9fb525d8a2b5d62aea96462f9ba069 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Tue, 20 Dec 2016 14:16:18 -0800 Subject: [PATCH 4/6] doc: use correct tls certificate property name Docs referred to an `issuer` property being optionally present, when it should have referred to the `issuerCertificate` property. PR-URL: https://github.com/nodejs/node/pull/10389 Reviewed-By: James M Snell Reviewed-By: Gibson Fahnestock Reviewed-By: Michael Dawson --- doc/api/tls.md | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/doc/api/tls.md b/doc/api/tls.md index ee3d37d6a43df2..0e47ddac0391be 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -583,13 +583,16 @@ For Example: `{ type: 'ECDH', name: 'prime256v1', size: 256 }` added: v0.11.4 --> -* `detailed` {boolean} Specify `true` to request that the full certificate - chain with the `issuer` property be returned; `false` to return only the - top certificate without the `issuer` property. +* `detailed` {boolean} Include the full certificate chain if `true`, otherwise + include just the peer's certificate. Returns an object representing the peer's certificate. The returned object has some properties corresponding to the fields of the certificate. +If the full certificate chain was requested, each certificate will include a +`issuerCertificate` property containing an object representing its issuer's +certificate. + For example: ```text @@ -600,15 +603,15 @@ For example: O: 'node.js', OU: 'Test TLS Certificate', CN: 'localhost' }, - issuerInfo: + issuer: { C: 'UK', ST: 'Acknack Ltd', L: 'Rhys Jones', O: 'node.js', OU: 'Test TLS Certificate', CN: 'localhost' }, - issuer: - { ... another certificate ... }, + issuerCertificate: + { ... another certificate, possibly with a .issuerCertificate ... }, raw: < RAW DER buffer >, valid_from: 'Nov 11 09:52:22 2009 GMT', valid_to: 'Nov 6 09:52:22 2029 GMT', @@ -616,8 +619,7 @@ For example: serialNumber: 'B9B0D332A1AA5635' } ``` -If the peer does not provide a certificate, `null` or an empty object will be -returned. +If the peer does not provide a certificate, an empty object will be returned. ### tlsSocket.getProtocol()