Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix calculating grants for introspection #9840

Merged
merged 1 commit into from
Mar 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
21 changes: 11 additions & 10 deletions dbms/src/Access/ContextAccess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -372,17 +372,17 @@ boost::shared_ptr<const AccessRights> ContextAccess::calculateResultAccess(bool

boost::shared_ptr<const AccessRights> ContextAccess::calculateResultAccess(bool grant_option, UInt64 readonly_, bool allow_ddl_, bool allow_introspection_) const
{
size_t cache_index = static_cast<size_t>(readonly_ != params.readonly)
size_t index = static_cast<size_t>(readonly_ != params.readonly)
+ static_cast<size_t>(allow_ddl_ != params.allow_ddl) * 2 +
+ static_cast<size_t>(allow_introspection_ != params.allow_introspection) * 3
+ static_cast<size_t>(grant_option) * 4;
assert(cache_index < std::size(result_access));
auto res = result_access[cache_index].load();
assert(index < std::size(result_access));
auto res = result_access[index].load();
if (res)
return res;

std::lock_guard lock{mutex};
res = result_access[cache_index].load();
res = result_access[index].load();
if (res)
return res;

Expand Down Expand Up @@ -412,10 +412,7 @@ boost::shared_ptr<const AccessRights> ContextAccess::calculateResultAccess(bool
| AccessType::DROP_ROLE | AccessType::DROP_POLICY | AccessType::DROP_QUOTA | AccessType::ROLE_ADMIN;

if (readonly_)
merged_access->revoke(write_table_access | all_dcl | AccessType::SYSTEM | AccessType::KILL_QUERY);

if (readonly_ || !allow_ddl_)
merged_access->revoke(table_and_dictionary_ddl);
merged_access->revoke(write_table_access | all_dcl | table_and_dictionary_ddl | AccessType::SYSTEM | AccessType::KILL_QUERY);

if (readonly_ == 1)
{
Expand All @@ -424,7 +421,10 @@ boost::shared_ptr<const AccessRights> ContextAccess::calculateResultAccess(bool
merged_access->revoke(AccessType::CREATE_TEMPORARY_TABLE | AccessType::TABLE_FUNCTIONS);
}

if (!allow_introspection_)
if (!allow_ddl_ && !grant_option)
merged_access->revoke(table_and_dictionary_ddl);

if (!allow_introspection_ && !grant_option)
merged_access->revoke(AccessType::INTROSPECTION);

/// Anyone has access to the "system" database.
Expand Down Expand Up @@ -452,10 +452,11 @@ boost::shared_ptr<const AccessRights> ContextAccess::calculateResultAccess(bool
"Current_roles: " << boost::algorithm::join(roles_info->getCurrentRolesNames(), ", ")
<< ", enabled_roles: " << boost::algorithm::join(roles_info->getEnabledRolesNames(), ", "));
}
LOG_TRACE(trace_log, "Settings: readonly=" << readonly_ << ", allow_ddl=" << allow_ddl_ << ", allow_introspection_functions=" << allow_introspection_);
}

res = std::move(merged_access);
result_access[cache_index].store(res);
result_access[index].store(res);
return res;
}

Expand Down
2 changes: 1 addition & 1 deletion dbms/src/Interpreters/Context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ void Context::setUser(const String & name, const String & password, const Poco::
std::shared_ptr<const ContextAccess> new_access;
if (new_user_id)
{
new_access = getAccessControlManager().getContextAccess(*new_user_id, {}, true, {}, current_database, client_info);
new_access = getAccessControlManager().getContextAccess(*new_user_id, {}, true, settings, current_database, client_info);
if (!new_access->isClientHostAllowed() || !new_access->isCorrectPassword(password))
{
new_user_id = {};
Expand Down
22 changes: 22 additions & 0 deletions dbms/tests/integration/test_settings_profile/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,25 @@ def test_alter_and_drop():
assert instance.query("SELECT value FROM system.settings WHERE name = 'max_memory_usage'", user="robin") == "10000000000\n"
instance.query("SET max_memory_usage = 80000000", user="robin")
instance.query("SET max_memory_usage = 120000000", user="robin")


def test_allow_introspection():
assert "Not enough privileges" in instance.query_and_get_error("SELECT demangle('a')", user="robin")

instance.query("GRANT ALL ON *.* TO robin")
assert "Introspection functions are disabled" in instance.query_and_get_error("SELECT demangle('a')", user="robin")

instance.query("ALTER USER robin SETTINGS allow_introspection_functions=1")
assert instance.query("SELECT demangle('a')", user="robin") == "signed char\n"

instance.query("ALTER USER robin SETTINGS NONE")
assert "Introspection functions are disabled" in instance.query_and_get_error("SELECT demangle('a')", user="robin")

instance.query("CREATE SETTINGS PROFILE xyz SETTINGS allow_introspection_functions=1 TO robin")
assert instance.query("SELECT demangle('a')", user="robin") == "signed char\n"

instance.query("DROP SETTINGS PROFILE xyz")
assert "Introspection functions are disabled" in instance.query_and_get_error("SELECT demangle('a')", user="robin")

instance.query("REVOKE ALL ON *.* FROM robin")
assert "Not enough privileges" in instance.query_and_get_error("SELECT demangle('a')", user="robin")