Merge pull request ClickHouse#78094 from ClickHouse/pwd-leak

Query masking rules are now able to throw on match

Author: nikitamikhaylov

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

@@ -1547,6 +1547,7 @@ ClickHouse использует потоки из глобального пул
         <name>hide SSN</name>
         <regexp>(^|\D)\d{3}-\d{2}-\d{4}($|\D)</regexp>
         <replace>000-00-0000</replace>
+        <throw_on_match>false</throw_on_match>
     </rule>
 </query_masking_rules>
 ```
@@ -1555,6 +1556,7 @@ ClickHouse использует потоки из глобального пул
 - `name` - имя правила (необязательно)
 - `regexp` - совместимое с RE2 регулярное выражение (обязательное)
 - `replace` - строка замены для конфиденциальных данных (опционально, по умолчанию - шесть звездочек)
+- `throw_on_match` - нужно ли кидать исключение, если такая строка обнаружилась в логах.
 
 Правила маскировки применяются ко всему запросу (для предотвращения утечки конфиденциальных данных из неправильно оформленных / не интерпретируемых запросов).
 

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

@@ -627,6 +627,7 @@ SSL客户端/服务器配置。
         <name>hide SSN</name>
         <regexp>(^|\D)\d{3}-\d{2}-\d{4}($|\D)</regexp>
         <replace>000-00-0000</replace>
+        <throw_on_match>false</throw_on_match>
     </rule>
 </query_masking_rules>
 ```
@@ -635,6 +636,7 @@ SSL客户端/服务器配置。
 - `name` -规则的名称（可选)
 - `regexp` -RE2兼容正则表达式（强制性)
 - `replace` -敏感数据的替换字符串（可选，默认情况下-六个星号)
+- `throw_on_match` -如果在日志中找到这样的字符串，我是否需要抛出异常？
 
 屏蔽规则应用于整个查询（以防止敏感数据从格式错误/不可解析的查询泄漏）。
 

src/Common/SensitiveDataMasker.cpp

@@ -41,6 +41,8 @@ class SensitiveDataMasker::MaskingRule
     const std::string replacement_string;
     const std::string regexp_string;
 
+    const bool throw_on_match;
+
     const RE2 regexp;
     const std::string_view replacement;
 
@@ -52,10 +54,16 @@ class SensitiveDataMasker::MaskingRule
     //* TODO: option with hyperscan? https://software.intel.com/en-us/articles/why-and-how-to-replace-pcre-with-hyperscan
     // re2::set should also work quite fast, but it doesn't return the match position, only which regexp was matched
 
-    MaskingRule(const std::string & name_, const std::string & regexp_string_, const std::string & replacement_string_)
+    MaskingRule(
+        const std::string & name_,
+        const std::string & regexp_string_,
+        const std::string & replacement_string_,
+        bool throw_on_match_
+    )
         : name(name_)
         , replacement_string(replacement_string_)
         , regexp_string(regexp_string_)
+        , throw_on_match(throw_on_match_)
         , regexp(regexp_string, RE2::Quiet)
         , replacement(replacement_string)
     {
@@ -69,6 +77,12 @@ class SensitiveDataMasker::MaskingRule
     uint64_t apply(std::string & data) const
     {
         auto m = RE2::GlobalReplace(&data, regexp, replacement);
+
+        if (throw_on_match && m > 0)
+            throw Exception(ErrorCodes::LOGICAL_ERROR,
+                            "The rule {} was triggered on the log line {}",
+                            name, data);
+
 #ifndef NDEBUG
         matches_count += m;
 #endif
@@ -140,10 +154,11 @@ SensitiveDataMasker::SensitiveDataMasker(const Poco::Util::AbstractConfiguration
             }
 
             auto replace = config.getString(rule_config_prefix + ".replace", "******");
+            auto throw_on_match = config.getBool(rule_config_prefix + ".throw_on_match", false);
 
             try
             {
-                addMaskingRule(rule_name, regexp, replace);
+                addMaskingRule(rule_name, regexp, replace, throw_on_match);
             }
             catch (DB::Exception & e)
             {
@@ -163,9 +178,12 @@ SensitiveDataMasker::SensitiveDataMasker(const Poco::Util::AbstractConfiguration
 }
 
 void SensitiveDataMasker::addMaskingRule(
-    const std::string & name, const std::string & regexp_string, const std::string & replacement_string)
+    const std::string & name,
+    const std::string & regexp_string,
+    const std::string & replacement_string,
+    bool throw_on_match)
 {
-    all_masking_rules.push_back(std::make_unique<MaskingRule>(name, regexp_string, replacement_string));
+    all_masking_rules.push_back(std::make_unique<MaskingRule>(name, regexp_string, replacement_string, throw_on_match));
 }
 
 

src/Common/SensitiveDataMasker.h

@@ -61,7 +61,12 @@ class SensitiveDataMasker
     static MaskerMultiVersion::Version getInstance();
 
     /// Used in tests.
-    void addMaskingRule(const std::string & name, const std::string & regexp_string, const std::string & replacement_string);
+    void addMaskingRule(
+        const std::string & name,
+        const std::string & regexp_string,
+        const std::string & replacement_string,
+        bool throw_on_match
+    );
 
 #ifndef NDEBUG
     void printStats();

src/Common/tests/gtest_sensitive_data_masker.cpp

@@ -9,7 +9,6 @@
 #pragma clang diagnostic ignored "-Wundef"
 
 #include <gtest/gtest.h>
-#include <chrono>
 
 
 namespace DB
@@ -28,11 +27,11 @@ TEST(Common, SensitiveDataMasker)
 
     Poco::AutoPtr<Poco::Util::XMLConfiguration> empty_xml_config = new Poco::Util::XMLConfiguration();
     DB::SensitiveDataMasker masker(*empty_xml_config, "");
-    masker.addMaskingRule("all a letters", "a+", "--a--");
-    masker.addMaskingRule("all b letters", "b+", "--b--");
-    masker.addMaskingRule("all d letters", "d+", "--d--");
-    masker.addMaskingRule("all x letters", "x+", "--x--");
-    masker.addMaskingRule("rule \"d\" result", "--d--", "*****"); // RE2 regexps are applied one-by-one in order
+    masker.addMaskingRule("all a letters", "a+", "--a--", /*throw_on_match=*/false);
+    masker.addMaskingRule("all b letters", "b+", "--b--", /*throw_on_match=*/false);
+    masker.addMaskingRule("all d letters", "d+", "--d--", /*throw_on_match=*/false);
+    masker.addMaskingRule("all x letters", "x+", "--x--", /*throw_on_match=*/false);
+    masker.addMaskingRule("rule \"d\" result", "--d--", "*****", /*throw_on_match=*/false); // RE2 regexps are applied one-by-one in order
     std::string x = "aaaaaaaaaaaaa   bbbbbbbbbb cccc aaaaaaaaaaaa d ";
     EXPECT_EQ(masker.wipeSensitiveData(x), 5);
     EXPECT_EQ(x, "--a--   --b-- cccc --a-- ***** ");
@@ -46,9 +45,9 @@ TEST(Common, SensitiveDataMasker)
 #endif
 
     DB::SensitiveDataMasker masker2(*empty_xml_config, "");
-    masker2.addMaskingRule("hide root password", "qwerty123", "******");
-    masker2.addMaskingRule("hide SSN", "[0-9]{3}-[0-9]{2}-[0-9]{4}", "000-00-0000");
-    masker2.addMaskingRule("hide email", "[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,4}", "[email protected]");
+    masker2.addMaskingRule("hide root password", "qwerty123", "******", /*throw_on_match=*/false);
+    masker2.addMaskingRule("hide SSN", "[0-9]{3}-[0-9]{2}-[0-9]{4}", "000-00-0000", /*throw_on_match=*/false);
+    masker2.addMaskingRule("hide email", "[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\\.[A-Za-z]{2,4}", "[email protected]", /*throw_on_match=*/false);
 
     std::string query = "SELECT id FROM mysql('localhost:3308', 'database', 'table', 'root', 'qwerty123') WHERE ssn='123-45-6789' or "
                         "email='[email protected]'";
@@ -63,7 +62,7 @@ TEST(Common, SensitiveDataMasker)
     // gtest has not good way to check exception content, so just do it manually (see https://github.com/google/googletest/issues/952 )
     try
     {
-        maskerbad.addMaskingRule("bad regexp", "**", "");
+        maskerbad.addMaskingRule("bad regexp", "**", "", /*throw_on_match=*/false);
         ADD_FAILURE() << "addMaskingRule() should throw an error" << std::endl;
     }
     catch (const DB::Exception & e)

tests/config/config.d/query_masking_rules.xml

@@ -5,5 +5,10 @@
             <regexp>TOPSECRET.TOPSECRET</regexp>
             <replace>[hidden]</replace>
         </rule>
+        <rule>
+            <name>Detect passwords in tests</name>
+            <regexp>(?i)P@ssw0rd</regexp>
+            <throw_on_match>true</throw_on_match>
+        </rule>
     </query_masking_rules>
 </clickhouse>