diff --git a/programs/server/config.xml b/programs/server/config.xml index 203684a9e00..343e8dc7093 100644 --- a/programs/server/config.xml +++ b/programs/server/config.xml @@ -604,9 +604,22 @@ if this setting is true the user B will see all rows, and if this setting is false the user B will see no rows. By default this setting is false for compatibility with earlier access configurations. --> false + false + + + false + + + false diff --git a/src/Access/AccessControl.cpp b/src/Access/AccessControl.cpp index abd481f0bb6..5d3fc558130 100644 --- a/src/Access/AccessControl.cpp +++ b/src/Access/AccessControl.cpp @@ -165,13 +165,12 @@ void AccessControl::setUpFromMainConfig(const Poco::Util::AbstractConfiguration setNoPasswordAllowed(config_.getBool("allow_no_password", true)); setPlaintextPasswordAllowed(config_.getBool("allow_plaintext_password", true)); - setEnabledUsersWithoutRowPoliciesCanReadRows(config_.getBool( - "access_control_improvements.users_without_row_policies_can_read_rows", - false /* false because we need to be compatible with earlier access configurations */)); - - setOnClusterQueriesRequireClusterGrant(config_.getBool( - "access_control_improvements.on_cluster_queries_require_cluster_grant", - false /* false because we need to be compatible with earlier access configurations */)); + /// Optional improvements in access control system. + /// The default values are false because we need to be compatible with earlier access configurations + setEnabledUsersWithoutRowPoliciesCanReadRows(config_.getBool("access_control_improvements.users_without_row_policies_can_read_rows", false)); + setOnClusterQueriesRequireClusterGrant(config_.getBool("access_control_improvements.on_cluster_queries_require_cluster_grant", false)); + setSelectFromSystemDatabaseRequiresGrant(config_.getBool("access_control_improvements.select_from_system_db_requires_grant", false)); + setSelectFromInformationSchemaDatabaseRequiresGrant(config_.getBool("access_control_improvements.select_from_information_schema_db_requires_grant", false)); addStoragesFromMainConfig(config_, config_path_, get_zookeeper_function_); } diff --git a/src/Access/AccessControl.h b/src/Access/AccessControl.h index 22ff0a488f7..e0571ad370e 100644 --- a/src/Access/AccessControl.h +++ b/src/Access/AccessControl.h @@ -152,6 +152,12 @@ public: void setOnClusterQueriesRequireClusterGrant(bool enable) { on_cluster_queries_require_cluster_grant = enable; } bool doesOnClusterQueriesRequireClusterGrant() const { return on_cluster_queries_require_cluster_grant; } + void setSelectFromSystemDatabaseRequiresGrant(bool enable) { select_from_system_db_requires_grant = enable; } + bool doesSelectFromSystemDatabaseRequireGrant() const { return select_from_system_db_requires_grant; } + + void setSelectFromInformationSchemaDatabaseRequiresGrant(bool enable) { select_from_information_schema_db_requires_grant = enable; } + bool doesSelectFromInformationSchemaDatabaseRequireGrant() const { return select_from_information_schema_db_requires_grant; } + std::shared_ptr getContextAccess( const UUID & user_id, const std::vector & current_roles, @@ -215,6 +221,8 @@ private: std::atomic_bool allow_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; + std::atomic_bool select_from_information_schema_db_requires_grant = false; }; } diff --git a/src/Access/AccessRights.cpp b/src/Access/AccessRights.cpp index b6fed3ac912..747e7a91b2c 100644 --- a/src/Access/AccessRights.cpp +++ b/src/Access/AccessRights.cpp @@ -388,11 +388,11 @@ public: return res; } - void modifyFlags(const ModifyFlagsFunction & function, bool & flags_added, bool & flags_removed) + void modifyFlags(const ModifyFlagsFunction & function, bool grant_option, bool & flags_added, bool & flags_removed) { flags_added = false; flags_removed = false; - modifyFlagsRec(function, flags_added, flags_removed); + modifyFlagsRec(function, grant_option, flags_added, flags_removed); if (flags_added || flags_removed) optimizeTree(); } @@ -669,11 +669,11 @@ private: } template - void modifyFlagsRec(const ModifyFlagsFunction & function, bool & flags_added, bool & flags_removed, const ParentNames & ... parent_names) + void modifyFlagsRec(const ModifyFlagsFunction & function, bool grant_option, bool & flags_added, bool & flags_removed, const ParentNames & ... parent_names) { - auto invoke = [&function](const AccessFlags & flags_, const AccessFlags & min_flags_with_children_, const AccessFlags & max_flags_with_children_, std::string_view database_ = {}, std::string_view table_ = {}, std::string_view column_ = {}) -> AccessFlags + auto invoke = [function, grant_option](const AccessFlags & flags_, const AccessFlags & min_flags_with_children_, const AccessFlags & max_flags_with_children_, std::string_view database_ = {}, std::string_view table_ = {}, std::string_view column_ = {}) -> AccessFlags { - return function(flags_, min_flags_with_children_, max_flags_with_children_, database_, table_, column_); + return function(flags_, min_flags_with_children_, max_flags_with_children_, database_, table_, column_, grant_option); }; if constexpr (sizeof...(ParentNames) < 3) @@ -683,7 +683,7 @@ private: for (auto & child : *children | boost::adaptors::map_values) { const String & child_name = *child.node_name; - child.modifyFlagsRec(function, flags_added, flags_removed, parent_names..., child_name); + child.modifyFlagsRec(function, grant_option, flags_added, flags_removed, parent_names..., child_name); } } } @@ -1062,24 +1062,21 @@ void AccessRights::modifyFlags(const ModifyFlagsFunction & function) { if (!root) return; + bool flags_added, flags_removed; - root->modifyFlags(function, flags_added, flags_removed); + root->modifyFlags(function, false, flags_added, flags_removed); if (flags_removed && root_with_grant_option) root_with_grant_option->makeIntersection(*root); -} - - -void AccessRights::modifyFlagsWithGrantOption(const ModifyFlagsFunction & function) -{ - if (!root_with_grant_option) - return; - bool flags_added, flags_removed; - root_with_grant_option->modifyFlags(function, flags_added, flags_removed); - if (flags_added) + + if (root_with_grant_option) { - if (!root) - root = std::make_unique(); - root->makeUnion(*root_with_grant_option); + root_with_grant_option->modifyFlags(function, true, flags_added, flags_removed); + if (flags_added) + { + if (!root) + root = std::make_unique(); + root->makeUnion(*root_with_grant_option); + } } } diff --git a/src/Access/AccessRights.h b/src/Access/AccessRights.h index 80e37561c2b..5efffc0037a 100644 --- a/src/Access/AccessRights.h +++ b/src/Access/AccessRights.h @@ -109,9 +109,9 @@ public: const AccessFlags & max_flags_with_children, std::string_view database, std::string_view table, - std::string_view column)>; + std::string_view column, + bool grant_option)>; void modifyFlags(const ModifyFlagsFunction & function); - void modifyFlagsWithGrantOption(const ModifyFlagsFunction & function); friend bool operator ==(const AccessRights & left, const AccessRights & right); friend bool operator !=(const AccessRights & left, const AccessRights & right) { return !(left == right); } diff --git a/src/Access/ContextAccess.cpp b/src/Access/ContextAccess.cpp index 92a5179d861..47f6c35ae32 100644 --- a/src/Access/ContextAccess.cpp +++ b/src/Access/ContextAccess.cpp @@ -44,9 +44,17 @@ namespace } - AccessRights addImplicitAccessRights(const AccessRights & access) + AccessRights addImplicitAccessRights(const AccessRights & access, const AccessControl & access_control) { - auto modifier = [&](const AccessFlags & flags, const AccessFlags & min_flags_with_children, const AccessFlags & max_flags_with_children, std::string_view database, std::string_view table, std::string_view column) -> AccessFlags + AccessFlags max_flags; + + auto modifier = [&](const AccessFlags & flags, + const AccessFlags & min_flags_with_children, + const AccessFlags & max_flags_with_children, + std::string_view database, + std::string_view table, + std::string_view column, + bool /* grant_option */) -> AccessFlags { size_t level = !database.empty() + !table.empty() + !column.empty(); AccessFlags res = flags; @@ -115,17 +123,55 @@ namespace res |= show_databases; } + max_flags |= max_flags_with_children; + return res; }; AccessRights res = access; res.modifyFlags(modifier); - res.modifyFlagsWithGrantOption(modifier); - /// Anyone has access to the "system" and "information_schema" database. - res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE); - res.grant(AccessType::SELECT, DatabaseCatalog::INFORMATION_SCHEMA); - res.grant(AccessType::SELECT, DatabaseCatalog::INFORMATION_SCHEMA_UPPERCASE); + if (access_control.doesSelectFromSystemDatabaseRequireGrant()) + { + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "one"); + + if (max_flags.contains(AccessType::SHOW_USERS)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "users"); + + if (max_flags.contains(AccessType::SHOW_ROLES)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "roles"); + + if (max_flags.contains(AccessType::SHOW_ROW_POLICIES)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "row_policies"); + + if (max_flags.contains(AccessType::SHOW_SETTINGS_PROFILES)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "settings_profiles"); + + if (max_flags.contains(AccessType::SHOW_QUOTAS)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "quotas"); + + if (max_flags.contains(AccessType::SHOW_COLUMNS)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "columns"); + + if (max_flags.contains(AccessType::SHOW_TABLES)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "tables"); + + if (max_flags.contains(AccessType::SHOW_DATABASES)) + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE, "databases"); + } + else + { + /// Anyone has access to the "system" database. + res.grant(AccessType::SELECT, DatabaseCatalog::SYSTEM_DATABASE); + } + + if (!access_control.doesSelectFromInformationSchemaDatabaseRequireGrant()) + { + /// Anyone has access to the "information_schema" database. + res.grant(AccessType::SELECT, DatabaseCatalog::INFORMATION_SCHEMA); + res.grant(AccessType::SELECT, DatabaseCatalog::INFORMATION_SCHEMA_UPPERCASE); + } + return res; } @@ -247,7 +293,7 @@ void ContextAccess::setRolesInfo(const std::shared_ptr & void ContextAccess::calculateAccessRights() const { access = std::make_shared(mixAccessRightsFromUserAndRoles(*user, *roles_info)); - access_with_implicit = std::make_shared(addImplicitAccessRights(*access)); + access_with_implicit = std::make_shared(addImplicitAccessRights(*access, *access_control)); if (trace_log) { @@ -342,7 +388,7 @@ std::shared_ptr ContextAccess::getFullAccess() auto full_access = std::shared_ptr(new ContextAccess); full_access->is_full_access = true; full_access->access = std::make_shared(AccessRights::getFullAccess()); - full_access->access_with_implicit = std::make_shared(addImplicitAccessRights(*full_access->access)); + full_access->access_with_implicit = full_access->access; return full_access; }(); return res; diff --git a/tests/config/config.d/enable_access_control_improvements.xml b/tests/config/config.d/enable_access_control_improvements.xml index 052858a9519..3bab0d95144 100644 --- a/tests/config/config.d/enable_access_control_improvements.xml +++ b/tests/config/config.d/enable_access_control_improvements.xml @@ -2,5 +2,7 @@ true true + true + true diff --git a/tests/integration/helpers/0_common_instance_config.xml b/tests/integration/helpers/0_common_instance_config.xml index b6ea21648bb..4bede7767c5 100644 --- a/tests/integration/helpers/0_common_instance_config.xml +++ b/tests/integration/helpers/0_common_instance_config.xml @@ -21,5 +21,7 @@ true + true + true