From 9a2893564a171dba85815fb39ae7ebdb8865ea33 Mon Sep 17 00:00:00 2001 From: HarshLunagariya <40052763+HarshLunagariya@users.noreply.github.com> Date: Tue, 17 Dec 2024 20:27:04 +0530 Subject: [PATCH] Babel: Avoid storing initial privileges for Babelfish objects created during initialization (#480) In Babelfish, Currently There are two patterns followed for initial privileges for system created objects: 1. Initial privileges are stored for the objects that are created during CREATE EXTENSION like system object, catalogs. 2. But for the objects that are created during `initialize_babelfish()` (which gets executed after creating extension) like dbo user, schema for system generated TSQL databases like master, msdb, tempdb, Initial privileges are not being recorded. When there are any GRANT/REVOKE on objects which are created during `initialize_babelfish()` in any upgrade script, they are falsely being considered as internal/system-generated object and initial_privileges are being stored for them which can cause inconsistency between initial privileges of a newly spawned database server and upgraded database server. To avoid this situation, we should not store initial privileges for GRANT/REVOKE execution during upgrade. For achieving this, * Extension Changes: This commit implements `pltsql_allow_storing_init_privs_hook` hook for checking whether to store initial privileges for given object or not. There are 3 category of handling storage of initial privileges: 1. SAVE_INIT_PRIVS : Check if it is objects created during CREATE extension and store initial privs for them. system, information_schema_tsql objects and pltsql language are the examples of it. 2. DISCARD_INIT_PRIVS : If it is schema contained object within system created TSQL schema like master, msdb or tempdb OR user created schema, Do not store initial privileges for them. 3. ERROR_INIT_PRIVS : The default case when above 2 conditions doesn't match then error out. To avoid error please classify it between above 2 conditions. * Engine Changes: Reverts the temporary fix 3f20518 and f9e9557. Added `pltsql_check_store_init_privs_flag_hook` hook for checking whether to store initial privileges or not. Extension PR : https://github.com/babelfish-for-postgresql/babelfish_extensions/pull/3136 Task: BABEL-5410 Signed-off-by: Harsh Lunagariya --- src/backend/catalog/aclchk.c | 32 ++++---------------------------- src/include/utils/acl.h | 3 +++ 2 files changed, 7 insertions(+), 28 deletions(-) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 9627bd404b4..a9a626db3c1 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -173,7 +173,7 @@ static void recordExtensionInitPrivWorker(Oid objoid, Oid classoid, int objsubid tsql_has_linked_srv_permissions_hook_type tsql_has_linked_srv_permissions_hook = NULL; bbf_execute_grantstmt_as_dbsecadmin_hook_type bbf_execute_grantstmt_as_dbsecadmin_hook = NULL; - +pltsql_allow_storing_init_privs_hook_type pltsql_allow_storing_init_privs_hook = NULL; /* * If is_grant is true, adds the given privileges for the list of * grantees to the existing old_acl. If is_grant is false, the @@ -4705,6 +4705,9 @@ recordExtensionInitPriv(Oid objoid, Oid classoid, int objsubid, Acl *new_acl) if (!creating_extension && !binary_upgrade_record_init_privs) return; + if (pltsql_allow_storing_init_privs_hook && !((*pltsql_allow_storing_init_privs_hook)(objoid, classoid, objsubid))) + return; + recordExtensionInitPrivWorker(objoid, classoid, objsubid, new_acl); } @@ -5039,32 +5042,6 @@ RemoveRoleFromInitPriv(Oid roleid, Oid classid, Oid objid, int32 objsubid) * Generate new ACL. Grantor of rights is always the same as the owner. */ if (old_acl != NULL) - { - Oid sysadminOid; - Acl *temp_acl; - const char *babelfish_db_name; - - /* - * For babelfish database, grantor will be sysadmin instead of object owner. - */ - if (bbf_get_sysadmin_oid_hook && - classid == DatabaseRelationId && - (babelfish_db_name = GetConfigOption("babelfishpg_tsql.database_name", true, false)) && - objid == get_database_oid(babelfish_db_name, true) && - is_member_of_role(GetUserId(), sysadminOid = (*bbf_get_sysadmin_oid_hook)())) - { - - temp_acl = merge_acl_with_grant(old_acl, - false, /* is_grant */ - false, /* grant_option */ - DROP_RESTRICT, - list_make1_oid(roleid), - ACLITEM_ALL_PRIV_BITS, - sysadminOid, - ownerId); - old_acl = temp_acl; - } - new_acl = merge_acl_with_grant(old_acl, false, /* is_grant */ false, /* grant_option */ @@ -5073,7 +5050,6 @@ RemoveRoleFromInitPriv(Oid roleid, Oid classid, Oid objid, int32 objsubid) ACLITEM_ALL_PRIV_BITS, ownerId, ownerId); - } else new_acl = NULL; /* this case shouldn't happen, probably */ diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index c43cd5d90ba..1df439f3692 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -306,6 +306,9 @@ extern PGDLLEXPORT pltsql_get_object_owner_hook_type pltsql_get_object_owner_hoo typedef bool (*is_bbf_db_ddladmin_operation_hook_type) (Oid namespaceId); extern PGDLLEXPORT is_bbf_db_ddladmin_operation_hook_type is_bbf_db_ddladmin_operation_hook; +typedef bool (*pltsql_allow_storing_init_privs_hook_type) (Oid objoid, Oid classoid, int objsubid); +extern PGDLLEXPORT pltsql_allow_storing_init_privs_hook_type pltsql_allow_storing_init_privs_hook; + #define IS_BBF_DB_DDLADMIN(namespaceId) \ (is_bbf_db_ddladmin_operation_hook && \ is_bbf_db_ddladmin_operation_hook(namespaceId))