From bc1f5a574e65f0f0609645ac477e1204ab178851 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Wed, 8 Jan 2025 18:37:21 +0000 Subject: [PATCH 1/7] Restrict grant to/from any babelfish created role --- .../babelfishpg_tds/src/backend/tds/tdsutils.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c index 58585d5dbc..87ea1bbbe0 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c @@ -949,11 +949,13 @@ is_babelfish_role(const char *role) Oid bbf_msdb_guest_oid; Oid securityadmin; Oid dbcreator; + Oid bbf_admin_oid; sysadmin_oid = get_role_oid(BABELFISH_SYSADMIN, true); /* missing OK */ role_oid = get_role_oid(role, true); /* missing OK */ securityadmin = get_role_oid(BABELFISH_SECURITYADMIN, true); /* missing OK */ dbcreator = get_role_oid(BABELFISH_DBCREATOR, true); /* missing OK */ + bbf_admin_oid = get_role_oid(BABELFISH_ROLE_ADMIN, true); /* missing OK */ if (!OidIsValid(sysadmin_oid) || !OidIsValid(role_oid) || !OidIsValid(securityadmin) || !OidIsValid(dbcreator)) @@ -973,7 +975,8 @@ is_babelfish_role(const char *role) && OidIsValid(bbf_msdb_guest_oid) && is_member_of_role(role_oid, bbf_master_guest_oid) && is_member_of_role(role_oid, bbf_tempdb_guest_oid) - && is_member_of_role(role_oid, bbf_msdb_guest_oid)) + && is_member_of_role(role_oid, bbf_msdb_guest_oid) + && is_member_of_role(bbf_admin_oid, role_oid)) return true; return false; @@ -1231,7 +1234,7 @@ handle_grant_role(GrantRoleStmt *grant_stmt) if (get_bbf_role_admin_oid() == GetUserId()) return true; - /* Restrict roles to added as a member of BBF default server roles */ + /* Restrict roles to added as a member of babelfish created roles */ foreach(item, grant_stmt->granted_roles) { AccessPriv *priv = (AccessPriv *) lfirst(item); @@ -1242,11 +1245,14 @@ handle_grant_role(GrantRoleStmt *grant_stmt) continue; roleid = get_role_oid(rolename, false); - if (OidIsValid(roleid) && IS_DEFAULT_BBF_SERVER_ROLE(rolename)) + if (OidIsValid(roleid) && is_babelfish_role(rolename)) + { + pfree(rolename); check_babelfish_alterrole_restictions(false); + } } - /* Restrict grant to/from bbf_role_admin, securityadmin or dbcreator role */ + /* Restrict grant to/from any babelfish created role */ foreach(item, grant_stmt->grantee_roles) { @@ -1254,8 +1260,10 @@ handle_grant_role(GrantRoleStmt *grant_stmt) Oid roleid; roleid = get_rolespec_oid(rolespec, false); - if (OidIsValid(roleid) && IS_DEFAULT_BBF_SERVER_ROLE(rolespec->rolename)) + if (OidIsValid(roleid) && is_babelfish_role(rolespec->rolename)) + { check_babelfish_alterrole_restictions(false); + } } return true; From e6b825d8d652123a82673025282fc20a070393c0 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Thu, 9 Jan 2025 03:46:21 +0000 Subject: [PATCH 2/7] Fix the error message --- .../expected/db_securityadmin-vu-verify.out | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/test/JDBC/expected/db_securityadmin-vu-verify.out b/test/JDBC/expected/db_securityadmin-vu-verify.out index 2cb9a4dde3..a324f3698d 100644 --- a/test/JDBC/expected/db_securityadmin-vu-verify.out +++ b/test/JDBC/expected/db_securityadmin-vu-verify.out @@ -1344,8 +1344,7 @@ GRANT db_securityadmin_restrictions_login TO master_db_securityadmin; GO ~~ERROR (Code: 0)~~ -~~ERROR (Message: ERROR: permission denied to grant role "db_securityadmin_restrictions_login" - Detail: Only roles with the ADMIN option on role "db_securityadmin_restrictions_login" may grant this role. +~~ERROR (Message: ERROR: Babelfish-created logins/users/roles cannot be altered outside of a Babelfish session Server SQLState: 42501)~~ @@ -1356,8 +1355,7 @@ REVOKE master_dbo FROM master_db_securityadmin; GO ~~ERROR (Code: 0)~~ -~~ERROR (Message: ERROR: permission denied to revoke role "master_dbo" - Detail: Only roles with the ADMIN option on role "master_dbo" may revoke this role. +~~ERROR (Message: ERROR: Babelfish-created logins/users/roles cannot be altered outside of a Babelfish session Server SQLState: 42501)~~ @@ -1418,8 +1416,7 @@ GRANT master_db_securityadmin TO db_securityadmin_restrictions_login; GO ~~ERROR (Code: 0)~~ -~~ERROR (Message: ERROR: permission denied to grant role "master_db_securityadmin" - Detail: Only roles with the ADMIN option on role "master_db_securityadmin" may grant this role. +~~ERROR (Message: ERROR: Babelfish-created logins/users/roles cannot be altered outside of a Babelfish session Server SQLState: 42501)~~ @@ -1427,8 +1424,7 @@ GRANT db_securityadmin_restrictions_login TO master_db_securityadmin; GO ~~ERROR (Code: 0)~~ -~~ERROR (Message: ERROR: permission denied to grant role "db_securityadmin_restrictions_login" - Detail: Only roles with the ADMIN option on role "db_securityadmin_restrictions_login" may grant this role. +~~ERROR (Message: ERROR: Babelfish-created logins/users/roles cannot be altered outside of a Babelfish session Server SQLState: 42501)~~ @@ -1436,8 +1432,7 @@ REVOKE master_db_securityadmin FROM master_dbo; GO ~~ERROR (Code: 0)~~ -~~ERROR (Message: ERROR: permission denied to revoke role "master_db_securityadmin" - Detail: Only roles with the ADMIN option on role "master_db_securityadmin" may revoke this role. +~~ERROR (Message: ERROR: Babelfish-created logins/users/roles cannot be altered outside of a Babelfish session Server SQLState: 42501)~~ @@ -1445,8 +1440,7 @@ REVOKE master_dbo FROM master_db_securityadmin; GO ~~ERROR (Code: 0)~~ -~~ERROR (Message: ERROR: permission denied to revoke role "master_dbo" - Detail: Only roles with the ADMIN option on role "master_dbo" may revoke this role. +~~ERROR (Message: ERROR: Babelfish-created logins/users/roles cannot be altered outside of a Babelfish session Server SQLState: 42501)~~ From bdf954861f0dce712efdce4a6d2e9e760c1c2f72 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Thu, 9 Jan 2025 04:19:49 +0000 Subject: [PATCH 3/7] Any role which is a member of bbf_role_admin is a bbf role --- .../src/backend/tds/tdsutils.c | 31 +++---------------- 1 file changed, 4 insertions(+), 27 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c index 87ea1bbbe0..b099023d74 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c @@ -942,41 +942,18 @@ check_babelfish_droprole_restrictions(char *role) static bool is_babelfish_role(const char *role) { - Oid sysadmin_oid; Oid role_oid; - Oid bbf_master_guest_oid; - Oid bbf_tempdb_guest_oid; - Oid bbf_msdb_guest_oid; - Oid securityadmin; - Oid dbcreator; Oid bbf_admin_oid; - sysadmin_oid = get_role_oid(BABELFISH_SYSADMIN, true); /* missing OK */ role_oid = get_role_oid(role, true); /* missing OK */ - securityadmin = get_role_oid(BABELFISH_SECURITYADMIN, true); /* missing OK */ - dbcreator = get_role_oid(BABELFISH_DBCREATOR, true); /* missing OK */ bbf_admin_oid = get_role_oid(BABELFISH_ROLE_ADMIN, true); /* missing OK */ - if (!OidIsValid(sysadmin_oid) || !OidIsValid(role_oid) - || !OidIsValid(securityadmin) || !OidIsValid(dbcreator)) - return false; - - if (is_member_of_role(sysadmin_oid, role_oid) || - is_member_of_role(securityadmin, role_oid) || - is_member_of_role(dbcreator, role_oid) || - pg_strcasecmp(role, BABELFISH_ROLE_ADMIN) == 0) /* check if it is bbf_role_admin */ + /* check if it is bbf_role_admin */ + if (pg_strcasecmp(role, BABELFISH_ROLE_ADMIN) == 0) return true; - bbf_master_guest_oid = get_role_oid("master_guest", true); - bbf_tempdb_guest_oid = get_role_oid("tempdb_guest", true); - bbf_msdb_guest_oid = get_role_oid("msdb_guest", true); - if (OidIsValid(bbf_master_guest_oid) - && OidIsValid(bbf_tempdb_guest_oid) - && OidIsValid(bbf_msdb_guest_oid) - && is_member_of_role(role_oid, bbf_master_guest_oid) - && is_member_of_role(role_oid, bbf_tempdb_guest_oid) - && is_member_of_role(role_oid, bbf_msdb_guest_oid) - && is_member_of_role(bbf_admin_oid, role_oid)) + /* If a role as 'bbf_role_admin' as a member, it's a Babelfish role. */ + if (is_member_of_role(bbf_admin_oid, role_oid)) return true; return false; From 29a6f45738509a1ff75beeef6014e4b0bc8ae253 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Thu, 9 Jan 2025 04:26:19 +0000 Subject: [PATCH 4/7] Nit: Fixed comments --- contrib/babelfishpg_tds/src/backend/tds/tdsutils.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c index b099023d74..f80cab3f0d 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c @@ -935,7 +935,7 @@ check_babelfish_droprole_restrictions(char *role) * that is only accessible in babelfish_db. * Since role related DDLs could be executed in any PG databases, * This function check the underlying assumption on the membership chain instead - * sysadmin <-- dbo* <--- db_owner* <--- users/roles + * bbf_admin_oid <-- dbo* <--- db_owner* <--- users/roles * * actual dbo and db_owner name varies across different babelfish logical databases */ @@ -943,7 +943,7 @@ static bool is_babelfish_role(const char *role) { Oid role_oid; - Oid bbf_admin_oid; + Oid bbf_admin_oid; role_oid = get_role_oid(role, true); /* missing OK */ bbf_admin_oid = get_role_oid(BABELFISH_ROLE_ADMIN, true); /* missing OK */ @@ -952,7 +952,7 @@ is_babelfish_role(const char *role) if (pg_strcasecmp(role, BABELFISH_ROLE_ADMIN) == 0) return true; - /* If a role as 'bbf_role_admin' as a member, it's a Babelfish role. */ + /* If a role has 'bbf_role_admin' as a member, it's a Babelfish role. */ if (is_member_of_role(bbf_admin_oid, role_oid)) return true; From 2031a2d2532c2ff7330efdcf75deb095a8eca1bd Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Thu, 9 Jan 2025 06:42:03 +0000 Subject: [PATCH 5/7] Update the check for bbf role --- .../babelfishpg_tds/src/backend/tds/tdsutils.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c index f80cab3f0d..8011c109cd 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c @@ -943,6 +943,9 @@ static bool is_babelfish_role(const char *role) { Oid role_oid; + Oid bbf_master_guest_oid; + Oid bbf_tempdb_guest_oid; + Oid bbf_msdb_guest_oid; Oid bbf_admin_oid; role_oid = get_role_oid(role, true); /* missing OK */ @@ -952,9 +955,17 @@ is_babelfish_role(const char *role) if (pg_strcasecmp(role, BABELFISH_ROLE_ADMIN) == 0) return true; - /* If a role has 'bbf_role_admin' as a member, it's a Babelfish role. */ - if (is_member_of_role(bbf_admin_oid, role_oid)) - return true; + bbf_master_guest_oid = get_role_oid("master_guest", true); + bbf_tempdb_guest_oid = get_role_oid("tempdb_guest", true); + bbf_msdb_guest_oid = get_role_oid("msdb_guest", true); + if (OidIsValid(bbf_master_guest_oid) + && OidIsValid(bbf_tempdb_guest_oid) + && OidIsValid(bbf_msdb_guest_oid) + && is_member_of_role(role_oid, bbf_master_guest_oid) + && is_member_of_role(role_oid, bbf_tempdb_guest_oid) + && is_member_of_role(role_oid, bbf_msdb_guest_oid) + && is_member_of_role(bbf_admin_oid, role_oid)) /* If a role has 'bbf_role_admin' as a member, it's a Babelfish role. */ + return true; return false; } From b88bec8edb798afc40371ebdf4fcc72535becb9c Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Thu, 9 Jan 2025 06:46:06 +0000 Subject: [PATCH 6/7] Return false if oid is invalid --- contrib/babelfishpg_tds/src/backend/tds/tdsutils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c index 8011c109cd..15039ab9a4 100644 --- a/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c +++ b/contrib/babelfishpg_tds/src/backend/tds/tdsutils.c @@ -951,6 +951,9 @@ is_babelfish_role(const char *role) role_oid = get_role_oid(role, true); /* missing OK */ bbf_admin_oid = get_role_oid(BABELFISH_ROLE_ADMIN, true); /* missing OK */ + if (!OidIsValid(role_oid) || !OidIsValid(bbf_admin_oid)) + return false; + /* check if it is bbf_role_admin */ if (pg_strcasecmp(role, BABELFISH_ROLE_ADMIN) == 0) return true; From 2f32a21c647ae3e82d621180ac32120049363d84 Mon Sep 17 00:00:00 2001 From: Shalini Lohia Date: Thu, 9 Jan 2025 07:04:26 +0000 Subject: [PATCH 7/7] Remove unused macro --- contrib/babelfishpg_tds/src/include/tds_int.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/contrib/babelfishpg_tds/src/include/tds_int.h b/contrib/babelfishpg_tds/src/include/tds_int.h index 624d47ead8..fadcb7d54a 100644 --- a/contrib/babelfishpg_tds/src/include/tds_int.h +++ b/contrib/babelfishpg_tds/src/include/tds_int.h @@ -259,12 +259,6 @@ extern ProcessUtility_hook_type next_ProcessUtility; #define BABELFISH_SECURITYADMIN "securityadmin" #define BABELFISH_DBCREATOR "dbcreator" -#define IS_DEFAULT_BBF_SERVER_ROLE(rolename) \ - ((strlen(rolename) == 13 && strncmp(rolename, BABELFISH_SECURITYADMIN, 13) == 0) || \ - (strlen(rolename) == 14 && strncmp(rolename, BABELFISH_ROLE_ADMIN, 14) == 0) || \ - (strlen(rolename) == 9 && strncmp(rolename, BABELFISH_DBCREATOR, 9) == 0) || \ - (strlen(rolename) == 8 && strncmp(rolename, BABELFISH_SYSADMIN, 8) == 0)) - /* Functions in backend/tds/tdscomm.c */ extern void TdsSetMessageType(uint8_t msgType); extern void TdsCommInit(uint32_t bufferSize,