Repository navigation
SNICallback doesn't appear to work with tls.createSecurePair() #4878
Description
Activity
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 intolib/_tls_legacy.js. That version ofSNICallbacknever had a callback argument, IIRC. It's implemented in terms ofSSL_CTX_set_tlsext_servername_callback(), its callback needs to accept or reject immediately. The non-legacySNICallbackis implemented throughSSL_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 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 how goes?
@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?
Looks like the issue is resolved in some sense.
Test case: https://gist.github.com/adammw/cf4327506d4293e69014
Testing hitting the server with cURL:
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:
I note that when removing the callback argument I get an OpenSSL error in my terminal:
Potentially related PR: #2441 /cc @socketpair