Skip to content

Commit

Permalink
tls: accept empty net.Sockets
Browse files Browse the repository at this point in the history
Accept `new net.Socket()` as a `socket` option to `tls.connect()`
without triggering an assertion error in C++.

This is done by wrapping it into a JSStream to ensure that there will be
a handle at the time of wrapping the socket into TLSSocket.

Fix: #987
PR-URL: #1046
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Rod Vagg <rod@vagg.org>
  • Loading branch information
indutny committed Mar 3, 2015
1 parent e1bf670 commit 7b3b8ac
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 11 deletions.
30 changes: 19 additions & 11 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,15 +227,20 @@ function TLSSocket(socket, options) {
this.authorizationError = null;

// Wrap plain JS Stream into StreamWrap
var wrap;
if (!(socket instanceof net.Socket) && socket instanceof Duplex)
socket = new StreamWrap(socket);
wrap = new StreamWrap(socket);
else if ((socket instanceof net.Socket) && !socket._handle)
wrap = new StreamWrap(socket);
else
wrap = socket;

// Just a documented property to make secure sockets
// distinguishable from regular ones.
this.encrypted = true;

net.Socket.call(this, {
handle: this._wrapHandle(socket && socket._handle),
handle: this._wrapHandle(wrap && wrap._handle),
allowHalfOpen: socket && socket.allowHalfOpen,
readable: false,
writable: false
Expand All @@ -246,7 +251,7 @@ function TLSSocket(socket, options) {

this.on('error', this._tlsError);

this._init(socket);
this._init(socket, wrap);

// Make sure to setup all required properties like: `_connecting` before
// starting the flow of the data
Expand Down Expand Up @@ -302,7 +307,7 @@ TLSSocket.prototype._wrapHandle = function(handle) {
return res;
};

TLSSocket.prototype._init = function(socket) {
TLSSocket.prototype._init = function(socket, wrap) {
var self = this;
var options = this._tlsOptions;
var ssl = this._handle;
Expand Down Expand Up @@ -394,24 +399,26 @@ TLSSocket.prototype._init = function(socket) {
ssl.receive(buf);
}

if (socket) {
if (socket instanceof net.Socket) {
this._parent = socket;

// To prevent assertion in afterConnect() and properly kick off readStart
this._connecting = socket._connecting;
this._connecting = socket._connecting || !socket._handle;
socket.once('connect', function() {
self._connecting = false;
self.emit('connect');
});

socket.on('error', function(err) {
self._tlsError(err);
});
}

// Assume `tls.connect()`
if (!socket)
if (wrap) {
wrap.on('error', function(err) {
self._tlsError(err);
});
} else {
assert(!socket);
this._connecting = true;
}
};

TLSSocket.prototype.renegotiate = function(options, callback) {
Expand Down Expand Up @@ -506,6 +513,7 @@ TLSSocket.prototype._start = function() {
return;
}

debug('start');
if (this._tlsOptions.requestOCSP)
this._handle.requestOCSP();
this._handle.start();
Expand Down
41 changes: 41 additions & 0 deletions test/parallel/test-tls-on-empty-socket.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
if (!process.versions.openssl) {
console.error('Skipping because node compiled without OpenSSL.');
process.exit(0);
}

var assert = require('assert');
var fs = require('fs');
var net = require('net');
var tls = require('tls');

var common = require('../common');

var out = '';

var server = tls.createServer({
key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'),
cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem')
}, function(c) {
c.end('hello');
}).listen(common.PORT, function() {
var socket = new net.Socket();

var s = tls.connect({
socket: socket,
rejectUnauthorized: false
}, function() {
s.on('data', function(chunk) {
out += chunk;
});
s.on('end', function() {
s.destroy();
server.close();
});
});

socket.connect(common.PORT);
});

process.on('exit', function() {
assert.equal(out, 'hello');
});

0 comments on commit 7b3b8ac

Please sign in to comment.