tls: make rejectUnauthorized default to true

rejectUnauthorized used to be false when the property was undefined or null, quietly allowing client connections for which certificates have been requested (requestCert is true) even when the client certificate was not authorized (signed by a trusted CA). Change this so rejectUnauthorized is always true unless it is explicitly set to false. PR-URL: #5923 Reviewed-By: Sam Roberts <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
nodejs · Mar 23, 2017 · 348cc80 · 348cc80
1 parent ee19e29
commit 348cc80
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 21 deletions.
diff --git a/doc/api/tls.md b/doc/api/tls.md
@@ -712,7 +712,10 @@ added: v0.11.8
 -->
 
 * `options` {Object}
-  * `rejectUnauthorized` {boolean}
+  * `rejectUnauthorized` {boolean} If not `false`, the server certificate is verified
+    against the list of supplied CAs. An `'error'` event is emitted if
+    verification fails; `err.code` contains the OpenSSL error code. Defaults to
+    `true`.
   * `requestCert`
 * `callback` {Function} A function that will be called when the renegotiation
   request has been completed.
@@ -769,7 +772,7 @@ changes:
     connection/disconnection/destruction of `socket` is the user's
     responsibility, calling `tls.connect()` will not cause `net.connect()` to be
     called.
-  * `rejectUnauthorized` {boolean} If `true`, the server certificate is verified
+  * `rejectUnauthorized` {boolean} If not `false`, the server certificate is verified
     against the list of supplied CAs. An `'error'` event is emitted if
     verification fails; `err.code` contains the OpenSSL error code. Defaults to
     `true`.
@@ -1012,9 +1015,9 @@ changes:
   * `requestCert` {boolean} If `true` the server will request a certificate from
     clients that connect and attempt to verify that certificate. Defaults to
     `false`.
-  * `rejectUnauthorized` {boolean} If `true` the server will reject any
+  * `rejectUnauthorized` {boolean} If not `false` the server will reject any
     connection which is not authorized with the list of supplied CAs. This
-    option only has an effect if `requestCert` is `true`. Defaults to `false`.
+    option only has an effect if `requestCert` is `true`. Defaults to `true`.
   * `NPNProtocols` {string[]|Buffer} An array of strings or a `Buffer` naming
     possible NPN protocols. (Protocols should be ordered by their priority.)
   * `ALPNProtocols` {string[]|Buffer} An array of strings or a `Buffer` naming
@@ -1190,9 +1193,8 @@ changes:
   opened as a server.
 * `requestCert` {boolean} `true` to specify whether a server should request a
   certificate from a connecting client. Only applies when `isServer` is `true`.
-* `rejectUnauthorized` {boolean} `true` to specify whether a server should
-  automatically reject clients with invalid certificates. Only applies when
-  `isServer` is `true`.
+* `rejectUnauthorized` {boolean} If not `false` a server automatically reject clients
+   with invalid certificates. Only applies when `isServer` is `true`.
 * `options`
   * `secureContext`: An optional TLS context object from
      [`tls.createSecureContext()`][]

diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -920,17 +920,8 @@ Server.prototype.setTicketKeys = function setTicketKeys(keys) {
 
 
 Server.prototype.setOptions = function(options) {
-  if (typeof options.requestCert === 'boolean') {
-    this.requestCert = options.requestCert;
-  } else {
-    this.requestCert = false;
-  }
-
-  if (typeof options.rejectUnauthorized === 'boolean') {
-    this.rejectUnauthorized = options.rejectUnauthorized;
-  } else {
-    this.rejectUnauthorized = false;
-  }
+  this.requestCert = options.requestCert === true;
+  this.rejectUnauthorized = options.rejectUnauthorized !== false;
 
   if (options.pfx) this.pfx = options.pfx;
   if (options.key) this.key = options.key;
@@ -1062,7 +1053,7 @@ exports.connect = function(...args /* [port,] [host,] [options,] [cb] */) {
     secureContext: context,
     isServer: false,
     requestCert: true,
-    rejectUnauthorized: options.rejectUnauthorized,
+    rejectUnauthorized: options.rejectUnauthorized !== false,
     session: options.session,
     NPNProtocols: NPN.NPNProtocols,
     ALPNProtocols: ALPN.ALPNProtocols,

diff --git a/test/parallel/test-https-foafssl.js b/test/parallel/test-https-foafssl.js
@@ -42,7 +42,8 @@ const https = require('https');
 const options = {
   key: fs.readFileSync(common.fixturesDir + '/agent.key'),
   cert: fs.readFileSync(common.fixturesDir + '/agent.crt'),
-  requestCert: true
+  requestCert: true,
+  rejectUnauthorized: false
 };
 
 const modulus = 'A6F44A9C25791431214F5C87AF9E040177A8BB89AC803F7E09BBC3A5519F' +

diff --git a/test/parallel/test-tls-session-cache.js b/test/parallel/test-tls-session-cache.js
@@ -56,7 +56,8 @@ function doTest(testOptions, callback) {
     key: key,
     cert: cert,
     ca: [cert],
-    requestCert: true
+    requestCert: true,
+    rejectUnauthorized: false
   };
   let requestCount = 0;
   let resumeCount = 0;