From: Michael Paquier <michael@paquier.xyz>
To: Bossart, Nathan <bossartn@amazon.com>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: andres@anarazel.de <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: robertmhaas@gmail.com <robertmhaas@gmail.com>
Cc: Schneider, Jeremy <schnjere@amazon.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: lalbin@scharp.org <lalbin@scharp.org>
Subject: Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack
Date: Sat, 11 Aug 2018 11:50:03 +0200
Message-ID: <20180811095003.GA2900@paquier.xyz> (raw)
In-Reply-To: <62728D88-85A0-4A9A-A4F8-67D96C1179E0@amazon.com>
References: <20180726204127.GA1754@paquier.xyz>
<3D4F2733-92AE-4B74-8912-3C611B027721@amazon.com>
<20180727023123.GE1754@paquier.xyz>
<20180727.123702.77822416.horiguchi.kyotaro@lab.ntt.co.jp>
<20180727041542.GH1754@paquier.xyz>
<62728D88-85A0-4A9A-A4F8-67D96C1179E0@amazon.com>
Hi Nathan,
On Fri, Jul 27, 2018 at 02:40:42PM +0000, Bossart, Nathan wrote:
> I think I'm essentially suggesting what you have in 0002 but without
> the new RangeVarGetRelidExtended() callback. I've attached a modified
> version of 0002 that seems to fix the originally reported issue. (I
> haven't looked into any extra handling needed for ANALYZE or
> partitioned tables.) Running the same checks for all VACUUMs would
> keep things simple and provide a more uniform user experience.
I have been looking at this patch for VACUUM (perhaps we ought to spawn
a new thread as the cases of REINDEX and TRUNCATE have been addressed),
and I can see a problem with this approach. If multiple transactions
are used with a list of relations vacuum'ed then simply moving around
the ACL checks would cause a new problem: if the relation vacuum'ed
disappears when processing it with vacuum_rel() after the list of
relations is built then we would get an ERROR instead of a skip because
of pg_class_ownercheck() which is not fail-safe.
Wouldn't it be enough to check just the ACLs in expand_vacuum_rel() and
get_all_vacuum_rels()? The first is rather similar to what happens for
TRUNCATE, and the second is similar to what has been fixed for VACUUM.
We should also remove the relkind checks out of vacuum_skip_rel() as
those checks are not completely consistent for all those code paths...
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../20180811095003.GA2900@paquier.xyz/2-signature.asc)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: michael@paquier.xyz, bossartn@amazon.com, horiguchi.kyotaro@lab.ntt.co.jp, andres@anarazel.de, pgsql-hackers@lists.postgresql.org, robertmhaas@gmail.com, schnjere@amazon.com, pgsql-hackers@postgresql.org, lalbin@scharp.org
Subject: Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack
In-Reply-To: <20180811095003.GA2900@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox