SNICallback doesn't appear to work with tls.createSecurePair()

@socketpair

Test case: https://gist.github.com/adammw/cf4327506d4293e69014

Testing hitting the server with cURL:

curl -v --resolve www.example.com:4443:127.0.0.1 https://www.example.com:4443/

Docs say that there should be two arguments passed to SNICallback, the servername and the callback to call with the secure context. The second argument doesn't appear to be passed through when the TLS connection is created as a stream with tls.createSecurePair().

I get the error:

TypeError: cb is not a function
    at Object.tls.createSecurePair.SNICallback [as onselect]

I note that when removing the callback argument I get an OpenSSL error in my terminal:

Error: 140735169483536:error:1408A0C1:SSL routines:ssl3_get_client_hello:no shared cipher:../deps/openssl/openssl/ssl/s3_srvr.c:1411:

Potentially related PR: #2441 /cc @socketpair

@indutny

Yea, confirmed...looks like https://github.com/nodejs/node/blob/master/src/node_crypto.cc#L2691 could be where the issue lies? /cc @indutny

Reproducing the test case here for posterity:

#!/usr/bin/env node
var fs = require('fs');
var net = require('net');
var tls = require('tls');

var secureContext = tls.createSecureContext({
  cert: fs.readFileSync('test_cert.pem'),
  key: fs.readFileSync('test_key.pem')
})

var server = net.Server(function(raw) {
  var pair = tls.createSecurePair(null, true, false, false, {
    SNICallback: function(servername, cb) {
      console.log('servername', servername);
      cb(null, secureContext); 
    }
  });
  raw.pipe(pair.encrypted).pipe(raw);
});

server.listen(4443, function() {
  var addr = server.address();
  console.log('Server listening on %s %s:%d', addr.family, addr.address, addr.port);
});

Here, tls.createSecurePair() calls into lib/_tls_legacy.js. That version of SNICallback never had a callback argument, IIRC. It's implemented in terms of SSL_CTX_set_tlsext_servername_callback(), its callback needs to accept or reject immediately. The non-legacy SNICallback is implemented through SSL_set_cert_cb(), which is asynchronous and can defer the action until a later time.

I think it should be possible to move the legacy implementation over to SSL_set_cert_cb(). It's a bit of work but it would reduce the line count. Alternatively, we could just document it.

@bnoordhuis

@bnoordhuis by saying

'I think it should be possible to move the legacy implementation over to SSL_set_cert_cb()'

Do you mean moving the whole createSecurePair() from _tls_legacy?
Sounds interesting and I would really like to work on that, but might require a bit of mentoring.

I'm on it.

@webertlima Looks to be the same issue, yes.

@jhamhader

@jhamhader how goes?

@jhamhader

@jhamhader let me know if you need any help on this. I would be more than happy to supply any amount of hints, or pick it over from you (if you are busy with other things).

I would like that and will contact you over IRC/mail.

Any progress on this?

tls.createSecurePair() has been deprecated (#6063)

Looks like the issue is resolved in some sense.

Navigation Menu

Repository navigation

SNICallback doesn't appear to work with `tls.createSecurePair()` #4878

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

SNICallback doesn't appear to work with tls.createSecurePair() #4878

Description

Activity

evanlucas commented on Jan 26, 2016

bnoordhuis commented on Jan 26, 2016

jhamhader commented on Feb 16, 2016

jhamhader commented on Feb 17, 2016

bnoordhuis commented on Feb 24, 2016

dcposch commented on Mar 7, 2016

indutny commented on Mar 9, 2016

jhamhader commented on Mar 9, 2016

webertrlz commented on Jun 17, 2016

jhamhader commented on Jun 17, 2016

indutny commented on Sep 16, 2016

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

SNICallback doesn't appear to work with `tls.createSecurePair()` #4878