Merge pull request #53907 from arenadata/ADQM-1126

Added validate_tcp_client_information server setting

docs/en/operations/settings/settings.md

@@ -4643,3 +4643,19 @@ SELECT toFloat64('1.7091'), toFloat64('1.5008753E7') SETTINGS precise_float_pars
 │              1.7091 │                 15008753 │
 └─────────────────────┴──────────────────────────┘
 ```
+
+## validate_tcp_client_information {#validate-tcp-client-information}
+
+Determines whether validation of client information enabled when query packet is received from a client using a TCP connection.
+
+If `true`, an exception will be thrown on invalid client information from the TCP client.
+
+If `false`, the data will not be validated. The server will work with clients of all versions.
+
+The default value is `false`.
+
+**Example**
+
+``` xml
+<validate_tcp_client_information>true</validate_tcp_client_information>
+```

docs/ru/operations/server-configuration-parameters/settings.md

@@ -1923,3 +1923,19 @@ ClickHouse использует ZooKeeper для хранения метадан
 -   Положительное целое число.
 
 Значение по умолчанию: `10000`.
+
+## validate_tcp_client_information {#validate-tcp-client-information}
+
+Включена ли валидация данных о клиенте при запросе от клиента, использующего TCP соединение.
+
+Если `true`, то на неверные данные от клиента будет выброшено исключение.
+
+Если `false`, то данные не будут валидироваться. Сервер будет работать с клиентами всех версий.
+
+Значение по умолчанию: `false`.
+
+**Пример**
+
+``` xml
+<validate_tcp_client_information>true</validate_tcp_client_information>
+```

programs/server/config.xml

@@ -419,6 +419,8 @@
     <!-- Cache size in elements for compiled expressions.-->
     <compiled_expression_cache_elements_size>10000</compiled_expression_cache_elements_size>
 
+    <validate_tcp_client_information>false</validate_tcp_client_information>
+
     <!-- Path to data directory, with trailing slash. -->
     <path>/var/lib/clickhouse/</path>
 

src/Core/ServerSettings.h

@@ -96,7 +96,8 @@ namespace DB
     M(UInt64, total_memory_profiler_step, 0, "Whenever server memory usage becomes larger than every next step in number of bytes the memory profiler will collect the allocating stack trace. Zero means disabled memory profiler. Values lower than a few megabytes will slow down server.", 0) \
     M(Double, total_memory_tracker_sample_probability, 0, "Collect random allocations and deallocations and write them into system.trace_log with 'MemorySample' trace_type. The probability is for every alloc/free regardless to the size of the allocation (can be changed with `memory_profiler_sample_min_allocation_size` and `memory_profiler_sample_max_allocation_size`). Note that sampling happens only when the amount of untracked memory exceeds 'max_untracked_memory'. You may want to set 'max_untracked_memory' to 0 for extra fine grained sampling.", 0) \
     M(UInt64, total_memory_profiler_sample_min_allocation_size, 0, "Collect random allocations of size greater or equal than specified value with probability equal to `total_memory_profiler_sample_probability`. 0 means disabled. You may want to set 'max_untracked_memory' to 0 to make this threshold to work as expected.", 0) \
-    M(UInt64, total_memory_profiler_sample_max_allocation_size, 0, "Collect random allocations of size less or equal than specified value with probability equal to `total_memory_profiler_sample_probability`. 0 means disabled. You may want to set 'max_untracked_memory' to 0 to make this threshold to work as expected.", 0)
+    M(UInt64, total_memory_profiler_sample_max_allocation_size, 0, "Collect random allocations of size less or equal than specified value with probability equal to `total_memory_profiler_sample_probability`. 0 means disabled. You may want to set 'max_untracked_memory' to 0 to make this threshold to work as expected.", 0) \
+    M(Bool, validate_tcp_client_information, false, "Validate client_information in the query packet over the native TCP protocol.", 0)
 
 DECLARE_SETTINGS_TRAITS(ServerSettingsTraits, SERVER_SETTINGS)
 

src/Interpreters/ClientInfo.cpp

@@ -213,6 +213,10 @@ String ClientInfo::getVersionStr() const
     return std::format("{}.{}.{} ({})", client_version_major, client_version_minor, client_version_patch, client_tcp_protocol_version);
 }
 
+VersionNumber ClientInfo::getVersionNumber() const
+{
+    return VersionNumber(client_version_major, client_version_minor, client_version_patch);
+}
 
 void ClientInfo::fillOSUserHostNameAndVersionInfo()
 {

src/Interpreters/ClientInfo.h

@@ -4,6 +4,7 @@
 #include <Poco/Net/SocketAddress.h>
 #include <base/types.h>
 #include <Common/OpenTelemetryTraceContext.h>
+#include <Common/VersionNumber.h>
 #include <boost/algorithm/string/trim.hpp>
 
 namespace DB
@@ -137,6 +138,7 @@ public:
     bool clientVersionEquals(const ClientInfo & other, bool compare_patch) const;
 
     String getVersionStr() const;
+    VersionNumber getVersionNumber() const;
 
 private:
     void fillOSUserHostNameAndVersionInfo();

src/Server/TCPHandler.cpp

@@ -35,6 +35,7 @@
 #include <Storages/MergeTree/MergeTreeDataPartUUID.h>
 #include <Storages/StorageS3Cluster.h>
 #include <Core/ExternalTable.h>
+#include <Core/ServerSettings.h>
 #include <Access/AccessControl.h>
 #include <Access/Credentials.h>
 #include <DataTypes/DataTypeLowCardinality.h>
@@ -114,6 +115,20 @@ NameToNameMap convertToQueryParameters(const Settings & passed_params)
     return query_parameters;
 }
 
+// This function corrects the wrong client_name from the old client.
+// Old clients 28.7 and some intermediate versions of 28.7 were sending different ClientInfo.client_name
+// "ClickHouse client" was sent with the hello message.
+// "ClickHouse" or "ClickHouse " was sent with the query message.
+void correctQueryClientInfo(const ClientInfo & session_client_info, ClientInfo & client_info)
+{
+    if (client_info.getVersionNumber() <= VersionNumber(23, 8, 1) &&
+        session_client_info.client_name == "ClickHouse client" &&
+        (client_info.client_name == "ClickHouse" || client_info.client_name == "ClickHouse "))
+    {
+        client_info.client_name = "ClickHouse client";
+    }
+}
+
 void validateClientInfo(const ClientInfo & session_client_info, const ClientInfo & client_info)
 {
     // Secondary query may contain different client_info.
@@ -1532,7 +1547,11 @@ void TCPHandler::receiveQuery()
     if (client_tcp_protocol_version >= DBMS_MIN_REVISION_WITH_CLIENT_INFO)
     {
         client_info.read(*in, client_tcp_protocol_version);
-        validateClientInfo(session->getClientInfo(), client_info);
+
+        correctQueryClientInfo(session->getClientInfo(), client_info);
+        const auto & config_ref = Context::getGlobalContextInstance()->getServerSettings();
+        if (config_ref.validate_tcp_client_information)
+            validateClientInfo(session->getClientInfo(), client_info);
     }
 
     /// Per query settings are also passed via TCP.

tests/config/config.d/validate_tcp_client_information.xml

@@ -0,0 +1,3 @@
+<clickhouse>
+    <validate_tcp_client_information>true</validate_tcp_client_information>
+</clickhouse>

tests/config/install.sh

@@ -61,6 +61,7 @@ ln -sf $SRC_PATH/config.d/disable_s3_env_credentials.xml $DEST_SERVER_PATH/confi
 ln -sf $SRC_PATH/config.d/enable_wait_for_shutdown_replicated_tables.xml $DEST_SERVER_PATH/config.d/
 ln -sf $SRC_PATH/config.d/backups.xml $DEST_SERVER_PATH/config.d/
 ln -sf $SRC_PATH/config.d/filesystem_caches_path.xml $DEST_SERVER_PATH/config.d/
+ln -sf $SRC_PATH/config.d/validate_tcp_client_information.xml $DEST_SERVER_PATH/config.d/
 
 # Not supported with fasttest.
 if [ "${DEST_SERVER_PATH}" = "/etc/clickhouse-server" ]

tests/integration/test_composable_protocols/configs/config.xml

@@ -61,4 +61,6 @@
     </protocols>
 
     <!--tcp_port>9010</tcp_port--> 
+
+    <validate_tcp_client_information>true</validate_tcp_client_information>
 </clickhouse>

tests/integration/test_composable_protocols/test.py

@@ -84,7 +84,7 @@ def test_connections():
 
     assert execute_query_https(server.ip_address, 8444, "SELECT 1") == "1\n"
 
-    data = "PROXY TCP4 255.255.255.255 255.255.255.255 65535 65535\r\n\0\021ClickHouse client\024\r\253\251\003\0\007default\0\004\001\0\001\0\0\t0.0.0.0:0\001\tmilovidov\021milovidov-desktop\vClickHouse \024\r\253\251\003\0\001\0\0\0\002\001\025SELECT 'Hello, world'\002\0\247\203\254l\325\\z|\265\254F\275\333\206\342\024\202\024\0\0\0\n\0\0\0\240\01\0\02\377\377\377\377\0\0\0"
+    data = "PROXY TCP4 255.255.255.255 255.255.255.255 65535 65535\r\n\0\021ClickHouse client\024\r\253\251\003\0\007default\0\004\001\0\001\0\0\t0.0.0.0:0\001\tmilovidov\021milovidov-desktop\21ClickHouse client\024\r\253\251\003\0\001\0\0\0\002\001\025SELECT 'Hello, world'\002\0\247\203\254l\325\\z|\265\254F\275\333\206\342\024\202\024\0\0\0\n\0\0\0\240\01\0\02\377\377\377\377\0\0\0"
     assert (
         netcat(server.ip_address, 9100, bytearray(data, "latin-1")).find(
             bytearray("Hello, world", "latin-1")
@@ -92,7 +92,7 @@ def test_connections():
         >= 0
     )
 
-    data_user_allowed = "PROXY TCP4 123.123.123.123 255.255.255.255 65535 65535\r\n\0\021ClickHouse client\024\r\253\251\003\0\007user123\0\004\001\0\001\0\0\t0.0.0.0:0\001\tmilovidov\021milovidov-desktop\vClickHouse \024\r\253\251\003\0\001\0\0\0\002\001\025SELECT 'Hello, world'\002\0\247\203\254l\325\\z|\265\254F\275\333\206\342\024\202\024\0\0\0\n\0\0\0\240\01\0\02\377\377\377\377\0\0\0"
+    data_user_allowed = "PROXY TCP4 123.123.123.123 255.255.255.255 65535 65535\r\n\0\021ClickHouse client\024\r\253\251\003\0\007user123\0\004\001\0\001\0\0\t0.0.0.0:0\001\tmilovidov\021milovidov-desktop\21ClickHouse client\024\r\253\251\003\0\001\0\0\0\002\001\025SELECT 'Hello, world'\002\0\247\203\254l\325\\z|\265\254F\275\333\206\342\024\202\024\0\0\0\n\0\0\0\240\01\0\02\377\377\377\377\0\0\0"
     assert (
         netcat(server.ip_address, 9100, bytearray(data_user_allowed, "latin-1")).find(
             bytearray("Hello, world", "latin-1")
@@ -100,7 +100,7 @@ def test_connections():
         >= 0
     )
 
-    data_user_restricted = "PROXY TCP4 127.0.0.1 255.255.255.255 65535 65535\r\n\0\021ClickHouse client\024\r\253\251\003\0\007user123\0\004\001\0\001\0\0\t0.0.0.0:0\001\tmilovidov\021milovidov-desktop\vClickHouse \024\r\253\251\003\0\001\0\0\0\002\001\025SELECT 'Hello, world'\002\0\247\203\254l\325\\z|\265\254F\275\333\206\342\024\202\024\0\0\0\n\0\0\0\240\01\0\02\377\377\377\377\0\0\0"
+    data_user_restricted = "PROXY TCP4 127.0.0.1 255.255.255.255 65535 65535\r\n\0\021ClickHouse client\024\r\253\251\003\0\007user123\0\004\001\0\001\0\0\t0.0.0.0:0\001\tmilovidov\021milovidov-desktop\21ClickHouse client\024\r\253\251\003\0\001\0\0\0\002\001\025SELECT 'Hello, world'\002\0\247\203\254l\325\\z|\265\254F\275\333\206\342\024\202\024\0\0\0\n\0\0\0\240\01\0\02\377\377\377\377\0\0\0"
     assert (
         netcat(
             server.ip_address, 9100, bytearray(data_user_restricted, "latin-1")

tests/queries/0_stateless/01601_proxy_protocol.reference

@@ -1 +1,3 @@
 Hello, world
+Hello, world
+Hello, world

tests/queries/0_stateless/01601_proxy_protocol.sh

@@ -6,4 +6,8 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 # shellcheck source=../shell_config.sh
 . "$CURDIR"/../shell_config.sh
 
+# Old clickhouse-client (with version 23.8-) sends "ClickHouse client" and then "ClickHouse" or "ClickHouse ".
+# For backward compatibility purposes, the server accepts both variants.
+printf "PROXY TCP4 255.255.255.255 255.255.255.255 65535 65535\r\n\0\21ClickHouse client\24\r\253\251\3\0\7default\0\4\1\0\1\0\0\t0.0.0.0:0\1\tmilovidov\21milovidov-desktop\nClickHouse\24\r\253\251\3\0\1\0\0\0\2\1\25SELECT 'Hello, world'\2\0\247\203\254l\325\\z|\265\254F\275\333\206\342\24\202\24\0\0\0\n\0\0\0\240\1\0\2\377\377\377\377\0\0\0" | nc "${CLICKHOUSE_HOST}" "${CLICKHOUSE_PORT_TCP_WITH_PROXY}" | head -c150 | grep --text -o -F 'Hello, world'
+printf "PROXY TCP4 255.255.255.255 255.255.255.255 65535 65535\r\n\0\21ClickHouse client\24\r\253\251\3\0\7default\0\4\1\0\1\0\0\t0.0.0.0:0\1\tmilovidov\21milovidov-desktop\vClickHouse \24\r\253\251\3\0\1\0\0\0\2\1\25SELECT 'Hello, world'\2\0\247\203\254l\325\\z|\265\254F\275\333\206\342\24\202\24\0\0\0\n\0\0\0\240\1\0\2\377\377\377\377\0\0\0" | nc "${CLICKHOUSE_HOST}" "${CLICKHOUSE_PORT_TCP_WITH_PROXY}" | head -c150 | grep --text -o -F 'Hello, world'
 printf "PROXY TCP4 255.255.255.255 255.255.255.255 65535 65535\r\n\0\21ClickHouse client\24\r\253\251\3\0\7default\0\4\1\0\1\0\0\t0.0.0.0:0\1\tmilovidov\21milovidov-desktop\21ClickHouse client\24\r\253\251\3\0\1\0\0\0\2\1\25SELECT 'Hello, world'\2\0\247\203\254l\325\\z|\265\254F\275\333\206\342\24\202\24\0\0\0\n\0\0\0\240\1\0\2\377\377\377\377\0\0\0" | nc "${CLICKHOUSE_HOST}" "${CLICKHOUSE_PORT_TCP_WITH_PROXY}" | head -c150 | grep --text -o -F 'Hello, world'