Skip to content

Commit 21fa1fc

Browse files
mattia-eleuterilexfrei
authored andcommitted
feat(postgres): add per-user createRole and bypassRls attributes
Without these, a tenant that needs to manage its own roles has no option but the postgres superuser, which also hands over filesystem access and command execution on the instance. Role attributes are not inherited through role membership, so the generated <db>_admin group role cannot carry them: they have to be set on the login user itself. users.<name>.createRole and users.<name>.bypassRls set CREATEROLE and BYPASSRLS on a managed login user. Both are reconciled on every init-script run, so setting one back to false emits the NO... form and revokes the attribute. createRole is refused on PostgreSQL 15 and older. Before PostgreSQL 16, CREATEROLE allowed granting membership in any non-superuser role, so a user holding it could grant itself pg_execute_server_program and reach command execution on the instance, which is exactly what not exposing the superuser prevents. PostgreSQL 16 restricted CREATEROLE to roles the grantee administers. bypassRls carries no such gate: BYPASSRLS only waives row-level security inside the tenant's own databases, whose owner already reads every row, so it adds no reach. The two attributes are needed together, not separately. PostgreSQL only lets a role create other roles carrying BYPASSRLS if it holds the attribute itself, so provisioning a privileged service role fails with "Only roles with the BYPASSRLS attribute may create roles with the BYPASSRLS attribute" when only createRole is granted. createRole also sets createrole_self_grant as a role-level default. PostgreSQL 16 grants a CREATEROLE holder only ADMIN OPTION on the roles it creates, governed by that parameter, whose default is empty. ADMIN OPTION allows granting the role onward but not SET ROLE to it, and creating an object owned by another role requires SET. The attribute therefore looks functional -- CREATE ROLE succeeds -- while the next statement a tenant writes, CREATE SCHEMA ... AUTHORIZATION <new role>, fails with "must be able to SET ROLE". The default is reset when the attribute is withdrawn. The init job runs as the superuser, and before this change it trusted two things a CREATEROLE holder controls for the roles it creates: role comments and role names. It picked the users and roles to drop by their comment, and a PostgreSQL 16 CREATEROLE holder may comment every role it created, then hand it to the chart for deletion. It adopted any role that already existed under a declared name, so a role created under a name an administrator declares later stayed under its creator's control. Neither can be caught from the catalog: creating a role through an intermediate role and then dropping that role leaves the squatted role with no member at all, while the objects, settings and default privileges planted in it stay. The chart therefore records the roles it manages as members of "cozystack:managed", a role only the superuser can grant, and the delete passes act only on those. Its first run marks every role the chart had commented before, when no role could yet hold CREATEROLE through it. Once any user has held createRole, recorded as a setting of the postgres database, the chart refuses to adopt a role it did not mark and fails with the role's name, rather than hand a squatted role REPLICATION or the ownership of a database. Verified against live PostgreSQL instances. On 17 a migration checklist replayed as the login user, with no superuser: role creation with attributes, CREATE SCHEMA ... AUTHORIZATION with the requested owner, extensions in a named schema, per-role search_path and timeouts, RLS including FORCE ROW LEVEL SECURITY, and SECURITY DEFINER functions all succeed. On 17 the forged-comment and squatting paths are refused or ignored, the upgrade from the previous chart marks the roles it already managed, and a fresh install still adopts the role CNPG bootstraps for users.app. On 13, 15 and 16 the init script runs and re-runs cleanly. Assisted-by: LLM Signed-off-by: Mattia Eleuteri <[email protected]>
1 parent 9734a16 commit 21fa1fc

9 files changed

Lines changed: 425 additions & 7 deletions

File tree

‎api/apps/v1alpha1/postgresql/types.go‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/apps/postgres/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,8 @@ For `sslmode=verify-full` to work, the CA bundle retrieved above must be saved t
264264
| ------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------- | ------- |
265265
| `users` | Users configuration map. Passwords are always auto-generated and stored in the `<release>-credentials` Secret; they cannot be set from values — read a user's password from that Secret. A `password` left over in values from before the field was removed is ignored by the render and draws an admission warning, but it is not inert on an upgraded release: the chart preserves whatever password is already in the Secret, which on the first upgrade is the value that was set before removal, so that value stays the live credential until it is rotated (dedicated rotation is tracked in cozystack/community#72). The live password lives only in the Secret. | `map[string]object` | `{}` |
266266
| `users[name].replication` | Whether the user has replication privileges. | `bool` | `false` |
267+
| `users[name].createRole` | Whether the user can create and manage roles (CREATEROLE). Requires PostgreSQL 16 or newer. Setting it back to `false` withdraws the attribute only: the roles the user created, and its ADMIN and SET memberships in them, stay. Once any user has held it, the chart no longer adopts an existing role it did not create, so a user or database declared later under the name of such a role fails the init job until a platform administrator drops that role. | `bool` | `false` |
268+
| `users[name].bypassRls` | Whether the user can bypass row-level security policies (BYPASSRLS). A role can only create other roles carrying BYPASSRLS if it holds the attribute itself, so this is required alongside `createRole` to provision a privileged service role. | `bool` | `false` |
267269

268270

269271
### Databases configuration

‎packages/apps/postgres/templates/init-script.yaml‎

Lines changed: 94 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,72 @@ stringData:
169169
sleep 5
170170
done
171171
172+
{{- /*
173+
Role comments cannot tell which roles this chart manages: on PostgreSQL 16 a
174+
CREATEROLE holder may COMMENT ON ROLE every role it created, and it may drop
175+
the intermediate role that created one, leaving no pg_auth_members trace of
176+
who did. Membership in "cozystack:managed" can only be granted by a holder of
177+
ADMIN on it, which no role but the superuser ever gets, so the delete passes
178+
and the adoption checks below trust that instead. The name sits outside the
179+
username and database charsets, so values can never declare it. Its first run
180+
marks every role the chart had commented before the marker existed; no role
181+
could have held CREATEROLE through the chart by then. On PostgreSQL 16 and
182+
later the membership carries neither INHERIT nor SET, so it is a record and
183+
nothing else; the role holds no privileges on older majors either.
184+
*/}}
185+
{{- $pgMajor := atoi (trimPrefix "v" .Values.version) }}
186+
{{- $markOptions := ternary " WITH INHERIT FALSE, SET FALSE" "" (ge $pgMajor 16) }}
187+
echo "== mark managed roles"
188+
psql -v ON_ERROR_STOP=1 --echo-all <<\EOT
189+
DO $$
190+
DECLARE
191+
managed record;
192+
BEGIN
193+
IF NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = 'cozystack:managed') THEN
194+
CREATE ROLE "cozystack:managed" NOLOGIN;
195+
FOR managed IN SELECT r.rolname FROM pg_roles r JOIN pg_shdescription s ON s.objoid = r.oid AND s.classoid = 'pg_authid'::regclass WHERE s.description IN ('user managed by helm', 'role managed by helm') LOOP
196+
EXECUTE format('GRANT %I TO %I{{ $markOptions }}', 'cozystack:managed', managed.rolname);
197+
END LOOP;
198+
END IF;
199+
END$$;
200+
EOT
201+
172202
echo "== create users"
173203
{{- if .Values.users }}
204+
{{- /*
205+
A CREATEROLE holder can create a role under a name values will declare later,
206+
plant objects owned by it, role settings and default privileges, then drop the
207+
intermediate role it did this through, so the squatted role shows no member at
208+
all by the time it is declared. No catalog check can tell that apart from a
209+
role a platform administrator created, so once any user has held createRole
210+
the chart refuses to adopt a role it did not mark. Before that, adopting is
211+
safe, and it is what a declared users.app does on install with the role CNPG
212+
bootstraps. The flag is read at the start of this session and set only after
213+
it, so the run that first grants createRole still adopts, and every user is
214+
created and marked before any of them holds CREATEROLE.
215+
*/}}
216+
{{- $grantsCreateRole := false }}
217+
psql -v ON_ERROR_STOP=1 --echo-all <<\EOT
218+
{{- range $user, $u := .Values.users }}
219+
{{- if and (default false $u.createRole) (lt $pgMajor 16) }}
220+
{{- fail (printf "SECURITY: users.%s.createRole requires PostgreSQL 16 or newer. On PostgreSQL 15 and older, a role holding CREATEROLE can grant itself any non-superuser role, including pg_execute_server_program, which escalates to command execution on the instance." $user) }}
221+
{{- end }}
222+
{{- if default false $u.createRole }}
223+
{{- $grantsCreateRole = true }}
224+
{{- end }}
225+
SELECT format('DO $$BEGIN RAISE EXCEPTION %L; END$$', 'role "{{ $user }}" already exists and this chart did not create it; a user of this instance has held createRole, so the role may be controlled by that user and users.{{ $user }} will not adopt it. A platform administrator must inspect the role and drop it as the superuser, or declare the user under another name')
226+
FROM pg_catalog.pg_roles r
227+
WHERE r.rolname = '{{ $user }}'
228+
AND current_setting('cozystack.createrole_granted', true) = 'on'
229+
AND NOT EXISTS (SELECT FROM pg_catalog.pg_auth_members m JOIN pg_catalog.pg_roles g ON g.oid = m.roleid WHERE g.rolname = 'cozystack:managed' AND m.member = r.oid)\gexec
230+
SELECT 'CREATE ROLE "{{ $user }}" LOGIN INHERIT;'
231+
WHERE NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = '{{ $user }}')\gexec
232+
GRANT "cozystack:managed" TO "{{ $user }}"{{ $markOptions }};
233+
{{- end }}
234+
EOT
235+
{{- if $grantsCreateRole }}
236+
psql -v ON_ERROR_STOP=1 --echo-all -c "ALTER DATABASE postgres SET cozystack.createrole_granted = 'on';"
237+
{{- end }}
174238
{{- /*
175239
Each password is read from the mounted <release>-credentials Secret at run
176240
time rather than rendered into this script: the init-script Secret is stored
@@ -187,9 +251,22 @@ stringData:
187251
{{- range $user, $u := .Values.users }}
188252
pw=$(cat "/credentials/{{ $user }}")
189253
psql -v ON_ERROR_STOP=1 -v pw="$pw" <<\EOT
190-
SELECT 'CREATE ROLE "{{ $user }}" LOGIN INHERIT;'
191-
WHERE NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = '{{ $user }}')\gexec
192-
ALTER ROLE "{{ $user }}" WITH PASSWORD :'pw' LOGIN INHERIT {{ ternary "REPLICATION" "NOREPLICATION" (default false $u.replication) }};
254+
ALTER ROLE "{{ $user }}" WITH PASSWORD :'pw' LOGIN INHERIT {{ ternary "REPLICATION" "NOREPLICATION" (default false $u.replication) }} {{ ternary "CREATEROLE" "NOCREATEROLE" (default false $u.createRole) }} {{ ternary "BYPASSRLS" "NOBYPASSRLS" (default false $u.bypassRls) }};
255+
{{- /*
256+
PostgreSQL 16 grants a CREATEROLE holder only ADMIN OPTION on the roles it
257+
creates, governed by createrole_self_grant, whose default is empty. ADMIN
258+
OPTION alone allows granting the role onward but not SET ROLE to it, and
259+
creating an object owned by another role requires SET. Without this the
260+
attribute looks functional — CREATE ROLE succeeds — yet the very next
261+
statement a tenant writes, CREATE SCHEMA ... AUTHORIZATION <new role>,
262+
fails with "must be able to SET ROLE". Set as a role-level default so it
263+
applies to every session, and reset when the attribute is withdrawn.
264+
*/}}
265+
{{- if default false $u.createRole }}
266+
ALTER ROLE "{{ $user }}" SET createrole_self_grant = 'set, inherit';
267+
{{- else }}
268+
ALTER ROLE "{{ $user }}" RESET createrole_self_grant;
269+
{{- end }}
193270
COMMENT ON ROLE "{{ $user }}" IS 'user managed by helm';
194271
EOT
195272
{{- end }}
@@ -218,7 +295,7 @@ stringData:
218295
take the name. In the user and role drop passes, names read from the
219296
catalog reach SQL only as psql variables.
220297
*/}}
221-
MANAGED_USERS=$(psql -v ON_ERROR_STOP=1 -t -A -c "SELECT r.rolname FROM pg_roles r JOIN pg_shdescription s ON s.objoid = r.oid AND s.classoid = 'pg_authid'::regclass WHERE s.description = 'user managed by helm'")
298+
MANAGED_USERS=$(psql -v ON_ERROR_STOP=1 -t -A -c "SELECT r.rolname FROM pg_roles r JOIN pg_shdescription s ON s.objoid = r.oid AND s.classoid = 'pg_authid'::regclass WHERE s.description = 'user managed by helm' AND r.oid IN (SELECT m.member FROM pg_auth_members m JOIN pg_roles g ON g.oid = m.roleid WHERE g.rolname = 'cozystack:managed')")
222299
DEFINED_USERS="{{ $defined := list }}{{ range $user, $u := .Values.users }}{{ $defined = append $defined (trunc 63 $user) }}{{ end }}{{ join " " $defined }}"
223300
DELETE_USERS=$(for user in $MANAGED_USERS; do case " $DEFINED_USERS " in *" $user "*) :;; *) echo $user;; esac; done)
224301
ALL_DBS=$(psql -v ON_ERROR_STOP=1 -t -A -c "SELECT datname FROM pg_database WHERE datallowconn AND NOT datistemplate")
@@ -279,7 +356,7 @@ stringData:
279356
fi
280357
281358
echo "== delete orphaned managed roles"
282-
MANAGED_ROLES=$(psql -v ON_ERROR_STOP=1 -t -A -c "SELECT rolname FROM pg_roles r JOIN pg_shdescription s ON r.oid = s.objoid WHERE s.description = 'role managed by helm'")
359+
MANAGED_ROLES=$(psql -v ON_ERROR_STOP=1 -t -A -c "SELECT rolname FROM pg_roles r JOIN pg_shdescription s ON r.oid = s.objoid WHERE s.description = 'role managed by helm' AND r.oid IN (SELECT m.member FROM pg_auth_members m JOIN pg_roles g ON g.oid = m.roleid WHERE g.rolname = 'cozystack:managed')")
283360
DEFINED_ROLES="{{ range $database, $d := .Values.databases }}{{ $database }}_admin {{ $database }}_readonly {{ end }}"
284361
DELETE_ROLES=$(for role in $MANAGED_ROLES; do case " $DEFINED_ROLES " in *" $role "*) :;; *) echo $role;; esac; done)
285362
@@ -315,17 +392,29 @@ stringData:
315392
316393
echo "== create databases and roles"
317394
{{- if .Values.databases }}
395+
{{- /*
396+
The same adoption check as for users. This session starts after the create
397+
users pass has recorded createRole, so the run that first grants it already
398+
refuses: its CREATEROLE holders exist from that pass on.
399+
*/}}
318400
psql -v ON_ERROR_STOP=1 --echo-all <<\EOT
319401
{{- range $database, $d := .Values.databases }}
402+
SELECT format('DO $$BEGIN RAISE EXCEPTION %L; END$$', format('role "%s" already exists and this chart did not create it; a user of this instance has held createRole, so the role may be controlled by that user and databases.{{ $database }} will not adopt it. A platform administrator must inspect the role and drop it as the superuser, or declare the database under another name', r.rolname))
403+
FROM pg_catalog.pg_roles r
404+
WHERE r.rolname IN ('{{ $database }}_admin', '{{ $database }}_readonly')
405+
AND current_setting('cozystack.createrole_granted', true) = 'on'
406+
AND NOT EXISTS (SELECT FROM pg_catalog.pg_auth_members m JOIN pg_catalog.pg_roles g ON g.oid = m.roleid WHERE g.rolname = 'cozystack:managed' AND m.member = r.oid)\gexec
320407
SELECT 'CREATE DATABASE "{{ $database }}"'
321408
WHERE NOT EXISTS (SELECT FROM pg_database WHERE datname = '{{ $database }}')\gexec
322409
COMMENT ON DATABASE "{{ $database }}" IS 'database managed by helm';
323410
SELECT 'CREATE ROLE "{{ $database }}_admin" NOINHERIT;'
324411
WHERE NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = '{{ $database }}_admin')\gexec
325412
COMMENT ON ROLE "{{ $database }}_admin" IS 'role managed by helm';
413+
GRANT "cozystack:managed" TO "{{ $database }}_admin"{{ $markOptions }};
326414
SELECT 'CREATE ROLE "{{ $database }}_readonly" NOINHERIT;'
327415
WHERE NOT EXISTS (SELECT FROM pg_catalog.pg_roles WHERE rolname = '{{ $database }}_readonly')\gexec
328416
COMMENT ON ROLE "{{ $database }}_readonly" IS 'role managed by helm';
417+
GRANT "cozystack:managed" TO "{{ $database }}_readonly"{{ $markOptions }};
329418
{{- end }}
330419
EOT
331420
{{- end }}

‎packages/apps/postgres/tests/delete_users_test.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ tests:
2828
asserts:
2929
- matchRegex:
3030
path: stringData["init.sh"]
31-
pattern: "MANAGED_USERS=\\$\\(psql -v ON_ERROR_STOP=1 -t -A -c \"SELECT r.rolname FROM pg_roles r JOIN pg_shdescription s ON s.objoid = r.oid AND s.classoid = 'pg_authid'::regclass WHERE s.description = 'user managed by helm'\"\\)"
31+
pattern: "MANAGED_USERS=\\$\\(psql -v ON_ERROR_STOP=1 -t -A -c \"SELECT r.rolname FROM pg_roles r JOIN pg_shdescription s ON s.objoid = r.oid AND s.classoid = 'pg_authid'::regclass WHERE s.description = 'user managed by helm' AND r.oid IN \\(SELECT m.member FROM pg_auth_members m JOIN pg_roles g ON g.oid = m.roleid WHERE g.rolname = 'cozystack:managed'\\)\"\\)"
3232
- notMatchRegex:
3333
path: stringData["init.sh"]
3434
pattern: 'du\+'

0 commit comments

Comments
 (0)