Skip to content

Commit a45f351

Browse files
committed
fix(query): the stage utilized by the history table needs to be restricted
The history table uses a stage named log_1f93b76af0bd4b1d8e018667865fbc65 to store intermediate raw log files, and access to this stage need also be controlled by the privilege system.
1 parent 715c54f commit a45f351

File tree

4 files changed

+114
-29
lines changed

4 files changed

+114
-29
lines changed

src/query/service/src/interpreters/access/privilege_access.rs

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ use databend_common_sql::plans::RewriteKind;
4646
use databend_common_sql::Planner;
4747
use databend_common_users::RoleCacheManager;
4848
use databend_common_users::UserApiProvider;
49+
use databend_common_users::BUILTIN_ROLE_ACCOUNT_ADMIN;
4950
use databend_enterprise_resources_management::ResourcesManagement;
5051
use databend_storages_common_table_meta::table::OPT_KEY_TEMP_PREFIX;
5152

@@ -165,8 +166,9 @@ impl PrivilegeAccess {
165166

166167
fn access_system_history(
167168
&self,
168-
catalog_name: &str,
169-
db_name: &str,
169+
catalog_name: Option<&str>,
170+
db_name: Option<&str>,
171+
stage_name: Option<&str>,
170172
privilege: UserPrivilegeType,
171173
) -> Result<()> {
172174
let cluster_id = GlobalConfig::instance().query.cluster_id.clone();
@@ -176,20 +178,50 @@ impl PrivilegeAccess {
176178
{
177179
return Ok(());
178180
}
179-
if catalog_name == CATALOG_DEFAULT
180-
&& db_name.eq_ignore_ascii_case(SENSITIVE_SYSTEM_RESOURCE)
181-
&& !matches!(
182-
privilege,
183-
UserPrivilegeType::Select | UserPrivilegeType::Drop
184-
)
185-
{
186-
return Err(ErrorCode::PermissionDenied(
187-
format!(
188-
"Permission Denied: Operation '{:?}' on database 'default.system_history' is not allowed. This sensitive system resource only supports 'SELECT' and 'DROP'",
189-
privilege
190-
),
191-
));
181+
match (catalog_name, db_name, stage_name) {
182+
(Some(catalog_name), Some(db_name), None) => {
183+
if catalog_name == CATALOG_DEFAULT
184+
&& db_name.eq_ignore_ascii_case(SENSITIVE_SYSTEM_RESOURCE)
185+
&& !matches!(
186+
privilege,
187+
UserPrivilegeType::Select | UserPrivilegeType::Drop
188+
)
189+
{
190+
return Err(ErrorCode::PermissionDenied(
191+
format!(
192+
"Permission Denied: Operation '{:?}' on database 'default.system_history' is not allowed. This sensitive system resource only supports 'SELECT' and 'DROP'",
193+
privilege
194+
),
195+
));
196+
}
197+
}
198+
(None, None, Some(stage_name)) => {
199+
let config = GlobalConfig::instance();
200+
let sensitive_system_stage = config.log.history.stage_name.clone();
201+
202+
return if stage_name.eq_ignore_ascii_case(&sensitive_system_stage) {
203+
if let Some(current_role) = self.ctx.get_current_role() {
204+
if current_role.name == BUILTIN_ROLE_ACCOUNT_ADMIN {
205+
Ok(())
206+
} else {
207+
Err(ErrorCode::PermissionDenied(format!(
208+
"Permission Denied: Operation '{:?}' on stage {sensitive_system_stage} is not allowed",
209+
privilege
210+
)))
211+
}
212+
} else {
213+
Err(ErrorCode::PermissionDenied(format!(
214+
"Permission Denied: Operation '{:?}' on stage {sensitive_system_stage} is not allowed",
215+
privilege
216+
)))
217+
}
218+
} else {
219+
Ok(())
220+
};
221+
}
222+
_ => unreachable!(),
192223
}
224+
193225
Ok(())
194226
}
195227

@@ -200,7 +232,7 @@ impl PrivilegeAccess {
200232
privileges: UserPrivilegeType,
201233
if_exists: bool,
202234
) -> Result<()> {
203-
self.access_system_history(catalog_name, db_name, privileges)?;
235+
self.access_system_history(Some(catalog_name), Some(db_name), None, privileges)?;
204236
let tenant = self.ctx.get_tenant();
205237
let check_current_role_only = match privileges {
206238
// create table/stream need check db's Create Privilege
@@ -305,7 +337,7 @@ impl PrivilegeAccess {
305337
return Ok(());
306338
}
307339

308-
self.access_system_history(catalog_name, db_name, privilege)?;
340+
self.access_system_history(Some(catalog_name), Some(db_name), None, privilege)?;
309341

310342
if self.ctx.is_temp_table(catalog_name, db_name, table_name) {
311343
return Ok(());
@@ -625,6 +657,9 @@ impl PrivilegeAccess {
625657
return Ok(());
626658
}
627659

660+
// History Config has a inner stage, can not be operator by any user.
661+
self.access_system_history(None, None, Some(&stage_info.stage_name), privilege)?;
662+
628663
// Note: validate_stage_access is not used for validate Create Stage privilege
629664
self.validate_access(
630665
&GrantObject::Stage(stage_info.stage_name.to_string()),

src/query/service/src/interpreters/interpreter_privilege_grant.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use std::sync::Arc;
1616

1717
use databend_common_base::base::GlobalInstance;
18+
use databend_common_config::GlobalConfig;
1819
use databend_common_exception::ErrorCode;
1920
use databend_common_exception::Result;
2021
use databend_common_meta_app::principal::GrantObject;
@@ -33,6 +34,7 @@ use log::info;
3334

3435
use crate::interpreters::common::validate_grant_object_exists;
3536
use crate::interpreters::util::check_system_history;
37+
use crate::interpreters::util::check_system_history_stage;
3638
use crate::interpreters::Interpreter;
3739
use crate::pipelines::PipelineBuildResult;
3840
use crate::sessions::QueryContext;
@@ -110,9 +112,14 @@ impl GrantPrivilegeInterpreter {
110112
db_id: *db_id,
111113
})
112114
},
113-
GrantObject::Stage(name) => Ok(OwnershipObject::Stage {
114-
name: name.to_string(),
115-
}),
115+
GrantObject::Stage(name) => {
116+
let config = GlobalConfig::instance();
117+
let sensitive_system_stage = config.log.history.stage_name.clone();
118+
check_system_history_stage(&name, &sensitive_system_stage)?;
119+
Ok(OwnershipObject::Stage {
120+
name: name.to_string(),
121+
})
122+
},
116123
GrantObject::UDF(name) => Ok(OwnershipObject::UDF {
117124
name: name.to_string(),
118125
}),

src/query/service/src/interpreters/util.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,18 @@ pub fn check_system_history(
4848
Ok(())
4949
}
5050

51+
pub fn check_system_history_stage(
52+
stage_name: &str,
53+
sensitive_system_stage: &str,
54+
) -> databend_common_exception::Result<()> {
55+
if stage_name.eq_ignore_ascii_case(sensitive_system_stage) {
56+
return Err(ErrorCode::IllegalGrant(
57+
"Can not modify system history stage {sensitive_system_stage} ownership",
58+
));
59+
}
60+
Ok(())
61+
}
62+
5163
#[allow(clippy::type_complexity)]
5264
pub fn generate_desc_schema(
5365
schema: TableSchemaRef,

tests/logging/check_logs_table.sh

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-
106106
response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d '{"sql": "grant select , drop on system_history.* to role ra"}')
107107
response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d '{"sql": "grant alter on system_history.* to role ra"}')
108108
response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d '{"sql": "grant role ra to a"}')
109+
response=$(curl -s -u root: -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d '{"sql": "grant write, read on stage log_1f93b76af0bd4b1d8e018667865fbc65 to a"}')
109110

110111
execute_and_verify() {
111112
local cmd_description="$1"
@@ -114,14 +115,16 @@ execute_and_verify() {
114115
local jq_expression="$4"
115116
local result
116117

117-
echo -n "Executing: $cmd_description ... "
118+
echo "Executing: $cmd_description ... "
118119

119120
result=$(curl -s -u "${user_cred}" -XPOST "http://localhost:8000/v1/query" -H 'Content-Type: application/json' -d "${json_payload}" | jq -r "${jq_expression}")
120121

121122
if [[ "$result" != "true" ]]; then
122123
echo "Failed! Expected result: true, actual result: $result."
123124
echo "$cmd_description failed."
124125
exit 1 # Exit script immediately if it fails
126+
else
127+
echo "Description: $cmd_description completed successfully."
125128
fi
126129
}
127130

@@ -140,32 +143,60 @@ check_system_history_permissions() {
140143
'{"sql": "alter table system_history.log_history add column id int"}' \
141144
'.state == "Failed"'
142145

143-
# Command 3: User 'a:123' attempts to drop 'system_history.access_history' table (expected to succeed)
146+
# Command 3: User 'a:123' drop 'system_history.access_history' table (expected to succeed)
144147
execute_and_verify \
145-
"User 'a:123' attempts to drop 'system_history.access_history' table" \
148+
"User 'a:123' drop 'system_history.access_history' table" \
146149
"a:123" \
147150
'{"sql": "drop table system_history.access_history"}' \
148151
'.state == "Succeeded"'
149152

150-
# Command 4: User 'root:' attempts to grant ownership on 'system_history.*' (expected to fail)
153+
# Command 4: User 'root:' grant ownership on 'system_history.*' (expected to fail)
151154
execute_and_verify \
152-
"User 'root:' attempts to grant ownership on 'system_history.*'" \
155+
"User 'root:' grant ownership on 'system_history.*'" \
153156
"root:" \
154157
'{"sql": "grant ownership on system_history.* to role ra"}' \
155158
'.state == "Failed"'
156159

157-
# Command 5: User 'root:' attempts to grant ownership on 'system_history.query_history' (expected to fail)
160+
# Command 5: User 'root:' grant ownership on 'system_history.query_history' (expected to fail)
158161
execute_and_verify \
159-
"User 'root:' attempts to grant ownership on 'system_history.query_history'" \
162+
"User 'root:' grant ownership on 'system_history.query_history'" \
160163
"root:" \
161164
'{"sql": "grant ownership on system_history.query_history to role ra"}' \
162165
'.state == "Failed"'
163-
# Command 6: User 'a:123' attempts to select 'system_history.query_history' table (expected to succeed)
166+
# Command 6: User 'a:123' select 'system_history.query_history' table (expected to succeed)
164167
execute_and_verify \
165-
"User 'a:123' attempts to query 'system_history.query_history' table" \
168+
"User 'a:123' query 'system_history.query_history' table" \
166169
"a:123" \
167170
'{"sql": "select count() from system_history.query_history"}' \
168171
'.state == "Succeeded"'
172+
173+
# Command 7: User 'a:123' drop stage 'log_1f93b76af0bd4b1d8e018667865fbc65' table (expected to failed)
174+
execute_and_verify \
175+
"User 'a:123' drop stage log_1f93b76af0bd4b1d8e018667865fbc65" \
176+
"a:123" \
177+
'{"sql": "drop stage log_1f93b76af0bd4b1d8e018667865fbc65"}' \
178+
'.state == "Failed"'
179+
180+
# Command 8: User 'a:123' copy into @log_1f93b76af0bd4b1d8e018667865fbc65 from (select * from system.one) (expected to failed)
181+
execute_and_verify \
182+
"User 'a:123' copy into @log_1f93b76af0bd4b1d8e018667865fbc65 from (select * from system.one);" \
183+
"a:123" \
184+
'{"sql": "copy into @log_1f93b76af0bd4b1d8e018667865fbc65 from (select * from system.one);"}' \
185+
'.state == "Failed"'
186+
187+
# Command 9: User 'a:123' copy into t from (select * from @log_1f93b76af0bd4b1d8e018667865fbc65) (expected to failed)
188+
execute_and_verify \
189+
"User 'a:123' copy into t from (select * from @log_1f93b76af0bd4b1d8e018667865fbc65);" \
190+
"a:123" \
191+
'{"sql": "copy into t from (select * from @log_1f93b76af0bd4b1d8e018667865fbc65);"}' \
192+
'.state == "Failed"'
193+
194+
# Command 10: User 'root' drop stage 'log_1f93b76af0bd4b1d8e018667865fbc65' (expected to succeed)
195+
execute_and_verify \
196+
"User 'root' drop stage log_1f93b76af0bd4b1d8e018667865fbc65" \
197+
"root:" \
198+
'{"sql": "drop stage log_1f93b76af0bd4b1d8e018667865fbc65"}' \
199+
'.state == "Succeeded"'
169200
}
170201

171202
check_system_history_permissions

0 commit comments

Comments
 (0)