From 61816ad0767d0eb85816af49150ee77a066aa6dd Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 13 Apr 2021 00:56:16 +0300 Subject: [PATCH 1/5] Add a test for #2719 --- .../01812_basic_auth_http_server.reference | 0 .../01812_basic_auth_http_server.sh | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+) create mode 100644 tests/queries/0_stateless/01812_basic_auth_http_server.reference create mode 100755 tests/queries/0_stateless/01812_basic_auth_http_server.sh diff --git a/tests/queries/0_stateless/01812_basic_auth_http_server.reference b/tests/queries/0_stateless/01812_basic_auth_http_server.reference new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/queries/0_stateless/01812_basic_auth_http_server.sh b/tests/queries/0_stateless/01812_basic_auth_http_server.sh new file mode 100755 index 00000000000..9b328911e39 --- /dev/null +++ b/tests/queries/0_stateless/01812_basic_auth_http_server.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash + +# In very old (e.g. 1.1.54385) versions of ClickHouse there was a bug in Poco HTTP library: +# Basic HTTP authentication headers was not parsed if the size of URL is exactly 4077 + something bytes. +# So, the user may get authentication error if valid credentials are passed. +# This is a minor issue because it does not have security implications (at worse the user will be not allowed to access). + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +# In this test we do the opposite: passing the invalid credentials while server is accepting default user without a password. +# And if the bug exists, they will be ignored (treat as empty credentials) and query succeed. + +for i in {3950..4100}; do ${CLICKHOUSE_CURL} --user default:12345 "${CLICKHOUSE_URL}&query=SELECT+1"$(perl -e "print '+'x$i") | grep -v -F 'password' && echo 'Fail'; done + +# You can check that the bug exists in old version by running the old server in Docker: +# docker run --network host -it --rm yandex/clickhouse-server:1.1.54385 From a29334de7c30adddc83304195af65870a50978f0 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 13 Apr 2021 20:23:14 +0300 Subject: [PATCH 2/5] Fix test --- tests/queries/0_stateless/01812_basic_auth_http_server.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/01812_basic_auth_http_server.sh b/tests/queries/0_stateless/01812_basic_auth_http_server.sh index 9b328911e39..b2caa32ef1a 100755 --- a/tests/queries/0_stateless/01812_basic_auth_http_server.sh +++ b/tests/queries/0_stateless/01812_basic_auth_http_server.sh @@ -12,7 +12,7 @@ CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # In this test we do the opposite: passing the invalid credentials while server is accepting default user without a password. # And if the bug exists, they will be ignored (treat as empty credentials) and query succeed. -for i in {3950..4100}; do ${CLICKHOUSE_CURL} --user default:12345 "${CLICKHOUSE_URL}&query=SELECT+1"$(perl -e "print '+'x$i") | grep -v -F 'password' && echo 'Fail'; done +for i in {3950..4100}; do ${CLICKHOUSE_CURL} --user default:12345 "${CLICKHOUSE_URL}&query=SELECT+1"$(perl -e "print '+'x$i") | grep -v -F 'password' && echo 'Fail' ||:; done # You can check that the bug exists in old version by running the old server in Docker: # docker run --network host -it --rm yandex/clickhouse-server:1.1.54385 From 2450b1e7ca4b08a431f88c06c07370aac14dfa19 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 13 Apr 2021 20:34:11 +0300 Subject: [PATCH 3/5] Add hilight for usability --- programs/install/Install.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/programs/install/Install.cpp b/programs/install/Install.cpp index ef72624e7ab..3cab7a0ce96 100644 --- a/programs/install/Install.cpp +++ b/programs/install/Install.cpp @@ -71,6 +71,9 @@ namespace ErrorCodes } +/// ANSI escape sequence for intense color in terminal. +#define HILITE "\033[1m" +#define END_HILITE "\033[0m" using namespace DB; namespace po = boost::program_options; @@ -563,12 +566,12 @@ int mainEntryClickHouseInstall(int argc, char ** argv) if (has_password_for_default_user) { - fmt::print("Password for default user is already specified. To remind or reset, see {} and {}.\n", + fmt::print(HILITE "Password for default user is already specified. To remind or reset, see {} and {}." END_HILITE, users_config_file.string(), users_d.string()); } else if (!is_interactive) { - fmt::print("Password for default user is empty string. See {} and {} to change it.\n", + fmt::print(HILITE "Password for default user is empty string. See {} and {} to change it." END_HILITE, users_config_file.string(), users_d.string()); } else From 39d7f50e8a06e91be81ff4abebbacf95739f7356 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 13 Apr 2021 22:15:15 +0300 Subject: [PATCH 4/5] Fix style --- tests/queries/0_stateless/01812_basic_auth_http_server.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/queries/0_stateless/01812_basic_auth_http_server.sh b/tests/queries/0_stateless/01812_basic_auth_http_server.sh index b2caa32ef1a..6f553596656 100755 --- a/tests/queries/0_stateless/01812_basic_auth_http_server.sh +++ b/tests/queries/0_stateless/01812_basic_auth_http_server.sh @@ -1,4 +1,5 @@ #!/usr/bin/env bash +# shellcheck disable=SC2046 # In very old (e.g. 1.1.54385) versions of ClickHouse there was a bug in Poco HTTP library: # Basic HTTP authentication headers was not parsed if the size of URL is exactly 4077 + something bytes. From 4914860f91d82e7bde6e741948d6f3c4d767e959 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Tue, 13 Apr 2021 22:41:07 +0300 Subject: [PATCH 5/5] Fix style --- tests/queries/0_stateless/01812_basic_auth_http_server.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/01812_basic_auth_http_server.sh b/tests/queries/0_stateless/01812_basic_auth_http_server.sh index 6f553596656..4b993137bbd 100755 --- a/tests/queries/0_stateless/01812_basic_auth_http_server.sh +++ b/tests/queries/0_stateless/01812_basic_auth_http_server.sh @@ -13,7 +13,7 @@ CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) # In this test we do the opposite: passing the invalid credentials while server is accepting default user without a password. # And if the bug exists, they will be ignored (treat as empty credentials) and query succeed. -for i in {3950..4100}; do ${CLICKHOUSE_CURL} --user default:12345 "${CLICKHOUSE_URL}&query=SELECT+1"$(perl -e "print '+'x$i") | grep -v -F 'password' && echo 'Fail' ||:; done +for i in {3950..4100}; do ${CLICKHOUSE_CURL} --user default:12345 "${CLICKHOUSE_URL}&query=SELECT+1"$(perl -e "print '+'x$i") | grep -v -F 'password' ||:; done # You can check that the bug exists in old version by running the old server in Docker: # docker run --network host -it --rm yandex/clickhouse-server:1.1.54385