From dcf8724adf32cf9c9734813e2cc00845fc1a5ef8 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Mon, 24 Oct 2022 21:31:11 +0200 Subject: [PATCH] Move prepareQueryForLogging() to a separate header. --- src/Interpreters/executeQuery.cpp | 39 +--------- ...=> maskSensitiveInfoInQueryForLogging.cpp} | 73 +++++++++++++++---- .../maskSensitiveInfoInQueryForLogging.h | 14 ++++ src/Interpreters/wipePasswordFromQuery.h | 19 ----- .../__init__.py | 0 .../test.py | 0 6 files changed, 74 insertions(+), 71 deletions(-) rename src/Interpreters/{wipePasswordFromQuery.cpp => maskSensitiveInfoInQueryForLogging.cpp} (89%) create mode 100644 src/Interpreters/maskSensitiveInfoInQueryForLogging.h delete mode 100644 src/Interpreters/wipePasswordFromQuery.h rename tests/integration/{test_mask_queries_in_logs => test_mask_sensitive_info_in_logs}/__init__.py (100%) rename tests/integration/{test_mask_queries_in_logs => test_mask_sensitive_info_in_logs}/test.py (100%) diff --git a/src/Interpreters/executeQuery.cpp b/src/Interpreters/executeQuery.cpp index f4c8d2bd0ca..087f3fd8887 100644 --- a/src/Interpreters/executeQuery.cpp +++ b/src/Interpreters/executeQuery.cpp @@ -55,10 +55,9 @@ #include #include #include -#include +#include #include -#include #include #include @@ -77,7 +76,6 @@ namespace ProfileEvents { - extern const Event QueryMaskingRulesMatch; extern const Event FailedQuery; extern const Event FailedInsertQuery; extern const Event FailedSelectQuery; @@ -109,37 +107,6 @@ static void checkASTSizeLimits(const IAST & ast, const Settings & settings) } -/// Makes a version of a query without sensitive information (e.g. passwords) for logging. -/// The parameter `parsed query` can be nullptr if the query cannot be parsed. -static String prepareQueryForLogging(const String & query, const ASTPtr & parsed_query, ContextPtr context) -{ - String res = query; - - // Wiping a password or hash from CREATE/ALTER USER query because we don't want it to go to logs. - if (parsed_query && canContainPassword(*parsed_query)) - { - ASTPtr ast_for_logging = parsed_query->clone(); - wipePasswordFromQuery(ast_for_logging, context); - res = serializeAST(*ast_for_logging); - } - - // Wiping sensitive data before cropping query by log_queries_cut_to_length, - // otherwise something like credit card without last digit can go to log. - if (auto * masker = SensitiveDataMasker::getInstance()) - { - auto matches = masker->wipeSensitiveData(res); - if (matches > 0) - { - ProfileEvents::increment(ProfileEvents::QueryMaskingRulesMatch, matches); - } - } - - res = res.substr(0, context->getSettingsRef().log_queries_cut_to_length); - - return res; -} - - /// Log query into text log (not into system table). static void logQuery(const String & query, ContextPtr context, bool internal, QueryProcessingStage::Enum stage) { @@ -425,14 +392,14 @@ static std::tuple executeQueryImpl( /// MUST go before any modification (except for prepared statements, /// since it substitute parameters and without them query does not contain /// parameters), to keep query as-is in query_log and server log. - query_for_logging = prepareQueryForLogging(query, ast, context); + query_for_logging = maskSensitiveInfoInQueryForLogging(query, ast, context); } catch (...) { /// Anyway log the query. if (query.empty()) query.assign(begin, std::min(end - begin, static_cast(max_query_size))); - query_for_logging = prepareQueryForLogging(query, ast, context); + query_for_logging = maskSensitiveInfoInQueryForLogging(query, ast, context); logQuery(query_for_logging, context, internal, stage); diff --git a/src/Interpreters/wipePasswordFromQuery.cpp b/src/Interpreters/maskSensitiveInfoInQueryForLogging.cpp similarity index 89% rename from src/Interpreters/wipePasswordFromQuery.cpp rename to src/Interpreters/maskSensitiveInfoInQueryForLogging.cpp index a81715d34da..00ea9d34ece 100644 --- a/src/Interpreters/wipePasswordFromQuery.cpp +++ b/src/Interpreters/maskSensitiveInfoInQueryForLogging.cpp @@ -1,6 +1,7 @@ -#include +#include #include +#include #include #include #include @@ -8,10 +9,19 @@ #include #include #include +#include #include +#include +#include #include +namespace ProfileEvents +{ + extern const Event QueryMaskingRulesMatch; +} + + namespace DB { @@ -513,26 +523,57 @@ namespace } } }; + + /// Checks the type of a specified AST and returns true if it can contain a password. + bool canContainPassword(const IAST & ast) + { + using WipingVisitor = PasswordWipingVisitor; + WipingVisitor::Data data; + WipingVisitor::Visitor visitor{data}; + ASTPtr ast_ptr = std::const_pointer_cast(ast.shared_from_this()); + visitor.visit(ast_ptr); + return data.can_contain_password; + } + + /// Removes a password or its hash from a query if it's specified there or replaces it with some placeholder. + /// This function is used to prepare a query for storing in logs (we don't want logs to contain sensitive information). + void wipePasswordFromQuery(ASTPtr ast, const ContextPtr & context) + { + using WipingVisitor = PasswordWipingVisitor; + WipingVisitor::Data data; + data.context = context; + WipingVisitor::Visitor visitor{data}; + visitor.visit(ast); + } } -bool canContainPassword(const IAST & ast) +String maskSensitiveInfoInQueryForLogging(const String & query, const ASTPtr & parsed_query, const ContextPtr & context) { - using WipingVisitor = PasswordWipingVisitor; - WipingVisitor::Data data; - WipingVisitor::Visitor visitor{data}; - ASTPtr ast_ptr = std::const_pointer_cast(ast.shared_from_this()); - visitor.visit(ast_ptr); - return data.can_contain_password; -} + String res = query; -void wipePasswordFromQuery(ASTPtr ast, const ContextPtr & context) -{ - using WipingVisitor = PasswordWipingVisitor; - WipingVisitor::Data data; - data.context = context; - WipingVisitor::Visitor visitor{data}; - visitor.visit(ast); + // Wiping a password or hash from CREATE/ALTER USER query because we don't want it to go to logs. + if (parsed_query && canContainPassword(*parsed_query)) + { + ASTPtr ast_without_password = parsed_query->clone(); + wipePasswordFromQuery(ast_without_password, context); + res = serializeAST(*ast_without_password); + } + + // Wiping sensitive data before cropping query by log_queries_cut_to_length, + // otherwise something like credit card without last digit can go to log. + if (auto * masker = SensitiveDataMasker::getInstance()) + { + auto matches = masker->wipeSensitiveData(res); + if (matches > 0) + { + ProfileEvents::increment(ProfileEvents::QueryMaskingRulesMatch, matches); + } + } + + res = res.substr(0, context->getSettingsRef().log_queries_cut_to_length); + + return res; } } diff --git a/src/Interpreters/maskSensitiveInfoInQueryForLogging.h b/src/Interpreters/maskSensitiveInfoInQueryForLogging.h new file mode 100644 index 00000000000..10d0a9b1c2f --- /dev/null +++ b/src/Interpreters/maskSensitiveInfoInQueryForLogging.h @@ -0,0 +1,14 @@ +#pragma once + +#include +#include + + +namespace DB +{ + +/// Makes a version of a query without sensitive information (e.g. passwords) for logging. +/// The parameter `parsed query` is allowed to be nullptr if the query cannot be parsed. +String maskSensitiveInfoInQueryForLogging(const String & query, const ASTPtr & parsed_query, const ContextPtr & context); + +} diff --git a/src/Interpreters/wipePasswordFromQuery.h b/src/Interpreters/wipePasswordFromQuery.h deleted file mode 100644 index 98c21556a2d..00000000000 --- a/src/Interpreters/wipePasswordFromQuery.h +++ /dev/null @@ -1,19 +0,0 @@ -#pragma once - -#include -#include - - -namespace DB -{ - -/// Checks the type of a specified AST and returns true if it can contain a password. -bool canContainPassword(const IAST & ast); - -/// Removes a password or its hash from a query if it's specified there or replaces it with some placeholder. -/// This function is used to prepare a query for storing in logs (we don't want logs to contain sensitive information). -/// The function changes only following types of queries: -/// CREATE/ALTER USER. -void wipePasswordFromQuery(ASTPtr ast, const ContextPtr & context); - -} diff --git a/tests/integration/test_mask_queries_in_logs/__init__.py b/tests/integration/test_mask_sensitive_info_in_logs/__init__.py similarity index 100% rename from tests/integration/test_mask_queries_in_logs/__init__.py rename to tests/integration/test_mask_sensitive_info_in_logs/__init__.py diff --git a/tests/integration/test_mask_queries_in_logs/test.py b/tests/integration/test_mask_sensitive_info_in_logs/test.py similarity index 100% rename from tests/integration/test_mask_queries_in_logs/test.py rename to tests/integration/test_mask_sensitive_info_in_logs/test.py