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

TLS connection over existing TLS socket causes assertion failure #6204

Closed
TooTallNate opened this issue Sep 11, 2013 · 4 comments
Closed

TLS connection over existing TLS socket causes assertion failure #6204

TooTallNate opened this issue Sep 11, 2013 · 4 comments
Assignees
Milestone

Comments

@TooTallNate
Copy link

This script works up until node v0.11.3. I can bisect the exact commit a bit later...

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

var options, a, b, portA, portB;
var gotHello = false;

options = {
  key: fs.readFileSync(__dirname + '/test/server.key'),
  cert: fs.readFileSync(__dirname + '/test/server.crt')
};

// the "proxy" server
a = tls.createServer(options, function (socket) {
  var options = {
    host: '127.0.0.1',
    port: b.address().port
  };
  var dest = net.connect(options);
  dest.pipe(socket);
  socket.pipe(dest);
});

options = {
  key: fs.readFileSync(__dirname + '/test/proxy.key'),
  cert: fs.readFileSync(__dirname + '/test/proxy.crt')
};

// the "target" server
b = tls.createServer(options, function (socket) {
  socket.end('hello');
});

process.on('exit', function () {
  assert(gotHello);
});

a.listen(function () {
  b.listen(function () {
    options = {
      host: '127.0.0.1',
      port: a.address().port,
      rejectUnauthorized: false
    };
    var socket = tls.connect(options);
    var ssl;
    ssl = tls.connect({
      socket: socket,
      rejectUnauthorized: false
    });
    ssl.setEncoding('utf8');
    ssl.once('data', function (data) {
      assert.equal('hello', data);
      gotHello = true;
    });
    ssl.on('end', function () {
      ssl.end();
      a.close();
      b.close();
    });
  });
});

Causes:

☮ ~ (master) ∴ node -v
v0.11.7
☮ ~ (master) ∴ node tls-over-tls-fail.js
Assertion failed: (uv__stream_fd(stream) >= 0), function uv__read_start_common, file ../deps/uv/src/unix/stream.c, line 1331.
Abort trap: 6
@bnoordhuis
Copy link
Member

I will bet good money that the offending commit turns out to be af80e7b.

/cc @indutny

@ghost ghost assigned indutny Sep 11, 2013
@TooTallNate
Copy link
Author

Additionally, if you modify the test to wait until after the "secureConnect" event to upgrade the socket to TLS, then I get a "segmentation fault":

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

var options, a, b, portA, portB;
var gotHello = false;

options = {
  key: fs.readFileSync(__dirname + '/test/server.key'),
  cert: fs.readFileSync(__dirname + '/test/server.crt')
};

// the "proxy" server
a = tls.createServer(options, function (socket) {
  var options = {
    host: '127.0.0.1',
    port: b.address().port
  };
  var dest = net.connect(options);
  dest.pipe(socket);
  socket.pipe(dest);
});

options = {
  key: fs.readFileSync(__dirname + '/test/proxy.key'),
  cert: fs.readFileSync(__dirname + '/test/proxy.crt')
};

// the "target" server
b = tls.createServer(options, function (socket) {
  socket.end('hello');
});

process.on('exit', function () {
  assert(gotHello);
});

a.listen(function () {
  b.listen(function () {
    options = {
      host: '127.0.0.1',
      port: a.address().port,
      rejectUnauthorized: false
    };
    var socket = tls.connect(options);
    var ssl;
    socket.on('secureConnect', function () {
      ssl = tls.connect({
        socket: socket,
        rejectUnauthorized: false
      });
      ssl.setEncoding('utf8');
      ssl.once('data', function (data) {
        assert.equal('hello', data);
        gotHello = true;
      });
      ssl.on('end', function () {
        ssl.end();
        a.close();
        b.close();
      });
    });

  });
});
☮ ~ (master) ∴ node tls-over-tls-segfault.js
Segmentation fault: 11

@TooTallNate
Copy link
Author

gdb gives me:

(no debugging symbols found)...done.
(gdb) run
Starting program: /usr/local/bin/node tls-over-tls-segfault.js
[New Thread 0x1a0b of process 56832]
[New Thread 0x1b03 of process 56832]
[New Thread 0x1c03 of process 56832]
[New Thread 0x1d03 of process 56832]
[New Thread 0x1e03 of process 56832]

Program received signal SIGSEGV, Segmentation fault.
0x00000001000543a9 in ssl3_read_bytes ()
(gdb) bt full
#0  0x00000001000543a9 in ssl3_read_bytes ()
No symbol table info available.
#1  0x00000001000535ea in ssl3_read_internal ()
No symbol table info available.
#2  0x00000001000405dc in node::TLSCallbacks::ClearOut() ()
No symbol table info available.
#3  0x0000000100040f72 in node::TLSCallbacks::DoRead(uv_stream_s*, long, uv_buf_t const*, uv_handle_type) ()
No symbol table info available.
#4  0x00000001000247d3 in node::StreamWrap::OnReadCommon(uv_stream_s*, long, uv_buf_t const*, uv_handle_type) ()
No symbol table info available.
#5  0x000000010012bf24 in uv__stream_io ()
No symbol table info available.
#6  0x0000000100132b7f in uv__io_poll ()
No symbol table info available.
#7  0x00000001001267f3 in uv_run ()
No symbol table info available.
#8  0x000000010000b862 in node::Start(int, char**) ()
No symbol table info available.
#9  0x0000000100000fb4 in start ()
No symbol table info available.

indutny added a commit to indutny/node that referenced this issue Sep 24, 2013
Allow wrapping TLSSocket inside another TLSSocket, emulate it using
SecurePair in legacy APIs.

fix nodejs#6204
@indutny
Copy link
Member

indutny commented Sep 24, 2013

Fixed in 42acbf8.

richardlau pushed a commit to ibmruntimes/node that referenced this issue Nov 5, 2015
Helps in implementation of nodejs#6204, where some options passed to
`createSecurePair()` are ignored before this patch.

These options are very helpful if someone wants to pass
`options.servername` or `options.SNICallback` to securepair.

PR-URL: nodejs/node#2441
Reviewed-By: Fedor Indutny <fedor@indutny.com>
richardlau pushed a commit to ibmruntimes/node that referenced this issue Feb 18, 2016
Helps in implementation of nodejs#6204, where some options passed to
`createSecurePair()` are ignored before this patch.

These options are very helpful if someone wants to pass
`options.servername` or `options.SNICallback` to securepair.

PR-URL: nodejs/node#2441
Reviewed-By: Fedor Indutny <fedor@indutny.com>
richardlau pushed a commit to ibmruntimes/node that referenced this issue Feb 18, 2016
Helps in implementation of nodejs#6204, where some options passed to
`createSecurePair()` are ignored before this patch.

These options are very helpful if someone wants to pass
`options.servername` or `options.SNICallback` to securepair.

PR-URL: nodejs/node#2441
Reviewed-By: Fedor Indutny <fedor@indutny.com>
richardlau pushed a commit to ibmruntimes/node that referenced this issue Mar 9, 2016
Helps in implementation of nodejs#6204, where some options passed to
`createSecurePair()` are ignored before this patch.

These options are very helpful if someone wants to pass
`options.servername` or `options.SNICallback` to securepair.

PR-URL: nodejs/node#2441
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants