Skip to content

Commit

Permalink
tls: handle empty cert in checkServerIndentity
Browse files Browse the repository at this point in the history
This resolves nodejs/node-v0.x-archive#9272. `tlsSocket.getPeerCertificate` will
return an empty object when the peer does not provide a certificate,
but, prior to this, when the certificate is empty, `checkServerIdentity`
would throw because the `subject` wasn't present on the cert.
`checkServerIdentity` must return an error, not throw one, so this
returns an error when the cert is empty instead of throwing
a `TypeError`.

PR-URL: #2343
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Shigeki Ohtsu <ohtsu@iij.ad.jp>
  • Loading branch information
Mike Atkins authored and rvagg committed Aug 21, 2015
1 parent 3f821b9 commit d9b70f9
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
host,
ips.join(', '));
}
} else {
} else if (cert.subject) {
// Transform hostname to canonical form
if (!/\.$/.test(host)) host += '.';

Expand Down Expand Up @@ -204,6 +204,8 @@ exports.checkServerIdentity = function checkServerIdentity(host, cert) {
cert.subject.CN);
}
}
} else {
reason = 'Cert is empty';
}

if (!valid) {
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-tls-check-server-identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ var tests = [
'DNS:omg.com'
},

// Empty Cert
{
host: 'a.com',
cert: { },
error: 'Cert is empty'
},

// Multiple CN fields
{
host: 'foo.com', cert: {
Expand Down

0 comments on commit d9b70f9

Please sign in to comment.