Skip to content

Commit

Permalink
tls: make rejectUnauthorized default to true
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
ghaiklor authored and sam-github committed Mar 23, 2017
1 parent ee19e29 commit 348cc80
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 21 deletions.
16 changes: 9 additions & 7 deletions doc/api/tls.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()`][]
Expand Down
15 changes: 3 additions & 12 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-https-foafssl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' +
Expand Down
3 changes: 2 additions & 1 deletion test/parallel/test-tls-session-cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 348cc80

Please sign in to comment.