From 9c0b2db107d1f3f0b3bc1f33cd8d67aa22ff49a3 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 19 Apr 2019 15:51:24 -0400 Subject: [PATCH 1/5] http: `servername === false` should disable SNI There is no way to disable SNI extension when sending a request to HTTPS server. Setting `options.servername` to a falsy value would make Node.js core override it with either hostname or ip address. This change introduces a way to disable SNI completely if this is required for user's application. Setting `options.servername` to `false` in `https.request` would disable overrides and thus disable the extension. --- lib/_http_agent.js | 4 ++-- test/parallel/test-https-agent-sni.js | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 32dbf27abc0dff..ae0f221e11c823 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -151,7 +151,7 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */, if (options.socketPath) options.path = options.socketPath; - if (!options.servername) + if (!options.servername && options.servername !== false) options.servername = calculateServerName(options, req); const name = this.getName(options); @@ -198,7 +198,7 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { if (options.socketPath) options.path = options.socketPath; - if (!options.servername) + if (!options.servername && options.servername !== false) options.servername = calculateServerName(options, req); const name = this.getName(options); diff --git a/test/parallel/test-https-agent-sni.js b/test/parallel/test-https-agent-sni.js index 80278ed2d8fe96..c7e0609e29a998 100644 --- a/test/parallel/test-https-agent-sni.js +++ b/test/parallel/test-https-agent-sni.js @@ -18,9 +18,12 @@ let waiting = TOTAL; const server = https.Server(options, function(req, res) { if (--waiting === 0) server.close(); - res.writeHead(200, { - 'x-sni': req.socket.servername - }); + const servername = req.socket.servername; + + if (servername !== false) { + res.setHeader('x-sni', servername); + } + res.end('hello world'); }); @@ -28,7 +31,8 @@ server.listen(0, function() { function expectResponse(id) { return common.mustCall(function(res) { res.resume(); - assert.strictEqual(res.headers['x-sni'], `sni.${id}`); + assert.strictEqual(res.headers['x-sni'], + id === false ? undefined : `sni.${id}`); }); } @@ -46,4 +50,13 @@ server.listen(0, function() { rejectUnauthorized: false }, expectResponse(j)); } + https.get({ + agent: agent, + + path: '/', + port: this.address().port, + host: '127.0.0.1', + servername: false, + rejectUnauthorized: false + }, expectResponse(false)); }); From 549b4bda917079c8cc951c676436f3f1b578014a Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 19 Apr 2019 17:16:33 -0400 Subject: [PATCH 2/5] doc: add description of `servername` --- doc/api/https.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/doc/api/https.md b/doc/api/https.md index 95e7e715c32ca5..4076efd334c61b 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -28,7 +28,11 @@ An [`Agent`][] object for HTTPS similar to [`http.Agent`][]. See * `options` {Object} Set of configurable options to set on the agent. Can have the same fields as for [`http.Agent(options)`][], and * `maxCachedSessions` {number} maximum number of TLS cached sessions. - Use `0` to disable TLS session caching. **Default:** `100`. + Use `0` to disable TLS session caching. **Default:** `100` + * `servername` {string | boolean} the value of + [Server Name Indication extension](https://en.wikipedia.org/wiki/Server_Name_Indication) + to be sent to the server. Use `false` to disable sending the extension. + **Default:** hostname or ip address of the target server See [`Session Resumption`][] for infomation about TLS session reuse. From 2ea3e4cf226948adb14c5d6938bef18e0738b933 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 20 Apr 2019 12:47:34 -0400 Subject: [PATCH 3/5] fix --- doc/api/https.md | 4 ++-- test/parallel/test-https-agent-sni.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/https.md b/doc/api/https.md index 4076efd334c61b..aca048c9bcd20e 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -28,11 +28,11 @@ An [`Agent`][] object for HTTPS similar to [`http.Agent`][]. See * `options` {Object} Set of configurable options to set on the agent. Can have the same fields as for [`http.Agent(options)`][], and * `maxCachedSessions` {number} maximum number of TLS cached sessions. - Use `0` to disable TLS session caching. **Default:** `100` + Use `0` to disable TLS session caching. **Default:** `100`. * `servername` {string | boolean} the value of [Server Name Indication extension](https://en.wikipedia.org/wiki/Server_Name_Indication) to be sent to the server. Use `false` to disable sending the extension. - **Default:** hostname or ip address of the target server + **Default:** hostname or IP address of the target server. See [`Session Resumption`][] for infomation about TLS session reuse. diff --git a/test/parallel/test-https-agent-sni.js b/test/parallel/test-https-agent-sni.js index c7e0609e29a998..e11cd6651d644e 100644 --- a/test/parallel/test-https-agent-sni.js +++ b/test/parallel/test-https-agent-sni.js @@ -32,7 +32,7 @@ server.listen(0, function() { return common.mustCall(function(res) { res.resume(); assert.strictEqual(res.headers['x-sni'], - id === false ? undefined : `sni.${id}`); + id === false ? undefined : `sni.${id}`); }); } From c85a7eb8d3f5f10bc82b9441b0a3e39e383a8761 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 27 Apr 2019 16:52:41 +0200 Subject: [PATCH 4/5] https: fix link --- doc/api/https.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/api/https.md b/doc/api/https.md index aca048c9bcd20e..98a62e7efc1a70 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -30,8 +30,8 @@ An [`Agent`][] object for HTTPS similar to [`http.Agent`][]. See * `maxCachedSessions` {number} maximum number of TLS cached sessions. Use `0` to disable TLS session caching. **Default:** `100`. * `servername` {string | boolean} the value of - [Server Name Indication extension](https://en.wikipedia.org/wiki/Server_Name_Indication) - to be sent to the server. Use `false` to disable sending the extension. + [Server Name Indication extension][sni wiki] to be sent to the server. Use + `false` to disable sending the extension. **Default:** hostname or IP address of the target server. See [`Session Resumption`][] for infomation about TLS session reuse. @@ -410,3 +410,4 @@ headers: max-age=0; pin-sha256="WoiWRyIOVNa9ihaBciRSC7XHjliYS9VwUGOIud4PB18="; p [`tls.createSecureContext()`]: tls.html#tls_tls_createsecurecontext_options [`tls.createServer()`]: tls.html#tls_tls_createserver_options_secureconnectionlistener [`Session Resumption`]: tls.html#tls_session_resumption +[sni wiki]: https://en.wikipedia.org/wiki/Server_Name_Indication From 1abce92affd74b03478a97359bbf80384d9f069e Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Sat, 27 Apr 2019 18:17:13 +0200 Subject: [PATCH 5/5] change `false` => `''` --- doc/api/https.md | 4 ++-- lib/_http_agent.js | 4 ++-- test/parallel/test-https-agent-sni.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/api/https.md b/doc/api/https.md index 98a62e7efc1a70..9b6ec83a6e7796 100644 --- a/doc/api/https.md +++ b/doc/api/https.md @@ -29,9 +29,9 @@ An [`Agent`][] object for HTTPS similar to [`http.Agent`][]. See Can have the same fields as for [`http.Agent(options)`][], and * `maxCachedSessions` {number} maximum number of TLS cached sessions. Use `0` to disable TLS session caching. **Default:** `100`. - * `servername` {string | boolean} the value of + * `servername` {string} the value of [Server Name Indication extension][sni wiki] to be sent to the server. Use - `false` to disable sending the extension. + empty string `''` to disable sending the extension. **Default:** hostname or IP address of the target server. See [`Session Resumption`][] for infomation about TLS session reuse. diff --git a/lib/_http_agent.js b/lib/_http_agent.js index ae0f221e11c823..eb98f2b0bd1ca4 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -151,7 +151,7 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */, if (options.socketPath) options.path = options.socketPath; - if (!options.servername && options.servername !== false) + if (!options.servername && options.servername !== '') options.servername = calculateServerName(options, req); const name = this.getName(options); @@ -198,7 +198,7 @@ Agent.prototype.createSocket = function createSocket(req, options, cb) { if (options.socketPath) options.path = options.socketPath; - if (!options.servername && options.servername !== false) + if (!options.servername && options.servername !== '') options.servername = calculateServerName(options, req); const name = this.getName(options); diff --git a/test/parallel/test-https-agent-sni.js b/test/parallel/test-https-agent-sni.js index e11cd6651d644e..1ddeff7ce205d9 100644 --- a/test/parallel/test-https-agent-sni.js +++ b/test/parallel/test-https-agent-sni.js @@ -56,7 +56,7 @@ server.listen(0, function() { path: '/', port: this.address().port, host: '127.0.0.1', - servername: false, + servername: '', rejectUnauthorized: false }, expectResponse(false)); });