From dbe7dc11852cbae6764ec77d2199cd1f91eb16e7 Mon Sep 17 00:00:00 2001 From: Yuriy Date: Thu, 5 Dec 2019 02:16:11 +0300 Subject: [PATCH 01/10] using SHA1 MySQL auth plugin for plaintext passwords --- dbms/programs/server/MySQLHandler.cpp | 3 ++- dbms/src/Access/Authentication.cpp | 6 +++++- dbms/src/Core/MySQLProtocol.h | 5 +++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/dbms/programs/server/MySQLHandler.cpp b/dbms/programs/server/MySQLHandler.cpp index c2de9eb74e0..6bab1724406 100644 --- a/dbms/programs/server/MySQLHandler.cpp +++ b/dbms/programs/server/MySQLHandler.cpp @@ -221,7 +221,8 @@ void MySQLHandler::authenticate(const String & user_name, const String & auth_pl { // For compatibility with JavaScript MySQL client, Native41 authentication plugin is used when possible (if password is specified using double SHA1). Otherwise SHA256 plugin is used. auto user = connection_context.getUser(user_name); - if (user->authentication.getType() != DB::Authentication::DOUBLE_SHA1_PASSWORD) + const DB::Authentication::Type user_auth_type = user->authentication.getType() + if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD) { authPluginSSL(); } diff --git a/dbms/src/Access/Authentication.cpp b/dbms/src/Access/Authentication.cpp index 5b641e2906e..ed3635decfd 100644 --- a/dbms/src/Access/Authentication.cpp +++ b/dbms/src/Access/Authentication.cpp @@ -168,7 +168,11 @@ bool Authentication::isCorrectPassword(const String & password_) const return true; case PLAINTEXT_PASSWORD: - return password_ == StringRef{reinterpret_cast(password_hash.data()), password_hash.size()}; + if (password_ == StringRef{reinterpret_cast(password_hash.data()), password_hash.size()}) + return true; + else + // For compatibility with MySQL clients which support only native authentication plugin, SHA1 can be passed instead of password. + return password_ == encodeSHA1(password_hash); case SHA256_PASSWORD: return encodeSHA256(password_) == password_hash; diff --git a/dbms/src/Core/MySQLProtocol.h b/dbms/src/Core/MySQLProtocol.h index db7a8dae2fa..7758096343f 100644 --- a/dbms/src/Core/MySQLProtocol.h +++ b/dbms/src/Core/MySQLProtocol.h @@ -953,8 +953,9 @@ public: auto user = context.getUser(user_name); - if (user->authentication.getType() != DB::Authentication::DOUBLE_SHA1_PASSWORD) - throw Exception("Cannot use " + getName() + " auth plugin for user " + user_name + " since its password isn't specified using double SHA1.", ErrorCodes::UNKNOWN_EXCEPTION); + DB::Authentication user_auth_type = user->authentication.getType(); + if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD) + throw Exception("Cannot use " + getName() + " auth plugin for user " + user_name + " since its password isn't specified using double SHA1 or plaintext.", ErrorCodes::UNKNOWN_EXCEPTION); Poco::SHA1Engine::Digest double_sha1_value = user->authentication.getPasswordHashBinary(); assert(double_sha1_value.size() == Poco::SHA1Engine::DIGEST_SIZE); From 9d0a0d0db08a8d112b4b2e2836c58338cc0f20a1 Mon Sep 17 00:00:00 2001 From: Yuriy Date: Thu, 5 Dec 2019 02:32:17 +0300 Subject: [PATCH 02/10] fixed type --- dbms/src/Core/MySQLProtocol.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Core/MySQLProtocol.h b/dbms/src/Core/MySQLProtocol.h index 7758096343f..6436f1f10b2 100644 --- a/dbms/src/Core/MySQLProtocol.h +++ b/dbms/src/Core/MySQLProtocol.h @@ -953,7 +953,7 @@ public: auto user = context.getUser(user_name); - DB::Authentication user_auth_type = user->authentication.getType(); + const DB::Authentication::Type user_auth_type = user->authentication.getType(); if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD) throw Exception("Cannot use " + getName() + " auth plugin for user " + user_name + " since its password isn't specified using double SHA1 or plaintext.", ErrorCodes::UNKNOWN_EXCEPTION); From 564b58d2f6261e69e6bdd6d4b12dda431adffff1 Mon Sep 17 00:00:00 2001 From: Yuriy Date: Thu, 5 Dec 2019 02:37:11 +0300 Subject: [PATCH 03/10] using mysql_native_password for passwordless users --- dbms/programs/server/MySQLHandler.cpp | 2 +- dbms/src/Core/MySQLProtocol.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dbms/programs/server/MySQLHandler.cpp b/dbms/programs/server/MySQLHandler.cpp index 6bab1724406..1092a8c1471 100644 --- a/dbms/programs/server/MySQLHandler.cpp +++ b/dbms/programs/server/MySQLHandler.cpp @@ -222,7 +222,7 @@ void MySQLHandler::authenticate(const String & user_name, const String & auth_pl // For compatibility with JavaScript MySQL client, Native41 authentication plugin is used when possible (if password is specified using double SHA1). Otherwise SHA256 plugin is used. auto user = connection_context.getUser(user_name); const DB::Authentication::Type user_auth_type = user->authentication.getType() - if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD) + if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD && user_auth_type != DB::Authentication::NO_PASSWORD) { authPluginSSL(); } diff --git a/dbms/src/Core/MySQLProtocol.h b/dbms/src/Core/MySQLProtocol.h index 6436f1f10b2..39ec02a6a1c 100644 --- a/dbms/src/Core/MySQLProtocol.h +++ b/dbms/src/Core/MySQLProtocol.h @@ -954,7 +954,7 @@ public: auto user = context.getUser(user_name); const DB::Authentication::Type user_auth_type = user->authentication.getType(); - if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD) + if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD && user_auth_type != DB::Authentication::NO_PASSWORD) throw Exception("Cannot use " + getName() + " auth plugin for user " + user_name + " since its password isn't specified using double SHA1 or plaintext.", ErrorCodes::UNKNOWN_EXCEPTION); Poco::SHA1Engine::Digest double_sha1_value = user->authentication.getPasswordHashBinary(); From 4b170ae7693e13fac7802e3b524b5a21d3cf4a64 Mon Sep 17 00:00:00 2001 From: Yuriy Date: Thu, 5 Dec 2019 03:19:36 +0300 Subject: [PATCH 04/10] fixed compilation error --- dbms/src/Access/Authentication.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dbms/src/Access/Authentication.cpp b/dbms/src/Access/Authentication.cpp index ed3635decfd..d8d181c5421 100644 --- a/dbms/src/Access/Authentication.cpp +++ b/dbms/src/Access/Authentication.cpp @@ -168,12 +168,14 @@ bool Authentication::isCorrectPassword(const String & password_) const return true; case PLAINTEXT_PASSWORD: + { if (password_ == StringRef{reinterpret_cast(password_hash.data()), password_hash.size()}) return true; - else - // For compatibility with MySQL clients which support only native authentication plugin, SHA1 can be passed instead of password. - return password_ == encodeSHA1(password_hash); + // For compatibility with MySQL clients which support only native authentication plugin, SHA1 can be passed instead of password. + auto password_sha1 = encodeSHA1(password_hash); + return password_ == StringRef{reinterpret_cast(password_hash.data()), password_hash.size()}; + } case SHA256_PASSWORD: return encodeSHA256(password_) == password_hash; From 9a222c6a2a3d29f4349427c074ba0f18c392749a Mon Sep 17 00:00:00 2001 From: Yuriy Date: Thu, 5 Dec 2019 03:23:12 +0300 Subject: [PATCH 05/10] compare with proper variable --- dbms/src/Access/Authentication.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Access/Authentication.cpp b/dbms/src/Access/Authentication.cpp index d8d181c5421..8c9990e394a 100644 --- a/dbms/src/Access/Authentication.cpp +++ b/dbms/src/Access/Authentication.cpp @@ -174,7 +174,7 @@ bool Authentication::isCorrectPassword(const String & password_) const // For compatibility with MySQL clients which support only native authentication plugin, SHA1 can be passed instead of password. auto password_sha1 = encodeSHA1(password_hash); - return password_ == StringRef{reinterpret_cast(password_hash.data()), password_hash.size()}; + return password_ == StringRef{reinterpret_cast(password_sha1.data()), password_sha1.size()}; } case SHA256_PASSWORD: return encodeSHA256(password_) == password_hash; From 67d72f35b981041aabc07fdb9d66ce6618057a8b Mon Sep 17 00:00:00 2001 From: Yuriy Baranov Date: Thu, 5 Dec 2019 03:35:52 +0300 Subject: [PATCH 06/10] Update Authentication.cpp --- dbms/src/Access/Authentication.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/dbms/src/Access/Authentication.cpp b/dbms/src/Access/Authentication.cpp index 8c9990e394a..5921e289e77 100644 --- a/dbms/src/Access/Authentication.cpp +++ b/dbms/src/Access/Authentication.cpp @@ -176,6 +176,7 @@ bool Authentication::isCorrectPassword(const String & password_) const auto password_sha1 = encodeSHA1(password_hash); return password_ == StringRef{reinterpret_cast(password_sha1.data()), password_sha1.size()}; } + case SHA256_PASSWORD: return encodeSHA256(password_) == password_hash; From 1df4250b2f335b84bf61094112fae9dee402d644 Mon Sep 17 00:00:00 2001 From: Yuriy Baranov Date: Thu, 5 Dec 2019 04:52:48 +0300 Subject: [PATCH 07/10] Update MySQLHandler.cpp --- dbms/programs/server/MySQLHandler.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/programs/server/MySQLHandler.cpp b/dbms/programs/server/MySQLHandler.cpp index 1092a8c1471..a147ccafba0 100644 --- a/dbms/programs/server/MySQLHandler.cpp +++ b/dbms/programs/server/MySQLHandler.cpp @@ -221,7 +221,7 @@ void MySQLHandler::authenticate(const String & user_name, const String & auth_pl { // For compatibility with JavaScript MySQL client, Native41 authentication plugin is used when possible (if password is specified using double SHA1). Otherwise SHA256 plugin is used. auto user = connection_context.getUser(user_name); - const DB::Authentication::Type user_auth_type = user->authentication.getType() + const DB::Authentication::Type user_auth_type = user->authentication.getType(); if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD && user_auth_type != DB::Authentication::NO_PASSWORD) { authPluginSSL(); From e91d4722a458f2d7289b7b5e72acfe1d03e364fe Mon Sep 17 00:00:00 2001 From: Yuriy Date: Fri, 6 Dec 2019 04:26:28 +0300 Subject: [PATCH 08/10] fixed occasional federated server test failures --- .../clients/mysql/docker_compose.yml | 5 +++ .../integration/test_mysql_protocol/test.py | 32 +++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/dbms/tests/integration/test_mysql_protocol/clients/mysql/docker_compose.yml b/dbms/tests/integration/test_mysql_protocol/clients/mysql/docker_compose.yml index 6e0558208e2..752e59a2d08 100644 --- a/dbms/tests/integration/test_mysql_protocol/clients/mysql/docker_compose.yml +++ b/dbms/tests/integration/test_mysql_protocol/clients/mysql/docker_compose.yml @@ -6,3 +6,8 @@ services: environment: MYSQL_ALLOW_EMPTY_PASSWORD: 1 command: --federated --socket /var/run/mysqld/mysqld.sock + healthcheck: + test: ["CMD", "mysqladmin" ,"ping", "-h", "localhost"] + interval: 1s + timeout: 2s + retries: 100 diff --git a/dbms/tests/integration/test_mysql_protocol/test.py b/dbms/tests/integration/test_mysql_protocol/test.py index d1ea106a70e..9b743ef6d6f 100644 --- a/dbms/tests/integration/test_mysql_protocol/test.py +++ b/dbms/tests/integration/test_mysql_protocol/test.py @@ -1,9 +1,10 @@ # coding: utf-8 -import os import docker +import os import pytest import subprocess +import time import pymysql.connections from docker.models.containers import Container @@ -36,6 +37,25 @@ def mysql_client(): yield docker.from_env().containers.get(cluster.project_name + '_mysql1_1') +@pytest.fixture(scope='module') +def mysql_server(mysql_client): + """Return MySQL container when it is healthy. + + :type mysql_client: Container + :rtype: Container + """ + retries = 30 + for i in range(retries): + info = mysql_client.client.api.inspect_container(mysql_client.name) + if info['State']['Health']['Status'] == 'healthy': + break + time.sleep(1) + else: + raise Exception('Mysql server is not started in %d seconds.' % retries) + + return mysql_client + + @pytest.fixture(scope='module') def golang_container(): docker_compose = os.path.join(SCRIPT_DIR, 'clients', 'golang', 'docker_compose.yml') @@ -108,14 +128,14 @@ def test_mysql_client(mysql_client, server_address): assert stdout == '\n'.join(['column', '0', '0', '1', '1', '5', '5', 'tmp_column', '0', '1', '']) -def test_mysql_federated(mysql_client, server_address): + +def test_mysql_federated(mysql_server, server_address): node.query('''DROP DATABASE IF EXISTS mysql_federated''', settings={"password": "123"}) node.query('''CREATE DATABASE mysql_federated''', settings={"password": "123"}) node.query('''CREATE TABLE mysql_federated.test (col UInt32) ENGINE = Log''', settings={"password": "123"}) node.query('''INSERT INTO mysql_federated.test VALUES (0), (1), (5)''', settings={"password": "123"}) - - code, (_, stderr) = mysql_client.exec_run(''' + code, (_, stderr) = mysql_server.exec_run(''' mysql -e "DROP SERVER IF EXISTS clickhouse;" -e "CREATE SERVER clickhouse FOREIGN DATA WRAPPER mysql OPTIONS (USER 'default', PASSWORD '123', HOST '{host}', PORT {port}, DATABASE 'mysql_federated');" @@ -125,7 +145,7 @@ def test_mysql_federated(mysql_client, server_address): assert code == 0 - code, (stdout, stderr) = mysql_client.exec_run(''' + code, (stdout, stderr) = mysql_server.exec_run(''' mysql -e "CREATE TABLE mysql_federated.test(`col` int UNSIGNED) ENGINE=FEDERATED CONNECTION='clickhouse';" -e "SELECT * FROM mysql_federated.test ORDER BY col;" @@ -133,7 +153,7 @@ def test_mysql_federated(mysql_client, server_address): assert stdout == '\n'.join(['col', '0', '1', '5', '']) - code, (stdout, stderr) = mysql_client.exec_run(''' + code, (stdout, stderr) = mysql_server.exec_run(''' mysql -e "INSERT INTO mysql_federated.test VALUES (0), (1), (5);" -e "SELECT * FROM mysql_federated.test ORDER BY col;" From 6c8e2d8b8594a2441db2ddb32a85bbc3afbdb285 Mon Sep 17 00:00:00 2001 From: Yuriy Date: Fri, 6 Dec 2019 04:30:01 +0300 Subject: [PATCH 09/10] fixed getting double SHA1 in mysql_native_password auth plugin --- dbms/src/Access/Authentication.cpp | 29 +++++++++++++++++++ dbms/src/Access/Authentication.h | 4 +++ dbms/src/Core/MySQLProtocol.h | 6 +--- .../test_mysql_protocol/configs/users.xml | 10 +++++++ .../integration/test_mysql_protocol/test.py | 5 ++-- 5 files changed, 46 insertions(+), 8 deletions(-) diff --git a/dbms/src/Access/Authentication.cpp b/dbms/src/Access/Authentication.cpp index 5921e289e77..88738dfc837 100644 --- a/dbms/src/Access/Authentication.cpp +++ b/dbms/src/Access/Authentication.cpp @@ -160,6 +160,35 @@ void Authentication::setPasswordHashBinary(const Digest & hash) } +Digest Authentication::getPasswordDoubleSHA1() const +{ + switch (type) + { + case NO_PASSWORD: + { + Poco::SHA1Engine engine; + return engine.digest(); + } + + case PLAINTEXT_PASSWORD: + { + Poco::SHA1Engine engine; + engine.update(getPassword()); + const Digest & first_sha1 = engine.digest(); + engine.update(first_sha1.data(), first_sha1.size()); + return engine.digest(); + } + + case SHA256_PASSWORD: + throw Exception("Cannot get password double SHA1 for user with 'SHA256_PASSWORD' authentication.", ErrorCodes::BAD_ARGUMENTS); + + case DOUBLE_SHA1_PASSWORD: + return password_hash; + } + throw Exception("Unknown authentication type: " + std::to_string(static_cast(type)), ErrorCodes::LOGICAL_ERROR); +} + + bool Authentication::isCorrectPassword(const String & password_) const { switch (type) diff --git a/dbms/src/Access/Authentication.h b/dbms/src/Access/Authentication.h index d8fae6e03eb..dc528fdedb8 100644 --- a/dbms/src/Access/Authentication.h +++ b/dbms/src/Access/Authentication.h @@ -49,6 +49,10 @@ public: void setPasswordHashBinary(const Digest & hash); const Digest & getPasswordHashBinary() const { return password_hash; } + /// Returns SHA1(SHA1(password)) used by MySQL compatibility server for authentication. + /// Allowed to use for Type::NO_PASSWORD, Type::PLAINTEXT_PASSWORD, Type::DOUBLE_SHA1_PASSWORD. + Digest getPasswordDoubleSHA1() const; + /// Checks if the provided password is correct. Returns false if not. bool isCorrectPassword(const String & password) const; diff --git a/dbms/src/Core/MySQLProtocol.h b/dbms/src/Core/MySQLProtocol.h index 39ec02a6a1c..25ce6213641 100644 --- a/dbms/src/Core/MySQLProtocol.h +++ b/dbms/src/Core/MySQLProtocol.h @@ -953,11 +953,7 @@ public: auto user = context.getUser(user_name); - const DB::Authentication::Type user_auth_type = user->authentication.getType(); - if (user_auth_type != DB::Authentication::DOUBLE_SHA1_PASSWORD && user_auth_type != DB::Authentication::PLAINTEXT_PASSWORD && user_auth_type != DB::Authentication::NO_PASSWORD) - throw Exception("Cannot use " + getName() + " auth plugin for user " + user_name + " since its password isn't specified using double SHA1 or plaintext.", ErrorCodes::UNKNOWN_EXCEPTION); - - Poco::SHA1Engine::Digest double_sha1_value = user->authentication.getPasswordHashBinary(); + Poco::SHA1Engine::Digest double_sha1_value = user->authentication.getPasswordDoubleSHA1(); assert(double_sha1_value.size() == Poco::SHA1Engine::DIGEST_SIZE); Poco::SHA1Engine engine; diff --git a/dbms/tests/integration/test_mysql_protocol/configs/users.xml b/dbms/tests/integration/test_mysql_protocol/configs/users.xml index ebcd1a297e1..b88dfbada37 100644 --- a/dbms/tests/integration/test_mysql_protocol/configs/users.xml +++ b/dbms/tests/integration/test_mysql_protocol/configs/users.xml @@ -15,6 +15,16 @@ default + + + 65e84be33532fb784c48129675f9eff3a682b27168c0ea744b2cf58ee02337c5 + + ::/0 + + default + default + + e395796d6546b1b65db9d665cd43f0e858dd4303 diff --git a/dbms/tests/integration/test_mysql_protocol/test.py b/dbms/tests/integration/test_mysql_protocol/test.py index 9b743ef6d6f..7b8d67f3cdf 100644 --- a/dbms/tests/integration/test_mysql_protocol/test.py +++ b/dbms/tests/integration/test_mysql_protocol/test.py @@ -250,13 +250,12 @@ def test_php_client(server_address, php_container): def test_mysqljs_client(server_address, nodejs_container): - code, (_, stderr) = nodejs_container.exec_run('node test.js {host} {port} default 123'.format(host=server_address, port=server_port), demux=True) + code, (_, stderr) = nodejs_container.exec_run('node test.js {host} {port} user_with_sha256 abacaba'.format(host=server_address, port=server_port), demux=True) assert code == 1 assert 'MySQL is requesting the sha256_password authentication method, which is not supported.' in stderr code, (_, stderr) = nodejs_container.exec_run('node test.js {host} {port} user_with_empty_password ""'.format(host=server_address, port=server_port), demux=True) - assert code == 1 - assert 'MySQL is requesting the sha256_password authentication method, which is not supported.' in stderr + assert code == 0 code, (_, _) = nodejs_container.exec_run('node test.js {host} {port} user_with_double_sha1 abacaba'.format(host=server_address, port=server_port), demux=True) assert code == 0 From 5237e3c0595936f0cf6422533c7bebe6f92112e4 Mon Sep 17 00:00:00 2001 From: Yuriy Date: Fri, 6 Dec 2019 04:42:12 +0300 Subject: [PATCH 10/10] better exception text --- dbms/tests/integration/test_mysql_protocol/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/tests/integration/test_mysql_protocol/test.py b/dbms/tests/integration/test_mysql_protocol/test.py index 7b8d67f3cdf..80912ac9f26 100644 --- a/dbms/tests/integration/test_mysql_protocol/test.py +++ b/dbms/tests/integration/test_mysql_protocol/test.py @@ -51,7 +51,7 @@ def mysql_server(mysql_client): break time.sleep(1) else: - raise Exception('Mysql server is not started in %d seconds.' % retries) + raise Exception('Mysql server has not started in %d seconds.' % retries) return mysql_client