Revoke all from public on c functions by default

Even though we do have some access control in most of our C functions,
it seems dangerous to not revoke them from public by default. A
whitelist of allowed functions seems safer than a black list. Also
include non-C functions which are security definer.

contrib/pg_tde/expected/access_control.out

@@ -75,7 +75,18 @@ SELECT pg_tde_verify_server_key();
 ERROR:  principal key not configured for current database
 SELECT pg_tde_verify_default_key();
 ERROR:  principal key not configured for current database
--- only superuser
+-- Only superusers can execute key management functions, regardless of role grants
+RESET ROLE;
+GRANT EXECUTE ON FUNCTION pg_tde_add_database_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_change_database_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_delete_database_key_provider(TEXT) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_delete_global_key_provider(TEXT) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_set_default_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_set_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_set_server_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) TO regress_pg_tde_access_control;
+SET ROLE regress_pg_tde_access_control;
 SELECT pg_tde_add_database_key_provider_file('local-file-provider', '/tmp/pg_tde_test_keyring.per');
 ERROR:  must be superuser to modify key providers
 SELECT pg_tde_change_global_key_provider_file('local-file-provider', '/tmp/pg_tde_test_keyring.per');

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

@@ -8,6 +8,7 @@ CREATE FUNCTION pg_tde_add_database_key_provider(provider_type TEXT, provider_na
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_add_database_key_provider(TEXT, TEXT, JSON) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_add_database_key_provider_file(provider_name TEXT, file_path TEXT)
 RETURNS VOID
@@ -109,6 +110,7 @@ CREATE FUNCTION pg_tde_list_all_database_key_providers
 RETURNS SETOF RECORD
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_list_all_database_key_providers() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_list_all_global_key_providers
     (OUT id INT,
@@ -118,12 +120,14 @@ CREATE FUNCTION pg_tde_list_all_global_key_providers
 RETURNS SETOF RECORD
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_list_all_global_key_providers() FROM PUBLIC;
 
 -- Global Tablespace Key Provider Management
 CREATE FUNCTION pg_tde_add_global_key_provider(provider_type TEXT, provider_name TEXT, options JSON)
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_add_global_key_provider(TEXT, TEXT, JSON) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_add_global_key_provider_file(provider_name TEXT, file_path TEXT)
 RETURNS VOID
@@ -222,6 +226,7 @@ CREATE FUNCTION pg_tde_change_database_key_provider(provider_type TEXT, provider
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_change_database_key_provider(TEXT, TEXT, JSON) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_change_database_key_provider_file(provider_name TEXT, file_path TEXT)
 RETURNS VOID
@@ -320,6 +325,7 @@ CREATE FUNCTION pg_tde_change_global_key_provider(provider_type TEXT, provider_n
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_change_global_key_provider(TEXT, TEXT, JSON) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_change_global_key_provider_file(provider_name TEXT, file_path TEXT)
 RETURNS VOID
@@ -423,36 +429,44 @@ CREATE FUNCTION pg_tde_set_key_using_database_key_provider(key_name TEXT, provid
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_set_key_using_database_key_provider(TEXT, TEXT, BOOLEAN) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_set_key_using_global_key_provider(key_name TEXT, provider_name TEXT, ensure_new_key BOOLEAN DEFAULT FALSE)
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_set_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_set_server_key_using_global_key_provider(key_name TEXT, provider_name TEXT, ensure_new_key BOOLEAN DEFAULT FALSE)
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_set_server_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) FROM PUBLIC;
+
 
 CREATE FUNCTION pg_tde_set_default_key_using_global_key_provider(key_name TEXT, provider_name TEXT, ensure_new_key BOOLEAN DEFAULT FALSE)
 RETURNS VOID
 AS 'MODULE_PATHNAME'
 LANGUAGE C;
+REVOKE ALL ON FUNCTION pg_tde_set_default_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_verify_key()
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_verify_key() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_verify_server_key()
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_verify_server_key() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_verify_default_key()
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_verify_default_key() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_key_info()
 RETURNS TABLE ( key_name TEXT,
@@ -461,6 +475,7 @@ RETURNS TABLE ( key_name TEXT,
                 key_creation_time TIMESTAMP WITH TIME ZONE)
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_key_info() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_server_key_info()
 RETURNS TABLE ( key_name TEXT,
@@ -469,6 +484,7 @@ RETURNS TABLE ( key_name TEXT,
                 key_creation_time TIMESTAMP WITH TIME ZONE)
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_server_key_info() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_default_key_info()
 RETURNS TABLE ( key_name TEXT,
@@ -477,16 +493,19 @@ RETURNS TABLE ( key_name TEXT,
                 key_creation_time TIMESTAMP WITH TIME ZONE)
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_default_key_info() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_delete_global_key_provider(provider_name TEXT)
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_delete_global_key_provider(TEXT) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_delete_database_key_provider(provider_name TEXT)
 RETURNS VOID
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_delete_database_key_provider(TEXT) FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_version() RETURNS TEXT LANGUAGE C AS 'MODULE_PATHNAME';
 
@@ -495,6 +514,7 @@ CREATE FUNCTION pg_tdeam_handler(internal)
 RETURNS TABLE_AM_HANDLER
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tdeam_handler(internal) FROM PUBLIC;
 
 CREATE ACCESS METHOD tde_heap TYPE TABLE HANDLER pg_tdeam_handler;
 COMMENT ON ACCESS METHOD tde_heap IS 'tde_heap table access method';
@@ -503,11 +523,13 @@ CREATE FUNCTION pg_tde_ddl_command_start_capture()
 RETURNS EVENT_TRIGGER
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_ddl_command_start_capture() FROM PUBLIC;
 
 CREATE FUNCTION pg_tde_ddl_command_end_capture()
 RETURNS EVENT_TRIGGER
 LANGUAGE C
 AS 'MODULE_PATHNAME';
+REVOKE ALL ON FUNCTION pg_tde_ddl_command_end_capture() FROM PUBLIC;
 
 CREATE EVENT TRIGGER pg_tde_ddl_start
 ON ddl_command_start
@@ -588,7 +610,3 @@ BEGIN
     EXECUTE format('REVOKE EXECUTE ON FUNCTION pg_tde_verify_default_key() FROM %I', target_role);
 END;
 $$;
-
--- Revoking all the privileges from the public role
-SELECT pg_tde_revoke_database_key_management_from_role('public');
-SELECT pg_tde_revoke_key_viewer_from_role('public');

contrib/pg_tde/sql/access_control.sql

@@ -34,7 +34,19 @@ SELECT pg_tde_verify_key();
 SELECT pg_tde_verify_server_key();
 SELECT pg_tde_verify_default_key();
 
--- only superuser
+-- Only superusers can execute key management functions, regardless of role grants
+RESET ROLE;
+GRANT EXECUTE ON FUNCTION pg_tde_add_database_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_add_global_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_change_database_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_change_global_key_provider(TEXT, TEXT, JSON) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_delete_database_key_provider(TEXT) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_delete_global_key_provider(TEXT) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_set_default_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_set_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) TO regress_pg_tde_access_control;
+GRANT EXECUTE ON FUNCTION pg_tde_set_server_key_using_global_key_provider(TEXT, TEXT, BOOLEAN) TO regress_pg_tde_access_control;
+
+SET ROLE regress_pg_tde_access_control;
 SELECT pg_tde_add_database_key_provider_file('local-file-provider', '/tmp/pg_tde_test_keyring.per');
 SELECT pg_tde_change_global_key_provider_file('local-file-provider', '/tmp/pg_tde_test_keyring.per');
 SELECT pg_tde_delete_database_key_provider('local-file-provider');
@@ -44,7 +56,6 @@ SELECT pg_tde_delete_global_key_provider('global-file-provider');
 SELECT pg_tde_set_key_using_global_key_provider('key1', 'global-file-provider');
 SELECT pg_tde_set_default_key_using_global_key_provider('key1', 'global-file-provider');
 SELECT pg_tde_set_server_key_using_global_key_provider('key1', 'global-file-provider');
-
 RESET ROLE;
 
 SELECT pg_tde_revoke_key_viewer_from_role('regress_pg_tde_access_control');

contrib/pg_tde/t/001_basic.pl

@@ -16,6 +16,23 @@ $node->start;
 
 PGTDE::psql($node, 'postgres', 'CREATE EXTENSION IF NOT EXISTS pg_tde;');
 
+# Only whitelisted C or security definer functions are granted to public by default
+PGTDE::psql(
+	$node, 'postgres',
+	q{
+		SELECT
+			pg_proc.oid::regprocedure
+		FROM
+			pg_catalog.pg_proc
+			JOIN pg_catalog.pg_language ON prolang = pg_language.oid
+			LEFT JOIN LATERAL aclexplode(proacl) ON TRUE
+		WHERE
+			proname LIKE 'pg_tde%' AND
+			(lanname = 'c' OR prosecdef) AND
+			(grantee IS NULL OR grantee = 0)
+		ORDER BY pg_proc.oid::regprocedure::text;
+	});
+
 PGTDE::psql($node, 'postgres',
 	"SELECT extname, extversion FROM pg_extension WHERE extname = 'pg_tde';");
 

contrib/pg_tde/t/expected/001_basic.out

@@ -1,4 +1,22 @@
 CREATE EXTENSION IF NOT EXISTS pg_tde;
+		SELECT
+			pg_proc.oid::regprocedure
+		FROM
+			pg_catalog.pg_proc
+			JOIN pg_catalog.pg_language ON prolang = pg_language.oid
+			LEFT JOIN LATERAL aclexplode(proacl) ON TRUE
+		WHERE
+			proname LIKE 'pg_tde%' AND
+			(lanname = 'c' OR prosecdef) AND
+			(grantee IS NULL OR grantee = 0)
+		ORDER BY pg_proc.oid::regprocedure::text;
+              oid              
+-------------------------------
+ pg_tde_is_encrypted(regclass)
+ pg_tde_version()
+(2 rows)
+
+	
 SELECT extname, extversion FROM pg_extension WHERE extname = 'pg_tde';
  extname | extversion 
 ---------+------------