Horton

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

Nathan Bossart <bossartn@amazon.com>

View thread
From: "Bossart, Nathan" <bossartn@amazon.com>
To: Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Robert Haas <robertmhaas@gmail.com>, "Schneider, Jeremy" <schnjere@amazon.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>, "Albin, Lloyd P" <lalbin@scharp.org>
Date: 2018-07-26T15:06:59Z
Lists: pgsql-bugs, pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Improve VACUUM and ANALYZE by avoiding early lock queue

  2. Improve TRUNCATE by avoiding early lock queue

  3. Restrict access to reindex of shared catalogs for non-privileged users

  4. Improve behavior of concurrent CLUSTER.

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