From e3aef72bf2b0f224e9cd4bb843ce7572c91a4850 Mon Sep 17 00:00:00 2001 From: Mir Mufaqam Ali Date: Wed, 20 Dec 2017 17:28:03 +0530 Subject: [PATCH 1/6] lib/_tls_wrap.js: Migrate errors to internal/errors.js This migrates the old style error messages to use internal/errors.js Fixes: https://github.com/nodejs/node/issues/17709 --- doc/api/errors.md | 6 ++++++ lib/_tls_wrap.js | 2 +- lib/internal/errors.js | 1 + test/parallel/test-tls-disable-renegotiation.js | 8 +++++--- 4 files changed, 13 insertions(+), 4 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 918cfa343136af..362f7d14a40b98 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1502,6 +1502,12 @@ a hostname in the first parameter. An excessive amount of TLS renegotiations is detected, which is a potential vector for denial-of-service attacks. + +### ERR_TLS_RENEGOTIATION_DISABLED + +This errors is triggered when attempts are made to renegotiate TLS on a socket +instance which has TLS disabled + ### ERR_TRANSFORM_ALREADY_TRANSFORMING diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index e30efa4159b57e..6d2c14c0bf27d8 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -70,7 +70,7 @@ function onhandshakestart() { } if (owner[kDisableRenegotiation] && this.handshakes > 0) { - const err = new Error('TLS session renegotiation disabled for this socket'); + const err = new errors.Error('ERR_TLS_RENEGOTIATION_DISABLED'); owner._emitTLSError(err); } } diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 7276bb6344a5bd..297ab496435bf6 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -474,6 +474,7 @@ E('ERR_TLS_RENEGOTIATION_FAILED', 'Failed to renegotiate'); E('ERR_TLS_REQUIRED_SERVER_NAME', '"servername" is required parameter for Server.addContext'); E('ERR_TLS_SESSION_ATTACK', 'TLS session renegotiation attack detected'); +E('ERR_TLS_RENEGOTIATION_DISABLED', 'TLS session renegotiation disabled for this socket'); E('ERR_TRANSFORM_ALREADY_TRANSFORMING', 'Calling transform done when still transforming'); E('ERR_TRANSFORM_WITH_LENGTH_0', diff --git a/test/parallel/test-tls-disable-renegotiation.js b/test/parallel/test-tls-disable-renegotiation.js index 7e64710da52575..7b54509eb9b302 100644 --- a/test/parallel/test-tls-disable-renegotiation.js +++ b/test/parallel/test-tls-disable-renegotiation.js @@ -17,9 +17,11 @@ const options = { const server = tls.Server(options, common.mustCall((socket) => { socket.on('error', common.mustCall((err) => { - assert.strictEqual( - err.message, - 'TLS session renegotiation disabled for this socket'); + common.expectsError({ + code: 'ERR_TLS_RENEGOTIATION_DISABLED', + message: 'TLS session renegotiation disabled for this socket', + type: Error + })(error); socket.destroy(); server.close(); })); From 226c4af0275c9622a999ab8c31c70f591108e46b Mon Sep 17 00:00:00 2001 From: Mir Mufaqam Ali Date: Wed, 20 Dec 2017 20:55:09 +0530 Subject: [PATCH 2/6] lib/_tls_wrap.js: Linting and error assertion fixes This commit follows the previous commit. Tested Linting and added this commit Fixes: https://github.com/nodejs/node/issues/17709 --- lib/_tls_wrap.js | 3 +-- lib/internal/errors.js | 3 ++- test/parallel/test-tls-disable-renegotiation.js | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index 6d2c14c0bf27d8..07d7cb72987b2a 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -70,8 +70,7 @@ function onhandshakestart() { } if (owner[kDisableRenegotiation] && this.handshakes > 0) { - const err = new errors.Error('ERR_TLS_RENEGOTIATION_DISABLED'); - owner._emitTLSError(err); + owner._emitTLSError(new errors.Error('ERR_TLS_RENEGOTIATION_DISABLED')); } } diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 297ab496435bf6..8c8f4efda90c8f 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -470,11 +470,12 @@ E('ERR_TLS_CERT_ALTNAME_INVALID', 'Hostname/IP does not match certificate\'s altnames: %s'); E('ERR_TLS_DH_PARAM_SIZE', 'DH parameter size %s is less than 2048'); E('ERR_TLS_HANDSHAKE_TIMEOUT', 'TLS handshake timeout'); +E('ERR_TLS_RENEGOTIATION_DISABLED', + 'TLS session renegotiation disabled for this socket'); E('ERR_TLS_RENEGOTIATION_FAILED', 'Failed to renegotiate'); E('ERR_TLS_REQUIRED_SERVER_NAME', '"servername" is required parameter for Server.addContext'); E('ERR_TLS_SESSION_ATTACK', 'TLS session renegotiation attack detected'); -E('ERR_TLS_RENEGOTIATION_DISABLED', 'TLS session renegotiation disabled for this socket'); E('ERR_TRANSFORM_ALREADY_TRANSFORMING', 'Calling transform done when still transforming'); E('ERR_TRANSFORM_WITH_LENGTH_0', diff --git a/test/parallel/test-tls-disable-renegotiation.js b/test/parallel/test-tls-disable-renegotiation.js index 7b54509eb9b302..b688079a58af50 100644 --- a/test/parallel/test-tls-disable-renegotiation.js +++ b/test/parallel/test-tls-disable-renegotiation.js @@ -18,10 +18,10 @@ const options = { const server = tls.Server(options, common.mustCall((socket) => { socket.on('error', common.mustCall((err) => { common.expectsError({ + type: Error, code: 'ERR_TLS_RENEGOTIATION_DISABLED', - message: 'TLS session renegotiation disabled for this socket', - type: Error - })(error); + message: 'TLS session renegotiation disabled for this socket' + })(err); socket.destroy(); server.close(); })); From 7395aa4418da3063de52290442867531597440e2 Mon Sep 17 00:00:00 2001 From: Mir Mufaqam Ali Date: Wed, 20 Dec 2017 17:28:03 +0530 Subject: [PATCH 3/6] lib/_tls_wrap.js: Migrate errors to internal/errors.js This migrates the old style error messages to use internal/errors.js Fixes: https://github.com/nodejs/node/issues/17709 --- lib/internal/errors.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 8c8f4efda90c8f..402192f3b96c9b 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -476,6 +476,7 @@ E('ERR_TLS_RENEGOTIATION_FAILED', 'Failed to renegotiate'); E('ERR_TLS_REQUIRED_SERVER_NAME', '"servername" is required parameter for Server.addContext'); E('ERR_TLS_SESSION_ATTACK', 'TLS session renegotiation attack detected'); +E('ERR_TLS_RENEGOTIATION_DISABLED', 'TLS session renegotiation disabled for this socket'); E('ERR_TRANSFORM_ALREADY_TRANSFORMING', 'Calling transform done when still transforming'); E('ERR_TRANSFORM_WITH_LENGTH_0', From d8978416fef920272a449fa8894bc8c0aac0ed5e Mon Sep 17 00:00:00 2001 From: Mir Mufaqam Ali Date: Wed, 20 Dec 2017 20:55:09 +0530 Subject: [PATCH 4/6] lib/_tls_wrap.js: Linting and error assertion fixes This commit follows the previous commit. Tested Linting and added this commit Fixes: https://github.com/nodejs/node/issues/17709 --- lib/internal/errors.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 402192f3b96c9b..8c8f4efda90c8f 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -476,7 +476,6 @@ E('ERR_TLS_RENEGOTIATION_FAILED', 'Failed to renegotiate'); E('ERR_TLS_REQUIRED_SERVER_NAME', '"servername" is required parameter for Server.addContext'); E('ERR_TLS_SESSION_ATTACK', 'TLS session renegotiation attack detected'); -E('ERR_TLS_RENEGOTIATION_DISABLED', 'TLS session renegotiation disabled for this socket'); E('ERR_TRANSFORM_ALREADY_TRANSFORMING', 'Calling transform done when still transforming'); E('ERR_TRANSFORM_WITH_LENGTH_0', From 55dabafe1cfe111665cbffd64b26206eb3a1a04c Mon Sep 17 00:00:00 2001 From: Mir Mufaqam Ali Date: Fri, 22 Dec 2017 17:49:41 +0530 Subject: [PATCH 5/6] errors.md: Paraphrase the error message in case of the ERR_TLS_RENEGOTIATION_DISABLED This commit paraphrases the error message corresponding to the ERR_TLS_RENEGOTIATION_DISABLED key, so as to ensure consistency with the other error messages. Fixes: https://github.com/nodejs/node/issues/17709 --- doc/api/errors.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 362f7d14a40b98..dd817d3710bf2d 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1505,8 +1505,7 @@ vector for denial-of-service attacks. ### ERR_TLS_RENEGOTIATION_DISABLED -This errors is triggered when attempts are made to renegotiate TLS on a socket -instance which has TLS disabled +An attempt was made to renegotiate TLS on a socket instance with TLS disabled. ### ERR_TRANSFORM_ALREADY_TRANSFORMING From b18af9137c1afc26f7321aa36e1baa1979dbb2ea Mon Sep 17 00:00:00 2001 From: Mir Ali Date: Sat, 23 Dec 2017 16:10:11 +0530 Subject: [PATCH 6/6] api/errors.md: Remove extra space in the message Remove the extra space in the message corresponding to the key ERR_TLS_RENEGOTIATION_DISABLED . Fixes: https://github.com/nodejs/node/issues/17709 --- doc/api/errors.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index dd817d3710bf2d..59a9b979fc84f2 100644 --- a/doc/api/errors.md +++ b/doc/api/errors.md @@ -1505,7 +1505,7 @@ vector for denial-of-service attacks. ### ERR_TLS_RENEGOTIATION_DISABLED -An attempt was made to renegotiate TLS on a socket instance with TLS disabled. +An attempt was made to renegotiate TLS on a socket instance with TLS disabled. ### ERR_TRANSFORM_ALREADY_TRANSFORMING