From: Bossart, Nathan <bossartn@amazon.com>
To: Michael Paquier <michael@paquier.xyz>
To: 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: Mon, 30 Jul 2018 15:42:55 +0000
Message-ID: <39D13161-4996-41B0-B097-BE87069469A8@amazon.com> (raw)
In-Reply-To: <20180730102131.GC2878@paquier.xyz>
References: <20180728001012.GC1882@paquier.xyz>
<6D3CBDB6-C358-49F0-8D00-32E58AD2D3D3@amazon.com>
<20180730003422.GA2878@paquier.xyz>
<20180730.175354.32712514.horiguchi.kyotaro@lab.ntt.co.jp>
<20180730102131.GC2878@paquier.xyz>
On 7/29/18, 7:35 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> Yeah, I was testing that yesterday night and bumped on this case when
> trying do a REINDEX SCHEMA pg_class. The point is that you can simplify
> the check and remove pg_database_ownercheck as there is already an ACL
> check on the database/system/schema at the top of the routine, so you
> are already sure that pg_database_ownercheck() or
> pg_namespace_ownercheck would return true. This shaves a few cycles as
> well for each relation checked.
Makes sense.
>> 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.
>
> That's intentional. I thought about that as well, but I am hesitant to
> do so as we don't bother mentioning the other relations skipped.
> REINDEX VERBOSE also shows up what are the tables processed, so it is
> easy to guess what are the tables skipped, still more painful. And the
> documentation changes added cover the gap.
Okay, that seems reasonable to me, too.
> +1, I have included your suggestions. The patch attached applies easily
> down to 9.5 where REINDEX SCHEMA was added. For 9.4 and 9.3, there is
> no schema case, still the new check is similar. The commit message is
> slightly changed so as there is no mention of REINDEX SCHEMA. 9.3 needs
> a slight change compared to 9.4 as well.
For 9.3 and 9.4, it might be nice to add the "even if the user is the
owner of the specified database" part, too.
> So attached are patches for 9.5~master, 9.4 and 9.3 with commit
> messages. Does that look fine to folks of this thread?
Looks good to me. Since REINDEX can be used to block calls to
load_critical_index() from new connections, back-patching seems appropriate.
Nathan
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: bossartn@amazon.com, michael@paquier.xyz, 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: <39D13161-4996-41B0-B097-BE87069469A8@amazon.com>
* 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