diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7bdb463..9139027 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - pg-version: ['13', '14', '15'] + pg-version: ['13', '14', '15', '16'] steps: - uses: actions/checkout@v3 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2d609d5..6cbf5a4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,7 +31,7 @@ jobs: matrix: extension_name: - supautils - postgres: [13, 14, 15] + postgres: [13, 14, 15, 16] box: - { runner: ubuntu-20.04, arch: amd64 } - { runner: arm-runner, arch: arm64 } diff --git a/Makefile b/Makefile index d2d5d66..32670db 100644 --- a/Makefile +++ b/Makefile @@ -10,12 +10,18 @@ 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 +PG_VERSION = $(strip $(shell $(PG_CONFIG) --version | $(GREP) -oP '(?<=PostgreSQL )[0-9]+')) +PG_GE16 = $(shell test $(PG_VERSION) -ge 16; echo $$?) SYSTEM = $(shell uname -s) +TESTS := $(wildcard test/sql/*.sql) ifneq ($(SYSTEM), Linux) -TESTS = $(filter-out test/sql/ge13_%.sql, $(wildcard test/sql/*.sql)) +TESTS := $(filter-out test/sql/linux_%.sql, $(TESTS)) +endif +ifneq ($(PG_GE16), 0) +TESTS := $(filter-out test/sql/ge16_%.sql, $(TESTS)) else -TESTS = $(wildcard test/sql/*.sql) +TESTS := $(filter-out test/sql/lt16_%.sql, $(TESTS)) endif REGRESS = $(patsubst test/sql/%.sql,%,$(TESTS)) diff --git a/README.md b/README.md index 381d7fa..89b7141 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Supautils is an extension that secures a PostgreSQL cluster on a cloud environme It doesn't require creating database objects. It's a shared library that modifies PostgreSQL behavior through "hooks", not through tables or functions. -It's tested to work on PostgreSQL 13, 14 and 15. +It's tested to work on PostgreSQL 13, 14, 15, and 16. ## Installation diff --git a/src/supautils.c b/src/supautils.c index c262511..aee132b 100644 --- a/src/supautils.c +++ b/src/supautils.c @@ -464,6 +464,19 @@ supautils_hook(PROCESS_UTILITY_PARAMS) if (hasrolemembers) confirm_reserved_memberships(created_role); + // We don't want to switch to superuser on PG16+ because the + // creating role is implicitly granted ADMIN on the new + // role: + // https://www.postgresql.org/docs/16/runtime-config-client.html#GUC-CREATEROLE-SELF-GRANT + // + // This ADMIN will be missing if we switch to superuser + // since the creating role becomes the superuser. + // + // We also no longer need superuser to grant BYPASSRLS & + // REPLICATION anyway. +#if PG16_GTE + run_process_utility_hook(prev_hook); +#else if (is_current_role_privileged()) { bool already_switched_to_superuser = false; @@ -479,6 +492,7 @@ supautils_hook(PROCESS_UTILITY_PARAMS) } else { run_process_utility_hook(prev_hook); } +#endif return; } diff --git a/test/expected/ge16_privileged_role.out b/test/expected/ge16_privileged_role.out new file mode 100644 index 0000000..b287d2b --- /dev/null +++ b/test/expected/ge16_privileged_role.out @@ -0,0 +1,246 @@ +-- non-superuser privileged role +set role privileged_role; +\echo + +-- can set privileged_role_allowed_configs +set session_replication_role to 'replica'; +begin; + set local session_replication_role to 'origin'; +commit; +reset session_replication_role; +\echo + +-- non-superuser non-privileged role cannot set privileged_role_allowed_configs +set role rolecreator; +set session_replication_role to 'replica'; +ERROR: permission denied to set parameter "session_replication_role" +begin; + set local session_replication_role to 'origin'; +ERROR: permission denied to set parameter "session_replication_role" +commit; +reset session_replication_role; +ERROR: permission denied to set parameter "session_replication_role" +set role privileged_role; +\echo + +-- superuser can set privileged_role_allowed_configs +set role postgres; +set session_replication_role to 'replica'; +begin; + set local session_replication_role to 'origin'; +commit; +reset session_replication_role; +set role privileged_role; +\echo + +-- can set privileged_role_allowed_configs for a role +create role r; +alter role r set session_replication_role to 'replica'; +drop role r; +\echo + +-- can manage bypassrls role attribute +create role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; + rolbypassrls +-------------- + t +(1 row) + +alter role r nobypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; + rolbypassrls +-------------- + f +(1 row) + +alter role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; + rolbypassrls +-------------- + t +(1 row) + +drop role r; +\echo + +-- can manage replication role attribute +create role r replication; +select rolreplication from pg_roles where rolname = 'r'; + rolreplication +---------------- + t +(1 row) + +alter role r noreplication; +select rolreplication from pg_roles where rolname = 'r'; + rolreplication +---------------- + f +(1 row) + +alter role r replication; +select rolreplication from pg_roles where rolname = 'r'; + rolreplication +---------------- + t +(1 row) + +drop role r; +\echo + +-- can manage foreign data wrappers +create extension postgres_fdw; +create foreign data wrapper new_fdw + handler postgres_fdw_handler + validator postgres_fdw_validator; +drop extension postgres_fdw cascade; +NOTICE: drop cascades to foreign-data wrapper new_fdw +\echo + +-- non-superuser non-privileged role cannot manage bypassrls role attribute +set role rolecreator; +\echo + +-- the error message changed in PG14 +do $$ +begin + create role r bypassrls; + exception when insufficient_privilege then null; +end; +$$ language plpgsql; +create role r; +alter role r nobypassrls; +ERROR: permission denied to alter role +DETAIL: Only roles with the BYPASSRLS attribute may change the BYPASSRLS attribute. +alter role r bypassrls; +ERROR: permission denied to alter role +DETAIL: Only roles with the BYPASSRLS attribute may change the BYPASSRLS attribute. +drop role r; +set role privileged_role; +\echo + +-- superuser can manage bypassrls role attribute +set role postgres; +create role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; + rolbypassrls +-------------- + t +(1 row) + +alter role r nobypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; + rolbypassrls +-------------- + f +(1 row) + +alter role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; + rolbypassrls +-------------- + t +(1 row) + +drop role r; +set role privileged_role; +\echo + +-- regression: https://github.com/supabase/supautils/issues/34 +create role tmp; +alter role tmp; +\echo + +-- privileged_role can modify reserved roles GUCs +set role privileged_role; +alter role authenticator set search_path to public; +alter role authenticator set statement_timeout = '15s'; +\echo + +-- privileged_role can do GRANT to +grant testme to authenticator; +\echo + +-- privileged_role can set wildcard privileged_role_allowed_configs +alter role authenticator set pgrst.db_plan_enabled to true; +alter role authenticator set pgrst.db_prepared_statements to false; +alter role authenticator set other.nested.foo to true; +alter role authenticator set other.nested.bar to true; +\echo + +-- privileged_role cannot drop, rename or nologin reserved role +drop role authenticator; +ERROR: "authenticator" is a reserved role, only superusers can modify it +alter role authenticator rename to authorized; +ERROR: "authenticator" is a reserved role, only superusers can modify it +alter role authenticator nologin; +ERROR: "authenticator" is a reserved role, only superusers can modify it +\echo + +-- privileged_role can create nosuperuser +create role r nosuperuser; +drop role r; +\echo + +-- privileged_role cannot create superuser or alter [no]superuser +create role r superuser; +ERROR: permission denied to create role +DETAIL: Only roles with the SUPERUSER attribute may create roles with the SUPERUSER attribute. +create role r; +alter role r superuser; +ERROR: permission denied to alter role +DETAIL: Only roles with the SUPERUSER attribute may alter roles with the SUPERUSER attribute. +alter role r nosuperuser; +ERROR: permission denied to alter role +DETAIL: Only roles with the SUPERUSER attribute may alter roles with the SUPERUSER attribute. +drop role r; +\echo + +-- member of privileged_role can do privileged role stuff +set role privileged_role_member; +grant testme to authenticator; +NOTICE: role "authenticator" has already been granted membership in role "testme" by role "privileged_role" +set role privileged_role; +\echo + +-- privileged_role can manage publications +create publication p for all tables; +drop publication p; +-- not testing `create publication ... for tables in schema ...` because it's PG15+ +\echo + +-- privileged_role can manage policies on tables in allowlist +set role postgres; +create schema allow_policies; +create table allow_policies.my_table (); +grant usage on schema allow_policies to privileged_role; +set role privileged_role; +create policy p on allow_policies.my_table for select using (true); +alter policy p on allow_policies.my_table using (false); +drop policy p on allow_policies.my_table; +set role postgres; +drop schema allow_policies cascade; +NOTICE: drop cascades to table allow_policies.my_table +set role privileged_role; +\echo + +-- privileged_role cannot manage policies on tables not in allowlist +set role postgres; +create schema deny_policies; +create table deny_policies.my_table (); +create policy p1 on deny_policies.my_table for select using (true); +grant usage on schema deny_policies to privileged_role; +set role privileged_role; +create policy p2 on deny_policies.my_table for select using (true); +ERROR: must be owner of table my_table +alter policy p1 on deny_policies.my_table using (false); +ERROR: must be owner of table my_table +drop policy p1 on deny_policies.my_table; +ERROR: must be owner of relation my_table +set role postgres; +drop schema deny_policies cascade; +NOTICE: drop cascades to table deny_policies.my_table +set role privileged_role; +\echo + diff --git a/test/expected/ge13_constrained_extensions.out b/test/expected/linux_constrained_extensions.out similarity index 100% rename from test/expected/ge13_constrained_extensions.out rename to test/expected/linux_constrained_extensions.out diff --git a/test/expected/privileged_role.out b/test/expected/lt16_privileged_role.out similarity index 98% rename from test/expected/privileged_role.out rename to test/expected/lt16_privileged_role.out index 7ba3595..d3eb533 100644 --- a/test/expected/privileged_role.out +++ b/test/expected/lt16_privileged_role.out @@ -186,15 +186,12 @@ create role r superuser; ERROR: permission denied to create role DETAIL: Only roles with the SUPERUSER attribute may create roles with the SUPERUSER attribute. create role r; -alter role r nosuperuser; -ERROR: permission denied to alter role -DETAIL: Only roles with the SUPERUSER attribute may alter roles with the SUPERUSER attribute. alter role r superuser; ERROR: permission denied to alter role DETAIL: Only roles with the SUPERUSER attribute may alter roles with the SUPERUSER attribute. -alter role postgres nosuperuser; +alter role r nosuperuser; ERROR: permission denied to alter role -DETAIL: Only superusers can alter privileged roles. +DETAIL: Only roles with the SUPERUSER attribute may alter roles with the SUPERUSER attribute. drop role r; \echo diff --git a/test/expected/reserved_roles.out b/test/expected/reserved_roles.out index 722e9fc..271273f 100644 --- a/test/expected/reserved_roles.out +++ b/test/expected/reserved_roles.out @@ -3,8 +3,10 @@ set role rolecreator; \echo -- cannot rename an existing role to a reserved role -alter role fake rename to reserved_but_not_yet_created; +create role r; +alter role r rename to reserved_but_not_yet_created; ERROR: "reserved_but_not_yet_created" is a reserved role, only superusers can modify it +drop role r; \echo -- cannot rename a reserved role @@ -56,9 +58,10 @@ ERROR: "reserved_but_not_yet_created" is a reserved role, only superusers can m create role anon; ERROR: role "anon" already exists -- ensure our hooks don't mess regular non-reserved roles functionality -alter role fake rename to new_fake; -alter role new_fake createrole createdb; -drop role new_fake; +create role r; +alter role r rename to new_r; +alter role new_r createrole createdb; +drop role new_r; create role non_reserved; \echo diff --git a/test/fixtures.sql b/test/fixtures.sql index b686259..dd2f4c1 100644 --- a/test/fixtures.sql +++ b/test/fixtures.sql @@ -3,14 +3,19 @@ create role supabase_storage_admin noinherit; create role anon noinherit; -- login role -create user rolecreator createrole noinherit nosuperuser; -grant anon to rolecreator; +create user rolecreator createrole createdb noinherit nosuperuser; +grant anon to rolecreator with admin option; +grant pg_monitor to rolecreator with admin option; +grant pg_read_all_settings to rolecreator with admin option; +grant pg_read_all_stats to rolecreator with admin option; grant supabase_storage_admin to rolecreator; -- other roles create role fake noinherit; -create role privileged_role login createrole; +create role privileged_role login createrole bypassrls replication; create role privileged_role_member login createrole in role privileged_role; create role testme noinherit; +grant testme to privileged_role with admin option; create role authenticator login noinherit; +grant authenticator to privileged_role with admin option; grant all on database postgres to privileged_role; diff --git a/test/sql/privileged_role.sql b/test/sql/ge16_privileged_role.sql similarity index 99% rename from test/sql/privileged_role.sql rename to test/sql/ge16_privileged_role.sql index 29b5366..d4ddb07 100644 --- a/test/sql/privileged_role.sql +++ b/test/sql/ge16_privileged_role.sql @@ -141,9 +141,8 @@ drop role r; -- privileged_role cannot create superuser or alter [no]superuser create role r superuser; create role r; -alter role r nosuperuser; alter role r superuser; -alter role postgres nosuperuser; +alter role r nosuperuser; drop role r; \echo diff --git a/test/sql/ge13_constrained_extensions.sql b/test/sql/linux_constrained_extensions.sql similarity index 100% rename from test/sql/ge13_constrained_extensions.sql rename to test/sql/linux_constrained_extensions.sql diff --git a/test/sql/lt16_privileged_role.sql b/test/sql/lt16_privileged_role.sql new file mode 100644 index 0000000..8a75565 --- /dev/null +++ b/test/sql/lt16_privileged_role.sql @@ -0,0 +1,191 @@ +-- non-superuser privileged role +set role privileged_role; +\echo + +-- can set privileged_role_allowed_configs +set session_replication_role to 'replica'; +begin; + set local session_replication_role to 'origin'; +commit; +reset session_replication_role; +\echo + +-- non-superuser non-privileged role cannot set privileged_role_allowed_configs +set role rolecreator; + +set session_replication_role to 'replica'; +begin; + set local session_replication_role to 'origin'; +commit; +reset session_replication_role; + +set role privileged_role; +\echo + +-- superuser can set privileged_role_allowed_configs +set role postgres; + +set session_replication_role to 'replica'; +begin; + set local session_replication_role to 'origin'; +commit; +reset session_replication_role; + +set role privileged_role; +\echo + +-- can set privileged_role_allowed_configs for a role +create role r; +alter role r set session_replication_role to 'replica'; +drop role r; +\echo + +-- can manage bypassrls role attribute +create role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; +alter role r nobypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; +alter role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; +drop role r; +\echo + +-- can manage replication role attribute +create role r replication; +select rolreplication from pg_roles where rolname = 'r'; +alter role r noreplication; +select rolreplication from pg_roles where rolname = 'r'; +alter role r replication; +select rolreplication from pg_roles where rolname = 'r'; + +drop role r; +\echo + +-- can manage foreign data wrappers +create extension postgres_fdw; +create foreign data wrapper new_fdw + handler postgres_fdw_handler + validator postgres_fdw_validator; + +drop extension postgres_fdw cascade; +\echo + +-- non-superuser non-privileged role cannot manage bypassrls role attribute +set role rolecreator; +\echo + +-- the error message changed in PG14 +do $$ +begin + create role r bypassrls; + exception when insufficient_privilege then null; +end; +$$ language plpgsql; + +create role r; +alter role r nobypassrls; +alter role r bypassrls; +drop role r; + +set role privileged_role; +\echo + +-- superuser can manage bypassrls role attribute +set role postgres; + +create role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; +alter role r nobypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; +alter role r bypassrls; +select rolbypassrls from pg_roles where rolname = 'r'; +drop role r; + +set role privileged_role; +\echo + +-- regression: https://github.com/supabase/supautils/issues/34 +create role tmp; +alter role tmp; +\echo + +-- privileged_role can modify reserved roles GUCs +set role privileged_role; +alter role authenticator set search_path to public; +alter role authenticator set statement_timeout = '15s'; +\echo + +-- privileged_role can do GRANT to +grant testme to authenticator; +\echo + +-- privileged_role can set wildcard privileged_role_allowed_configs +alter role authenticator set pgrst.db_plan_enabled to true; +alter role authenticator set pgrst.db_prepared_statements to false; +alter role authenticator set other.nested.foo to true; +alter role authenticator set other.nested.bar to true; +\echo + +-- privileged_role cannot drop, rename or nologin reserved role +drop role authenticator; +alter role authenticator rename to authorized; +alter role authenticator nologin; +\echo + +-- privileged_role can create nosuperuser +create role r nosuperuser; + +drop role r; +\echo + +-- privileged_role cannot create superuser or alter [no]superuser +create role r superuser; +create role r; +alter role r superuser; +alter role r nosuperuser; + +drop role r; +\echo + +-- member of privileged_role can do privileged role stuff +set role privileged_role_member; + +grant testme to authenticator; + +set role privileged_role; +\echo + +-- privileged_role can manage publications +create publication p for all tables; +drop publication p; +-- not testing `create publication ... for tables in schema ...` because it's PG15+ +\echo + +-- privileged_role can manage policies on tables in allowlist +set role postgres; +create schema allow_policies; +create table allow_policies.my_table (); +grant usage on schema allow_policies to privileged_role; +set role privileged_role; +create policy p on allow_policies.my_table for select using (true); +alter policy p on allow_policies.my_table using (false); +drop policy p on allow_policies.my_table; +set role postgres; +drop schema allow_policies cascade; +set role privileged_role; +\echo + +-- privileged_role cannot manage policies on tables not in allowlist +set role postgres; +create schema deny_policies; +create table deny_policies.my_table (); +create policy p1 on deny_policies.my_table for select using (true); +grant usage on schema deny_policies to privileged_role; +set role privileged_role; +create policy p2 on deny_policies.my_table for select using (true); +alter policy p1 on deny_policies.my_table using (false); +drop policy p1 on deny_policies.my_table; +set role postgres; +drop schema deny_policies cascade; +set role privileged_role; +\echo diff --git a/test/sql/reserved_roles.sql b/test/sql/reserved_roles.sql index 691a218..e35a7d0 100644 --- a/test/sql/reserved_roles.sql +++ b/test/sql/reserved_roles.sql @@ -3,7 +3,10 @@ set role rolecreator; \echo -- cannot rename an existing role to a reserved role -alter role fake rename to reserved_but_not_yet_created; +create role r; +alter role r rename to reserved_but_not_yet_created; + +drop role r; \echo -- cannot rename a reserved role @@ -41,9 +44,10 @@ create role reserved_but_not_yet_created; create role anon; -- ensure our hooks don't mess regular non-reserved roles functionality -alter role fake rename to new_fake; -alter role new_fake createrole createdb; -drop role new_fake; +create role r; +alter role r rename to new_r; +alter role new_r createrole createdb; +drop role new_r; create role non_reserved; \echo