Skip to content

Commit

Permalink
Babel: Avoid storing initial privileges for Babelfish objects created…
Browse files Browse the repository at this point in the history
… 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 : babelfish-for-postgresql/babelfish_extensions#3136

Task: BABEL-5410
Signed-off-by: Harsh Lunagariya <[email protected]>
  • Loading branch information
HarshLunagariya authored Dec 17, 2024
1 parent ac51c42 commit 9a28935
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 28 deletions.
32 changes: 4 additions & 28 deletions src/backend/catalog/aclchk.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 */
Expand All @@ -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 */

Expand Down
3 changes: 3 additions & 0 deletions src/include/utils/acl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down

0 comments on commit 9a28935

Please sign in to comment.