tls: veryify server's identity · nodejs/node@eb2ca10 · GitHub
Skip to content

Commit eb2ca10

Browse files
indutnyindutny
committed
tls: veryify server's identity
1 parent 53716eb commit eb2ca10

4 files changed

Lines changed: 306 additions & 13 deletions

File tree

‎lib/http.js‎

Lines changed: 6 additions & 8 deletions

‎lib/tls.js‎

Lines changed: 108 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
var crypto = require('crypto');
2323
var util = require('util');
2424
var net = require('net');
25+
var url = require('url');
2526
var events = require('events');
2627
var Stream = require('stream');
2728
var END_OF_FILE = 42;
@@ -77,6 +78,99 @@ function convertNPNProtocols(NPNProtocols, out) {
7778
}
7879
}
7980

81+
82+
function checkServerIdentity(host, cert) {
83+
// Create regexp to much hostnames
84+
function regexpify(host, wildcards) {
85+
// Add trailing dot (make hostnames uniform)
86+
if (!/\.$/.test(host)) host += '.';
87+
88+
// Host names with less than one dots are considered too broad,
89+
// and should not be allowed
90+
if (!/^.+\..+$/.test(host)) return /$./;
91+
92+
// The same applies to hostname with more than one wildcard,
93+
// if hostname has wildcard when wildcards are not allowed,
94+
// or if there are less than two dots after wildcard (i.e. *.com or *d.com)
95+
if (/\*.*\*/.test(host) || !wildcards && /\*/.test(host) ||
96+
/\*/.test(host) && !/\*.*\..+\..+/.test(host)) {
97+
return /$./;
98+
}
99+
100+
// Replace wildcard chars with regexp's wildcard and
101+
// escape all characters that have special meaning in regexps
102+
// (i.e. '.', '[', '{', '*', and others)
103+
var re = host.replace(
104+
/\*([a-z0-9\\-_\.])|[\.,\-\\\^\$+?*\[\]\(\):!\|{}]/g,
105+
function(all, sub) {
106+
if (sub) return '[a-z0-9\\-_]*' + (sub === '-' ? '\\-' : sub);
107+
return '\\' + all;
108+
}
109+
);
110+
111+
return new RegExp('^' + re + '$', 'i');
112+
}
113+
114+
var dnsNames = [],
115+
uriNames = [],
116+
ips = [],
117+
valid = false;
118+
119+
// There're several names to perform check against:
120+
// CN and altnames in certificate extension
121+
// (DNS names, IP addresses, and URIs)
122+
//
123+
// Walk through altnames and generate lists of those names
124+
if (cert.subjectaltname) {
125+
cert.subjectaltname.split(/, /g).forEach(function(altname) {
126+
if (/^DNS:/.test(altname)) {
127+
dnsNames.push(altname.slice(4));
128+
} else if (/^IP Address:/.test(altname)) {
129+
ips.push(altname.slice(11));
130+
} else if (/^URI:/.test(altname)) {
131+
var uri = url.parse(altname.slice(4));
132+
if (uri) uriNames.push(uri.hostname);
133+
}
134+
});
135+
}
136+
137+
// If hostname is an IP address, it should be present in the list of IP
138+
// addresses.
139+
if (net.isIP(host)) {
140+
valid = ips.some(function(ip) {
141+
return ip === host;
142+
});
143+
} else {
144+
// Transform hostname to canonical form
145+
if (!/\.$/.test(host)) host += '.';
146+
147+
// Otherwise check all DNS/URI records from certificate
148+
// (with allowed wildcards)
149+
dnsNames = dnsNames.map(function(name) {
150+
return regexpify(name, true);
151+
});
152+
153+
// Wildcards ain't allowed in URI names
154+
uriNames = uriNames.map(function(name) {
155+
return regexpify(name, false);
156+
});
157+
158+
dnsNames = dnsNames.concat(uriNames);
159+
160+
// And only after check if hostname matches CN
161+
// (because CN is deprecated, but should be used for compatiblity anyway)
162+
dnsNames.push(regexpify(cert.subject.CN, false));
163+
164+
valid = dnsNames.some(function(re) {
165+
return re.test(host);
166+
});
167+
}
168+
169+
return valid;
170+
};
171+
exports.checkServerIdentity = checkServerIdentity;
172+
173+
80174
// Base class of both CleartextStream and EncryptedStream
81175
function CryptoStream(pair) {
82176
Stream.call(this);
@@ -1118,11 +1212,12 @@ exports.connect = function(/* [port, host], options, cb */) {
11181212
var sslcontext = crypto.createCredentials(options);
11191213

11201214
convertNPNProtocols(options.NPNProtocols, this);
1121-
var pair = new SecurePair(sslcontext, false, true,
1215+
var hostname = options.servername || options.host,
1216+
pair = new SecurePair(sslcontext, false, true,
11221217
options.rejectUnauthorized === true ? true : false,
11231218
{
11241219
NPNProtocols: this.NPNProtocols,
1125-
servername: options.servername || options.host
1220+
servername: hostname
11261221
});
11271222

11281223
if (options.session) {
@@ -1147,9 +1242,19 @@ exports.connect = function(/* [port, host], options, cb */) {
11471242

11481243
cleartext.npnProtocol = pair.npnProtocol;
11491244

1245+
// Verify that server's identity matches it's certificate's names
1246+
if (!verifyError) {
1247+
var validCert = checkServerIdentity(hostname,
1248+
pair.cleartext.getPeerCertificate());
1249+
if (!validCert) {
1250+
verifyError = new Error('Hostname/IP doesn\'t match certificate\'s ' +
1251+
'altnames');
1252+
}
1253+
}
1254+
11501255
if (verifyError) {
11511256
cleartext.authorized = false;
1152-
cleartext.authorizationError = verifyError;
1257+
cleartext.authorizationError = verifyError.message;
11531258

11541259
if (pair._rejectUnauthorized) {
11551260
cleartext.emit('error', verifyError);

‎src/node_crypto.cc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1547,7 +1547,8 @@ Handle<Value> Connection::VerifyError(const Arguments& args) {
15471547
// We requested a certificate and they did not send us one.
15481548
// Definitely an error.
15491549
// XXX is this the right error message?
1550-
return scope.Close(String::New("UNABLE_TO_GET_ISSUER_CERT"));
1550+
return scope.Close(Exception::Error(
1551+
String::New("UNABLE_TO_GET_ISSUER_CERT")));
15511552
}
15521553
X509_free(peer_cert);
15531554

@@ -1673,7 +1674,7 @@ Handle<Value> Connection::VerifyError(const Arguments& args) {
16731674
break;
16741675
}
16751676

1676-
return scope.Close(s);
1677+
return scope.Close(Exception::Error(s));
16771678
}
16781679

16791680

‎test/simple/test-tls-check-server-identity.js‎

Lines changed: 189 additions & 0 deletions

0 commit comments

Comments
 (0)