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 <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Trivikram Kamat <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
nodejs · Nov 15, 2018 · 9b2ffff · bnoordhuis · Nov 15, 2018 · oyyd
1 parent c347e77
commit 9b2ffff
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 1 deletion.
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();
+  }));
+}