Skip to content

Commit

Permalink
tls: emit a warning when servername is an IP address
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Rodger Combs authored and oyyd committed Nov 15, 2018
1 parent c347e77 commit 9b2ffff
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 1 deletion.
15 changes: 15 additions & 0 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

This comment has been minimized.

Copy link
@bnoordhuis

bnoordhuis Nov 15, 2018

Member

This should have been a link to #23329, shouldn't it?

This comment has been minimized.

Copy link
@oyyd

oyyd Nov 16, 2018

Contributor

Yeah, sorry for the mistake. Will create another PR.

This comment has been minimized.

Copy link
@oyyd

oyyd Nov 18, 2018

Contributor

Fixed in 34eccb2.

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
Expand Down Expand Up @@ -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
14 changes: 13 additions & 1 deletion lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const kSNICallback = Symbol('snicallback');

const noop = () => {};

let ipServernameWarned = false;

function onhandshakestart(now) {
debug('onhandshakestart');

Expand Down Expand Up @@ -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();
Expand Down
41 changes: 41 additions & 0 deletions test/parallel/test-tls-ip-servername-deprecation.js
Original file line number Diff line number Diff line change
@@ -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();
}));
}

0 comments on commit 9b2ffff

Please sign in to comment.