Skip to content

Commit 4716caa

Browse files
oyydtargos
authored andcommitted
tls: set tlsSocket.servername as early as possible
This commit makes `TLSSocket` set the `servername` property on `SSL_CTX_set_tlsext_servername_callback` so that we could get it later even if errors happen. Fixes: #27699 PR-URL: #27759 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
1 parent 1a8e67c commit 4716caa

File tree

3 files changed

+68
-1
lines changed

3 files changed

+68
-1
lines changed

‎lib/_tls_wrap.js‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,10 @@ TLSSocket.prototype._finishInit = function(){
774774
return;
775775

776776
this.alpnProtocol=this._handle.getALPNNegotiatedProtocol();
777-
this.servername=this._handle.getServername();
777+
// The servername could be set by TLSWrap::SelectSNIContextCallback().
778+
if(this.servername===null){
779+
this.servername=this._handle.getServername();
780+
}
778781

779782
debug('%s _finishInit',
780783
this._tlsOptions.isServer ? 'server' : 'client',

‎src/tls_wrap.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,6 +1033,14 @@ int TLSWrap::SelectSNIContextCallback(SSL* s, int* ad, void* arg){
10331033
Local<Object> object = p->object();
10341034
Local<Value> ctx;
10351035

1036+
// Set the servername as early as possible
1037+
Local<Object> owner = p->GetOwner();
1038+
if (!owner->Set(env->context(),
1039+
env->servername_string(),
1040+
OneByteString(env->isolate(), servername)).FromMaybe(false)){
1041+
return SSL_TLSEXT_ERR_NOACK;
1042+
}
1043+
10361044
if (!object->Get(env->context(), env->sni_context_string()).ToLocal(&ctx))
10371045
return SSL_TLSEXT_ERR_NOACK;
10381046

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
'use strict';
2+
constcommon=require('../common');
3+
if(!common.hasCrypto)
4+
common.skip('missing crypto');
5+
6+
constassert=require('assert');
7+
consttls=require('tls');
8+
9+
// We could get the `tlsSocket.servername` even if the event of "tlsClientError"
10+
// is emitted.
11+
12+
constserverOptions={
13+
requestCert: true,
14+
rejectUnauthorized: false,
15+
SNICallback: function(servername,callback){
16+
if(servername==='c.another.com'){
17+
callback(null,{});
18+
}else{
19+
callback(newError('Invalid SNI context'),null);
20+
}
21+
}
22+
};
23+
24+
functiontest(options){
25+
constserver=tls.createServer(serverOptions,common.mustNotCall());
26+
27+
server.on('tlsClientError',common.mustCall((err,socket)=>{
28+
assert.strictEqual(err.message,'Invalid SNI context');
29+
// The `servername` should match.
30+
assert.strictEqual(socket.servername,options.servername);
31+
}));
32+
33+
server.listen(0,()=>{
34+
options.port=server.address().port;
35+
constclient=tls.connect(options,common.mustNotCall());
36+
37+
client.on('error',common.mustCall((err)=>{
38+
assert.strictEqual(err.message,'Client network socket'+
39+
' disconnected before secure TLS connection was established');
40+
}));
41+
42+
client.on('close',common.mustCall(()=>server.close()));
43+
});
44+
}
45+
46+
test({
47+
port: undefined,
48+
servername: 'c.another.com',
49+
rejectUnauthorized: false
50+
});
51+
52+
test({
53+
port: undefined,
54+
servername: 'c.wrong.com',
55+
rejectUnauthorized: false
56+
});

0 commit comments

Comments
(0)