Re: BUG #15182: Canceling authentication due to timeout aka Denialof Service Attack

On Tue, Jul 31, 2018 at 10:34:57AM -0400, Robert Haas wrote:
> Just as a general statement, I don't think we should, as part of a
> patch for the issue discussed on this thread, make any changes AT ALL
> to who has permission to perform which operations.  We *certainly*
> should not back-patch such changes, but we really also should not make
> them on master unless they are discussed on a separate thread with a
> clear subject line and agreed by a clear consensus.

Well, perhaps spawning one thread for each command would make the most
sense then?  I am feeling a bit of confusion here.  There have been
three cases discussed up to now:
1) TRUNCATE, which results in a patch that has no behavioral changes.
2) VACUUM [FULL], where we are also heading toward a patch that has no
behavioral change per the last arguments exchanged, where we make sure
that permission checks are done before acquiring any locks.
3) REINDEX, where a database or a schema owner is able to take an
exclusive lock on a shared catalog with limited permissions.  That
sucks as this block calls to load_critical_index, but I would be ready
to buy to not back-patch such a change if the consensus reached is to
not skip shared catalogs if a database/schema owner has no ownership on
the shared catalog directly.

> This patch should only be about detecting cases where we lack
> permission earlier, before we try to acquire a lock.

Yeah, the REINDEX case is the only one among the three where the set of
relations worked on changes.  Please note that ownership checks happen
in this case for indexes, tables, database and schemas before taking a
lock on them.  Databases/systems and schemas just check for ownership of
respectively the database and the schema.

I'll be happy to create one thread per patch if that helps.  Thinking
about it this would attract more attention to each individual problem
reported.
--
Michael