tls: type checking for key, cert and ca options

PR-URL: https://github.com/nodejs/node/pull/14807
Fixes: https://github.com/nodejs/node/issues/12802
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>

lib/_tls_common.js

@@ -22,6 +22,7 @@
 'use strict';
 
 const tls = require('tls');
+const errors = require('internal/errors');
 
 const SSL_OP_CIPHER_SERVER_PREFERENCE =
     process.binding('constants').crypto.SSL_OP_CIPHER_SERVER_PREFERENCE;
@@ -52,6 +53,14 @@ function SecureContext(secureProtocol, secureOptions, context) {
   if (secureOptions) this.context.setOptions(secureOptions);
 }
 
+function validateKeyCert(value, type) {
+  if (typeof value !== 'string' && !ArrayBuffer.isView(value))
+    throw new errors.TypeError(
+      'ERR_INVALID_ARG_TYPE', type,
+      ['string', 'Buffer', 'TypedArray', 'DataView']
+    );
+}
+
 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++) {
+      options.ca.forEach((ca) => {
-        c.context.addCACert(options.ca[i]);
+        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++)
+      options.cert.forEach((cert) => {
-        c.context.setCert(options.cert[i]);
+        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++) {
+      options.key.forEach((k) => {
-        const key = options.key[i];
+        validateKeyCert(k.pem || k, 'key');
-        const passphrase = key.passphrase || options.passphrase;
+        c.context.setKey(k.pem || k, k.passphrase || options.passphrase);
-        c.context.setKey(key.pem || key, passphrase);
+      });
-      }
     } else {
+      validateKeyCert(options.key, 'key');
       c.context.setKey(options.key, options.passphrase);
     }
   }

test/parallel/test-https-options-boolean-check.js

@@ -0,0 +1,156 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const https = require('https');
+
+function toArrayBuffer(buf) {
+  const ab = new ArrayBuffer(buf.length);
+  const view = new Uint8Array(ab);
+  return buf.map((b, i) => view[i] = b);
+}
+
+function toDataView(buf) {
+  const ab = new ArrayBuffer(buf.length);
+  const view = new DataView(ab);
+  return buf.map((b, i) => view[i] = b);
+}
+
+const keyBuff = fixtures.readKey('agent1-key.pem');
+const certBuff = fixtures.readKey('agent1-cert.pem');
+const keyBuff2 = fixtures.readKey('ec-key.pem');
+const certBuff2 = fixtures.readKey('ec-cert.pem');
+const caCert = fixtures.readKey('ca1-cert.pem');
+const caCert2 = fixtures.readKey('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 keyArrBuff = toArrayBuffer(keyBuff);
+const certArrBuff = toArrayBuffer(certBuff);
+const caArrBuff = toArrayBuffer(caCert);
+const keyDataView = toDataView(keyBuff);
+const certDataView = toDataView(certBuff);
+const caArrDataView = toDataView(caCert);
+const invalidKeyRE = /^The "key" argument must be one of type string, Buffer, TypedArray, or DataView$/;
+const invalidCertRE = /^The "cert" argument must be one of type string, Buffer, TypedArray, or DataView$/;
+
+// 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],
+  [keyArrBuff, certArrBuff],
+  [keyArrBuff, false],
+  [false, certArrBuff],
+  [keyDataView, certDataView],
+  [keyDataView, false],
+  [false, certDataView],
+  [[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]
+    });
+  });
+});
+
+// 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, certArrBuff, invalidKeyRE],
+  [keyArrBuff, true, invalidCertRE],
+  [true, certDataView, invalidKeyRE],
+  [keyDataView, 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]
+  }));
+});
+
+// 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, caArrBuff],
+  [keyBuff, certBuff, caArrDataView],
+  [keyBuff, certBuff, false],
+].map((params) => {
+  assert.doesNotThrow(() => {
+    https.createServer({
+      key: params[0],
+      cert: params[1],
+      ca: params[2]
+    });
+  });
+});
+
+// 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, Buffer, TypedArray, or DataView$/
+  }));
+});

test/parallel/test-tls-options-boolean-check.js

@@ -0,0 +1,156 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+const assert = require('assert');
+const tls = require('tls');
+
+function toArrayBuffer(buf) {
+  const ab = new ArrayBuffer(buf.length);
+  const view = new Uint8Array(ab);
+  return buf.map((b, i) => view[i] = b);
+}
+
+function toDataView(buf) {
+  const ab = new ArrayBuffer(buf.length);
+  const view = new DataView(ab);
+  return buf.map((b, i) => view[i] = b);
+}
+
+const keyBuff = fixtures.readKey('agent1-key.pem');
+const certBuff = fixtures.readKey('agent1-cert.pem');
+const keyBuff2 = fixtures.readKey('ec-key.pem');
+const certBuff2 = fixtures.readKey('ec-cert.pem');
+const caCert = fixtures.readKey('ca1-cert.pem');
+const caCert2 = fixtures.readKey('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 keyArrBuff = toArrayBuffer(keyBuff);
+const certArrBuff = toArrayBuffer(certBuff);
+const caArrBuff = toArrayBuffer(caCert);
+const keyDataView = toDataView(keyBuff);
+const certDataView = toDataView(certBuff);
+const caArrDataView = toDataView(caCert);
+const invalidKeyRE = /^The "key" argument must be one of type string, Buffer, TypedArray, or DataView$/;
+const invalidCertRE = /^The "cert" argument must be one of type string, Buffer, TypedArray, or DataView$/;
+
+// 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],
+  [keyArrBuff, certArrBuff],
+  [keyArrBuff, false],
+  [false, certArrBuff],
+  [keyDataView, certDataView],
+  [keyDataView, false],
+  [false, certDataView],
+  [[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, certArrBuff, invalidKeyRE],
+  [keyArrBuff, true, invalidCertRE],
+  [true, certDataView, invalidKeyRE],
+  [keyDataView, 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, caArrBuff],
+  [keyBuff, certBuff, caArrDataView],
+  [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, Buffer, TypedArray, or DataView$/
+  }));
+});