Skip to content

Commit eb2ca10

Browse files
indutnyindutny
committedJul 19, 2012
tls: veryify server's identity
·
v9.2.0jenkins-accept-commit-temp2
1 parent 53716eb commit eb2ca10

File tree

4 files changed

+306
-13
lines changed

4 files changed

+306
-13
lines changed
 

‎lib/http.js

+6-8
Original file line numberDiff line numberDiff line change
@@ -1123,13 +1123,11 @@ Agent.prototype.removeSocket = function(s, name, host, port, localAddress) {
11231123
}
11241124
if (this.requests[name] && this.requests[name].length) {
11251125
// If we have pending requests and a socket gets closed a new one
1126-
this.createSocket(
1127-
name,
1128-
host,
1129-
port,
1130-
localAddress,
1131-
this.requests[name][0]
1132-
).emit('free');
1126+
this.createSocket(name,
1127+
host,
1128+
port,
1129+
localAddress,
1130+
this.requests[name][0]).emit('free');
11331131
}
11341132
};
11351133

@@ -1141,7 +1139,7 @@ function ClientRequest(options, cb) {
11411139
var self = this;
11421140
OutgoingMessage.call(self);
11431141

1144-
this.options = options;
1142+
this.options = util._extend({}, options);
11451143
self.agent = options.agent === undefined ? globalAgent : options.agent;
11461144

11471145
var defaultPort = options.defaultPort || 80;

‎lib/tls.js

+108-3
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

+3-2
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

+189
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
var util = require('util');
25+
var tls = require('tls');
26+
27+
var tests = [
28+
// Basic CN handling
29+
{ host: 'a.com', cert: { subject: { CN: 'a.com' } }, result: true },
30+
{ host: 'a.com', cert: { subject: { CN: 'A.COM' } }, result: true },
31+
{ host: 'a.com', cert: { subject: { CN: 'b.com' } }, result: false },
32+
{ host: 'a.com', cert: { subject: { CN: 'a.com.' } }, result: true },
33+
34+
// No wildcards in CN
35+
{ host: 'b.a.com', cert: { subject: { CN: '*.a.com' } }, result: false },
36+
37+
// DNS names and CN
38+
{
39+
host: 'a.com', cert: {
40+
subjectaltname: 'DNS:*',
41+
subject: { CN: 'b.com' }
42+
},
43+
result: false
44+
},
45+
{
46+
host: 'a.com', cert: {
47+
subjectaltname: 'DNS:*.com',
48+
subject: { CN: 'b.com' }
49+
},
50+
result: false
51+
},
52+
{
53+
host: 'a.co.uk', cert: {
54+
subjectaltname: 'DNS:*.co.uk',
55+
subject: { CN: 'b.com' }
56+
},
57+
result: true
58+
},
59+
{
60+
host: 'a.com', cert: {
61+
subjectaltname: 'DNS:*.a.com',
62+
subject: { CN: 'a.com' }
63+
},
64+
result: true
65+
},
66+
{
67+
host: 'a.com', cert: {
68+
subjectaltname: 'DNS:*.a.com',
69+
subject: { CN: 'b.com' }
70+
},
71+
result: false
72+
},
73+
{
74+
host: 'a.com', cert: {
75+
subjectaltname: 'DNS:a.com',
76+
subject: { CN: 'b.com' }
77+
},
78+
result: true
79+
},
80+
{
81+
host: 'a.com', cert: {
82+
subjectaltname: 'DNS:A.COM',
83+
subject: { CN: 'b.com' }
84+
},
85+
result: true
86+
},
87+
88+
// DNS names
89+
{
90+
host: 'a.com', cert: {
91+
subjectaltname: 'DNS:*.a.com',
92+
subject: {}
93+
},
94+
result: false
95+
},
96+
{
97+
host: 'b.a.com', cert: {
98+
subjectaltname: 'DNS:*.a.com',
99+
subject: {}
100+
},
101+
result: true
102+
},
103+
{
104+
host: 'c.b.a.com', cert: {
105+
subjectaltname: 'DNS:*.a.com',
106+
subject: {}
107+
},
108+
result: false
109+
},
110+
{
111+
host: 'b.a.com', cert: {
112+
subjectaltname: 'DNS:*b.a.com',
113+
subject: {}
114+
},
115+
result: true
116+
},
117+
{
118+
host: 'a-cb.a.com', cert: {
119+
subjectaltname: 'DNS:*b.a.com',
120+
subject: {}
121+
},
122+
result: true
123+
},
124+
{
125+
host: 'a.b.a.com', cert: {
126+
subjectaltname: 'DNS:*b.a.com',
127+
subject: {}
128+
},
129+
result: false
130+
},
131+
// Mutliple DNS names
132+
{
133+
host: 'a.b.a.com', cert: {
134+
subjectaltname: 'DNS:*b.a.com, DNS:a.b.a.com',
135+
subject: {}
136+
},
137+
result: true
138+
},
139+
// URI names
140+
{
141+
host: 'a.b.a.com', cert: {
142+
subjectaltname: 'URI:http://a.b.a.com/',
143+
subject: {}
144+
},
145+
result: true
146+
},
147+
{
148+
host: 'a.b.a.com', cert: {
149+
subjectaltname: 'URI:http://*.b.a.com/',
150+
subject: {}
151+
},
152+
result: false
153+
},
154+
// IP addresses
155+
{
156+
host: 'a.b.a.com', cert: {
157+
subjectaltname: 'IP Address:127.0.0.1',
158+
subject: {}
159+
},
160+
result: false
161+
},
162+
{
163+
host: '127.0.0.1', cert: {
164+
subjectaltname: 'IP Address:127.0.0.1',
165+
subject: {}
166+
},
167+
result: true
168+
},
169+
{
170+
host: '127.0.0.2', cert: {
171+
subjectaltname: 'IP Address:127.0.0.1',
172+
subject: {}
173+
},
174+
result: false
175+
},
176+
{
177+
host: '127.0.0.1', cert: {
178+
subjectaltname: 'DNS:a.com',
179+
subject: {}
180+
},
181+
result: false
182+
},
183+
];
184+
185+
tests.forEach(function(test, i) {
186+
assert.equal(tls.checkServerIdentity(test.host, test.cert),
187+
test.result,
188+
'Test#' + i + ' failed: ' + util.inspect(test));
189+
});

0 commit comments

Comments
 (0)
Please sign in to comment.