From 8688bea3a12978144e27be135e5389101b05c335 Mon Sep 17 00:00:00 2001 From: Noah Gregory Date: Wed, 29 May 2019 15:54:15 -0400 Subject: [PATCH 01/10] change handling of user-set flags Originally, would only allow user-set flags that disable a flag enabled by default. Now it will also allow user-set flags that enable a flag that is disabled by default. --- lib/ConnectionConfig.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ConnectionConfig.js b/lib/ConnectionConfig.js index 06f4399c5..be7174df7 100644 --- a/lib/ConnectionConfig.js +++ b/lib/ConnectionConfig.js @@ -69,7 +69,7 @@ ConnectionConfig.mergeFlags = function mergeFlags(defaultFlags, userFlags) { // Merge the new flags for (var flag in newFlags) { - if (allFlags[flag] !== false) { + if (allFlags[flag] !== newFlags[flag]) { allFlags[flag] = newFlags[flag]; } } From cced61f08552dd1304d082f712580195db2ccba1 Mon Sep 17 00:00:00 2001 From: Noah Gregory Date: Wed, 29 May 2019 15:56:49 -0400 Subject: [PATCH 02/10] add support for 'mysql_clear_password' auth plugin --- lib/protocol/Auth.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/protocol/Auth.js b/lib/protocol/Auth.js index a1033d1b9..d2617c31a 100644 --- a/lib/protocol/Auth.js +++ b/lib/protocol/Auth.js @@ -8,6 +8,8 @@ function auth(name, data, options) { switch (name) { case 'mysql_native_password': return Auth.token(options.password, data.slice(0, 20)); + case 'mysql_clear_password': + return Buffer.from(options.password); default: return undefined; } From fe3cef274f7c555443c3c167571bf4b652a58f84 Mon Sep 17 00:00:00 2001 From: Noah Gregory Date: Thu, 30 May 2019 20:36:35 -0400 Subject: [PATCH 03/10] add tests --- test/integration/connection/test-clear-auth-bad.js | 6 ++++++ test/integration/connection/test-clear-auth.js | 7 +++++++ 2 files changed, 13 insertions(+) create mode 100644 test/integration/connection/test-clear-auth-bad.js create mode 100644 test/integration/connection/test-clear-auth.js diff --git a/test/integration/connection/test-clear-auth-bad.js b/test/integration/connection/test-clear-auth-bad.js new file mode 100644 index 000000000..f6e7b9bc3 --- /dev/null +++ b/test/integration/connection/test-clear-auth-bad.js @@ -0,0 +1,6 @@ +var assert = require('assert'); +var common = require('../../common'); + +common.getTestConnection({ flags: ['-PLUGIN_AUTH'] }, function (err) { + assert.ok(err, 'got error'); +}); diff --git a/test/integration/connection/test-clear-auth.js b/test/integration/connection/test-clear-auth.js new file mode 100644 index 000000000..46c9cc808 --- /dev/null +++ b/test/integration/connection/test-clear-auth.js @@ -0,0 +1,7 @@ +var assert = require('assert'); +var common = require('../../common'); + +common.getTestConnection({ flags: ['+PLUGIN_AUTH'] }, function (err, connection) { + assert.ifError(err, 'got error'); + connection.destroy(); +}); \ No newline at end of file From 9fb9c88ac0293e3e6190bae180b3ff9fd264cb1b Mon Sep 17 00:00:00 2001 From: Noah Gregory Date: Sun, 2 Jun 2019 21:28:26 -0400 Subject: [PATCH 04/10] Delete test as it only passes for specific servers --- test/integration/connection/test-clear-auth-bad.js | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 test/integration/connection/test-clear-auth-bad.js diff --git a/test/integration/connection/test-clear-auth-bad.js b/test/integration/connection/test-clear-auth-bad.js deleted file mode 100644 index f6e7b9bc3..000000000 --- a/test/integration/connection/test-clear-auth-bad.js +++ /dev/null @@ -1,6 +0,0 @@ -var assert = require('assert'); -var common = require('../../common'); - -common.getTestConnection({ flags: ['-PLUGIN_AUTH'] }, function (err) { - assert.ok(err, 'got error'); -}); From 56428ebf0f8dd6da17d7b37bfc50ff25adf3a165 Mon Sep 17 00:00:00 2001 From: Ziggy Jonsson Date: Sat, 28 Mar 2020 17:01:31 -0400 Subject: [PATCH 05/10] Add unit test --- .../unit/connection/test-auth-switch-clear.js | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 test/unit/connection/test-auth-switch-clear.js diff --git a/test/unit/connection/test-auth-switch-clear.js b/test/unit/connection/test-auth-switch-clear.js new file mode 100644 index 000000000..4a2a8291e --- /dev/null +++ b/test/unit/connection/test-auth-switch-clear.js @@ -0,0 +1,41 @@ +var assert = require('assert'); +var common = require('../../common'); +var connection = common.createConnection({ + port : common.fakeServerPort, + password : 'authswitch' +}); + +var server = common.createFakeServer(); + +var connected; +server.listen(common.fakeServerPort, function (err) { + assert.ifError(err); + + connection.connect(function (err, result) { + assert.ifError(err); + + connected = result; + + connection.destroy(); + server.destroy(); + }); +}); + +server.on('connection', function(incomingConnection) { + incomingConnection.on('authSwitchResponse', function (packet) { + this._sendAuthResponse(packet.data, Buffer.from('authswitch')); + }); + + incomingConnection.on('clientAuthentication', function () { + this.authSwitchRequest({ + authMethodName : 'mysql_clear_password', + authMethodData : Buffer.alloc(0) + }); + }); + + incomingConnection.handshake(); +}); + +process.on('exit', function() { + assert.equal(connected.fieldCount, 0); +}); From 6a785ae95ac517e0e46a724201177ffa0477d876 Mon Sep 17 00:00:00 2001 From: Ziggy Jonsson Date: Sat, 28 Mar 2020 18:19:19 -0400 Subject: [PATCH 06/10] Ensure that `mysql_clear_password` is never used over insecure connections. --- lib/protocol/Auth.js | 8 +++- lib/protocol/sequences/Handshake.js | 14 +++++-- test/FakeServer.js | 2 +- .../test-auth-switch-clear-insecure.js | 38 +++++++++++++++++++ ...ar.js => test-auth-switch-clear-secure.js} | 5 ++- 5 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 test/unit/connection/test-auth-switch-clear-insecure.js rename test/unit/connection/{test-auth-switch-clear.js => test-auth-switch-clear-secure.js} (92%) diff --git a/lib/protocol/Auth.js b/lib/protocol/Auth.js index d2617c31a..c8c50e7e1 100644 --- a/lib/protocol/Auth.js +++ b/lib/protocol/Auth.js @@ -2,14 +2,18 @@ var Buffer = require('safe-buffer').Buffer; var Crypto = require('crypto'); var Auth = exports; -function auth(name, data, options) { +function auth(name, data, options, isSecure) { options = options || {}; switch (name) { case 'mysql_native_password': return Auth.token(options.password, data.slice(0, 20)); case 'mysql_clear_password': - return Buffer.from(options.password); + if (!isSecure) { + throw new Error('Authentication method mysql_clear_password not supported on insecure connections'); + } else { + return Buffer.from(options.password); + } default: return undefined; } diff --git a/lib/protocol/sequences/Handshake.js b/lib/protocol/sequences/Handshake.js index 8fad0fcf3..5297d804f 100644 --- a/lib/protocol/sequences/Handshake.js +++ b/lib/protocol/sequences/Handshake.js @@ -35,16 +35,21 @@ Handshake.prototype.determinePacket = function determinePacket(firstByte, parser Handshake.prototype['AuthSwitchRequestPacket'] = function (packet) { var name = packet.authMethodName; - var data = Auth.auth(name, packet.authMethodData, { - password: this._config.password - }); + var data, error; + try { + data = Auth.auth(name, packet.authMethodData, { + password: this._config.password + }, this._tls); + } catch (e) { + error = e; + } if (data !== undefined) { this.emit('packet', new Packets.AuthSwitchResponsePacket({ data: data })); } else { - var err = new Error('MySQL is requesting the ' + name + ' authentication method, which is not supported.'); + var err = error || new Error('MySQL is requesting the ' + name + ' authentication method, which is not supported.'); err.code = 'UNSUPPORTED_AUTH_METHOD'; err.fatal = true; this.end(err); @@ -82,6 +87,7 @@ Handshake.prototype['HandshakeInitializationPacket'] = function(packet) { }; Handshake.prototype._tlsUpgradeCompleteHandler = function() { + this._tls = true; this._sendCredentials(); }; diff --git a/test/FakeServer.js b/test/FakeServer.js index 28f67f257..045677d5b 100644 --- a/test/FakeServer.js +++ b/test/FakeServer.js @@ -95,7 +95,7 @@ FakeConnection.prototype.handshake = function(options) { var packetOptions = common.extend({ scrambleBuff1 : Buffer.from('1020304050607080', 'hex'), scrambleBuff2 : Buffer.from('0102030405060708090A0B0C', 'hex'), - serverCapabilities1 : 512, // only 1 flag, PROTOCOL_41 + serverCapabilities1 : 512 | 1 << 11, // only 2 flags, PROTOCOL_41 and SSL protocol41 : true }, this._handshakeOptions); diff --git a/test/unit/connection/test-auth-switch-clear-insecure.js b/test/unit/connection/test-auth-switch-clear-insecure.js new file mode 100644 index 000000000..c054331bb --- /dev/null +++ b/test/unit/connection/test-auth-switch-clear-insecure.js @@ -0,0 +1,38 @@ +var assert = require('assert'); +var common = require('../../common'); +var connection = common.createConnection({ + port : common.fakeServerPort, + password : 'authswitch' +}); + +var server = common.createFakeServer(); + +var error; +server.listen(common.fakeServerPort, function (err) { + assert.ifError(err); + + connection.connect(function (err) { + error = err; + connection.destroy(); + server.destroy(); + }); +}); + +server.on('connection', function(incomingConnection) { + incomingConnection.on('authSwitchResponse', function (packet) { + this._sendAuthResponse(packet.data, Buffer.from('authswitch')); + }); + + incomingConnection.on('clientAuthentication', function () { + this.authSwitchRequest({ + authMethodName : 'mysql_clear_password', + authMethodData : Buffer.alloc(0) + }); + }); + + incomingConnection.handshake(); +}); + +process.on('exit', function() { + assert.equal(error.message, 'Authentication method mysql_clear_password not supported on insecure connections'); +}); diff --git a/test/unit/connection/test-auth-switch-clear.js b/test/unit/connection/test-auth-switch-clear-secure.js similarity index 92% rename from test/unit/connection/test-auth-switch-clear.js rename to test/unit/connection/test-auth-switch-clear-secure.js index 4a2a8291e..47b51f3af 100644 --- a/test/unit/connection/test-auth-switch-clear.js +++ b/test/unit/connection/test-auth-switch-clear-secure.js @@ -2,7 +2,10 @@ var assert = require('assert'); var common = require('../../common'); var connection = common.createConnection({ port : common.fakeServerPort, - password : 'authswitch' + password : 'authswitch', + ssl : { + rejectUnauthorized: false + } }); var server = common.createFakeServer(); From 65368e4882f7ad12159c68bd114bfb65d075ae4a Mon Sep 17 00:00:00 2001 From: Ziggy Jonsson Date: Sat, 28 Mar 2020 20:19:20 -0400 Subject: [PATCH 07/10] Rename this._tls to this.secureConnection and fold into options Allow insecure cleartext if options.insecureAuth == true --- lib/protocol/Auth.js | 4 ++-- lib/protocol/sequences/Handshake.js | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/protocol/Auth.js b/lib/protocol/Auth.js index c8c50e7e1..45a786edc 100644 --- a/lib/protocol/Auth.js +++ b/lib/protocol/Auth.js @@ -2,14 +2,14 @@ var Buffer = require('safe-buffer').Buffer; var Crypto = require('crypto'); var Auth = exports; -function auth(name, data, options, isSecure) { +function auth(name, data, options) { options = options || {}; switch (name) { case 'mysql_native_password': return Auth.token(options.password, data.slice(0, 20)); case 'mysql_clear_password': - if (!isSecure) { + if (!options.secureConnection && !options.insecureAuth) { throw new Error('Authentication method mysql_clear_password not supported on insecure connections'); } else { return Buffer.from(options.password); diff --git a/lib/protocol/sequences/Handshake.js b/lib/protocol/sequences/Handshake.js index 5297d804f..75a89e6ba 100644 --- a/lib/protocol/sequences/Handshake.js +++ b/lib/protocol/sequences/Handshake.js @@ -38,8 +38,10 @@ Handshake.prototype['AuthSwitchRequestPacket'] = function (packet) { var data, error; try { data = Auth.auth(name, packet.authMethodData, { - password: this._config.password - }, this._tls); + password : this._config.password, + insecureAuth : this._config.insecureAuth, + secureConnection : this._secureConnection + }); } catch (e) { error = e; } @@ -87,7 +89,7 @@ Handshake.prototype['HandshakeInitializationPacket'] = function(packet) { }; Handshake.prototype._tlsUpgradeCompleteHandler = function() { - this._tls = true; + this._secureConnection = true; this._sendCredentials(); }; From 810dfb0e7c346b3ca7ce029042003448e581de1e Mon Sep 17 00:00:00 2001 From: Ziggy Jonsson Date: Sat, 28 Mar 2020 20:24:00 -0400 Subject: [PATCH 08/10] * Revert generic SSL flag * Specific SSL flag set for clear pwd tests --- test/FakeServer.js | 2 +- test/unit/connection/test-auth-switch-clear-secure.js | 4 +++- test/unit/connection/test-auth-switch-native.js | 4 +++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/test/FakeServer.js b/test/FakeServer.js index 045677d5b..28f67f257 100644 --- a/test/FakeServer.js +++ b/test/FakeServer.js @@ -95,7 +95,7 @@ FakeConnection.prototype.handshake = function(options) { var packetOptions = common.extend({ scrambleBuff1 : Buffer.from('1020304050607080', 'hex'), scrambleBuff2 : Buffer.from('0102030405060708090A0B0C', 'hex'), - serverCapabilities1 : 512 | 1 << 11, // only 2 flags, PROTOCOL_41 and SSL + serverCapabilities1 : 512, // only 1 flag, PROTOCOL_41 protocol41 : true }, this._handshakeOptions); diff --git a/test/unit/connection/test-auth-switch-clear-secure.js b/test/unit/connection/test-auth-switch-clear-secure.js index 47b51f3af..edb247a19 100644 --- a/test/unit/connection/test-auth-switch-clear-secure.js +++ b/test/unit/connection/test-auth-switch-clear-secure.js @@ -36,7 +36,9 @@ server.on('connection', function(incomingConnection) { }); }); - incomingConnection.handshake(); + incomingConnection.handshake({ + serverCapabilities1: 512 | 1 << 11 // PROTOCOL_41 and SSL + }); }); process.on('exit', function() { diff --git a/test/unit/connection/test-auth-switch-native.js b/test/unit/connection/test-auth-switch-native.js index 0d31dccb5..6c486f1ae 100644 --- a/test/unit/connection/test-auth-switch-native.js +++ b/test/unit/connection/test-auth-switch-native.js @@ -38,7 +38,9 @@ server.on('connection', function(incomingConnection) { }); }); - incomingConnection.handshake(); + incomingConnection.handshake({ + serverCapabilities1: 512 | 1 << 11 // PROTOCOL_41 and SSL + }); }); }); From 63f6bfe6e5679bee5fc43a64267cc526087edddd Mon Sep 17 00:00:00 2001 From: Ziggy Jonsson Date: Sat, 28 Mar 2020 20:27:17 -0400 Subject: [PATCH 09/10] Revert flag merging to use blacklist --- lib/ConnectionConfig.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ConnectionConfig.js b/lib/ConnectionConfig.js index be7174df7..06f4399c5 100644 --- a/lib/ConnectionConfig.js +++ b/lib/ConnectionConfig.js @@ -69,7 +69,7 @@ ConnectionConfig.mergeFlags = function mergeFlags(defaultFlags, userFlags) { // Merge the new flags for (var flag in newFlags) { - if (allFlags[flag] !== newFlags[flag]) { + if (allFlags[flag] !== false) { allFlags[flag] = newFlags[flag]; } } From 6753e1bba6adc9125b2afc1e253f97fd7c045e5b Mon Sep 17 00:00:00 2001 From: Ziggy Jonsson Date: Sun, 29 Mar 2020 06:38:49 -0400 Subject: [PATCH 10/10] use safe-buffer (compatibility with older node versions) --- test/unit/connection/test-auth-switch-clear-insecure.js | 1 + test/unit/connection/test-auth-switch-clear-secure.js | 1 + 2 files changed, 2 insertions(+) diff --git a/test/unit/connection/test-auth-switch-clear-insecure.js b/test/unit/connection/test-auth-switch-clear-insecure.js index c054331bb..b8fc20f8e 100644 --- a/test/unit/connection/test-auth-switch-clear-insecure.js +++ b/test/unit/connection/test-auth-switch-clear-insecure.js @@ -1,4 +1,5 @@ var assert = require('assert'); +var Buffer = require('safe-buffer').Buffer; var common = require('../../common'); var connection = common.createConnection({ port : common.fakeServerPort, diff --git a/test/unit/connection/test-auth-switch-clear-secure.js b/test/unit/connection/test-auth-switch-clear-secure.js index edb247a19..08f420eda 100644 --- a/test/unit/connection/test-auth-switch-clear-secure.js +++ b/test/unit/connection/test-auth-switch-clear-secure.js @@ -1,4 +1,5 @@ var assert = require('assert'); +var Buffer = require('safe-buffer').Buffer; var common = require('../../common'); var connection = common.createConnection({ port : common.fakeServerPort,