From d0fc6c5849ba96fabca2131b8c674f138180cf7a Mon Sep 17 00:00:00 2001 From: Bobbie Soedirgo Date: Wed, 25 Sep 2024 16:00:31 +0100 Subject: [PATCH] feat: dropping table triggers w/o being owners --- Makefile | 2 +- README.md | 9 ++ nix/withTmpDb.sh.in | 3 +- src/drop_trigger_grants.c | 204 ++++++++++++++++++++++++++++++ src/drop_trigger_grants.h | 42 ++++++ src/supautils.c | 143 ++++++++++++++++----- test/expected/privileged_role.out | 35 +++++ test/sql/privileged_role.sql | 30 +++++ 8 files changed, 435 insertions(+), 33 deletions(-) create mode 100644 src/drop_trigger_grants.c create mode 100644 src/drop_trigger_grants.h diff --git a/Makefile b/Makefile index 32670db..9ffbed4 100644 --- a/Makefile +++ b/Makefile @@ -8,7 +8,7 @@ else endif MODULE_big = supautils -OBJS = src/supautils.o src/privileged_extensions.o src/constrained_extensions.o src/extensions_parameter_overrides.o src/policy_grants.o src/utils.o +OBJS = src/supautils.o src/privileged_extensions.o src/drop_trigger_grants.o src/constrained_extensions.o src/extensions_parameter_overrides.o src/policy_grants.o src/utils.o PG_VERSION = $(strip $(shell $(PG_CONFIG) --version | $(GREP) -oP '(?<=PostgreSQL )[0-9]+')) PG_GE16 = $(shell test $(PG_VERSION) -ge 16; echo $$?) diff --git a/README.md b/README.md index 89b7141..8a04c95 100644 --- a/README.md +++ b/README.md @@ -194,6 +194,15 @@ supautils.policy_grants = '{ "my_role": ["public.not_my_table", "public.also_not This allows `my_role` to manage policies for `public.not_my_table` and `public.also_not_my_table` without being an owner of these tables. +## Dropping Triggers Without Table Ownership + +In addition to managing policies, you can also allow certain roles to drop +triggers on a table without being the table owner: + +``` +supautils.drop_trigger_grants = '{ "my_role": ["public.not_my_table", "public.also_not_my_table"] }' +``` + ## Development [Nix](https://nixos.org/download.html) is required to set up the environment. diff --git a/nix/withTmpDb.sh.in b/nix/withTmpDb.sh.in index 6fb9d34..2662625 100644 --- a/nix/withTmpDb.sh.in +++ b/nix/withTmpDb.sh.in @@ -27,9 +27,10 @@ placeholder_stuff_options='-c supautils.placeholders="response.headers, another. cexts_option='-c supautils.constrained_extensions="{\"adminpack\": { \"cpu\": 64}, \"cube\": { \"mem\": \"17 GB\"}, \"lo\": { \"disk\": \"20 GB\"}, \"amcheck\": { \"cpu\": 2, \"mem\": \"100 MB\", \"disk\": \"100 MB\"}}"' epos_option='-c supautils.extensions_parameter_overrides="{\"sslinfo\":{\"schema\":\"pg_catalog\"}}"' +drop_trigger_grants_option='-c supautils.drop_trigger_grants="{\"privileged_role\":[\"allow_drop_triggers.my_table\"]}"' policy_grants_option='-c supautils.policy_grants="{\"privileged_role\":[\"allow_policies.my_table\",\"allow_policies.nonexistent_table\"]}"' -pg_ctl start -o "$options" -o "$reserved_stuff_options" -o "$placeholder_stuff_options" -o "$cexts_option" -o "$epos_option" -o "$policy_grants_option" +pg_ctl start -o "$options" -o "$reserved_stuff_options" -o "$placeholder_stuff_options" -o "$cexts_option" -o "$epos_option" -o "$drop_trigger_grants_option" -o "$policy_grants_option" # print notice when creating a TLE mkdir -p "$tmpdir/privileged_extensions_custom_scripts" diff --git a/src/drop_trigger_grants.c b/src/drop_trigger_grants.c new file mode 100644 index 0000000..817e29a --- /dev/null +++ b/src/drop_trigger_grants.c @@ -0,0 +1,204 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "drop_trigger_grants.h" +#include "utils.h" + +static JSON_ACTION_RETURN_TYPE json_array_start(void *state) { + json_drop_trigger_grants_parse_state *parse = state; + + switch (parse->state) { + case JDTG_EXPECT_TABLES_START: + parse->state = JDTG_EXPECT_TABLE; + break; + + case JDTG_EXPECT_TOPLEVEL_START: + parse->state = JDTG_UNEXPECTED_ARRAY; + parse->error_msg = "unexpected array"; + break; + + case JDTG_EXPECT_TABLE: + parse->state = JDTG_UNEXPECTED_ARRAY; + parse->error_msg = "unexpected array"; + break; + + default: + break; + } + JSON_ACTION_RETURN; +} + +static JSON_ACTION_RETURN_TYPE json_array_end(void *state) { + json_drop_trigger_grants_parse_state *parse = state; + + switch (parse->state) { + case JDTG_EXPECT_TABLE: + parse->state = JDTG_EXPECT_TOPLEVEL_FIELD; + parse->total_dtgs++; + break; + + default: + break; + } + JSON_ACTION_RETURN; +} + +static JSON_ACTION_RETURN_TYPE json_object_start(void *state) { + json_drop_trigger_grants_parse_state *parse = state; + + switch (parse->state) { + case JDTG_EXPECT_TOPLEVEL_START: + parse->state = JDTG_EXPECT_TOPLEVEL_FIELD; + break; + + case JDTG_EXPECT_TABLES_START: + parse->error_msg = "unexpected object for tables, expected an array"; + parse->state = JDTG_UNEXPECTED_OBJECT; + break; + + case JDTG_EXPECT_TABLE: + parse->error_msg = "unexpected object for table, expected a string"; + parse->state = JDTG_UNEXPECTED_OBJECT; + break; + + default: + break; + } + JSON_ACTION_RETURN; +} + +static JSON_ACTION_RETURN_TYPE json_object_field_start(void *state, char *fname, + bool isnull) { + json_drop_trigger_grants_parse_state *parse = state; + drop_trigger_grants *x = &parse->dtgs[parse->total_dtgs]; + + switch (parse->state) { + case JDTG_EXPECT_TOPLEVEL_FIELD: + x->role_name = MemoryContextStrdup(TopMemoryContext, fname); + parse->state = JDTG_EXPECT_TABLES_START; + break; + + default: + break; + } + JSON_ACTION_RETURN; +} + +static JSON_ACTION_RETURN_TYPE json_scalar(void *state, char *token, + JsonTokenType tokentype) { + json_drop_trigger_grants_parse_state *parse = state; + drop_trigger_grants *x = &parse->dtgs[parse->total_dtgs]; + + switch (parse->state) { + case JDTG_EXPECT_TABLE: + if (tokentype == JSON_TOKEN_STRING) { + x->table_names[x->total_tables] = + MemoryContextStrdup(TopMemoryContext, token); + x->total_tables++; + } else { + parse->state = JDTG_UNEXPECTED_TABLE_VALUE; + parse->error_msg = "unexpected table value, expected a string"; + } + break; + + case JDTG_EXPECT_TOPLEVEL_START: + parse->state = JDTG_UNEXPECTED_SCALAR; + parse->error_msg = "unexpected scalar, expected an object"; + break; + + case JDTG_EXPECT_TABLES_START: + parse->state = JDTG_UNEXPECTED_SCALAR; + parse->error_msg = "unexpected scalar, expected an array"; + break; + + default: + break; + } + JSON_ACTION_RETURN; +} + +json_drop_trigger_grants_parse_state +parse_drop_trigger_grants(const char *str, drop_trigger_grants *dtgs) { + JsonLexContext *lex; + JsonParseErrorType json_error; + JsonSemAction sem; + + json_drop_trigger_grants_parse_state state = {JDTG_EXPECT_TOPLEVEL_START, + NULL, 0, dtgs}; + + lex = + makeJsonLexContextCstringLen(pstrdup(str), strlen(str), PG_UTF8, true); + + sem.semstate = &state; + sem.object_start = json_object_start; + sem.object_end = NULL; + sem.array_start = json_array_start; + sem.array_end = json_array_end; + sem.object_field_start = json_object_field_start; + sem.object_field_end = NULL; + sem.array_element_start = NULL; + sem.array_element_end = NULL; + sem.scalar = json_scalar; + + json_error = pg_parse_json(lex, &sem); + + if (json_error != JSON_SUCCESS) + state.error_msg = "invalid json"; + + return state; +} + +bool is_current_role_granted_table_drop_trigger(const RangeVar *table_range_var, + const drop_trigger_grants *dtgs, + const size_t total_dtgs) { + + Oid target_table_id = + RangeVarGetRelid(table_range_var, AccessExclusiveLock, false); + char *current_role_name = GetUserNameFromId(GetUserId(), false); + + for (size_t i = 0; i < total_dtgs; i++) { + const drop_trigger_grants *dtg = &dtgs[i]; + + if (strcmp(dtg->role_name, current_role_name) != 0) { + continue; + } + + for (size_t j = 0; j < dtg->total_tables; j++) { + const char *table_name = dtg->table_names[j]; + List *qual_name_list; + RangeVar *range_var; + Oid table_id; +#if PG16_GTE + qual_name_list = stringToQualifiedNameList(table_name, NULL); +#else + qual_name_list = stringToQualifiedNameList(table_name); +#endif + if (qual_name_list == NULL) { + list_free(qual_name_list); + continue; + } + + range_var = makeRangeVarFromNameList(qual_name_list); + table_id = RangeVarGetRelid(range_var, AccessExclusiveLock, true); + if (!OidIsValid(table_id)) { + continue; + } + + if (table_id == target_table_id) { + return true; + } + } + } + + return false; +} diff --git a/src/drop_trigger_grants.h b/src/drop_trigger_grants.h new file mode 100644 index 0000000..47b4be3 --- /dev/null +++ b/src/drop_trigger_grants.h @@ -0,0 +1,42 @@ +#ifndef DROP_TRIGGER_GRANTS_H +#define DROP_TRIGGER_GRANTS_H + +#include + +#include + +#define MAX_DROP_TRIGGER_GRANT_TABLES 100 + +typedef struct { + char *role_name; + char *table_names[MAX_DROP_TRIGGER_GRANT_TABLES]; + int total_tables; +} drop_trigger_grants; + +typedef enum { + JDTG_EXPECT_TOPLEVEL_START, + JDTG_EXPECT_TOPLEVEL_FIELD, + JDTG_EXPECT_TABLES_START, + JDTG_EXPECT_TABLE, + JDTG_UNEXPECTED_ARRAY, + JDTG_UNEXPECTED_SCALAR, + JDTG_UNEXPECTED_OBJECT, + JDTG_UNEXPECTED_TABLE_VALUE +} json_drop_trigger_grants_semantic_state; + +typedef struct { + json_drop_trigger_grants_semantic_state state; + char *error_msg; + int total_dtgs; + drop_trigger_grants *dtgs; +} json_drop_trigger_grants_parse_state; + +extern json_drop_trigger_grants_parse_state +parse_drop_trigger_grants(const char *str, drop_trigger_grants *dtgs); + +extern bool +is_current_role_granted_table_drop_trigger(const RangeVar *table_range_var, + const drop_trigger_grants *dtgs, + const size_t total_dtgs); + +#endif diff --git a/src/supautils.c b/src/supautils.c index aee132b..ab1afe8 100644 --- a/src/supautils.c +++ b/src/supautils.c @@ -22,6 +22,7 @@ #include #include "constrained_extensions.h" +#include "drop_trigger_grants.h" #include "extensions_parameter_overrides.h" #include "policy_grants.h" #include "privileged_extensions.h" @@ -47,6 +48,7 @@ #define MAX_CONSTRAINED_EXTENSIONS 100 #define MAX_EXTENSIONS_PARAMETER_OVERRIDES 100 +#define MAX_DROP_TRIGGER_GRANTS 100 #define MAX_POLICY_GRANTS 100 /* required macro for extension libraries to work */ @@ -82,6 +84,12 @@ static size_t total_pgs = 0; static bool policy_grants_check_hook(char **newval, void **extra, GucSource source); +static char *drop_trigger_grants_str = NULL; +static drop_trigger_grants dtgs[MAX_DROP_TRIGGER_GRANTS] = {0}; +static size_t total_dtgs = 0; +static bool +drop_trigger_grants_check_hook(char **newval, void **extra, GucSource source); + void _PG_init(void); void _PG_fini(void); @@ -242,6 +250,16 @@ _PG_init(void) constrained_extensions_assign_hook, NULL); + DefineCustomStringVariable("supautils.drop_trigger_grants", + "Allow non-owners to drop triggers on tables", + NULL, + &drop_trigger_grants_str, + NULL, + PGC_SIGHUP, 0, + &drop_trigger_grants_check_hook, + NULL, + NULL); + DefineCustomStringVariable("supautils.policy_grants", "Allow non-owners to manage policies on tables", NULL, @@ -845,45 +863,81 @@ supautils_hook(PROCESS_UTILITY_PARAMS) break; } - /* - * DROP EXTENSION - */ - if (stmt->removeType == OBJECT_EXTENSION) { - if (privileged_extensions == NULL) { - break; + switch (stmt->removeType) + { + /* + * DROP EXTENSION + */ + case OBJECT_EXTENSION: + { + if (privileged_extensions == NULL) { + break; + } + handle_drop_extension(prev_hook, + PROCESS_UTILITY_ARGS, + privileged_extensions, + privileged_extensions_superuser); + return; } - handle_drop_extension(prev_hook, - PROCESS_UTILITY_ARGS, - privileged_extensions, - privileged_extensions_superuser); - return; - } - /* - * DROP POLICY - */ - if (stmt->removeType == OBJECT_POLICY) { - // DROP POLICY always has one object. - ListCell *object_cell = list_head(stmt->objects); - List *object = castNode(List, lfirst(object_cell)); - // Last element is the policy name, the rest is the table name. - // Take everything but the last. - List *table_name_list = list_truncate(list_copy(object), list_length(object) - 1); - RangeVar *table_range_var = makeRangeVarFromNameList(table_name_list); - - if (is_current_role_granted_table_policy(table_range_var, pgs, total_pgs)) { - bool already_switched_to_superuser = false; + /* + * DROP POLICY + */ + case OBJECT_POLICY: + { + // DROP POLICY always has one object. + ListCell *object_cell = list_head(stmt->objects); + List *object = castNode(List, lfirst(object_cell)); + // Last element is the policy name, the rest is the table name. + // Take everything but the last. + List *table_name_list = list_truncate(list_copy(object), list_length(object) - 1); + RangeVar *table_range_var = makeRangeVarFromNameList(table_name_list); + + if (is_current_role_granted_table_policy(table_range_var, pgs, total_pgs)) { + bool already_switched_to_superuser = false; - switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser); + switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser); - run_process_utility_hook(prev_hook); + run_process_utility_hook(prev_hook); - if (!already_switched_to_superuser) { - switch_to_original_role(); + if (!already_switched_to_superuser) { + switch_to_original_role(); + } + + return; } + } - return; + /* + * DROP TRIGGER + */ + case OBJECT_TRIGGER: + { + // DROP TRIGGER always has one object. + ListCell *object_cell = list_head(stmt->objects); + List *object = castNode(List, lfirst(object_cell)); + // Last element is the trigger name, the rest is the table name. + // Take everything but the last. + List *table_name_list = list_truncate(list_copy(object), list_length(object) - 1); + RangeVar *table_range_var = makeRangeVarFromNameList(table_name_list); + + if (is_current_role_granted_table_drop_trigger(table_range_var, dtgs, total_dtgs)) { + bool already_switched_to_superuser = false; + + switch_to_superuser(privileged_extensions_superuser, &already_switched_to_superuser); + + run_process_utility_hook(prev_hook); + + if (!already_switched_to_superuser) { + switch_to_original_role(); + } + + return; + } } + + default: + break; } break; @@ -1014,6 +1068,33 @@ policy_grants_check_hook(char **newval, void **extra, GucSource source) return true; } +static bool +drop_trigger_grants_check_hook(char **newval, void **extra, GucSource source) +{ + char *val = *newval; + + for (size_t i = 0; i < total_dtgs; i++){ + pfree(dtgs[i].role_name); + for (size_t j = 0; j < dtgs[i].total_tables; j++) { + pfree(dtgs[i].table_names[j]); + } + dtgs[i].total_tables = 0; + } + total_dtgs = 0; + + if (val) { + json_drop_trigger_grants_parse_state state = parse_drop_trigger_grants(val, dtgs); + if (state.error_msg) { + ereport(ERROR, + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("supautils.drop_trigger_grants: %s", state.error_msg))); + } + total_dtgs = state.total_dtgs; + } + + return true; +} + static bool reserved_roles_check_hook(char **newval, void **extra, GucSource source) { diff --git a/test/expected/privileged_role.out b/test/expected/privileged_role.out index 8025f11..49c274a 100644 --- a/test/expected/privileged_role.out +++ b/test/expected/privileged_role.out @@ -211,3 +211,38 @@ NOTICE: drop cascades to table deny_policies.my_table set role privileged_role; \echo +-- privileged_role can drop triggers on tables in allowlist +set role postgres; +create schema allow_drop_triggers; +create table allow_drop_triggers.my_table (); +create function allow_drop_triggers.f() returns trigger as 'begin return null; end' language plpgsql; +create trigger tr after insert on allow_drop_triggers.my_table execute function allow_drop_triggers.f(); +grant usage on schema allow_drop_triggers to privileged_role; +set role privileged_role; +drop trigger tr on allow_drop_triggers.my_table; +set role postgres; +drop schema allow_drop_triggers cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table allow_drop_triggers.my_table +drop cascades to function allow_drop_triggers.f() +set role privileged_role; +\echo + +-- privileged_role cannot drop triggers on tables not in allowlist +set role postgres; +create schema deny_drop_triggers; +create table deny_drop_triggers.my_table (); +create function deny_drop_triggers.f() returns trigger as 'begin return null; end' language plpgsql; +create trigger tr after insert on deny_drop_triggers.my_table execute function deny_drop_triggers.f(); +grant usage on schema deny_drop_triggers to privileged_role; +set role privileged_role; +drop trigger tr on deny_drop_triggers.my_table; +ERROR: must be owner of relation my_table +set role postgres; +drop schema deny_drop_triggers cascade; +NOTICE: drop cascades to 2 other objects +DETAIL: drop cascades to table deny_drop_triggers.my_table +drop cascades to function deny_drop_triggers.f() +set role privileged_role; +\echo + diff --git a/test/sql/privileged_role.sql b/test/sql/privileged_role.sql index e4ca3bf..91a3231 100644 --- a/test/sql/privileged_role.sql +++ b/test/sql/privileged_role.sql @@ -159,3 +159,33 @@ set role postgres; drop schema deny_policies cascade; set role privileged_role; \echo + +-- privileged_role can drop triggers on tables in allowlist +set role postgres; +create schema allow_drop_triggers; +create table allow_drop_triggers.my_table (); +create function allow_drop_triggers.f() returns trigger as 'begin return null; end' language plpgsql; +create trigger tr after insert on allow_drop_triggers.my_table execute function allow_drop_triggers.f(); +grant usage on schema allow_drop_triggers to privileged_role; +set role privileged_role; +drop trigger tr on allow_drop_triggers.my_table; + +set role postgres; +drop schema allow_drop_triggers cascade; +set role privileged_role; +\echo + +-- privileged_role cannot drop triggers on tables not in allowlist +set role postgres; +create schema deny_drop_triggers; +create table deny_drop_triggers.my_table (); +create function deny_drop_triggers.f() returns trigger as 'begin return null; end' language plpgsql; +create trigger tr after insert on deny_drop_triggers.my_table execute function deny_drop_triggers.f(); +grant usage on schema deny_drop_triggers to privileged_role; +set role privileged_role; +drop trigger tr on deny_drop_triggers.my_table; + +set role postgres; +drop schema deny_drop_triggers cascade; +set role privileged_role; +\echo