postgres/timescaledb
1
0
Fork 0
mirror of https://github.com/timescale/timescaledb.git synced 2025-05-16 18:43:18 +08:00
Code Issues Packages Projects Releases Wiki Activity

Better superuser handling for move_chunk

Browse Source 
The current code was assuming the bootstrap superuser for the actual
move chunk operation. However, we can make it further flexible by using
the logged in credentials if those happen to have superuser privileges.
This commit is contained in:
Nikhil Sontakke 2022-06-24 13:19:31 +05:30 committed by Nikhil
parent 0b76a8e5c3
commit 8bf6c887a9
3 changed files with 66 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
tsl/src/chunk_copy.c
View File

@ -269,12 +269,6 @@ chunk_copy_setup(ChunkCopy *cc, Oid chunk_relid, const char *src_node, const cha
Cache *hcache;
MemoryContext old, mcxt;
if (!superuser() && !has_rolreplication(GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(
"must be superuser or replication role to copy/move chunk to data node"))));
if (dist_util_membership() != DIST_MEMBER_ACCESS_NODE)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@ -300,9 +294,16 @@ chunk_copy_setup(ChunkCopy *cc, Oid chunk_relid, const char *src_node, const cha
CACHE_FLAG_NONE,
&hcache);
if (!superuser() && !has_rolreplication(GetUserId()) &&
(ts_rel_get_owner(ht->main_table_relid) != GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg("must be superuser, replication role, or hypertable owner to copy/move "
"chunk to data node"))));
/*
* We have already checked above for superuser or replication perms. There's no
* need to check if the user has "ownership" on the hypertable now.
* We have already checked above for superuser/replication/owner perms. There's no
* need to check again if the user has "ownership" on the hypertable now.
*/
if (!hypertable_is_distributed(ht))
ereport(ERROR,
@ -1051,6 +1052,7 @@ chunk_copy_execute(ChunkCopy *cc)
{
int sec_ctx;
Oid saved_uid;
bool is_superuser;
/*
* A chunk copy/move operation involves a lot of stages. Many of these
@ -1062,13 +1064,22 @@ chunk_copy_execute(ChunkCopy *cc)
*
* The move_chunk/copy_chunk functions can only be invoked by superuser
* or REPLICATION users. To keep things manageable, we switch to the
* bootstrap superuser for each stage of the execution. Care is taken to
* create the chunks with original hypertable ownership.
* bootstrap superuser (or if the current logged in user is a superuser)
* for each stage of the execution. Care is taken to create the chunks
* with original hypertable ownership.
*/
SPI_start_transaction();
GetUserIdAndSecContext(&saved_uid, &sec_ctx);
if (BOOTSTRAP_SUPERUSERID != saved_uid)
is_superuser = superuser();
/*
* Note that if the current logged in user is superuser then we use those
* credentials instead of implicit bootstrap superuser
*/
if (!is_superuser)
{
GetUserIdAndSecContext(&saved_uid, &sec_ctx);
SetUserIdAndSecContext(BOOTSTRAP_SUPERUSERID, sec_ctx | SECURITY_LOCAL_USERID_CHANGE);
}
cc->stage = stage;
@ -1080,7 +1091,7 @@ chunk_copy_execute(ChunkCopy *cc)
DEBUG_ERROR_INJECTION(stage->name);
if (BOOTSTRAP_SUPERUSERID != saved_uid)
if (!is_superuser)
SetUserIdAndSecContext(saved_uid, sec_ctx);
SPI_commit();
}
@ -1223,6 +1234,7 @@ chunk_copy_cleanup_internal(ChunkCopy *cc, int stage_idx)
{
int sec_ctx;
Oid saved_uid;
bool is_superuser = superuser();
/*
* A chunk copy/move cleanup operation involves a lot of stages. Many of these
@ -1236,9 +1248,16 @@ chunk_copy_cleanup_internal(ChunkCopy *cc, int stage_idx)
* bootstrap superuser for each stage of the execution.
*/
SPI_start_transaction();
GetUserIdAndSecContext(&saved_uid, &sec_ctx);
if (BOOTSTRAP_SUPERUSERID != saved_uid)
/*
* Note that if the current logged in user is superuser then we use those
* credentials instead of implicit bootstrap superuser
*/
if (!is_superuser)
{
GetUserIdAndSecContext(&saved_uid, &sec_ctx);
SetUserIdAndSecContext(BOOTSTRAP_SUPERUSERID, sec_ctx | SECURITY_LOCAL_USERID_CHANGE);
}
cc->stage = &chunk_copy_stages[stage_idx];
if (cc->stage->function_cleanup)
@ -1250,7 +1269,7 @@ chunk_copy_cleanup_internal(ChunkCopy *cc, int stage_idx)
else
first = false;
if (BOOTSTRAP_SUPERUSERID != saved_uid)
if (!is_superuser)
SetUserIdAndSecContext(saved_uid, sec_ctx);
SPI_commit();
} while (--stage_idx >= 0);
@ -1265,12 +1284,6 @@ chunk_copy_cleanup(const char *operation_id)
bool found = false;
int stage_idx;
if (!superuser() && !has_rolreplication(GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg(
"must be superuser or replication role to cleanup a chunk copy operation"))));
if (dist_util_membership() != DIST_MEMBER_ACCESS_NODE)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@ -1302,6 +1315,13 @@ chunk_copy_cleanup(const char *operation_id)
}
}
if (!superuser() && !has_rolreplication(GetUserId()) &&
(ts_rel_get_owner(cc->chunk->hypertable_relid) != GetUserId()))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
(errmsg("must be superuser, replication role, or hypertable owner to cleanup a "
"chunk copy/move operation"))));
/* should always find an entry, add ereport to quell compiler warning */
Assert(found == true);
if (!found)

2
tsl/test/expected/dist_move_chunk.out
View File

@ -133,7 +133,7 @@ ROLLBACK;
SET ROLE :ROLE_DEFAULT_PERM_USER;
\set ON_ERROR_STOP 0
CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'data_node_1', destination_node => 'data_node_2');
ERROR: must be superuser or replication role to copy/move chunk to data node
ERROR: must be superuser, replication role, or hypertable owner to copy/move chunk to data node
\set ON_ERROR_STOP 1
SET ROLE :ROLE_1;
-- can't run copy/move chunk on a data node

27
tsl/test/t/002_chunk_copy_move.pl
View File

@ -8,7 +8,7 @@ use warnings;
use AccessNode;
use DataNode;
use TestLib;
use Test::More tests => 286;
use Test::More tests => 287;
#Initialize all the multi-node instances
my $an = AccessNode->create('an');
@ -18,10 +18,18 @@ my $dn2 = DataNode->create('dn2', allows_streaming => 'logical');
$an->add_data_node($dn1);
$an->add_data_node($dn2);
$an->safe_psql('postgres',
"GRANT USAGE ON FOREIGN SERVER dn1, dn2 TO PUBLIC;");
for my $node ($an, $dn1, $dn2)
{
$node->safe_psql('postgres', "CREATE ROLE htowner LOGIN");
}
#Create few distributed hypertables with default and specified schema names and insert a few rows
$an->safe_psql(
'postgres',
qq[
SET ROLE htowner;
CREATE TABLE test(time timestamp NOT NULL, device int, temp float);
SELECT create_distributed_hypertable('test', 'time', 'device', 3);
INSERT INTO test SELECT t, (abs(timestamp_hash(t::timestamp)) % 10) + 1, 0.10 FROM generate_series('2018-03-02 1:00'::TIMESTAMPTZ, '2018-03-08 1:00', '1 hour') t;
@ -65,7 +73,7 @@ while ($curr_index < $arrSize)
#We provide the operation id ourselves
$operation_id = "ts_cloud_" . $curr_index . "_1";
($ret, $stdout, $stderr) = $an->psql('postgres',
"SELECT error_injection_on('$stages[$curr_index]'); CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'dn1', destination_node => 'dn2', operation_id => '$operation_id');"
"SELECT error_injection_on('$stages[$curr_index]'); SET ROLE htowner; CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'dn1', destination_node => 'dn2', operation_id => '$operation_id');"
);
is($ret, 3,
"move_chunk fails as expected in stage '$stages[$curr_index]'");
@ -77,7 +85,7 @@ while ($curr_index < $arrSize)
#The earlier debug error point gets released automatically since it's a session lock
#Call the cleanup procedure to make things right
$an->safe_psql('postgres',
"CALL timescaledb_experimental.cleanup_copy_chunk_operation(operation_id=>'$operation_id');"
"SET ROLE htowner; CALL timescaledb_experimental.cleanup_copy_chunk_operation(operation_id=>'$operation_id');"
);
#Check chunk state is as before the move
@ -103,7 +111,7 @@ for my $node ($an, $dn1, $dn2)
like(
$stderr,
qr/must be superuser or replication role to copy\/move chunk to data node/,
qr/must be superuser, replication role, or hypertable owner to copy\/move chunk to data node/,
'Expected failure due to no credentials');
#Provide REPLICATION creds to this user now
@ -141,6 +149,17 @@ $an->psql_is(
"ts_copy_1_1|complete|dn1|dn2|t",
"AN catalog is as expected");
# Cleanup with a role which doesn't have superuser, replication or ownernership should fail
$an->safe_psql('postgres', "CREATE ROLE testrole2 LOGIN;");
($ret, $stdout, $stderr) = $an->psql('postgres',
"SET ROLE testrole2; CALL timescaledb_experimental.move_chunk(chunk=>'_timescaledb_internal._dist_hyper_1_1_chunk', source_node=> 'dn1', destination_node => 'dn2')"
);
like(
$stderr,
qr/must be superuser, replication role, or hypertable owner to copy\/move chunk to data node/,
'Expected failure due to no credentials');
#
#Run cleanup on this operstion. It should just delete the catalog entry since the
#activity has completed successfully. Rest of the checks below should succeed
#Try this cleanup as the REPLICATION user