pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Michael Paquier <michael@paquier.xyz>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Jeremy Schneider <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: Tue, 24 Jul 2018 13:14:03 +0900
Message-ID: <20180724041403.GF4035@paquier.xyz> (raw)
In-Reply-To: <CA+TgmobgJ9b0LpwhcCdj_gMDgp9UKmR_1pfX6WpWxGGGVrOG7A@mail.gmail.com>
References: <152512087100.19803.12733865831237526317@wrigleys.postgresql.org>
	<9145334b-9583-124c-012e-7d58039ad417@amazon.com>
	<CA+TgmobgJ9b0LpwhcCdj_gMDgp9UKmR_1pfX6WpWxGGGVrOG7A@mail.gmail.com>

On Mon, Jul 23, 2018 at 11:29:33AM -0400, Robert Haas wrote:
> ExecuteTruncate needs to be refactored to use RangeVarGetRelidExtended
> with a non-NULL callback rather than heap_openrv, and
> expand_vacuum_rel needs to use RangeVarGetRelidExtended with a
> callback instead of RangeVarGetRelid.  See
> cbe24a6dd8fb224b9585f25b882d5ffdb55a0ba5 as an example of what to do.
> I fixed a large number of cases of this problem back around that time,
> but then ran out of steam and had to move onto other things before I
> got them all.  Patches welcome.

Thanks for pointing those out, I looked at both code paths recently for
some other work...  The amount of work does not consist just in using
for example RangeVarCallbackOwnsRelation for VACUUM and TRUNCATE.  There
are a couple of reasons behind that:
- While it would make sense, at least to me, to make VACUUM fall into if
allow_system_table_mods is allowed, that's not the case of ANALYZE as I
think that we should be able to call ANALYZE on a system catalog as
well.  So we would basically a new flavor of
RangeVarCallbackOwnsRelation for VACUUM which makes this difference
between vacuum and analyze with an argument in the callback, the options
of VacuumStmt would be nice.  This would not be used by autovacuum
anyway, but adding an assertion and mentioning that in the comments
would not hurt.  There is an argument for just restricting VACUUM FULL
as well and not plain VACUUM, as that's the one hurting here.
- TRUNCATE is closer to a solution, as it has its own flavor of relation
checks with truncate_check_rel.  So the callback would replace
truncate_check_rel but CheckTableNotInUse should be moved out of it.
TRUNCATE already uses allow_system_table_mods for its checks.

Thoughts?
--
Michael

Attachments:

  [application/pgp-signature] signature.asc (832B, ../20180724041403.GF4035@paquier.xyz/2-signature.asc)
  download

view thread (36+ messages)  latest in thread

Message-ID: <20180724041403.GF4035@paquier.xyz>
Permalink:  ../20180724041403.GF4035@paquier.xyz/
Also on:    postgresql.org/message-id/20180724041403.GF4035@paquier.xyz

 · 

reply

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, 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: <20180724041403.GF4035@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