From 9e1357dd7eb733466fcfe00715490b00ef7d44ee Mon Sep 17 00:00:00 2001 From: Mikhail Koviazin Date: Tue, 7 Nov 2023 13:44:52 +0200 Subject: [PATCH] Update `query_masking_rules` when reloading the config Fixes #56449 --- programs/server/Server.cpp | 2 + src/Common/SensitiveDataMasker.cpp | 4 + src/Interpreters/Context.cpp | 16 ++++ src/Interpreters/Context.h | 2 + .../__init__.py | 0 .../configs/changed_settings.xml | 19 +++++ .../configs/empty_settings.xml | 12 +++ .../test_reload_query_masking_rules/test.py | 74 +++++++++++++++++++ 8 files changed, 129 insertions(+) create mode 100644 tests/integration/test_reload_query_masking_rules/__init__.py create mode 100644 tests/integration/test_reload_query_masking_rules/configs/changed_settings.xml create mode 100644 tests/integration/test_reload_query_masking_rules/configs/empty_settings.xml create mode 100644 tests/integration/test_reload_query_masking_rules/test.py diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index 9e974e796e0..ca091dbeceb 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -1372,6 +1372,8 @@ try global_context->reloadAuxiliaryZooKeepersConfigIfChanged(config); + global_context->reloadQueryMaskingRulesIfChanged(config); + std::lock_guard lock(servers_lock); updateServers(*config, server_pool, async_metrics, servers, servers_to_start_before_tables); } diff --git a/src/Common/SensitiveDataMasker.cpp b/src/Common/SensitiveDataMasker.cpp index 2b21c223bd8..5fc5c3618cc 100644 --- a/src/Common/SensitiveDataMasker.cpp +++ b/src/Common/SensitiveDataMasker.cpp @@ -104,6 +104,10 @@ void SensitiveDataMasker::setInstance(std::unique_ptr sensi { sensitive_data_masker = std::move(sensitive_data_masker_); } + else + { + sensitive_data_masker.reset(); + } } SensitiveDataMasker * SensitiveDataMasker::getInstance() diff --git a/src/Interpreters/Context.cpp b/src/Interpreters/Context.cpp index 75cc5f8366c..8bd4d619349 100644 --- a/src/Interpreters/Context.cpp +++ b/src/Interpreters/Context.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -196,6 +197,9 @@ struct ContextSharedPart : boost::noncopyable mutable zkutil::ZooKeeperPtr zookeeper TSA_GUARDED_BY(zookeeper_mutex); /// Client for ZooKeeper. ConfigurationPtr zookeeper_config TSA_GUARDED_BY(zookeeper_mutex); /// Stores zookeeper configs + mutable std::mutex sensitive_data_masker_mutex; + ConfigurationPtr sensitive_data_masker_config; + #if USE_NURAFT mutable std::mutex keeper_dispatcher_mutex; mutable std::shared_ptr keeper_dispatcher TSA_GUARDED_BY(keeper_dispatcher_mutex); @@ -3198,6 +3202,18 @@ bool Context::hasAuxiliaryZooKeeper(const String & name) const return getConfigRef().has("auxiliary_zookeepers." + name); } +void Context::reloadQueryMaskingRulesIfChanged(const ConfigurationPtr & config) const +{ + std::lock_guard lock(shared->sensitive_data_masker_mutex); + + const auto old_config = shared->sensitive_data_masker_config; + if (old_config && isSameConfiguration(*config, *old_config, "query_masking_rules")) + return; + + SensitiveDataMasker::setInstance(std::make_unique(*config, "query_masking_rules")); + shared->sensitive_data_masker_config = config; +} + InterserverCredentialsPtr Context::getInterserverCredentials() const { return shared->interserver_io_credentials.get(); diff --git a/src/Interpreters/Context.h b/src/Interpreters/Context.h index e12a5c4b69b..f90812df8c1 100644 --- a/src/Interpreters/Context.h +++ b/src/Interpreters/Context.h @@ -946,6 +946,8 @@ public: // Reload Zookeeper void reloadZooKeeperIfChanged(const ConfigurationPtr & config) const; + void reloadQueryMaskingRulesIfChanged(const ConfigurationPtr & config) const; + void setSystemZooKeeperLogAfterInitializationIfNeeded(); /// --- Caches ------------------------------------------------------------------------------------------ diff --git a/tests/integration/test_reload_query_masking_rules/__init__.py b/tests/integration/test_reload_query_masking_rules/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_reload_query_masking_rules/configs/changed_settings.xml b/tests/integration/test_reload_query_masking_rules/configs/changed_settings.xml new file mode 100644 index 00000000000..d681496d843 --- /dev/null +++ b/tests/integration/test_reload_query_masking_rules/configs/changed_settings.xml @@ -0,0 +1,19 @@ + + + system + query_log
+ toYYYYMM(event_date) + 7500 + 1048576 + 8192 + 524288 + false + + + + + TOPSECRET.TOPSECRET + [hidden] + + + diff --git a/tests/integration/test_reload_query_masking_rules/configs/empty_settings.xml b/tests/integration/test_reload_query_masking_rules/configs/empty_settings.xml new file mode 100644 index 00000000000..82647ff82b5 --- /dev/null +++ b/tests/integration/test_reload_query_masking_rules/configs/empty_settings.xml @@ -0,0 +1,12 @@ + + + system + query_log
+ toYYYYMM(event_date) + 7500 + 1048576 + 8192 + 524288 + false + + diff --git a/tests/integration/test_reload_query_masking_rules/test.py b/tests/integration/test_reload_query_masking_rules/test.py new file mode 100644 index 00000000000..0f29bd0825e --- /dev/null +++ b/tests/integration/test_reload_query_masking_rules/test.py @@ -0,0 +1,74 @@ +import pytest +import os +from helpers.cluster import ClickHouseCluster +from helpers.test_tools import assert_eq_with_retry, assert_logs_contain_with_retry + +SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) +cluster = ClickHouseCluster(__file__) +node = cluster.add_instance("node", user_configs=["configs/empty_settings.xml"]) + + +@pytest.fixture(scope="module", autouse=True) +def started_cluster(): + try: + cluster.start() + yield cluster + finally: + cluster.shutdown() + + +@pytest.fixture(autouse=True) +def reset_to_normal_settings_after_test(): + try: + node.copy_file_to_container( + os.path.join(SCRIPT_DIR, "configs/empty_settings.xml"), + "/etc/clickhouse-server/config.d/z.xml", + ) + node.query("SYSTEM RELOAD CONFIG") + yield + finally: + pass + + +# @pytest.mark.parametrize("reload_strategy", ["force", "timeout"]) +def test_reload_query_masking_rules(): + # At first, empty configuration is fed to ClickHouse. The query + # "SELECT 'TOPSECRET.TOPSECRET'" will not be redacted, and the new masking + # event will not be registered + node.query("SELECT 'TOPSECRET.TOPSECRET'") + assert_logs_contain_with_retry(node, "SELECT 'TOPSECRET.TOPSECRET'") + + # If there were no 'QueryMaskingRulesMatch' events, the query below returns + # 0 rows + assert ( + node.query( + "SELECT count(value) FROM system.events WHERE name = 'QueryMaskingRulesMatch'" + ) + == "0\n" + ) + + node.copy_file_to_container( + os.path.join(SCRIPT_DIR, "configs/changed_settings.xml"), + "/etc/clickhouse-server/config.d/z.xml", + ) + + node.query("SYSTEM RELOAD CONFIG") + + # Now the same query will be redacted in the logs and the counter of events + # will be incremented + node.query("SELECT 'TOPSECRET.TOPSECRET'") + + assert_eq_with_retry( + node, + "SELECT count(value) FROM system.events WHERE name = 'QueryMaskingRulesMatch'", + "1", + ) + assert_logs_contain_with_retry(node, r"SELECT '\[hidden\]'") + assert ( + node.query( + "SELECT value FROM system.events WHERE name = 'QueryMaskingRulesMatch'" + ) + == "1\n" + ) + + node.rotate_logs()