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 does not emit error event on handshake failure #8803

Closed
lekoder opened this issue Sep 27, 2016 · 0 comments
Closed

tls.TLSSocket does not emit error event on handshake failure #8803

lekoder opened this issue Sep 27, 2016 · 0 comments
Labels
tls Issues and PRs related to the tls subsystem.

Comments

@lekoder
Copy link
Contributor

lekoder commented Sep 27, 2016

  • Version: v6.5.0
  • Platform: Linux 4.4.0-38-generic
  • Subsystem: TLS/SSL

When upgrading net.Socket to tls.TLSSocket, if there is a handshake failure, no (documented) event is emited by the TLSSocket.

To reproduce:

var tls = require("tls");
var net = require("net");

var netServer = net.createServer(function (netSocket) {
    var TLSSocket = new tls.TLSSocket(netSocket, {
        isServer: true,
        server: netServer
    });
    TLSSocket.on("error", function (e) {
        console.log("TLS error", e);
    });
    netSocket.on("close", function () {
        console.log("Socket closed");
    });
}).listen(1337);
openssl s_client -connect 127.0.0.1:1337

Expected result:

TLS error Error: 140079908828992:error:1408A0C1:SSL routines:ssl3_get_client_hello:no shared cipher:../deps/openssl/openssl/ssl/s3_srvr.c:1417
Socket closed

Actual result:

Socket closed

There is undocumented event _tlsError emited, but it is both not documented not propagated to socket's error event.

@mscdex mscdex added the tls Issues and PRs related to the tls subsystem. label Sep 27, 2016
lekoder added a commit to lekoder/node that referenced this issue Oct 7, 2016
lekoder added a commit to lekoder/node that referenced this issue Oct 7, 2016
lekoder added a commit to lekoder/node that referenced this issue Oct 7, 2016
jasnell pushed a commit that referenced this issue Oct 10, 2016
Removes branch that would make TLSSocket emit '_tlsError' event if error
occured on handshake and control was not released, as it was never happening.
Addedd test for tls.Server to ensure it still emits 'tlsClientError' as expected.

Fixes: #8803
PR-URL: #8805
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Fishrock123 pushed a commit that referenced this issue Oct 11, 2016
Removes branch that would make TLSSocket emit '_tlsError' event if error
occured on handshake and control was not released, as it was never happening.
Addedd test for tls.Server to ensure it still emits 'tlsClientError' as expected.

Fixes: #8803
PR-URL: #8805
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
bnoordhuis pushed a commit to bnoordhuis/io.js that referenced this issue Nov 22, 2016
Removes branch that would make TLSSocket emit '_tlsError' event if
error occured on handshake and control was not released, as it was
never happening.  Added test for tls.Server to ensure it still emits
'tlsClientError' as expected.

Note that 'tlsClientError' does not exist in the v4.x branch so this
back-port emits 'clientError' instead.  See also pull request nodejs#4557.

Fixes: nodejs#8803
PR-URL: nodejs#8805
Refs: nodejs#4557
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
MylesBorins pushed a commit that referenced this issue Mar 28, 2017
Removes branch that would make TLSSocket emit '_tlsError' event if
error occured on handshake and control was not released, as it was
never happening.  Added test for tls.Server to ensure it still emits
'tlsClientError' as expected.

Note that 'tlsClientError' does not exist in the v4.x branch so this
back-port emits 'clientError' instead.  See also pull request #4557.

Fixes: #8803
PR-URL: #8805
Refs: #4557
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
MylesBorins pushed a commit that referenced this issue Apr 4, 2017
Removes branch that would make TLSSocket emit '_tlsError' event if
error occured on handshake and control was not released, as it was
never happening.  Added test for tls.Server to ensure it still emits
'tlsClientError' as expected.

Note that 'tlsClientError' does not exist in the v4.x branch so this
back-port emits 'clientError' instead.  See also pull request #4557.

Fixes: #8803
PR-URL: #8805
Refs: #4557
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
MylesBorins pushed a commit that referenced this issue Apr 17, 2017
Removes branch that would make TLSSocket emit '_tlsError' event if
error occured on handshake and control was not released, as it was
never happening.  Added test for tls.Server to ensure it still emits
'tlsClientError' as expected.

Note that 'tlsClientError' does not exist in the v4.x branch so this
back-port emits 'clientError' instead.  See also pull request #4557.

Fixes: #8803
PR-URL: #8805
Refs: #4557
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
MylesBorins pushed a commit that referenced this issue Apr 19, 2017
Removes branch that would make TLSSocket emit '_tlsError' event if
error occured on handshake and control was not released, as it was
never happening.  Added test for tls.Server to ensure it still emits
'tlsClientError' as expected.

Note that 'tlsClientError' does not exist in the v4.x branch so this
back-port emits 'clientError' instead.  See also pull request #4557.

Fixes: #8803
PR-URL: #8805
Refs: #4557
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants