Re: BUG #15182: Canceling authentication due to timeout aka Denial ofService Attack - Mailing list pgsql-hackers

On 7/27/18, 7:10 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> No problem.  If there are no objections, I am going to fix the REINDEX
> issue first and back-patch.  Its patch is the least invasive of the
> set.

This seems like a reasonable place to start.  I took a closer look at
0003.

+        /*
+         * We allow the user to reindex a table if he is superuser, the table
+         * owner, or the database owner (but in the latter case, only if it's not
+         * a shared relation).
+         */
+        if (!(pg_class_ownercheck(relid, GetUserId()) ||
+              (pg_database_ownercheck(MyDatabaseId, GetUserId()) &&
+               !classtuple->relisshared)))
+            continue;

This is added to ReindexMultipleTables(), which is used for REINDEX
SCHEMA, REINDEX SYSTEM, and REINDEX DATABASE.  Presently, REINDEX
SCHEMA requires being the owner of the schema, and REINDEX SYSTEM and
REINDEX DATABASE require being the owner of the database.  So, if a
role is an owner of a database or the pg_catalog schema, they are able
to reindex shared catalogs like pg_authid.

This patch changes things so that only superusers or the owner of the
table can reindex shared relations.  This works as expected for
REINDEX SYSTEM and REINDEX DATABASE, but I think there is a problem
for REINDEX SCHEMA.  If the user is not the owner of the database but
is the owner of the schema, they will not be able to use REINDEX
SCHEMA to reindex any tables that they do not own.  To fix this, we
will likely need to use pg_namespace_ownercheck() instead of
pg_database_ownercheck() for the REINDEX_OBJECT_SCHEMA case.

I also noticed that this patch causes shared relations to be skipped
silently.  Perhaps we should emit a WARNING or DEBUG message when this
happens, at least for REINDEXOPT_VERBOSE.

    Reindexing a single index or table requires being the owner of that
    index or table.  Reindexing a database requires being the owner of
    the database (note that the owner can therefore rebuild indexes of
-   tables owned by other users).  Of course, superusers can always
+   tables owned by other users).  Reindexing a shared catalog requires
+   being the owner of that shared catalog.  Of course, superusers can always
    reindex anything.

I noticed that there is no mention that the owner of a schema can do
REINDEX SCHEMA, which seems important to note.  Also, the proposed
wording might seem slightly ambiguous for the REINDEX DATABASE case.
It might be clearer to say something like the following:

        Reindexing a single index or table requires being the owner of
        that index of table.  REINDEX DATABASE and REINDEX SYSTEM
        require being the owner of the database, and REINDEX SCHEMA
        requires being the owner of the schema (note that the user can
        therefore rebuild indexes of tables owned by other users).
        Reindexing a shared catalog requires being the owner of the
        shared catalog, even if the user is the owner of the specified
        database or schema.  Of course, superusers can always reindex
        anything.

Nathan