Revert MAINTAIN privilege and pg_maintain predefined role.

This reverts the following commits: 4dbdb82, c2122aa, 5b1a879, 9e1e9d6, ff9618e, 60684dd, 4441fc7, and b5d6382. A role with the MAINTAIN privilege may be able to use search_path tricks to escalate privileges to the table owner. Unfortunately, it is too late in the v16 development cycle to apply the proposed fix, i.e., restricting search_path when running maintenance commands. Bumps catversion. Reviewed-by: Jeff Davis Discussion: https://postgr.es/m/E1q7j7Y-000z1H-Hr%40gemulon.postgresql.org Backpatch-through: 16
postgres · Jul 7, 2023 · 9574459 · 9574459
1 parent 1124cb2
commit 9574459
Show file tree

Hide file tree

Showing 41 changed files with 179 additions and 445 deletions.
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
@@ -1718,8 +1718,8 @@ ALTER TABLE products RENAME TO items;
    <literal>INSERT</literal>, <literal>UPDATE</literal>, <literal>DELETE</literal>,
    <literal>TRUNCATE</literal>, <literal>REFERENCES</literal>, <literal>TRIGGER</literal>,
    <literal>CREATE</literal>, <literal>CONNECT</literal>, <literal>TEMPORARY</literal>,
-   <literal>EXECUTE</literal>, <literal>USAGE</literal>, <literal>SET</literal>,
-   <literal>ALTER SYSTEM</literal>, and <literal>MAINTAIN</literal>.
+   <literal>EXECUTE</literal>, <literal>USAGE</literal>, <literal>SET</literal>
+   and <literal>ALTER SYSTEM</literal>.
    The privileges applicable to a particular
    object vary depending on the object's type (table, function, etc.).
    More detail about the meanings of these privileges appears below.
@@ -2010,19 +2010,7 @@ REVOKE ALL ON accounts FROM PUBLIC;
       </para>
      </listitem>
     </varlistentry>
-
-   <varlistentry id="ddl-priv-maintain">
-    <term><literal>MAINTAIN</literal></term>
-    <listitem>
-     <para>
-      Allows <command>VACUUM</command>, <command>ANALYZE</command>,
-      <command>CLUSTER</command>, <command>REFRESH MATERIALIZED VIEW</command>,
-      <command>REINDEX</command>, and <command>LOCK TABLE</command> on a
-      relation.
-     </para>
-    </listitem>
-   </varlistentry>
-  </variablelist>
+   </variablelist>
 
    The privileges required by other commands are listed on the
    reference page of the respective command.
@@ -2171,11 +2159,6 @@ REVOKE ALL ON accounts FROM PUBLIC;
       <entry><literal>A</literal></entry>
       <entry><literal>PARAMETER</literal></entry>
      </row>
-     <row>
-      <entry><literal>MAINTAIN</literal></entry>
-      <entry><literal>m</literal></entry>
-      <entry><literal>TABLE</literal></entry>
-     </row>
      </tbody>
    </tgroup>
   </table>
@@ -2266,7 +2249,7 @@ REVOKE ALL ON accounts FROM PUBLIC;
      </row>
      <row>
       <entry><literal>TABLE</literal> (and table-like objects)</entry>
-      <entry><literal>arwdDxtm</literal></entry>
+      <entry><literal>arwdDxt</literal></entry>
       <entry>none</entry>
       <entry><literal>\dp</literal></entry>
      </row>
@@ -2325,11 +2308,11 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO miriam_rw;
 <programlisting>
 =&gt; \dp mytable
                                   Access privileges
- Schema |  Name   | Type  |   Access privileges    |   Column privileges   | Policies
---------+---------+-------+------------------------+-----------------------+----------
- public | mytable | table | miriam=arwdDxtm/miriam+| col1:                +|
-        |         |       | =r/miriam             +|   miriam_rw=rw/miriam |
-        |         |       | admin=arw/miriam       |                       |
+ Schema |  Name   | Type  |   Access privileges   |   Column privileges   | Policies
+--------+---------+-------+-----------------------+-----------------------+----------
+ public | mytable | table | miriam=arwdDxt/miriam+| col1:                +|
+        |         |       | =r/miriam            +|   miriam_rw=rw/miriam |
+        |         |       | admin=arw/miriam      |                       |
 (1 row)
 </programlisting>
   </para>

diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml
@@ -23545,7 +23545,7 @@ SELECT has_function_privilege('joeuser', 'myfunc(int, text)', 'execute');
         are <literal>SELECT</literal>, <literal>INSERT</literal>,
         <literal>UPDATE</literal>, <literal>DELETE</literal>,
         <literal>TRUNCATE</literal>, <literal>REFERENCES</literal>,
-        <literal>TRIGGER</literal>, and <literal>MAINTAIN</literal>.
+        and <literal>TRIGGER</literal>.
        </para></entry>
       </row>
 

diff --git a/doc/src/sgml/ref/alter_default_privileges.sgml b/doc/src/sgml/ref/alter_default_privileges.sgml
@@ -28,7 +28,7 @@ ALTER DEFAULT PRIVILEGES
 
 <phrase>where <replaceable class="parameter">abbreviated_grant_or_revoke</replaceable> is one of:</phrase>
 
-GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | MAINTAIN }
+GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
     [, ...] | ALL [ PRIVILEGES ] }
     ON TABLES
     TO { [ GROUP ] <replaceable class="parameter">role_name</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ]
@@ -51,7 +51,7 @@ GRANT { USAGE | CREATE | ALL [ PRIVILEGES ] }
     TO { [ GROUP ] <replaceable class="parameter">role_name</replaceable> | PUBLIC } [, ...] [ WITH GRANT OPTION ]
 
 REVOKE [ GRANT OPTION FOR ]
-    { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | MAINTAIN }
+    { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
     [, ...] | ALL [ PRIVILEGES ] }
     ON TABLES
     FROM { [ GROUP ] <replaceable class="parameter">role_name</replaceable> | PUBLIC } [, ...]

diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
@@ -182,9 +182,11 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
   <title>Notes</title>
 
   <para>
-   To analyze a table, one must ordinarily have the <literal>MAINTAIN</literal>
-   privilege on the table.  However, database owners are allowed to
+   To analyze a table, one must ordinarily be the table's owner or a
+   superuser.  However, database owners are allowed to
    analyze all tables in their databases, except shared catalogs.
+   (The restriction for shared catalogs means that a true database-wide
+   <command>ANALYZE</command> can only be performed by a superuser.)
    <command>ANALYZE</command> will skip over any tables that the calling user
    does not have permission to analyze.
   </para>

diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml
@@ -70,8 +70,9 @@ CLUSTER [VERBOSE]
    <command>CLUSTER</command> without a
    <replaceable class="parameter">table_name</replaceable> reclusters all the
    previously-clustered tables in the current database that the calling user
-   has privileges for.  This form of <command>CLUSTER</command> cannot be
-   executed inside a transaction block.
+   owns, or all such tables if called by a superuser.  This
+   form of <command>CLUSTER</command> cannot be executed inside a transaction
+   block.
   </para>
 
   <para>
@@ -132,11 +133,6 @@ CLUSTER [VERBOSE]
  <refsect1>
   <title>Notes</title>
 
-   <para>
-    To cluster a table, one must have the <literal>MAINTAIN</literal> privilege
-    on the table.
-   </para>
-
    <para>
     In cases where you are accessing single rows randomly
     within a table, the actual order of the data in the

diff --git a/doc/src/sgml/ref/grant.sgml b/doc/src/sgml/ref/grant.sgml
@@ -21,7 +21,7 @@ PostgreSQL documentation
 
  <refsynopsisdiv>
 <synopsis>
-GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | MAINTAIN }
+GRANT { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
     [, ...] | ALL [ PRIVILEGES ] }
     ON { [ TABLE ] <replaceable class="parameter">table_name</replaceable> [, ...]
          | ALL TABLES IN SCHEMA <replaceable class="parameter">schema_name</replaceable> [, ...] }
@@ -193,7 +193,6 @@ GRANT <replaceable class="parameter">role_name</replaceable> [, ...] TO <replace
      <term><literal>USAGE</literal></term>
      <term><literal>SET</literal></term>
      <term><literal>ALTER SYSTEM</literal></term>
-     <term><literal>MAINTAIN</literal></term>
      <listitem>
       <para>
        Specific types of privileges, as defined in <xref linkend="ddl-priv"/>.

diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml
@@ -166,8 +166,8 @@ LOCK [ TABLE ] [ ONLY ] <replaceable class="parameter">name</replaceable> [ * ]
 
    <para>
     To lock a table, the user must have the right privilege for the specified
-    <replaceable class="parameter">lockmode</replaceable>.
-    If the user has <literal>MAINTAIN</literal>,
+    <replaceable class="parameter">lockmode</replaceable>, or be the table's
+    owner or a superuser.  If the user has
     <literal>UPDATE</literal>, <literal>DELETE</literal>, or
     <literal>TRUNCATE</literal> privileges on the table, any <replaceable
     class="parameter">lockmode</replaceable> is permitted. If the user has

diff --git a/doc/src/sgml/ref/refresh_materialized_view.sgml b/doc/src/sgml/ref/refresh_materialized_view.sgml
@@ -31,9 +31,8 @@ REFRESH MATERIALIZED VIEW [ CONCURRENTLY ] <replaceable class="parameter">name</
 
   <para>
    <command>REFRESH MATERIALIZED VIEW</command> completely replaces the
-   contents of a materialized view.  To execute this command you must have the
-   <literal>MAINTAIN</literal>
-   privilege on the materialized view.  The old contents are discarded.  If
+   contents of a materialized view.  To execute this command you must be the
+   owner of the materialized view.  The old contents are discarded.  If
    <literal>WITH DATA</literal> is specified (or defaults) the backing query
    is executed to provide the new data, and the materialized view is left in a
    scannable state.  If <literal>WITH NO DATA</literal> is specified no new

diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
@@ -292,21 +292,16 @@ REINDEX [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] { DA
   </para>
 
   <para>
-   Reindexing a single index or table requires
-   having the <literal>MAINTAIN</literal> privilege on the
-   table.  Note that while <command>REINDEX</command> on a partitioned index or
-   table requires having the <literal>MAINTAIN</literal> privilege on the
-   partitioned table, such commands skip the privilege checks when processing
-   the individual partitions.  Reindexing a schema or database requires being the
-   owner of that schema or database or having privileges of the
-   <link linkend="predefined-roles-table"><literal>pg_maintain</literal></link>
-   role.  Note specifically that it's thus
+   Reindexing a single index or table requires being the owner of that
+   index or table.  Reindexing a schema or database requires being the
+   owner of that schema or database.  Note specifically that it's thus
    possible for non-superusers to rebuild indexes of tables owned by
-   other users.  However, as a special exception,
-   <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>,
-   and <command>REINDEX SYSTEM</command> will skip indexes on shared catalogs
-   unless the user has the <literal>MAINTAIN</literal> privilege on the
-   catalog.
+   other users.  However, as a special exception, when
+   <command>REINDEX DATABASE</command>, <command>REINDEX SCHEMA</command>
+   or <command>REINDEX SYSTEM</command> is issued by a non-superuser,
+   indexes on shared catalogs will be skipped unless the user owns the
+   catalog (which typically won't be the case).  Of course, superusers
+   can always reindex anything.
   </para>
 
   <para>

diff --git a/doc/src/sgml/ref/revoke.sgml b/doc/src/sgml/ref/revoke.sgml
@@ -22,7 +22,7 @@ PostgreSQL documentation
  <refsynopsisdiv>
 <synopsis>
 REVOKE [ GRANT OPTION FOR ]
-    { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER | MAINTAIN }
+    { { SELECT | INSERT | UPDATE | DELETE | TRUNCATE | REFERENCES | TRIGGER }
     [, ...] | ALL [ PRIVILEGES ] }
     ON { [ TABLE ] <replaceable class="parameter">table_name</replaceable> [, ...]
          | ALL TABLES IN SCHEMA <replaceable>schema_name</replaceable> [, ...] }

diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
@@ -444,9 +444,11 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
   <title>Notes</title>
 
    <para>
-    To vacuum a table, one must ordinarily have the <literal>MAINTAIN</literal>
-    privilege on the table.  However, database owners are allowed to
+    To vacuum a table, one must ordinarily be the table's owner or a
+    superuser.  However, database owners are allowed to
     vacuum all tables in their databases, except shared catalogs.
+    (The restriction for shared catalogs means that a true database-wide
+    <command>VACUUM</command> can only be performed by a superuser.)
     <command>VACUUM</command> will skip over any tables that the calling user
     does not have permission to vacuum.
    </para>

diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml
@@ -683,18 +683,6 @@ DROP ROLE doomed_role;
        the <link linkend="sql-checkpoint"><command>CHECKPOINT</command></link>
        command.</entry>
       </row>
-      <row>
-       <entry>pg_maintain</entry>
-       <entry>Allow executing
-       <link linkend="sql-vacuum"><command>VACUUM</command></link>,
-       <link linkend="sql-analyze"><command>ANALYZE</command></link>,
-       <link linkend="sql-cluster"><command>CLUSTER</command></link>,
-       <link linkend="sql-refreshmaterializedview"><command>REFRESH MATERIALIZED VIEW</command></link>,
-       <link linkend="sql-reindex"><command>REINDEX</command></link>,
-       and <link linkend="sql-lock"><command>LOCK TABLE</command></link> on all
-       relations, as if having <literal>MAINTAIN</literal> rights on those
-       objects, even without having it explicitly.</entry>
-      </row>
       <row>
        <entry>pg_use_reserved_connections</entry>
        <entry>Allow use of connection slots reserved via

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
@@ -2612,8 +2612,6 @@ string_to_privilege(const char *privname)
 		return ACL_SET;
 	if (strcmp(privname, "alter system") == 0)
 		return ACL_ALTER_SYSTEM;
-	if (strcmp(privname, "maintain") == 0)
-		return ACL_MAINTAIN;
 	if (strcmp(privname, "rule") == 0)
 		return 0;				/* ignore old RULE privileges */
 	ereport(ERROR,
@@ -2655,8 +2653,6 @@ privilege_to_string(AclMode privilege)
 			return "SET";
 		case ACL_ALTER_SYSTEM:
 			return "ALTER SYSTEM";
-		case ACL_MAINTAIN:
-			return "MAINTAIN";
 		default:
 			elog(ERROR, "unrecognized privilege: %d", (int) privilege);
 	}
@@ -3388,17 +3384,6 @@ pg_class_aclmask_ext(Oid table_oid, Oid roleid, AclMode mask,
 		has_privs_of_role(roleid, ROLE_PG_WRITE_ALL_DATA))
 		result |= (mask & (ACL_INSERT | ACL_UPDATE | ACL_DELETE));
 
-	/*
-	 * Check if ACL_MAINTAIN is being checked and, if so, and not already set
-	 * as part of the result, then check if the user is a member of the
-	 * pg_maintain role, which allows VACUUM, ANALYZE, CLUSTER, REFRESH
-	 * MATERIALIZED VIEW, and REINDEX on all relations.
-	 */
-	if (mask & ACL_MAINTAIN &&
-		!(result & ACL_MAINTAIN) &&
-		has_privs_of_role(roleid, ROLE_PG_MAINTAIN))
-		result |= ACL_MAINTAIN;
-
 	return result;
 }
 

diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
@@ -159,15 +159,16 @@ analyze_rel(Oid relid, RangeVar *relation,
 		return;
 
 	/*
-	 * Check if relation needs to be skipped based on privileges.  This check
+	 * Check if relation needs to be skipped based on ownership.  This check
 	 * happens also when building the relation list to analyze for a manual
 	 * operation, and needs to be done additionally here as ANALYZE could
-	 * happen across multiple transactions where privileges could have changed
-	 * in-between.  Make sure to generate only logs for ANALYZE in this case.
+	 * happen across multiple transactions where relation ownership could have
+	 * changed in-between.  Make sure to generate only logs for ANALYZE in
+	 * this case.
 	 */
-	if (!vacuum_is_permitted_for_relation(RelationGetRelid(onerel),
-										  onerel->rd_rel,
-										  params->options & ~VACOPT_VACUUM))
+	if (!vacuum_is_relation_owner(RelationGetRelid(onerel),
+								  onerel->rd_rel,
+								  params->options & VACOPT_ANALYZE))
 	{
 		relation_close(onerel, ShareUpdateExclusiveLock);
 		return;