tls: emit a warning when servername is an IP address

Setting the TLS ServerName to an IP address is not permitted by RFC6066. This will be ignored in a future version. Refs: #18127 PR-URL: #23329 Fixes: #18071 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
nodejs · Jan 12, 2018 · 9b2ffff62cdbfe6ab538e87aafa5828bfbaaa196 · bnoordhuis · Nov 15, 2018 · oyyd
1 parent c347e77
commit 9b2ffff62cdbfe6ab538e87aafa5828bfbaaa196
Unified Split

Showing with 69 additions and 1 deletion.

+15 −0 doc/api/deprecations.md

+13 −1 lib/_tls_wrap.js

+41 −0 test/parallel/test-tls-ip-servername-deprecation.js
diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
@@ -2293,6 +2293,20 @@ Type: Runtime
 Please use `Server.prototype.setSecureContext()` instead.
+  
+<a id="DEP0123"></a>
+  
+### DEP0123: setting the TLS ServerName to an IP address
+  
+<!-- YAML
+  
+changes:
+  
+  - version: REPLACEME
+  
+    pr-url: https://github.com/nodejs/node/pull/REPLACEME
+  
+    description: Runtime deprecation.
+  
+-->
+  
+
+  
+Type: Runtime
+  
+
+  
+Setting the TLS ServerName to an IP address is not permitted by
+  
+[RFC 6066][]. This will be ignored in a future version.
+  
+
 [`--pending-deprecation`]: cli.html#cli_pending_deprecation
 [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
 [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
@@ -2393,3 +2407,4 @@ Please use `Server.prototype.setSecureContext()` instead.
 [legacy `urlObject`]: url.html#url_legacy_urlobject
 [NIST SP 800-38D]: https://nvlpubs.nist.gov/nistpubs/Legacy/SP/nistspecialpublication800-38d.pdf
 [WHATWG URL API]: url.html#url_the_whatwg_url_api
+  
+[RFC 6066]: https://tools.ietf.org/html/rfc6066#section-3
diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js
@@ -59,6 +59,8 @@ const kSNICallback = Symbol('snicallback');
 const noop = () => {};
+  
+let ipServernameWarned = false;
+  
+
 function onhandshakestart(now) {
  debug('onhandshakestart');
@@ -1240,8 +1242,18 @@ exports.connect = function connect(...args) {
  if (options.session)
    socket.setSession(options.session);
-  
-  if (options.servername)
+  
+  if (options.servername) {
+  
+    if (!ipServernameWarned && net.isIP(options.servername)) {
+  
+      process.emitWarning(
+  
+        'Setting the TLS ServerName to an IP address is not permitted by ' +
+  
+        'RFC 6066. This will be ignored in a future version.',
+  
+        'DeprecationWarning',
+  
+        'DEP0123'
+  
+      );
+  
+      ipServernameWarned = true;
+  
+    }
    socket.setServername(options.servername);
+  
+  }
  if (options.socket)
    socket._start();

diff --git a/test/parallel/test-tls-ip-servername-deprecation.js b/test/parallel/test-tls-ip-servername-deprecation.js
@@ -0,0 +1,41 @@
+  
+'use strict';
+  
+
+  
+const common = require('../common');
+  
+const fixtures = require('../common/fixtures');
+  
+
+  
+if (!common.hasCrypto)
+  
+  common.skip('missing crypto');
+  
+
+  
+const tls = require('tls');
+  
+
+  
+// This test expects `tls.connect()` to emit a warning when
+  
+// `servername` of options is an IP address.
+  
+common.expectWarning(
+  
+  'DeprecationWarning',
+  
+  'Setting the TLS ServerName to an IP address is not permitted by ' +
+  
+  'RFC 6066. This will be ignored in a future version.',
+  
+  'DEP0123'
+  
+);
+  
+
+  
+{
+  
+  const options = {
+  
+    key: fixtures.readKey('agent1-key.pem'),
+  
+    cert: fixtures.readKey('agent1-cert.pem')
+  
+  };
+  
+
+  
+  const server = tls.createServer(options, function(s) {
+  
+    s.end('hello');
+  
+  }).listen(0, function() {
+  
+    const client = tls.connect({
+  
+      port: this.address().port,
+  
+      rejectUnauthorized: false,
+  
+      servername: '127.0.0.1',
+  
+    }, function() {
+  
+      client.end();
+  
+    });
+  
+  });
+  
+
+  
+  server.on('connection', common.mustCall(function(socket) {
+  
+    server.close();
+  
+  }));
+  
+}