Skip to content

Commit

Permalink
Revert disallow ALTER ROLE in PG for Babelfish-created roles
Browse files Browse the repository at this point in the history 
Previously, for a PG user to break a Babelfish instance by using
ALTER ROLE to block logins, change role membership, change passwords,
and other changes that would have a dramatic effect on Babelfish
functionality. ALTER ROLE been disabled for Babelfish-created PG
roles.

Now reverting the disabling of ALTER ROLE for Babelfish-created PG
roles changes as there were other interoperability related operations to
be considered.

Signed-off-by: vasavi suthapalli <svasusri@amazon.com>
  • Loading branch information
@Suthapalli-Ramya-satya-vasavi-srija
Suthapalli-Ramya-satya-vasavi-srija authored Nov 2, 2022
1 parent d89c020 commit b82cac1
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 320 deletions.
84 changes: 4 additions & 80 deletions contrib/babelfishpg_tds/src/backend/tds/tdsutils.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void tdsutils_ProcessUtility (PlannedStmt *pstmt, const char *queryString, bool
ProcessUtility_hook_type next_ProcessUtility = NULL;
static void call_next_ProcessUtility (PlannedStmt *pstmt, const char *queryString, bool readOnlyTree, ProcessUtilityContext context, ParamListInfo params, QueryEnvironment *queryEnv, DestReceiver *dest, QueryCompletion *completionTag);
static void check_babelfish_droprole_restrictions(char *role);
static void check_babelfish_alterrole_restictions(char *role);
static void check_babelfish_renamerole_restrictions(char *role);
static void check_babelfish_renamedb_restrictions(Oid target_db_id);
static void check_babelfish_dropdb_restrictions(Oid target_db_id);
static bool is_babelfish_ownership_enabled(ArrayType *array);
Expand All @@ -57,8 +57,6 @@ static bool is_babelfish_role(const char *role);
/* Role specific handlers */
static bool handle_drop_role (DropRoleStmt* drop_role_stmt);
static bool handle_rename(RenameStmt* rename_stmt);
static bool handle_alter_role(AlterRoleStmt* alter_role_stmt);
static bool handle_alter_role_set (AlterRoleSetStmt* alter_role_set_stmt);

/* Drop database handler */
static bool handle_dropdb(DropdbStmt *dropdb_stmt);
Expand Down Expand Up @@ -593,7 +591,6 @@ babelfish_object_access(ObjectAccessType access,

switch (access)
{
case OAT_POST_ALTER:
case OAT_DROP:
{
switch (classId)
Expand Down Expand Up @@ -699,12 +696,6 @@ void tdsutils_ProcessUtility(PlannedStmt *pstmt,
case T_RenameStmt:
handle_result = handle_rename((RenameStmt *)parsetree);
break;
case T_AlterRoleStmt:
handle_result = handle_alter_role((AlterRoleStmt*)parsetree);
break;
case T_AlterRoleSetStmt:
handle_result = handle_alter_role_set((AlterRoleSetStmt*)parsetree);
break;
/* Case that deal with Drop Database */
case T_DropdbStmt:
handle_result = handle_dropdb((DropdbStmt *)parsetree);
Expand Down Expand Up @@ -948,7 +939,7 @@ handle_rename(RenameStmt* rename_stmt)
* (obviously) event triggers, so we need to ignore those.
*/
if (OBJECT_ROLE == rename_stmt->renameType)
check_babelfish_alterrole_restictions(rename_stmt->subname);
check_babelfish_renamerole_restrictions(rename_stmt->subname);

else if (OBJECT_DATABASE == rename_stmt->renameType)
{
Expand Down Expand Up @@ -976,13 +967,13 @@ handle_rename(RenameStmt* rename_stmt)
}

/*
* check_babelfish_alterrole_restictions
* check_babelfish_renamerole_restrictions
*
* Implements following one additional limitation to drop role stmt
* block renaming an active babelfish role/user
*/
static void
check_babelfish_alterrole_restictions(char *role)
check_babelfish_renamerole_restrictions(char *role)
{
if (sql_dialect == SQL_DIALECT_TSQL)
return;
Expand Down Expand Up @@ -1157,70 +1148,3 @@ is_babelfish_ownership_enabled(ArrayType *array)
}
return false;
}

/*
* handle_alter_role
*
* Description: This function handles dealing with ALTER ROLE <role> WITH.
*
* Returns: true - We're not attempting to modify something we shouldn't have access to. Normal security checks.
* false - We've reported an error and should not continue executing this call.
*/
static bool
handle_alter_role(AlterRoleStmt* alter_role_stmt)
{
char *name = get_role_name(alter_role_stmt->role);

/* If the role does not exist, just let the normal Postgres checks happen. */
if (name == NULL)
return true;

check_babelfish_alterrole_restictions(name);

/* We don't need "name" anymore */
pfree(name);
return true;
}

/* handle_alter_role_set
*
* Description: This function handles dealing with ALTER ROLE <role> SET.
*
* Returns: true - We're not attempting to modify something we shouldn't have access to, continue on.
* false - We've reported an error and should not continue executing this call.
*/
static bool
handle_alter_role_set (AlterRoleSetStmt* alter_role_set_stmt)
{
char *name;

/*
* If this is an ALTER ROLE ALL [ IN DATABASE ] SET statement,
* alter_role_set_stmt->role will be NULL. While we don't want users
* altering our "protected" roles, we can pass through here because
* PostgreSQL already handles those situations correctly.
*
* The ALTER ROLE ALL SET variant of this command can only be run by
* superusers, and the ALTER ROLE ALL IN DATABASE SET variant is the same as
* ALTER DATABASE SET, which is handled via the regular database ownership
* checks. (Customers should not be able to obtain ownership of our
* "protected" databases thanks to handle_alter_owner().)
*/
if (alter_role_set_stmt->role == NULL)
return true;

name = get_role_name(alter_role_set_stmt->role);

/* If the role does not exist, just let the normal Postgres checks happen.*/
if (NULL == name)
return true;

check_babelfish_alterrole_restictions(name);

/*
* Reaching here does not mean that this user has permission to modify the role.
* Those permissions checks are done through normal handling.
*/
pfree(name);
return true;
}
169 changes: 9 additions & 160 deletions test/JDBC/expected/ownership_restrictions_from_pg.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,107 +18,6 @@ GO
Server SQLState: 42501)~~


ALTER ROLE master_ownership_restrictions_from_pg_role1 RENAME TO master_ownership_restrictions_from_pg_role2;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


-- Give a bbf role the ability to create other roles, login, nologin and new databases should fail
ALTER ROLE master_ownership_restrictions_from_pg_role1 LOGIN;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


ALTER ROLE master_ownership_restrictions_from_pg_role1 NOLOGIN;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


ALTER ROLE master_ownership_restrictions_from_pg_role1 CREATEDB;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


ALTER ROLE master_ownership_restrictions_from_pg_role1 CREATEROLE CREATEDB;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


-- Change bbf role's password should fail
ALTER ROLE master_ownership_restrictions_from_pg_role1 PASSWORD '123';
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


ALTER ROLE master_ownership_restrictions_from_pg_role1 WITH PASSWORD '123';
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


ALTER ROLE master_ownership_restrictions_from_pg_role1 WITH PASSWORD NULL;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


-- Make a password valid until a particular period for bbf role should fail
ALTER ROLE master_ownership_restrictions_from_pg_role1 VALID UNTIL 'NOV 4 12:00:00 2022 +1';
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


ALTER ROLE master_ownership_restrictions_from_pg_role1 VALID UNTIL 'infinity';
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


-- Give a bbf role a non-default setting of the x parameter should fail
ALTER ROLE master_ownership_restrictions_from_pg_role1 SET x =false;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


-- Give a bbf role a non-default, database-specific setting of the x parameter should fail
ALTER ROLE master_ownership_restrictions_from_pg_role1 IN DATABASE postgres set x=false;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created users/roles cannot be dropped or altered outside of a Babelfish session
Server SQLState: 42501)~~


-- psql
-- Dropping login from psql port should fail
DROP ROLE ownership_restrictions_from_pg_login1;
Expand All @@ -129,22 +28,6 @@ GO
Server SQLState: 55006)~~


ALTER ROLE ownership_restrictions_from_pg_login1 RENAME TO ownership_restrictions_from_pg_login2;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created login cannot be dropped or altered outside of a Babelfish session
Server SQLState: 55006)~~


ALTER ROLE ownership_restrictions_from_pg_login1 CREATEDB;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created login cannot be dropped or altered outside of a Babelfish session
Server SQLState: 55006)~~


-- Create a non babelfish role that is a member of master_guest
-- and enable dropping
CREATE ROLE ownership_restrictions_from_pg_role2 IN ROLE master_guest, tempdb_guest, msdb_guest;
Expand All @@ -158,50 +41,25 @@ GO
Server SQLState: 55006)~~


ALTER ROLE ownership_restrictions_from_pg_role2 RENAME TO ownership_restrictions_from_pg_role3;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created login cannot be dropped or altered outside of a Babelfish session
Server SQLState: 55006)~~


SET enable_drop_babelfish_role = true;
GO

ALTER ROLE ownership_restrictions_from_pg_role2 RENAME TO ownership_restrictions_from_pg_role3;
GO

SET enable_drop_babelfish_role = false;
GO

SET enable_drop_babelfish_role = true;
GO

DROP ROLE ownership_restrictions_from_pg_role3;
DROP ROLE ownership_restrictions_from_pg_role2;
GO

SET enable_drop_babelfish_role = false;
GO


CREATE ROLE ownership_restrictions_from_pg_role4;
GO

GRANT master_guest TO ownership_restrictions_from_pg_role4;
GRANT tempdb_guest TO ownership_restrictions_from_pg_role4;
GRANT msdb_guest TO ownership_restrictions_from_pg_role4;
CREATE ROLE ownership_restrictions_from_pg_role3;
GO

DROP ROLE ownership_restrictions_from_pg_role4;
GRANT master_guest TO ownership_restrictions_from_pg_role3;
GRANT tempdb_guest TO ownership_restrictions_from_pg_role3;
GRANT msdb_guest TO ownership_restrictions_from_pg_role3;
GO
~~ERROR (Code: 0)~~

~~ERROR (Message: ERROR: Babelfish-created login cannot be dropped or altered outside of a Babelfish session
Server SQLState: 55006)~~


ALTER ROLE ownership_restrictions_from_pg_role4 RENAME TO ownership_restrictions_from_pg_role5;
DROP ROLE ownership_restrictions_from_pg_role3;
GO
~~ERROR (Code: 0)~~

Expand All @@ -212,26 +70,17 @@ GO
SET enable_drop_babelfish_role = true;
GO

ALTER ROLE ownership_restrictions_from_pg_role4 RENAME TO ownership_restrictions_from_pg_role5;
GO

SET enable_drop_babelfish_role = false;
GO

SET enable_drop_babelfish_role = true;
GO

DROP ROLE ownership_restrictions_from_pg_role5;
DROP ROLE ownership_restrictions_from_pg_role3;
GO

SET enable_drop_babelfish_role = false;
GO

-- Test a regular role
CREATE ROLE ownership_restrictions_from_pg_role6;
CREATE ROLE ownership_restrictions_from_pg_role4;
GO

DROP ROLE ownership_restrictions_from_pg_role6;
DROP ROLE ownership_restrictions_from_pg_role4;
GO

DROP USER ownership_restrictions_from_pg_test_user;
Expand Down
Loading

0 comments on commit b82cac1

Please sign in to comment.