diff --git a/lib/_tls_common.js b/lib/_tls_common.js index 36b2ebdad68d0b..c9616e4ca1e01f 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -22,6 +22,8 @@ 'use strict'; const tls = require('tls'); +const Buffer = require('buffer').Buffer; +const errors = require('internal/errors'); const SSL_OP_CIPHER_SERVER_PREFERENCE = process.binding('constants').crypto.SSL_OP_CIPHER_SERVER_PREFERENCE; @@ -52,6 +54,13 @@ function SecureContext(secureProtocol, secureOptions, context) { if (secureOptions) this.context.setOptions(secureOptions); } +function validateKeyCert(value, type) { + if (typeof value !== 'string' && !(value instanceof Buffer)) + throw new errors.TypeError( + 'ERR_INVALID_ARG_TYPE', type, ['string', 'buffer'] + ); +} + exports.SecureContext = SecureContext; @@ -71,10 +80,12 @@ exports.createSecureContext = function createSecureContext(options, context) { // cert's issuer in C++ code. if (options.ca) { if (Array.isArray(options.ca)) { - for (i = 0; i < options.ca.length; i++) { - c.context.addCACert(options.ca[i]); - } + options.ca.map((ca) => { + validateKeyCert(ca, 'ca'); + c.context.addCACert(ca); + }); } else { + validateKeyCert(options.ca, 'ca'); c.context.addCACert(options.ca); } } else { @@ -83,9 +94,12 @@ exports.createSecureContext = function createSecureContext(options, context) { if (options.cert) { if (Array.isArray(options.cert)) { - for (i = 0; i < options.cert.length; i++) - c.context.setCert(options.cert[i]); + options.cert.map((cert) => { + validateKeyCert(cert, 'cert'); + c.context.setCert(cert); + }); } else { + validateKeyCert(options.cert, 'cert'); c.context.setCert(options.cert); } } @@ -96,12 +110,12 @@ exports.createSecureContext = function createSecureContext(options, context) { // which leads to the crash later on. if (options.key) { if (Array.isArray(options.key)) { - for (i = 0; i < options.key.length; i++) { - const key = options.key[i]; - const passphrase = key.passphrase || options.passphrase; - c.context.setKey(key.pem || key, passphrase); - } + options.key.map((k) => { + validateKeyCert(k.pem || k, 'key'); + c.context.setKey(k.pem || k, k.passphrase || options.passphrase); + }); } else { + validateKeyCert(options.key, 'key'); c.context.setKey(options.key, options.passphrase); } } diff --git a/lib/https.js b/lib/https.js index 38ae2fa9b93578..fb220872598315 100644 --- a/lib/https.js +++ b/lib/https.js @@ -40,12 +40,6 @@ function Server(opts, requestListener) { } opts = util._extend({}, opts); - if (opts && typeof opts.key === 'boolean') - throw new Error('"options.key" must not be a boolean'); - - if (opts && typeof opts.cert === 'boolean') - throw new Error('"options.cert" must not be a boolean'); - if (process.features.tls_npn && !opts.NPNProtocols) { opts.NPNProtocols = ['http/1.1', 'http/1.0']; } diff --git a/test/parallel/test-https-options-boolean-check.js b/test/parallel/test-https-options-boolean-check.js index 123b4635b9aa82..0415f500f2a5ff 100644 --- a/test/parallel/test-https-options-boolean-check.js +++ b/test/parallel/test-https-options-boolean-check.js @@ -1,25 +1,5 @@ -// Copyright Joyent, Inc. and other Node contributors. -// -// Permission is hereby granted, free of charge, to any person obtaining a -// copy of this software and associated documentation files (the -// "Software"), to deal in the Software without restriction, including -// without limitation the rights to use, copy, modify, merge, publish, -// distribute, sublicense, and/or sell copies of the Software, and to permit -// persons to whom the Software is furnished to do so, subject to the -// following conditions: -// -// The above copyright notice and this permission notice shall be included -// in all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS -// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF -// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN -// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, -// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE -// USE OR OTHER DEALINGS IN THE SOFTWARE. - 'use strict'; + const common = require('../common'); if (!common.hasCrypto) @@ -29,56 +9,118 @@ const assert = require('assert'); const https = require('https'); const fs = require('fs'); -assert.doesNotThrow(() => - https.createServer({ - key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`), - cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`) - })); - -assert.throws(() => - https.createServer({ - key: true, - cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`) - }), /"options\.key" must not be a boolean/); - -assert.throws(() => - https.createServer({ - key: false, - cert: fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`) - }), /"options\.key" must not be a boolean/); +const keyBuff = fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`); +const certBuff = fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`); +const keyBuff2 = fs.readFileSync(`${common.fixturesDir}/keys/ec-key.pem`); +const certBuff2 = fs.readFileSync(`${common.fixturesDir}/keys/ec-cert.pem`); +const caCert = fs.readFileSync(`${common.fixturesDir}/keys/ca1-cert.pem`); +const caCert2 = fs.readFileSync(`${common.fixturesDir}/keys/ca2-cert.pem`); +const keyStr = keyBuff.toString(); +const certStr = certBuff.toString(); +const keyStr2 = keyBuff2.toString(); +const certStr2 = certBuff2.toString(); +const caCertStr = caCert.toString(); +const caCertStr2 = caCert2.toString(); +const invalidKeyRE = /^The "key" argument must be one of type string or buffer$/; +const invalidCertRE = /^The "cert" argument must be one of type string or buffer$/; -assert.throws(() => - https.createServer({ - key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`), - cert: true - }), /"options\.cert" must not be a boolean/); +// Checks to ensure https.createServer doesn't throw an error +// Format ['key', 'cert'] +[ + [keyBuff, certBuff], + [false, certBuff], + [keyBuff, false], + [keyStr, certStr], + [false, certStr], + [keyStr, false], + [false, false], + [[keyBuff, keyBuff2], [certBuff, certBuff2]], + [[keyStr, keyStr2], [certStr, certStr2]], + [[keyStr, keyStr2], false], + [false, [certStr, certStr2]], + [[{ pem: keyBuff }], false], + [[{ pem: keyBuff }, { pem: keyBuff }], false] +].map((params) => { + assert.doesNotThrow(() => { + https.createServer({ + key: params[0], + cert: params[1] + }); + }); +}); -assert.throws(() => - https.createServer({ - key: fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`), - cert: false - }), /"options\.cert" must not be a boolean/); - -assert.throws(() => - https.createServer({ - key: false, - cert: false - }), /"options\.key" must not be a boolean/); - -assert.throws(() => - https.createServer({ - key: true, - cert: true - }), /"options\.key" must not be a boolean/); +// Checks to ensure https.createServer predictably throws an error +// Format ['key', 'cert', 'expected message'] +[ + [true, certBuff, invalidKeyRE], + [keyBuff, true, invalidCertRE], + [true, certStr, invalidKeyRE], + [keyStr, true, invalidCertRE], + [true, true, invalidCertRE], + [true, false, invalidKeyRE], + [false, true, invalidCertRE], + [true, false, invalidKeyRE], + [{ pem: keyBuff }, false, invalidKeyRE], + [false, { pem: keyBuff }, invalidCertRE], + [1, false, invalidKeyRE], + [false, 1, invalidCertRE], + [[keyBuff, true], [certBuff, certBuff2], invalidKeyRE], + [[true, keyStr2], [certStr, certStr2], invalidKeyRE], + [[keyBuff, keyBuff2], [true, certBuff2], invalidCertRE], + [[keyStr, keyStr2], [certStr, true], invalidCertRE], + [[true, false], [certBuff, certBuff2], invalidKeyRE], + [[keyStr, keyStr2], [true, false], invalidCertRE], + [[keyStr, keyStr2], true, invalidCertRE], + [true, [certBuff, certBuff2], invalidKeyRE] +].map((params) => { + assert.throws(() => { + https.createServer({ + key: params[0], + cert: params[1] + }); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: params[2] + })); +}); -assert.throws(() => - https.createServer({ - key: true, - cert: false - }), /"options\.key" must not be a boolean/); +// Checks to ensure https.createServer works with the CA parameter +// Format ['key', 'cert', 'ca'] +[ + [keyBuff, certBuff, caCert], + [keyBuff, certBuff, [caCert, caCert2]], + [keyBuff, certBuff, caCertStr], + [keyBuff, certBuff, [caCertStr, caCertStr2]], + [keyBuff, certBuff, false], +].map((params) => { + assert.doesNotThrow(() => { + https.createServer({ + key: params[0], + cert: params[1], + ca: params[2] + }); + }); +}); -assert.throws(() => - https.createServer({ - key: false, - cert: true - }), /"options\.key" must not be a boolean/); +// Checks to ensure https.createServer throws an error for CA assignment +// Format ['key', 'cert', 'ca'] +[ + [keyBuff, certBuff, true], + [keyBuff, certBuff, {}], + [keyBuff, certBuff, 1], + [keyBuff, certBuff, true], + [keyBuff, certBuff, [caCert, true]] +].map((params) => { + assert.throws(() => { + https.createServer({ + key: params[0], + cert: params[1], + ca: params[2] + }); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "ca" argument must be one of type string or buffer$/ + })); +}); diff --git a/test/parallel/test-tls-options-boolean-check.js b/test/parallel/test-tls-options-boolean-check.js new file mode 100644 index 00000000000000..3451d9ce7cc2a7 --- /dev/null +++ b/test/parallel/test-tls-options-boolean-check.js @@ -0,0 +1,126 @@ +'use strict'; + +const common = require('../common'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const tls = require('tls'); +const fs = require('fs'); + +const keyBuff = fs.readFileSync(`${common.fixturesDir}/keys/agent1-key.pem`); +const certBuff = fs.readFileSync(`${common.fixturesDir}/keys/agent1-cert.pem`); +const keyBuff2 = fs.readFileSync(`${common.fixturesDir}/keys/ec-key.pem`); +const certBuff2 = fs.readFileSync(`${common.fixturesDir}/keys/ec-cert.pem`); +const caCert = fs.readFileSync(`${common.fixturesDir}/keys/ca1-cert.pem`); +const caCert2 = fs.readFileSync(`${common.fixturesDir}/keys/ca2-cert.pem`); +const keyStr = keyBuff.toString(); +const certStr = certBuff.toString(); +const keyStr2 = keyBuff2.toString(); +const certStr2 = certBuff2.toString(); +const caCertStr = caCert.toString(); +const caCertStr2 = caCert2.toString(); +const invalidKeyRE = /^The "key" argument must be one of type string or buffer$/; +const invalidCertRE = /^The "cert" argument must be one of type string or buffer$/; + +// Checks to ensure tls.createServer doesn't throw an error +// Format ['key', 'cert'] +[ + [keyBuff, certBuff], + [false, certBuff], + [keyBuff, false], + [keyStr, certStr], + [false, certStr], + [keyStr, false], + [false, false], + [[keyBuff, keyBuff2], [certBuff, certBuff2]], + [[keyStr, keyStr2], [certStr, certStr2]], + [[keyStr, keyStr2], false], + [false, [certStr, certStr2]], + [[{ pem: keyBuff }], false], + [[{ pem: keyBuff }, { pem: keyBuff }], false] +].map((params) => { + assert.doesNotThrow(() => { + tls.createServer({ + key: params[0], + cert: params[1] + }); + }); +}); + +// Checks to ensure tls.createServer predictably throws an error +// Format ['key', 'cert', 'expected message'] +[ + [true, certBuff, invalidKeyRE], + [keyBuff, true, invalidCertRE], + [true, certStr, invalidKeyRE], + [keyStr, true, invalidCertRE], + [true, true, invalidCertRE], + [true, false, invalidKeyRE], + [false, true, invalidCertRE], + [true, false, invalidKeyRE], + [{ pem: keyBuff }, false, invalidKeyRE], + [false, { pem: keyBuff }, invalidCertRE], + [1, false, invalidKeyRE], + [false, 1, invalidCertRE], + [[keyBuff, true], [certBuff, certBuff2], invalidKeyRE], + [[true, keyStr2], [certStr, certStr2], invalidKeyRE], + [[keyBuff, keyBuff2], [true, certBuff2], invalidCertRE], + [[keyStr, keyStr2], [certStr, true], invalidCertRE], + [[true, false], [certBuff, certBuff2], invalidKeyRE], + [[keyStr, keyStr2], [true, false], invalidCertRE], + [[keyStr, keyStr2], true, invalidCertRE], + [true, [certBuff, certBuff2], invalidKeyRE] +].map((params) => { + assert.throws(() => { + tls.createServer({ + key: params[0], + cert: params[1] + }); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: params[2] + })); +}); + +// Checks to ensure tls.createServer works with the CA parameter +// Format ['key', 'cert', 'ca'] +[ + [keyBuff, certBuff, caCert], + [keyBuff, certBuff, [caCert, caCert2]], + [keyBuff, certBuff, caCertStr], + [keyBuff, certBuff, [caCertStr, caCertStr2]], + [keyBuff, certBuff, false], +].map((params) => { + assert.doesNotThrow(() => { + tls.createServer({ + key: params[0], + cert: params[1], + ca: params[2] + }); + }); +}); + +// Checks to ensure tls.createServer throws an error for CA assignment +// Format ['key', 'cert', 'ca'] +[ + [keyBuff, certBuff, true], + [keyBuff, certBuff, {}], + [keyBuff, certBuff, 1], + [keyBuff, certBuff, true], + [keyBuff, certBuff, [caCert, true]] +].map((params) => { + assert.throws(() => { + tls.createServer({ + key: params[0], + cert: params[1], + ca: params[2] + }); + }, common.expectsError({ + code: 'ERR_INVALID_ARG_TYPE', + type: TypeError, + message: /^The "ca" argument must be one of type string or buffer$/ + })); +});