From f75dd93965307cca57d6153cd8ce73262f2dc498 Mon Sep 17 00:00:00 2001 From: Vitaly Baranov Date: Mon, 19 Sep 2022 02:02:09 +0200 Subject: [PATCH] Mask some information in logs. --- src/Interpreters/executeQuery.cpp | 8 +- src/Parsers/wipePasswordFromQuery.cpp | 32 ++++++ src/Parsers/wipePasswordFromQuery.h | 14 +++ .../test_mask_queries_in_logs/__init__.py | 0 .../test_mask_queries_in_logs/test.py | 99 +++++++++++++++++++ .../01702_system_query_log.reference | 2 +- 6 files changed, 152 insertions(+), 3 deletions(-) create mode 100644 src/Parsers/wipePasswordFromQuery.cpp create mode 100644 src/Parsers/wipePasswordFromQuery.h create mode 100644 tests/integration/test_mask_queries_in_logs/__init__.py create mode 100644 tests/integration/test_mask_queries_in_logs/test.py diff --git a/src/Interpreters/executeQuery.cpp b/src/Interpreters/executeQuery.cpp index c501c1722ba..05f5568a472 100644 --- a/src/Interpreters/executeQuery.cpp +++ b/src/Interpreters/executeQuery.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -111,8 +112,11 @@ static String prepareQueryForLogging(const String & query, ContextPtr context) { String res = query; - // wiping sensitive data before cropping query by log_queries_cut_to_length, - // otherwise something like credit card without last digit can go to log + // Wiping a password or its hash from CREATE/ALTER USER query because we don't want it to go to logs. + res = wipePasswordFromQuery(res); + + // 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); diff --git a/src/Parsers/wipePasswordFromQuery.cpp b/src/Parsers/wipePasswordFromQuery.cpp new file mode 100644 index 00000000000..87b297d6fcc --- /dev/null +++ b/src/Parsers/wipePasswordFromQuery.cpp @@ -0,0 +1,32 @@ +#include +#include +#include +#include +#include +#include + + +namespace DB +{ + +String wipePasswordFromQuery(const String & query) +{ + String error_message; + const char * begin = query.data(); + const char * end = begin + query.size(); + + { + ParserCreateUserQuery parser; + const char * pos = begin; + if (auto ast = tryParseQuery(parser, pos, end, error_message, false, "", false, 0, 0)) + { + auto create_query = typeid_cast>(ast); + create_query->show_password = false; + return serializeAST(*create_query); + } + } + + return query; +} + +} diff --git a/src/Parsers/wipePasswordFromQuery.h b/src/Parsers/wipePasswordFromQuery.h new file mode 100644 index 00000000000..cdd8518de53 --- /dev/null +++ b/src/Parsers/wipePasswordFromQuery.h @@ -0,0 +1,14 @@ +#pragma once +#include + + +namespace DB +{ + +/// 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. +String wipePasswordFromQuery(const String & query); + +} diff --git a/tests/integration/test_mask_queries_in_logs/__init__.py b/tests/integration/test_mask_queries_in_logs/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_mask_queries_in_logs/test.py b/tests/integration/test_mask_queries_in_logs/test.py new file mode 100644 index 00000000000..9ab13f41c9e --- /dev/null +++ b/tests/integration/test_mask_queries_in_logs/test.py @@ -0,0 +1,99 @@ +import pytest +from helpers.cluster import ClickHouseCluster + +cluster = ClickHouseCluster(__file__) +node = cluster.add_instance("node") + + +@pytest.fixture(scope="module", autouse=True) +def started_cluster(): + try: + cluster.start() + yield cluster + + finally: + cluster.shutdown() + + +# Passwords in CREATE/ALTER queries must be hidden in logs. +def test_create_alter_user(): + node.query("CREATE USER u1 IDENTIFIED BY 'qwe123' SETTINGS custom_a = 'a'") + node.query("ALTER USER u1 IDENTIFIED BY '123qwe' SETTINGS custom_b = 'b'") + node.query( + "CREATE USER u2 IDENTIFIED WITH plaintext_password BY 'plainpasswd' SETTINGS custom_c = 'c'" + ) + + assert ( + node.query("SHOW CREATE USER u1") + == "CREATE USER u1 IDENTIFIED WITH sha256_password SETTINGS custom_b = \\'b\\'\n" + ) + assert ( + node.query("SHOW CREATE USER u2") + == "CREATE USER u2 IDENTIFIED WITH plaintext_password SETTINGS custom_c = \\'c\\'\n" + ) + + node.query("SYSTEM FLUSH LOGS") + + assert node.contains_in_log("CREATE USER u1") + assert node.contains_in_log("ALTER USER u1") + assert node.contains_in_log("CREATE USER u2") + assert not node.contains_in_log("qwe123") + assert not node.contains_in_log("123qwe") + assert not node.contains_in_log("plainpasswd") + assert not node.contains_in_log("IDENTIFIED WITH sha256_password BY") + assert not node.contains_in_log("IDENTIFIED WITH sha256_hash BY") + assert not node.contains_in_log("IDENTIFIED WITH plaintext_password BY") + + assert ( + int( + node.query( + "SELECT COUNT() FROM system.query_log WHERE query LIKE 'CREATE USER u1%IDENTIFIED WITH sha256_password%'" + ).strip() + ) + >= 1 + ) + + assert ( + int( + node.query( + "SELECT COUNT() FROM system.query_log WHERE query LIKE 'CREATE USER u1%IDENTIFIED WITH sha256_password BY%'" + ).strip() + ) + == 0 + ) + + assert ( + int( + node.query( + "SELECT COUNT() FROM system.query_log WHERE query LIKE 'ALTER USER u1%IDENTIFIED WITH sha256_password%'" + ).strip() + ) + >= 1 + ) + + assert ( + int( + node.query( + "SELECT COUNT() FROM system.query_log WHERE query LIKE 'ALTER USER u1%IDENTIFIED WITH sha256_password BY%'" + ).strip() + ) + == 0 + ) + + assert ( + int( + node.query( + "SELECT COUNT() FROM system.query_log WHERE query LIKE 'CREATE USER u2%IDENTIFIED WITH plaintext_password%'" + ).strip() + ) + >= 1 + ) + + assert ( + int( + node.query( + "SELECT COUNT() FROM system.query_log WHERE query LIKE 'CREATE USER u2%IDENTIFIED WITH plaintext_password BY%'" + ).strip() + ) + == 0 + ) diff --git a/tests/queries/0_stateless/01702_system_query_log.reference b/tests/queries/0_stateless/01702_system_query_log.reference index 1f329feac22..4b9eeb139f4 100644 --- a/tests/queries/0_stateless/01702_system_query_log.reference +++ b/tests/queries/0_stateless/01702_system_query_log.reference @@ -21,7 +21,7 @@ Create CREATE DATABASE sqllt; Create CREATE TABLE sqllt.table\n(\n i UInt8, s String\n)\nENGINE = MergeTree PARTITION BY tuple() ORDER BY tuple(); Create CREATE VIEW sqllt.view AS SELECT i, s FROM sqllt.table; Create CREATE DICTIONARY sqllt.dictionary (key UInt64, value UInt64) PRIMARY KEY key SOURCE(CLICKHOUSE(DB \'sqllt\' TABLE \'table\' HOST \'localhost\' PORT 9001)) LIFETIME(0) LAYOUT(FLAT()); - CREATE USER sqllt_user IDENTIFIED WITH PLAINTEXT_PASSWORD BY \'password\'; + CREATE USER sqllt_user IDENTIFIED WITH plaintext_password CREATE ROLE sqllt_role; CREATE POLICY sqllt_policy ON sqllt.table, sqllt.view, sqllt.dictionary AS PERMISSIVE TO ALL; CREATE POLICY sqllt_row_policy ON sqllt.table, sqllt.view, sqllt.dictionary AS PERMISSIVE TO ALL;