Merge pull request #41341 from evillique/mandatory-identification

Add a setting requiring no_password to be explicitly specified when creating a user

programs/server/config.xml

@@ -462,8 +462,9 @@
     <tmp_path>/var/lib/clickhouse/tmp/</tmp_path>
 
     <!-- Disable AuthType plaintext_password and no_password for ACL. -->
-    <!-- <allow_plaintext_password>0</allow_plaintext_password> -->
-    <!-- <allow_no_password>0</allow_no_password> -->`
+    <allow_plaintext_password>1</allow_plaintext_password>
+    <allow_no_password>1</allow_no_password>
+    <allow_implicit_no_password>1</allow_implicit_no_password>
 
     <!-- Policy from the <storage_configuration> for the temporary files.
          If not set <tmp_path> is used, otherwise <tmp_path> is ignored.

src/Access/AccessControl.cpp

@@ -162,6 +162,7 @@ void AccessControl::setUpFromMainConfig(const Poco::Util::AbstractConfiguration
     if (config_.has("custom_settings_prefixes"))
         setCustomSettingsPrefixes(config_.getString("custom_settings_prefixes"));
 
+    setImplicitNoPasswordAllowed(config_.getBool("allow_implicit_no_password", true));
     setNoPasswordAllowed(config_.getBool("allow_no_password", true));
     setPlaintextPasswordAllowed(config_.getBool("allow_plaintext_password", true));
 
@@ -499,6 +500,15 @@ void AccessControl::checkSettingNameIsAllowed(const std::string_view setting_nam
     custom_settings_prefixes->checkSettingNameIsAllowed(setting_name);
 }
 
+void AccessControl::setImplicitNoPasswordAllowed(bool allow_implicit_no_password_)
+{
+    allow_implicit_no_password = allow_implicit_no_password_;
+}
+
+bool AccessControl::isImplicitNoPasswordAllowed() const
+{
+    return allow_implicit_no_password;
+}
 
 void AccessControl::setNoPasswordAllowed(bool allow_no_password_)
 {

src/Access/AccessControl.h

@@ -134,6 +134,11 @@ public:
     bool isSettingNameAllowed(const std::string_view name) const;
     void checkSettingNameIsAllowed(const std::string_view name) const;
 
+    /// Allows implicit user creation without password (by default it's allowed).
+    /// In other words, allow 'CREATE USER' queries without 'IDENTIFIED WITH' clause.
+    void setImplicitNoPasswordAllowed(const bool allow_implicit_no_password_);
+    bool isImplicitNoPasswordAllowed() const;
+
     /// Allows users without password (by default it's allowed).
     void setNoPasswordAllowed(const bool allow_no_password_);
     bool isNoPasswordAllowed() const;
@@ -222,6 +227,7 @@ private:
     std::unique_ptr<AccessChangesNotifier> changes_notifier;
     std::atomic_bool allow_plaintext_password = true;
     std::atomic_bool allow_no_password = true;
+    std::atomic_bool allow_implicit_no_password = true;
     std::atomic_bool users_without_row_policies_can_read_rows = false;
     std::atomic_bool on_cluster_queries_require_cluster_grant = false;
     std::atomic_bool select_from_system_db_requires_grant = false;

src/Interpreters/Access/InterpreterCreateUserQuery.cpp

@@ -100,9 +100,14 @@ BlockIO InterpreterCreateUserQuery::execute()
     auto & access_control = getContext()->getAccessControl();
     auto access = getContext()->getAccess();
     access->checkAccess(query.alter ? AccessType::ALTER_USER : AccessType::CREATE_USER);
+    bool implicit_no_password_allowed = access_control.isImplicitNoPasswordAllowed();
     bool no_password_allowed = access_control.isNoPasswordAllowed();
     bool plaintext_password_allowed = access_control.isPlaintextPasswordAllowed();
 
+     if (!query.attach && !query.alter && !query.auth_data && !implicit_no_password_allowed)
+        throw Exception(ErrorCodes::BAD_ARGUMENTS,
+            "Authentication type NO_PASSWORD must be explicitly specified, check the setting allow_implicit_no_password in the server configuration");
+
     std::optional<RolesOrUsersSet> default_roles_from_query;
     if (query.default_roles)
     {

tests/queries/0_stateless/02422_allow_implicit_no_password.config.xml

@@ -0,0 +1,22 @@
+<clickhouse>
+    <logger>
+        <level>trace</level>
+        <console>true</console>
+    </logger>
+
+    <tcp_port>9000</tcp_port>
+    <allow_implicit_no_password>0</allow_implicit_no_password>
+    <path>.</path>
+    <mark_cache_size>0</mark_cache_size>
+   <!-- Sources to read users, roles, access rights, profiles of settings, quotas. -->
+    <user_directories>
+	    <users_xml>
+            <!-- Path to configuration file with predefined users. -->
+            <path>users.xml</path>
+        </users_xml>
+        <local_directory>
+            <!-- Path to folder where users created by SQL commands are stored. -->
+	<path>./</path>
+        </local_directory>
+    </user_directories>
+</clickhouse>

tests/queries/0_stateless/02422_allow_implicit_no_password.sh

@@ -0,0 +1,85 @@
+#!/usr/bin/env bash
+# Tags: no-tsan, no-asan, no-ubsan, no-msan, no-parallel, no-fasttest
+# Tag no-tsan: requires jemalloc to track small allocations
+# Tag no-asan: requires jemalloc to track small allocations
+# Tag no-ubsan: requires jemalloc to track small allocations
+# Tag no-msan: requires jemalloc to track small allocations
+
+CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+# shellcheck source=../shell_config.sh
+. "$CURDIR"/../shell_config.sh
+
+cp /etc/clickhouse-server/users.xml "$CURDIR"/users.xml
+sed -i 's/<password><\/password>/<password_sha256_hex>c64c5e4e53ea1a9f1427d2713b3a22bbebe8940bc807adaf654744b1568c70ab<\/password_sha256_hex>/g' "$CURDIR"/users.xml
+ sed -i 's/<!-- <access_management>1<\/access_management> -->/<access_management>1<\/access_management>/g' "$CURDIR"/users.xml
+
+server_opts=(
+    "--config-file=$CURDIR/$(basename "${BASH_SOURCE[0]}" .sh).config.xml"
+    "--"
+    # to avoid multiple listen sockets (complexity for port discovering)
+    "--listen_host=127.1"
+    # we will discover the real port later.
+    "--tcp_port=0"
+    "--shutdown_wait_unfinished=0"
+)
+
+CLICKHOUSE_WATCHDOG_ENABLE=0 $CLICKHOUSE_SERVER_BINARY "${server_opts[@]}" &> clickhouse-server.stderr &
+server_pid=$!
+
+server_port=
+i=0 retries=300
+# wait until server will start to listen (max 30 seconds)
+while [[ -z $server_port ]] && [[ $i -lt $retries ]]; do
+    server_port=$(lsof -n -a -P -i tcp -s tcp:LISTEN -p $server_pid 2>/dev/null | awk -F'[ :]' '/LISTEN/ { print $(NF-1) }')
+    ((++i))
+    sleep 0.1
+    if ! kill -0 $server_pid >& /dev/null; then
+        echo "No server (pid $server_pid)"
+        break
+    fi
+done
+if [[ -z $server_port ]]; then
+    echo "Cannot wait for LISTEN socket" >&2
+    exit 1
+fi
+
+# wait for the server to start accepting tcp connections (max 30 seconds)
+i=0 retries=300
+while ! $CLICKHOUSE_CLIENT_BINARY  -u default --password='1w2swhb1' --host 127.1 --port "$server_port" --format Null -q 'select 1' 2>/dev/null && [[ $i -lt $retries ]]; do
+    sleep 0.1
+    if ! kill -0 $server_pid >& /dev/null; then
+        echo "No server (pid $server_pid)"
+        break
+    fi
+done
+
+
+if ! $CLICKHOUSE_CLIENT_BINARY  -u default --password='1w2swhb1' --host 127.1 --port "$server_port" --format Null -q 'select 1'; then
+    echo "Cannot wait until server will start accepting connections on <tcp_port>" >&2
+    exit 1
+fi
+
+$CLICKHOUSE_CLIENT_BINARY  -u default --password='1w2swhb1' --host 127.1 --port "$server_port" -q "DROP USER IF EXISTS u1_02422, u2_02422, u3_02422";
+
+$CLICKHOUSE_CLIENT_BINARY  -u default --password='1w2swhb1' --host 127.1 --port "$server_port" -q "CREATE USER u1_02422" " -- { serverError 516 } --" &> /dev/null ;
+
+$CLICKHOUSE_CLIENT_BINARY  -u default --password='1w2swhb1' --host 127.1 --port "$server_port" -q "CREATE USER u2_02422 IDENTIFIED WITH no_password "
+
+$CLICKHOUSE_CLIENT_BINARY  -u default --password='1w2swhb1' --host 127.1 --port "$server_port" -q "CREATE USER u3_02422 IDENTIFIED BY 'qwe123'";
+
+$CLICKHOUSE_CLIENT_BINARY  -u default --password='1w2swhb1' --host 127.1 --port "$server_port" -q "DROP USER u2_02422, u3_02422";
+
+
+# no sleep, since flushing to stderr should not be buffered.
+ grep  'User is not allowed to Create users' clickhouse-server.stderr
+
+
+# send TERM and save the error code to ensure that it is 0 (EXIT_SUCCESS)
+kill $server_pid
+wait $server_pid
+return_code=$?
+
+rm -f clickhouse-server.stderr
+rm -f "$CURDIR"/users.xml
+
+exit $return_code