REINDEX and shared catalogs

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: REINDEX and shared catalogs
Date: 2018-08-05 21:10:59
Message-ID: 20180805211059.GA2185@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

This is a continuation of the thread "Canceling authentication due to
timeout aka Denial of Service Attack", which is here to focus on the
case of REINDEX:
https://www.postgresql.org/message-id/20180730003422.GA2878%40paquier.xyz

As visibly the set of patches I proposed on this thread is not
attracting the proper attention, I have preferred beginning a new thread
so as this can get a proper review and agreement.

Per the original thread, it is not difficult to block loading of
critical indexes, authentication being one, with a couple of SQLs:
TRUNCATE, VACUUM and REINDEX as reported.

VACUUM and TRUNCATE will have their own thread later depending on my
time available, and actually those refer to the problem where a relation
lock is attempted to be taken before checking if the user running the
command has the privileges to do so, and if the user has no privilege on
the relation, then the session would wait on a lock but fail later.
However REINDEX is different.

In the case of REINDEX, we *allow* shared catalogs to be reindexed.
Hence, if a user is a database owner, he would also be able to reindex
critical indexes on shared catalogs, where blocking authentication is
possible just with sessions connected to the database reindexed. For a
schema, the situation is basically worse since 9.5 as a schema owner can
do the same with lighter permissions. One can just run "SELECT * FROM
pg_stat_activity" in a transaction block in session 1, run REINDEX in
session 2, and cause the system to refuse new connections. This is
documented as well.

Attached is a set of patches I proposed on the original thread, which
skips shared catalogs if the user running REINDEX is not an owner of
it. This is a behavior change, and as I have a hard time believing that
anybody can take advantage of the current situation, I would like also
to see this stuff back-patched, as anybody doing shared hosting of
Postgres is most likely fixing the hole one way or another. However, I
am sure as well that many folks here would not like that.

This thread is here to gather opinions and to help reaching a consensus,
as I would like to do something at least on HEAD for future releases.

reindex-priv-93.patch is for REL9_3_STABLE, reindex-priv-94.patch for
REL9_4_STABLE and reindex-priv-95-master.patch for 9.5~master.

Thanks for reading!
--
Michael

Attachment Content-Type Size
reindex-priv-93.patch text/x-diff 2.7 KB
reindex-priv-94.patch text/x-diff 2.7 KB
reindex-priv-95-master.patch text/x-diff 3.3 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Gierth 2018-08-05 22:03:37 Re: Should contrib modules install .h files?
Previous Message Andrew Gierth 2018-08-05 20:22:31 Re: Should contrib modules install .h files?