Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Allow tls.connect to start on server-side sockets and add tls.start as a simple wrapper for tls.connect #3653

Closed
wants to merge 4 commits 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
50 changes: 49 additions & 1 deletion lib/tls.js
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,8 @@ exports.connect = function(/* [port, host], options, cb */) {
var sslcontext = crypto.createCredentials(options);

convertNPNProtocols(options.NPNProtocols, this);
var pair = new SecurePair(sslcontext, false, true,

var pair = new SecurePair(sslcontext, options.asServer?true:false, true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style.

options.rejectUnauthorized === true ? true : false,
{
NPNProtocols: this.NPNProtocols,
Expand Down Expand Up @@ -1171,6 +1172,53 @@ exports.connect = function(/* [port, host], options, cb */) {
return cleartext;
};

// starts TLS on a given socket, be it client or server
// for a server-side socket, asServer has to be passed as a boolean
// var assert = require('assert');
// var net = require('net');
// var tls = require('tls');
// var client = net.connect(443, 'google.com', function() {
// var encryptedStream = tls.start(client, function() {
// // secure connection in here!
// client.end();
// });
//});
exports.start = function(socket /* [options], [asServer], cb */) {
var cb, options = {}, asServer;

if(!socket) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style.

throw new Error("First parameter as socket is required");
}


// callback is always last argument
if (typeof arguments[arguments.length - 1] === 'function') {
cb = arguments[arguments.length - 1];
}
// options passed as second argument
if (typeof arguments[1] === 'object') {
options = arguments[1];
if (typeof arguments[2] == 'boolean') {
asServer = arguments[2];
}
}
// no options passed
else if(typeof arguments[1] == 'boolean') {
asServer = arguments[1];
}

// revert encoding as TLS requires a binary stream
socket.setEncoding(undefined);

options = util._extend({
socket: socket,
// allow asServer to be passed either through argument
// or through options
asServer: asServer || options.asServer
}, options || {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The || {} is superfluous, _extend() handles that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly I was basing this on line 1111 of the same file, in the connect function. Could I remove the "|| {}" there as well?

return exports.connect(options, cb);
};


function pipe(pair, socket) {
pair.encrypted.pipe(socket);
Expand Down
46 changes: 46 additions & 0 deletions test/simple/test-tls-start-client-server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
var assert = require('assert');
var net = require('net');
var common = require('../common');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be the first require() statement. This file needs the copyright boilerplate as well (copy it from one of the other tests).

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

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

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

var serverOptions = {
key: loadPEM('agent2-key'),
cert: loadPEM('agent2-cert')
};


var serverSecurelyConnected = false;
var clientSecurelyConnected = false;

net.createServer(function(socket) {
var encryptedStream = tls.start(socket, serverOptions, true, function() {
serverSecurelyConnected = true;
});
}).listen(9999);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use common.PORT here.




var client = net.connect(9999, function() {
var encryptedStream = tls.start(client, function() {
clientSecurelyConnected = true;
if(serverSecurelyConnected) {
process.exit(0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Close the client/server, don't exit.

}
assert.fail("Client connected securely to the server but server does "
+ "not know this", "Both client and server should be "
+ "connected", "tls.start failed");
});
});

setTimeout(function() {
assert.fail("tls.start failed, client has not been able to upgrade connection");
}, 100);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is timing sensitive. Use process.on('exit', ...).

11 changes: 11 additions & 0 deletions test/simple/test-tls-start-client.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
var assert = require('assert');
var net = require('net');
var tls = require('tls');

var client = net.connect(443, 'google.com', function() {
var encryptedStream = tls.start(client, function() {
assert.ok(encryptedStream.authorized, "When connected, connection should yield authorized");
client.end();
});

});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this test to test/internet/