From: Bossart, Nathan <bossartn@amazon.com>
To: Michael Paquier <michael@paquier.xyz>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Schneider, Jeremy <schnjere@amazon.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Albin, Lloyd P <lalbin@scharp.org>
Subject: Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack
Date: Thu, 26 Jul 2018 15:06:59 +0000
Message-ID: <E5CF1BFB-322D-41AE-ACFC-237730A9E110@amazon.com> (raw)
In-Reply-To: <20180726052415.GC3927@paquier.xyz>
References: <152512087100.19803.12733865831237526317@wrigleys.postgresql.org>
<9145334b-9583-124c-012e-7d58039ad417@amazon.com>
<CA+TgmobgJ9b0LpwhcCdj_gMDgp9UKmR_1pfX6WpWxGGGVrOG7A@mail.gmail.com>
<20180724041403.GF4035@paquier.xyz>
<F4D1FB53-AAFB-4605-9336-F37A762A7738@anarazel.de>
<20180724045010.GA4736@paquier.xyz>
<B8A865D7-DDD0-4E04-9077-02C76B206FF5@anarazel.de>
<20180726052415.GC3927@paquier.xyz>
I took a look at 0001.
On 7/26/18, 12:24 AM, "Michael Paquier" <michael@paquier.xyz> wrote:
> 1) 0001 does the work for TRUNCATE, but using RangeVarGetRelidExtended
> with a custom callback based on the existing truncate_check_rel(). I
> had to split the session-level checks into a separate routine as no
> Relation is available, but that finishes by being a neat result without
> changing any user-facing behavior.
Splitting the checks like this seems reasonable. As you pointed out,
it doesn't change the behavior of the session checks, which AFAICT
aren't necessary for the kind of permissions checks we want to add to
the RangeVarGetRelidExtended() call.
- myrelid = RelationGetRelid(rel);
+ myrelid = RangeVarGetRelidExtended(rv, AccessExclusiveLock,
+ false, RangeVarCallbackForTruncate,
+ NULL);
Should the flags argument be 0 instead of false?
+ /* Nothing to do if the relation was not found. */
+ if (!OidIsValid(relId))
+ return;
+
+ tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relId));
+ if (!HeapTupleIsValid(tuple)) /* should not happen */
+ elog(ERROR, "cache lookup failed for relation %u", relId);
The first time we use this callback, the relation won't be locked, so
isn't it possible that we won't get a valid tuple here? I did notice
that callbacks like RangeVarCallbackForRenameRule,
RangeVarCallbackForPolicy, and RangeVarCallbackForRenameTrigger assume
that the relation can be concurrently dropped, but
RangeVarCallbackOwnsRelation does not. Instead, we assume that the
syscache search will succeed if the given OID is valid. Is this a
bug, or am I missing something?
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, 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: <E5CF1BFB-322D-41AE-ACFC-237730A9E110@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