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

tlsSocket.servername is false (sometimes) #4466

Closed
2 tasks done
siggi-gross opened this issue Aug 18, 2024 · 6 comments
Closed
2 tasks done

tlsSocket.servername is false (sometimes) #4466

siggi-gross opened this issue Aug 18, 2024 · 6 comments
Labels
needs more info issues that need more info from the author

Comments

@siggi-gross
Copy link

siggi-gross commented Aug 18, 2024

Node.js Version

v18.19.0

NPM Version

v9

Operating System

linux

Subsystem

https, tls

Description

https server serving multiple domains, SNICallback in options.

On initializing the server, no default context is given, only the SNICallback.
(see code below)

What I want to achieve is:

  • only valid domains (including subdomains) will be served - requests via IP or invalid domain will be rejected.
  • prevent domain spoofing (SNI servername and header hostname must match)

To decide, which content to serve to the client, the response is processed by the hostname.

Works as expected

The server is running sucessfully for a while now,
rejecting all unwanted and malformed requests (mostly from bots etc).
SNICallback Errors : OnSecureEstablish() -> OnSecureFailed()

request.socket.servername -> false

Every now and then - the servername is false after (obviously) successfull SNICallback.
SNICallback Success: OnSecureEstabish() -> OnSecureConnect()

As the [servername - hostname] match fails, those requests are responded with 400 - Bad Request.

I have done a lot of extensive logging and investigation,
but still could not find the reason for this happening.

Observations

While some requests are clearly malicious,
some are valid (the servername for SNICallback was fine and matching to the header hostname) .

The "false negatives" often appear after a client sent multiple valid and successfull requests in a short period of time.
The majority come from user-agents (bytespider, bingbot etc.) but not all of them

Questions

Did I miss something in SNICallback ?
Is it something with TLS 1.3 session reuse, i am not aware of ?

Once in OnSecureConnect() how can i find out:

  • which TLS-version is used ?
  • wich secureContext is used ?

Minimal Reproduction

/* create server and start up */
let _https_ = require("https");
var httpsOptions = {
	SNICallback: onSecureEstablish,
	minVersion: 'TLSv1.2'
};
_server = _https_.createServer(httpsOptions, onSecureConnect);
    _server.on('error',onServerError);
    _server.on('connection',onServerConnection)
    _server.on('tlsClientError',onSecureFailed);
    _server.on('listening',onServerListening);
_server.listen(443);

var onServerConnection = function onServerConnection(socket) {
    console.log("HTTPS - new Connection ");
    /* -> SNICallback -> onSecureEstablish */
};

var onSecureEstablish = function onSecureEstablish(servername, callback) {
      var _ctx = byHostName(servername);
      /* returns:  ctx: { name:"..", cert:"..", key:"..", ca:".."} || null */
      
      if (!_ctx) {
	      console.log("ERR - SSL-Establish - HTTPS Connect for [ "+servername+" ] not allowed !");
      
	      let _err = new Error("HOST_NOT_ALLOWED");
	      return callback(_err,undefined);
	      /* -> onSecureFailed */
      };
      
      console.log("SSL-Establish - HTTPS Connect for [ "+servername+" ] -> SecureContext:"+_ctx.name);
      let _tlsOptions = {
	      cert : _ctx.cert,
	      key  : _ctx.key,
	      ca   : _ctx.ca
      };
      let _tlsContext = new _tls_.createSecureContext(_tlsOptions);
      return callback(null, _tlsContext );
      /* -> onSecureConnect */
};

var onSecureFailed = function onSecureFailed(err,socket) {
      if (!socket && !socket._parent) { //SSL Error occured
	      console.log("HTTPS Connect ERR - SSL-Establish - "+err);
	      return;
      }
      var _servername = socket.servername; // _servername already rejected by SNICallback/onSecureEstablish
      if (err.message == "HOST_NOT_ALLOWED") {
	      console.log("HTTPS Connect ERR - SSL-Establish - Host [ "+_servername+" ] not allowed !");
	      return;
      }
      
      console.log("HTTPS Connect ERR - SSL-Establish - [ "+_servername+" ] - "+err);
};

var onSecureConnect = function onSecureConnect(request, response) {
    // request.socket is the tlsSocket
    console.log(request.socket._secureEstablished); // -> true
    console.log(request.socket.encrypted); // -> true
    console.log(request.socket.servername); // sometimes -> FALSE!!
    
    /* verify host & servername */
    var _servername = request.socket.servername;
    var _host = request.headers.host;
    
    if (!_host) {
        console.log("HTTPS Connect ERR -> missing required header [ Host ] !");
        
        response.writeHead( 400, "Bad Request");
        response.write("ERR - 400 Bad Request");
        response.end();
        return;
    };
    // trim trailing ":443" ?
    if (_host.substring(_host.length-4) == ":443") { _host = _host.substring(0,_host.length-4); };
    
    // servername & host MUST match !!
    if (_servername !== _host) {
        console.log("HTTPS Connect ERR -> requested Servername [ "+_servername+" ] and Host [ "+_host+" ] do not match !");
        
        response.writeHead( 400, "Bad Request");
        response.write("ERR - 400 Bad Request");
        response.end();
        return;
    };
    
    /* process request -> response */
        response.writeHead( 200, "OK");
        response.write("response from "+_host);
        response.end();
        return;

}; 

Output

var onSecureConnect = function onSecureConnect(request, response) {
    // request.socket is the tlsSocket
    console.log(request.socket._secureEstablished); // -> true
    console.log(request.socket.encrypted); // -> true
    console.log(request.socket.servername); // sometimes -> FALSE!!
    ...
}; 

Before You Submit

  • I have looked for issues that already exist before submitting this
  • My issue follows the guidelines in the README file, and follows the 'How to ask a good question' guide at https://stackoverflow.com/help/how-to-ask
@RedYetiDev
Copy link
Member

RedYetiDev commented Aug 18, 2024

Can you simplify that reproduction to be smaller?

Your reproduction has a lot of moving parts, it's easier if you have a one-and-done script that showcases this behavior.

@RedYetiDev RedYetiDev added the needs more info issues that need more info from the author label Aug 18, 2024
@siggi-gross
Copy link
Author

Hello RedYetiDev,

i'm afraid a fully reproducible https-setup as a one-and-done script isn't that easy to provide.

Nevertheless I have tried to simplify the above example as much as possible and
reduce it to the 3 essential callback functions for the https server.

let _https_ = require("https");
let _fs_ = require("fs");

var _tlsOptions = {
	name : "foo.com",
	cert : _fs_.readFileSync('test_cert.pem'),
	key  : _fs_.readFileSync('test_key.pem'),
	ca   : _fs_.readFileSync('test_ca.pem')
};

var httpsOptions = {
	SNICallback: onSecureEstablish,
	minVersion: 'TLSv1.2'
};

var _server = _https_.createServer(httpsOptions, onSecureConnect);
	_server.on('tlsClientError',onSecureFailed);
_server.listen(443);


var onSecureEstablish = function onSecureEstablish(servername, callback) {
	/* SNICallback function (simplified) */
	
	if (servername === "foo.com") { /* servername valid */
		console.log("SSL-Establish - for [ foo.com ]");
		return callback(null, new _tls_.createSecureContext(_tlsOptions) );
		/* -> onSecureConnect */
	};
	
	/* else - ERR */
		let _err = new Error("HOST_NOT_ALLOWED");
		return callback(_err,undefined);
		/* -> onSecureFailed */
};

var onSecureFailed = function onSecureFailed(err,socket) {
	/* tlsClienError function (simplified) */
	console.log("HTTPS Connect ERR -> SSL-Establish failed : "+err);
};

var onSecureConnect = function onSecureConnect(request, response) {
	// SNICallback sucsessfully done at this point
	// request.socket is the tlsSocket

	/* verify host & servername */
	var _servername = request.socket.servername;
	var _host = request.headers.host;
	
	if (!_servername || _servername === false) { /* THIS is where the ERROR occurs ! */
		console.log("HTTPS Connect ERR -> missing required SNI !");
		
		response.writeHead(400);
		response.end("ERR - 400 Bad Request");
		return;
	}

	if (!_host) {
		console.log("HTTPS Connect ERR -> missing required header [ Host ] !");
		
		response.writeHead(400);
		response.end("ERR - 400 Bad Request");
		return;
	};

	// servername & host MUST match !!
	if (_servername !== _host) {
		console.log("HTTPS Connect ERR -> SNI [ "+_servername+" ] and Host [ "+_host+" ] do not match !");
		
		response.writeHead(400);
		response.end("ERR - 400 Bad Request");
		return;
	};

	/* OK - process request -> response */
		response.writeHead(200);
		response.end("response from "+_host);
		return;
}; 

@siggi-gross
Copy link
Author

siggi-gross commented Aug 19, 2024

According to : https://nodejs.org/docs/latest/api/tls.html#event-secureconnection and https://nodejs.org/docs/latest/api/tls.html#event-tlsclienterror
the tlsSocket.servername should be set on successfull SSL-Handshake.

My extensive logging showed:

  • SSL-Handshake was successfull
  • the provided cert for the client is valid
  • SNICallback was executed sucessfully

but still - sometimes the tlsSocket.servername is false

somewhere in between the SNICallback execution, the tls.secureConnectionEvent and the call to https.server.requestListener the servername got lost with no tlsClientError being raised.

I am aware of server.addContext() https://nodejs.org/docs/latest/api/tls.html#serveraddcontexthostname-context

  • which implicitely does what I do in explicit form with the SNICallback and check in the https.server.requestListener
  • and fails silently if there is no SNI - Hostname match

Related:

nodejs/node#27699 and
nodejs/node#28985 (although it's HTTP/2)

@indutny
Copy link
Member

indutny commented Aug 19, 2024

@siggi-gross could you check if the session was reused? I think it is a know behavior if so.

@siggi-gross
Copy link
Author

@indutny could you please point me in the right direction how to check that?

This one? https://nodejs.org/docs/latest/api/tls.html#session-resumption

In OnSecureConnect() ?
Or do I need to chatch theserver's newSession and resumeSession Events?

Thank You in advance

@siggi-gross
Copy link
Author

@indutny - yes, session was resued...

After two more days of extensive logging and investigating into the topic,
nodejs/node#27167
nodejs/node#28985
and different other non-nodejs-related TLS session resumption issues
struggling with the same or very similar problems:

As it turns out, some clients try to reuse tlsSession on IP basis (for good) or on
domain basis - including multiple subdomains (for good)
while others reuse tlsSessions for "not so good reasons"..

I think it is a know behavior if so.

which effectively means - in OnSecureConnect (the https.server.requestListener) there is
no way I can get the "real" SNI servername once it is set to false.

As I strictly need the SNI servername there to proceed with the request,
I decided to respond with an 421 Misdirected Request
and leave it to the client to decide if he wants to retry with a new connection.

@siggi-gross siggi-gross changed the title tlsSocket.servername is false after SNICallback (sometimes) tlsSocket.servername is false (sometimes) Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info issues that need more info from the author
Projects
None yet
Development

No branches or pull requests

3 participants