[#25348] YSQL: add flag to block dangerous roles

Summary:
Add a new tserver flag --ysql_block_dangerous_roles to prevent roles
from becoming members of the following system roles:

- pg_read_all_data
- pg_write_all_data
- pg_read_server_files
- pg_write_server_files
- pg_execute_server_program

In an environment that does not allow superusers and uses yb_db_admin
(such as multitenancy or YBM), having the abilities of the above system
roles can potentially escalate privileges.

Since it is not problematic to have the above system roles become
members of other roles, leave that unblocked.  Also, permit REVOKE of
these dangerous roles from user roles.
Jira: DB-14566

Test Plan:
On Almalinux 8:

    ./yb_build.sh fastdebug --gcc11 \
      --gtest_filter PgLibPqTest.BlockDangerousRoles

Close: #25348

Reviewers: smishra, telgersma

Reviewed By: telgersma

Subscribers: ybase, yql

Differential Revision: https://phorge.dev.yugabyte.com/D40693

src/postgres/src/backend/commands/user.c

@@ -1604,6 +1604,16 @@ AddRoleMems(const char *rolename, Oid roleid,
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("must be superuser to set grantor")));
 
+	if (!superuser() && *YBCGetGFlags()->ysql_block_dangerous_roles &&
+		(roleid == ROLE_PG_EXECUTE_SERVER_PROGRAM ||
+		 roleid == ROLE_PG_READ_ALL_DATA ||
+		 roleid == ROLE_PG_READ_SERVER_FILES ||
+		 roleid == ROLE_PG_WRITE_ALL_DATA ||
+		 roleid == ROLE_PG_WRITE_SERVER_FILES))
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("read/write data/files roles are disabled")));
+
 	pg_authmem_rel = table_open(AuthMemRelationId, RowExclusiveLock);
 	pg_authmem_dsc = RelationGetDescr(pg_authmem_rel);
 

src/yb/util/test_macros.h

@@ -346,6 +346,13 @@ inline std::string FindFirstDiff(const std::string& lhs, const std::string& rhs)
   } while (false)
   /**/
 
+#define ASSERT_NOK_PG_ERROR_CODE(expr, pg_error_code) \
+  do { \
+    auto&& _status = (expr); \
+    ASSERT_NOK(_status); \
+    ASSERT_EQ(PgsqlError(_status), pg_error_code); \
+  } while (false)
+
 #define ASSERT_NOK_STR_CONTAINS(expr, expected_failure_substr) \
   do { \
     auto&& _result = (expr); \

src/yb/yql/pggate/ybc_pg_typedefs.h

@@ -408,6 +408,7 @@ typedef struct PgGFlagsAccessor {
   const bool*     TEST_ysql_log_perdb_allocated_new_objectid;
   const bool*     ysql_conn_mgr_version_matching;
   const bool*     ysql_conn_mgr_version_matching_connect_higher_version;
+  const bool*     ysql_block_dangerous_roles;
 } YBCPgGFlagsAccessor;
 
 typedef struct YbTablePropertiesData {

src/yb/yql/pggate/ybc_pggate.cc

@@ -145,6 +145,11 @@ DEFINE_RUNTIME_PREVIEW_bool(
     "If ysql_conn_mgr_version_matching is enabled is enabled, then connect to higher version "
     "server if this flag is set to true");
 
+DEFINE_NON_RUNTIME_bool(ysql_block_dangerous_roles, false,
+    "Block roles that can potentially be used to escalate to superuser privileges. Intended to be "
+    "used with superuser login disabled, such as in YBM. When true, this assumes those blocked "
+    "roles are not already in use.");
+
 DECLARE_bool(TEST_ash_debug_aux);
 DECLARE_bool(TEST_generate_ybrowid_sequentially);
 DECLARE_bool(TEST_ysql_log_perdb_allocated_new_objectid);
@@ -2065,6 +2070,7 @@ const YBCPgGFlagsAccessor* YBCGetGFlags() {
       .ysql_conn_mgr_version_matching = &FLAGS_ysql_conn_mgr_version_matching,
       .ysql_conn_mgr_version_matching_connect_higher_version =
           &FLAGS_ysql_conn_mgr_version_matching_connect_higher_version,
+      .ysql_block_dangerous_roles = &FLAGS_ysql_block_dangerous_roles,
   };
   // clang-format on
   return &accessor;

src/yb/yql/pgwrapper/pg_libpq-test.cc

@@ -4455,5 +4455,59 @@ TEST_F(PgLibPqTest, CollationWithPartitionedTable) {
   conn = ASSERT_RESULT(ConnectToDB("a"));
 }
 
+class PgLibPqBlockDangerousRolesTest : public PgLibPqTest {
+ protected:
+  void UpdateMiniClusterOptions(ExternalMiniClusterOptions* options) override {
+    PgLibPqTest::UpdateMiniClusterOptions(options);
+    options->extra_tserver_flags.push_back("--ysql_block_dangerous_roles=true");
+  }
+};
+
+TEST_F_EX(PgLibPqTest, BlockDangerousRoles, PgLibPqBlockDangerousRolesTest) {
+  PGConn conn_yugabyte = ASSERT_RESULT(Connect());
+  ASSERT_OK(conn_yugabyte.Execute("CREATE USER admin WITH CREATEROLE ROLE yb_db_admin"));
+  PGConn conn_admin = ASSERT_RESULT(ConnectToDBAsUser("" /* db_name */, "admin"));
+  ASSERT_OK(conn_admin.Execute("CREATE ROLE r"));
+
+  for (const auto& granted_role : {"pg_read_all_data", "pg_write_all_data", "pg_read_server_files",
+                                   "pg_write_server_files", "pg_execute_server_program"}) {
+    LOG(INFO) << "granted_role: " << granted_role;
+    ASSERT_NOK_PG_ERROR_CODE(conn_admin.ExecuteFormat("GRANT $0 TO r", granted_role),
+                             YBPgErrorCode::YB_PG_INSUFFICIENT_PRIVILEGE);
+    ASSERT_OK(conn_yugabyte.ExecuteFormat("GRANT $0 to r", granted_role));
+    ASSERT_OK(conn_admin.ExecuteFormat("REVOKE $0 FROM r", granted_role));
+    // Avoid catalog version mismatch by refreshing session.
+    conn_yugabyte.Reset();
+    for (const auto& option : {"IN ROLE", "IN GROUP"}) {
+      LOG(INFO) << "option: " << option;
+      ASSERT_NOK_PG_ERROR_CODE(conn_admin.ExecuteFormat("CREATE ROLE invalid WITH $0 $1",
+                                                        option, granted_role),
+                               YBPgErrorCode::YB_PG_INSUFFICIENT_PRIVILEGE);
+      ASSERT_OK(conn_yugabyte.ExecuteFormat("CREATE ROLE \"invalid$0$1\" WITH $0 $1",
+                                            option, granted_role));
+    }
+    for (const auto& option : {"ROLE", "ADMIN"}) {
+      LOG(INFO) << "option: " << option;
+      // While it is bad if a user role has the privileges of a dangerous role, the opposite is ok.
+      ASSERT_OK(conn_admin.ExecuteFormat("CREATE ROLE valid$0$1 WITH $0 $1", option, granted_role));
+    }
+  }
+
+  for (const auto& granted_role : {"pg_checkpoint", "pg_read_all_stats", "yb_db_admin", "admin"}) {
+    LOG(INFO) << "granted_role: " << granted_role;
+    ASSERT_OK(conn_admin.ExecuteFormat("GRANT $0 TO r", granted_role));
+  }
+
+  // Avoid catalog version mismatch by refreshing session.
+  conn_admin.Reset();
+  ASSERT_OK(conn_admin.Execute("ALTER GROUP r ADD USER pg_read_all_data"));
+  ASSERT_OK(conn_admin.Execute("ALTER GROUP r DROP USER pg_read_all_data"));
+  ASSERT_OK(conn_admin.Execute("ALTER ROLE r WITH USER pg_read_all_data"));
+  ASSERT_NOK_PG_ERROR_CODE(conn_admin.Execute("ALTER GROUP pg_read_all_data ADD USER r"),
+                           YBPgErrorCode::YB_PG_RESERVED_NAME);
+  ASSERT_NOK_PG_ERROR_CODE(conn_admin.Execute("ALTER ROLE pg_read_all_data WITH USER r"),
+                           YBPgErrorCode::YB_PG_RESERVED_NAME);
+}
+
 } // namespace pgwrapper
 } // namespace yb