Skip to content

Commit 99ef6b2

Browse files
jeltzjeltz
committed
PG-1440 Only let superusers modify the global key proviers
Since as soon as we have installed pg_tde the database owner can call any function created by the extension so any database owner can meddle with any global key provider. The only way to prevent the database owner to do whatever they want add permissions checks to the C code and here we keep that check simple by limiting modifying the global key provider to only the super user. Additionally we also protect the function for settting the WAL key, for setting the default key and to be paranoid also the function for using a global key provider to set the database key. The third is not obvious if it is necessary or not but I chose to be paranoid and relax that restirction later once we have demed it to be secure.
1 parent dcdcebb commit 99ef6b2

File tree

6 files changed

+53
-75
lines changed

6 files changed

+53
-75
lines changed

‎contrib/pg_tde/documentation/docs/functions.md

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ The `pg_tde` extension provides functions for managing different aspects of its
44

55
## Permission management
66

7-
By default, `pg_tde` is restrictive. It doesn't allow any operations until permissions are granted to the user. Only superusers can run permission management functions to manage user permissions.
7+
By default, `pg_tde` is restrictive. It doesn't allow any operations until permissions are granted to the user. Only superusers can run permission management functions to manage user permissions. Operations on the global scope are limited to superusers only.
88

99
Permissions are based on the normal `EXECUTE` permission on the functions provided by `pg_tde`. Superusers manage them using the `GRANT EXECUTE` and `REVOKE EXECUTE` commands.
1010

@@ -19,10 +19,7 @@ Use these functions to grant or revoke permissions to manage permissions for the
1919

2020
### Global scope key management
2121

22-
Use these functions to grant or revoke permissions to manage permissions for the global scope - the entire PostgreSQL instance. They enable or disable all functions related to the providers and keys for the global scope:
23-
24-
* `pg_tde_grant_global_key_management_to_role(role)`
25-
* `pg_tde_revoke_global_key_management_from_role(role)`
22+
Managment of the global scope is restricted to superusers only.
2623

2724
### Permission management
2825

‎contrib/pg_tde/expected/access_control.out

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,19 @@ SELECT key_name, key_provider_name, key_provider_id FROM pg_tde_key_info();
5252
test-db-key | file-vault | 1
5353
(1 row)
5454

55+
-- only superuser
56+
SELECT pg_tde_add_global_key_provider_file('file-vault', '/tmp/pg_tde_test_keyring.per');
57+
ERROR: must be superuser to modify global key providers
58+
SELECT pg_tde_change_global_key_provider_file('file-vault', '/tmp/pg_tde_test_keyring.per');
59+
ERROR: must be superuser to modify global key providers
60+
SELECT pg_tde_delete_global_key_provider('file-vault');
61+
ERROR: must be superuser to modify global key providers
62+
SELECT pg_tde_set_key_using_global_key_provider('key1', 'file-vault');
63+
ERROR: must be superuser to access global key providers
64+
SELECT pg_tde_set_default_key_using_global_key_provider('key1', 'file-vault');
65+
ERROR: must be superuser to access global key providers
66+
SELECT pg_tde_set_server_key_using_global_key_provider('key1', 'file-vault');
67+
ERROR: must be superuser to access global key providers
5568
RESET ROLE;
5669
SELECT pg_tde_revoke_key_viewer_from_role('regress_pg_tde_access_control');
5770
pg_tde_revoke_key_viewer_from_role

‎contrib/pg_tde/pg_tde--1.0-rc.sql

Lines changed: 0 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -528,39 +528,6 @@ ALTER EVENT TRIGGER pg_tde_trigger_create_index_2 ENABLE ALWAYS;
528528
-- Per database extension initialization
529529
SELECT pg_tde_extension_initialize();
530530

531-
CREATE FUNCTION pg_tde_grant_global_key_management_to_role(
532-
target_role TEXT)
533-
RETURNS VOID
534-
LANGUAGE plpgsql
535-
SET search_path = @extschema@
536-
AS $$
537-
BEGIN
538-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider(text, text, JSON) TO %I', target_role);
539-
540-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider_file(text, json) TO %I', target_role);
541-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider_file(text, text) TO %I', target_role);
542-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider_vault_v2(text, text, text, text, text) TO %I', target_role);
543-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider_vault_v2(text, JSON, JSON, JSON, JSON) TO %I', target_role);
544-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider_kmip(text, text, int, text, text) TO %I', target_role);
545-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider_kmip(text, JSON, JSON, JSON, JSON) TO %I', target_role);
546-
547-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider(text, text, JSON) TO %I', target_role);
548-
549-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider_file(text, json) TO %I', target_role);
550-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider_file(text, text) TO %I', target_role);
551-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider_vault_v2(text, text, text, text, text) TO %I', target_role);
552-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider_vault_v2(text, JSON, JSON, JSON, JSON) TO %I', target_role);
553-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider_kmip(text, text, int, text, text) TO %I', target_role);
554-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider_kmip(text, JSON, JSON, JSON, JSON) TO %I', target_role);
555-
556-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_delete_global_key_provider(text) TO %I', target_role);
557-
558-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_set_key_using_global_key_provider(text, text, BOOLEAN) TO %I', target_role);
559-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_set_server_key_using_global_key_provider(text, text, BOOLEAN) TO %I', target_role);
560-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_set_default_key_using_global_key_provider(text, text, BOOLEAN) TO %I', target_role);
561-
END;
562-
$$;
563-
564531
CREATE FUNCTION pg_tde_grant_database_key_management_to_role(
565532
target_role TEXT)
566533
RETURNS VOID
@@ -612,39 +579,6 @@ BEGIN
612579
END;
613580
$$;
614581

615-
CREATE FUNCTION pg_tde_revoke_global_key_management_from_role(
616-
target_role TEXT)
617-
RETURNS VOID
618-
LANGUAGE plpgsql
619-
SET search_path = @extschema@
620-
AS $$
621-
BEGIN
622-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_add_global_key_provider(text, text, JSON) FROM %I', target_role);
623-
624-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_add_global_key_provider_file(text, json) FROM %I', target_role);
625-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_add_global_key_provider_file(text, text) FROM %I', target_role);
626-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_add_global_key_provider_vault_v2(text, text, text, text, text) FROM %I', target_role);
627-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_add_global_key_provider_vault_v2(text, JSON, JSON, JSON, JSON) FROM %I', target_role);
628-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_add_global_key_provider_kmip(text, text, int, text, text) FROM %I', target_role);
629-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_add_global_key_provider_kmip(text, JSON, JSON, JSON, JSON) FROM %I', target_role);
630-
631-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_change_global_key_provider(text, text, JSON) FROM %I', target_role);
632-
633-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_change_global_key_provider_file(text, json) FROM %I', target_role);
634-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_change_global_key_provider_file(text, text) FROM %I', target_role);
635-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_change_global_key_provider_vault_v2(text, text, text, text, text) FROM %I', target_role);
636-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_change_global_key_provider_vault_v2(text, JSON, JSON, JSON, JSON) FROM %I', target_role);
637-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_change_global_key_provider_kmip(text, text, int, text, text) FROM %I', target_role);
638-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_change_global_key_provider_kmip(text, JSON, JSON, JSON, JSON) FROM %I', target_role);
639-
640-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_delete_global_key_provider(text) FROM %I', target_role);
641-
642-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_set_key_using_global_key_provider(text, text, BOOLEAN) FROM %I', target_role);
643-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_set_server_key_using_global_key_provider(text, text, BOOLEAN) FROM %I', target_role);
644-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_set_default_key_using_global_key_provider(text, text, BOOLEAN) FROM %I', target_role);
645-
END;
646-
$$;
647-
648582
CREATE FUNCTION pg_tde_revoke_database_key_management_from_role(
649583
target_role TEXT)
650584
RETURNS VOID
@@ -703,7 +637,6 @@ LANGUAGE plpgsql
703637
SET search_path = @extschema@
704638
AS $$
705639
BEGIN
706-
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_grant_global_key_management_to_role(TEXT) TO %I', target_role);
707640
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_grant_database_key_management_to_role(TEXT) TO %I', target_role);
708641
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_grant_grant_management_to_role(TEXT) TO %I', target_role);
709642
EXECUTE format('GRANT EXECUTE ON FUNCTION pg_tde_grant_key_viewer_to_role(TEXT) TO %I', target_role);
@@ -722,12 +655,10 @@ LANGUAGE plpgsql
722655
SET search_path = @extschema@
723656
AS $$
724657
BEGIN
725-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_grant_global_key_management_to_role(TEXT) FROM %I', target_role);
726658
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_grant_database_key_management_to_role(TEXT) FROM %I', target_role);
727659
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_grant_grant_management_to_role(TEXT) FROM %I', target_role);
728660
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_grant_key_viewer_to_role(TEXT) FROM %I', target_role);
729661

730-
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_revoke_global_key_management_from_role(TEXT) FROM %I', target_role);
731662
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_revoke_database_key_management_from_role(TEXT) FROM %I', target_role);
732663
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_revoke_grant_management_from_role(TEXT) FROM %I', target_role);
733664
EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_revoke_key_viewer_from_role(TEXT) FROM %I', target_role);
@@ -736,6 +667,5 @@ $$;
736667

737668
-- Revoking all the privileges from the public role
738669
SELECT pg_tde_revoke_database_key_management_from_role('public');
739-
SELECT pg_tde_revoke_global_key_management_from_role('public');
740670
SELECT pg_tde_revoke_grant_management_from_role('public');
741671
SELECT pg_tde_revoke_key_viewer_from_role('public');

‎contrib/pg_tde/sql/access_control.sql

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,14 @@ SELECT pg_tde_set_key_using_database_key_provider('test-db-key', 'file-vault');
2222
SELECT * FROM pg_tde_list_all_database_key_providers();
2323
SELECT key_name, key_provider_name, key_provider_id FROM pg_tde_key_info();
2424

25+
-- only superuser
26+
SELECT pg_tde_add_global_key_provider_file('file-vault', '/tmp/pg_tde_test_keyring.per');
27+
SELECT pg_tde_change_global_key_provider_file('file-vault', '/tmp/pg_tde_test_keyring.per');
28+
SELECT pg_tde_delete_global_key_provider('file-vault');
29+
SELECT pg_tde_set_key_using_global_key_provider('key1', 'file-vault');
30+
SELECT pg_tde_set_default_key_using_global_key_provider('key1', 'file-vault');
31+
SELECT pg_tde_set_server_key_using_global_key_provider('key1', 'file-vault');
32+
2533
RESET ROLE;
2634

2735
SELECT pg_tde_revoke_key_viewer_from_role('regress_pg_tde_access_control');

‎contrib/pg_tde/src/catalog/tde_keyring.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,11 @@ pg_tde_change_database_key_provider(PG_FUNCTION_ARGS)
214214
Datum
215215
pg_tde_change_global_key_provider(PG_FUNCTION_ARGS)
216216
{
217+
if (!superuser())
218+
ereport(ERROR,
219+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
220+
errmsg("must be superuser to modify global key providers")));
221+
217222
return pg_tde_change_key_provider_internal(fcinfo, GLOBAL_DATA_TDE_OID);
218223
}
219224

@@ -264,6 +269,11 @@ pg_tde_add_database_key_provider(PG_FUNCTION_ARGS)
264269
Datum
265270
pg_tde_add_global_key_provider(PG_FUNCTION_ARGS)
266271
{
272+
if (!superuser())
273+
ereport(ERROR,
274+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
275+
errmsg("must be superuser to modify global key providers")));
276+
267277
return pg_tde_add_key_provider_internal(fcinfo, GLOBAL_DATA_TDE_OID);
268278
}
269279

‎contrib/pg_tde/src/catalog/tde_principal_key.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,11 @@ pg_tde_set_default_key_using_global_key_provider(PG_FUNCTION_ARGS)
485485
char *provider_name = PG_ARGISNULL(1) ? NULL : text_to_cstring(PG_GETARG_TEXT_PP(1));
486486
bool ensure_new_key = PG_GETARG_BOOL(2);
487487

488+
if (!superuser())
489+
ereport(ERROR,
490+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
491+
errmsg("must be superuser to access global key providers")));
492+
488493
pg_tde_set_principal_key_internal(principal_key_name, GS_DEFAULT, provider_name, ensure_new_key);
489494

490495
PG_RETURN_VOID();
@@ -509,6 +514,11 @@ pg_tde_set_key_using_global_key_provider(PG_FUNCTION_ARGS)
509514
char *provider_name = PG_ARGISNULL(1) ? NULL : text_to_cstring(PG_GETARG_TEXT_PP(1));
510515
bool ensure_new_key = PG_GETARG_BOOL(2);
511516

517+
if (!superuser())
518+
ereport(ERROR,
519+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
520+
errmsg("must be superuser to access global key providers")));
521+
512522
pg_tde_set_principal_key_internal(principal_key_name, GS_GLOBAL, provider_name, ensure_new_key);
513523

514524
PG_RETURN_VOID();
@@ -521,6 +531,11 @@ pg_tde_set_server_key_using_global_key_provider(PG_FUNCTION_ARGS)
521531
char *provider_name = PG_ARGISNULL(1) ? NULL : text_to_cstring(PG_GETARG_TEXT_PP(1));
522532
bool ensure_new_key = PG_GETARG_BOOL(2);
523533

534+
if (!superuser())
535+
ereport(ERROR,
536+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
537+
errmsg("must be superuser to access global key providers")));
538+
524539
pg_tde_set_principal_key_internal(principal_key_name, GS_SERVER, provider_name, ensure_new_key);
525540

526541
PG_RETURN_VOID();
@@ -1045,6 +1060,11 @@ pg_tde_delete_database_key_provider(PG_FUNCTION_ARGS)
10451060
Datum
10461061
pg_tde_delete_global_key_provider(PG_FUNCTION_ARGS)
10471062
{
1063+
if (!superuser())
1064+
ereport(ERROR,
1065+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
1066+
errmsg("must be superuser to modify global key providers")));
1067+
10481068
return pg_tde_delete_key_provider_internal(fcinfo, 1);
10491069
}
10501070

0 commit comments

Comments
 (0)