From 5f9e88b260a492fb755f823541a5b2581e5d0d29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Tue, 27 Aug 2019 17:08:26 +0000 Subject: [PATCH 1/3] multi_multiuser: test that worker_merge_files_and_query doesn't allow privilege escalation --- src/test/regress/expected/multi_multiuser.out | 21 +++++++++++++++---- .../regress/expected/multi_multiuser_0.out | 19 ++++++++++++++--- src/test/regress/sql/multi_multiuser.sql | 12 +++++++++++ 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/test/regress/expected/multi_multiuser.out b/src/test/regress/expected/multi_multiuser.out index bd4eb312f..3a85444bd 100644 --- a/src/test/regress/expected/multi_multiuser.out +++ b/src/test/regress/expected/multi_multiuser.out @@ -402,7 +402,7 @@ INSERT INTO full_access_user_schema.t1 VALUES (1),(2),(3); -- not allowed to create a table SELECT create_distributed_table('full_access_user_schema.t1', 'id'); ERROR: permission denied for schema full_access_user_schema -CONTEXT: while executing command on localhost:57638 +CONTEXT: while executing command on localhost:57637 RESET ROLE; -- now we distribute the table as super user SELECT create_distributed_table('full_access_user_schema.t1', 'id'); @@ -536,7 +536,7 @@ ERROR: could not receive file "base/pgsql_job_cache/job_0042/task_000001/p_0000 -- different user should not be able to fetch partition file SET ROLE usage_access; SELECT worker_fetch_partition_file(42, 1, 1, 1, 'localhost', :worker_1_port); -WARNING: could not open file "base/pgsql_job_cache/job_0042/task_000001/p_00001.18007": No such file or directory +WARNING: could not open file "base/pgsql_job_cache/job_0042/task_000001/p_00001.17981": No such file or directory CONTEXT: while executing command on localhost:57637 ERROR: could not receive file "base/pgsql_job_cache/job_0042/task_000001/p_00001" from localhost:57637 -- only the user whom created the files should be able to fetch @@ -575,7 +575,7 @@ RESET ROLE; -- test that the super user is unable to read the contents of the intermediate file, -- although it does create the table SELECT worker_merge_files_into_table(42, 1, ARRAY['a'], ARRAY['integer']); -WARNING: Task file "task_000001.18003" does not have expected suffix ".10" +WARNING: Task file "task_000001.17977" does not have expected suffix ".10" worker_merge_files_into_table ------------------------------- @@ -617,7 +617,7 @@ SELECT worker_merge_files_and_run_query(42, 1, 'CREATE TABLE task_000001_merge(merge_column_0 int)', 'CREATE TABLE task_000001 (a) AS SELECT sum(merge_column_0) FROM task_000001_merge' ); -WARNING: Task file "task_000001.18003" does not have expected suffix ".10" +WARNING: Task file "task_000001.17977" does not have expected suffix ".10" worker_merge_files_and_run_query ---------------------------------- @@ -647,6 +647,19 @@ SELECT worker_merge_files_and_run_query(42, 1, (1 row) +-- test that owner of task cannot execute arbitrary sql +SELECT worker_merge_files_and_run_query(42, 1, + 'CREATE TABLE task_000002_merge(merge_column_0 int)', + 'DROP USER usage_access' +); +ERROR: permission denied to drop role +CONTEXT: SQL statement "DROP USER usage_access" +SELECT worker_merge_files_and_run_query(42, 1, + 'DROP USER usage_access', + 'CREATE TABLE task_000002 (a) AS SELECT sum(merge_column_0) FROM task_000002_merge' +); +ERROR: permission denied to drop role +CONTEXT: SQL statement "DROP USER usage_access" SELECT count(*) FROM pg_merge_job_0042.task_000001_merge; count ------- diff --git a/src/test/regress/expected/multi_multiuser_0.out b/src/test/regress/expected/multi_multiuser_0.out index 9759d0fe6..0264ac8ca 100644 --- a/src/test/regress/expected/multi_multiuser_0.out +++ b/src/test/regress/expected/multi_multiuser_0.out @@ -536,7 +536,7 @@ ERROR: could not receive file "base/pgsql_job_cache/job_0042/task_000001/p_0000 -- different user should not be able to fetch partition file SET ROLE usage_access; SELECT worker_fetch_partition_file(42, 1, 1, 1, 'localhost', :worker_1_port); -WARNING: could not open file "base/pgsql_job_cache/job_0042/task_000001/p_00001.18058": No such file or directory +WARNING: could not open file "base/pgsql_job_cache/job_0042/task_000001/p_00001.18007": No such file or directory CONTEXT: while executing command on localhost:57637 ERROR: could not receive file "base/pgsql_job_cache/job_0042/task_000001/p_00001" from localhost:57637 -- only the user whom created the files should be able to fetch @@ -575,7 +575,7 @@ RESET ROLE; -- test that the super user is unable to read the contents of the intermediate file, -- although it does create the table SELECT worker_merge_files_into_table(42, 1, ARRAY['a'], ARRAY['integer']); -WARNING: Task file "task_000001.18054" does not have expected suffix ".10" +WARNING: Task file "task_000001.18003" does not have expected suffix ".10" worker_merge_files_into_table ------------------------------- @@ -617,7 +617,7 @@ SELECT worker_merge_files_and_run_query(42, 1, 'CREATE TABLE task_000001_merge(merge_column_0 int)', 'CREATE TABLE task_000001 (a) AS SELECT sum(merge_column_0) FROM task_000001_merge' ); -WARNING: Task file "task_000001.18054" does not have expected suffix ".10" +WARNING: Task file "task_000001.18003" does not have expected suffix ".10" worker_merge_files_and_run_query ---------------------------------- @@ -647,6 +647,19 @@ SELECT worker_merge_files_and_run_query(42, 1, (1 row) +-- test that owner of task cannot execute arbitrary sql +SELECT worker_merge_files_and_run_query(42, 1, + 'CREATE TABLE task_000002_merge(merge_column_0 int)', + 'DROP USER usage_access' +); +ERROR: permission denied to drop role +CONTEXT: SQL statement "DROP USER usage_access" +SELECT worker_merge_files_and_run_query(42, 1, + 'DROP USER usage_access', + 'CREATE TABLE task_000002 (a) AS SELECT sum(merge_column_0) FROM task_000002_merge' +); +ERROR: permission denied to drop role +CONTEXT: SQL statement "DROP USER usage_access" SELECT count(*) FROM pg_merge_job_0042.task_000001_merge; count ------- diff --git a/src/test/regress/sql/multi_multiuser.sql b/src/test/regress/sql/multi_multiuser.sql index 6ed300ae2..4ce88471d 100644 --- a/src/test/regress/sql/multi_multiuser.sql +++ b/src/test/regress/sql/multi_multiuser.sql @@ -411,6 +411,18 @@ SELECT worker_merge_files_and_run_query(42, 1, 'CREATE TABLE task_000001_merge(merge_column_0 int)', 'CREATE TABLE task_000001 (a) AS SELECT sum(merge_column_0) FROM task_000001_merge' ); + +-- test that owner of task cannot execute arbitrary sql +SELECT worker_merge_files_and_run_query(42, 1, + 'CREATE TABLE task_000002_merge(merge_column_0 int)', + 'DROP USER usage_access' +); + +SELECT worker_merge_files_and_run_query(42, 1, + 'DROP USER usage_access', + 'CREATE TABLE task_000002 (a) AS SELECT sum(merge_column_0) FROM task_000002_merge' +); + SELECT count(*) FROM pg_merge_job_0042.task_000001_merge; SELECT count(*) FROM pg_merge_job_0042.task_000001; DROP TABLE pg_merge_job_0042.task_000001, pg_merge_job_0042.task_000001_merge; -- drop table so we can reuse the same files for more tests From 8979fd038bfef2e3354ca00d0dce2739a608e573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Tue, 27 Aug 2019 17:23:12 +0000 Subject: [PATCH 2/3] worker_check_invalid_arguments: invalid task/job ids --- .../worker_check_invalid_arguments.out | 21 ++++++++++++++++++- .../sql/worker_check_invalid_arguments.sql | 20 ++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/test/regress/expected/worker_check_invalid_arguments.out b/src/test/regress/expected/worker_check_invalid_arguments.out index 058539fce..5bd826195 100644 --- a/src/test/regress/expected/worker_check_invalid_arguments.out +++ b/src/test/regress/expected/worker_check_invalid_arguments.out @@ -67,8 +67,27 @@ ERROR: column name array size: 2 and type array size: 3 do not match SELECT worker_merge_files_into_table(:JobId, :TaskId, ARRAY['textcolumn', 'binarycolumn'], ARRAY['text', 'integer']); -ERROR: invalid input syntax for integer: "\x0b50" +ERROR: invalid input syntax for type integer: "\x0b50" CONTEXT: COPY task_101108, line 1, column binarycolumn: "\x0b50" +-- Check that we fail to merge when ids are wrong +SELECT worker_merge_files_into_table(-1, :TaskId, + ARRAY['textcolumn', 'binarycolumn'], + ARRAY['text', 'bytea']); +ERROR: could not open directory "base/pgsql_job_cache/job_18446744073709551615/task_101108": No such file or directory +SELECT worker_merge_files_into_table(:JobId, -1, + ARRAY['textcolumn', 'binarycolumn'], + ARRAY['text', 'bytea']); +ERROR: could not open directory "base/pgsql_job_cache/job_201010/task_4294967295": No such file or directory +SELECT worker_merge_files_and_run_query(-1, :TaskId, + 'CREATE TABLE task_000001_merge(merge_column_0 int)', + 'CREATE TABLE task_000001 (a) AS SELECT sum(merge_column_0) FROM task_000001_merge' +); +ERROR: could not open directory "base/pgsql_job_cache/job_18446744073709551615/task_101108": No such file or directory +SELECT worker_merge_files_and_run_query(:JobId, -1, + 'CREATE TABLE task_000001_merge(merge_column_0 int)', + 'CREATE TABLE task_000001 (a) AS SELECT sum(merge_column_0) FROM task_000001_merge' +); +ERROR: could not open directory "base/pgsql_job_cache/job_201010/task_4294967295": No such file or directory -- Finally, merge partitioned files using valid arguments SELECT worker_merge_files_into_table(:JobId, :TaskId, ARRAY['textcolumn', 'binarycolumn'], diff --git a/src/test/regress/sql/worker_check_invalid_arguments.sql b/src/test/regress/sql/worker_check_invalid_arguments.sql index c851d8b16..0c0a252da 100644 --- a/src/test/regress/sql/worker_check_invalid_arguments.sql +++ b/src/test/regress/sql/worker_check_invalid_arguments.sql @@ -75,6 +75,26 @@ SELECT worker_merge_files_into_table(:JobId, :TaskId, ARRAY['textcolumn', 'binarycolumn'], ARRAY['text', 'integer']); +-- Check that we fail to merge when ids are wrong + +SELECT worker_merge_files_into_table(-1, :TaskId, + ARRAY['textcolumn', 'binarycolumn'], + ARRAY['text', 'bytea']); + +SELECT worker_merge_files_into_table(:JobId, -1, + ARRAY['textcolumn', 'binarycolumn'], + ARRAY['text', 'bytea']); + +SELECT worker_merge_files_and_run_query(-1, :TaskId, + 'CREATE TABLE task_000001_merge(merge_column_0 int)', + 'CREATE TABLE task_000001 (a) AS SELECT sum(merge_column_0) FROM task_000001_merge' +); + +SELECT worker_merge_files_and_run_query(:JobId, -1, + 'CREATE TABLE task_000001_merge(merge_column_0 int)', + 'CREATE TABLE task_000001 (a) AS SELECT sum(merge_column_0) FROM task_000001_merge' +); + -- Finally, merge partitioned files using valid arguments SELECT worker_merge_files_into_table(:JobId, :TaskId, From b301cf628ae37405518c9f7eeb3bf4a0f181b302 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philip=20Dub=C3=A9?= Date: Tue, 27 Aug 2019 17:32:00 +0000 Subject: [PATCH 3/3] Test worker_cleanup_job_schema_cache actually drops schemas --- .../expected/task_tracker_cleanup_job.out | 25 +++++++++++++++++++ .../regress/sql/task_tracker_cleanup_job.sql | 6 +++++ 2 files changed, 31 insertions(+) diff --git a/src/test/regress/expected/task_tracker_cleanup_job.out b/src/test/regress/expected/task_tracker_cleanup_job.out index fd6a5d77e..664084a77 100644 --- a/src/test/regress/expected/task_tracker_cleanup_job.out +++ b/src/test/regress/expected/task_tracker_cleanup_job.out @@ -5,6 +5,31 @@ SET citus.next_shard_id TO 1060000; \set JobId 401010 \set CompletedTaskId 801107 \set RunningTaskId 801108 +-- Test worker_cleanup_job_schema_cache +SELECT * FROM task_tracker_assign_task(2, 2, ''); + task_tracker_assign_task +-------------------------- + +(1 row) + +SELECT count(*) FROM pg_catalog.pg_namespace WHERE nspname = 'pg_merge_job_0002'; + count +------- + 1 +(1 row) + +SELECT worker_cleanup_job_schema_cache(); + worker_cleanup_job_schema_cache +--------------------------------- + +(1 row) + +SELECT count(*) FROM pg_catalog.pg_namespace WHERE nspname = 'pg_merge_job_0002'; + count +------- + 0 +(1 row) + -- We assign two tasks to the task tracker. The first task should complete and -- the second task should continue to keep running. SELECT task_tracker_assign_task(:JobId, :CompletedTaskId, diff --git a/src/test/regress/sql/task_tracker_cleanup_job.sql b/src/test/regress/sql/task_tracker_cleanup_job.sql index 153fa7078..9a8b9af82 100644 --- a/src/test/regress/sql/task_tracker_cleanup_job.sql +++ b/src/test/regress/sql/task_tracker_cleanup_job.sql @@ -10,6 +10,12 @@ SET citus.next_shard_id TO 1060000; \set CompletedTaskId 801107 \set RunningTaskId 801108 +-- Test worker_cleanup_job_schema_cache +SELECT * FROM task_tracker_assign_task(2, 2, ''); +SELECT count(*) FROM pg_catalog.pg_namespace WHERE nspname = 'pg_merge_job_0002'; +SELECT worker_cleanup_job_schema_cache(); +SELECT count(*) FROM pg_catalog.pg_namespace WHERE nspname = 'pg_merge_job_0002'; + -- We assign two tasks to the task tracker. The first task should complete and -- the second task should continue to keep running.