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: https://github.com/nodejs/node/pull/18127

PR-URL: https://github.com/nodejs/node/pull/23329
Fixes: https://github.com/nodejs/node/issues/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>

3 files changed, 69 insertions, 1 deletions

diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md
index 681fac92de..a7294145ed 100644
--- 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
index 2e32366028..0cd500617f 100644
--- 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
new file mode 100644
index 0000000000..b747caa03d
--- /dev/null
+++ 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();
+  }));
+}