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

tls: TLSSocket emits 'error' on handshake failure #9742

Closed
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/_tls_wrap.js
Expand Up @@ -455,7 +455,9 @@ TLSSocket.prototype._init = function(socket, wrap) {

// Destroy socket if error happened before handshake's finish
if (!self._secureEstablished) {
self.destroy(self._tlsError(err));
// When handshake fails control is not yet released,
// so self._tlsError will return null instead of actual error
self.destroy(err);
} else if (options.isServer &&
rejectUnauthorized &&
/peer did not return a certificate/.test(err.message)) {
Expand Down
@@ -0,0 +1,36 @@
'use strict';
const common = require('../common');

Copy link
Contributor

Choose a reason for hiding this comment

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

if (!common.hasCrypto) {
common.skip('missing crypto');
return;
}
const tls = require('tls');
const net = require('net');
const assert = require('assert');
Copy link
Contributor

Choose a reason for hiding this comment

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

sort requires?


const bonkers = Buffer.alloc(1024, 42);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a comment, "Construct a buffer that is not a valid TLS handshake.", or just change var name -
s/bonkers/invalidTlsHandshake/?


let clientErrorEmited = false;

const server = tls.createServer({})
.listen(0, function() {
const c = net.connect({ port: this.address().port }, function() {
c.write(bonkers);
});

}).on('clientError', function(e) {
clientErrorEmited = true;
assert.ok(e instanceof Error,
'Instance of Error should be passed to error handler');
assert.ok(e.message.match(
/SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/),
'Expecting SSL unknown protocol');
});

setTimeout(function() {
server.close();

assert.ok(clientErrorEmited, 'clientError should be emited');
Copy link
Contributor

Choose a reason for hiding this comment

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

"emitted"


}, common.platformTimeout(200));
38 changes: 38 additions & 0 deletions test/parallel/test-tls-socket-failed-handshake-emits-error.js
@@ -0,0 +1,38 @@
'use strict';
const common = require('../common');

Copy link
Contributor

Choose a reason for hiding this comment

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

description, etc.

if (!common.hasCrypto) {
common.skip('missing crypto');
return;
}
const tls = require('tls');
const net = require('net');
const assert = require('assert');

const bonkers = Buffer.alloc(1024, 42);

const server = net.createServer(function(c) {
setTimeout(function() {
const s = new tls.TLSSocket(c, {
Copy link
Contributor

Choose a reason for hiding this comment

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

this version of the test exists because the error handling code paths under test differ in behaviour depending on how the server is created, or just to be very thorough?

isServer: true,
server: server
});

s.on('error', common.mustCall(function(e) {
assert.ok(e instanceof Error,
'Instance of Error should be passed to error handler');
assert.ok(e.message.match(
/SSL routines:SSL23_GET_CLIENT_HELLO:unknown protocol/),
'Expecting SSL unknown protocol');
}));

s.on('close', function() {
server.close();
s.destroy();
});
}, common.platformTimeout(200));
}).listen(0, function() {
const c = net.connect({port: this.address().port}, function() {
c.write(bonkers);
});
});