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

On Thu, Jul 26, 2018 at 11:14:31PM +0000, Bossart, Nathan wrote:
> On 7/26/18, 3:42 PM, "Michael Paquier" <michael@paquier.xyz> wrote:
> Minus a possible documentation update, 0003 seems almost ready, too.

The docs mentioned that shared catalogs are processed, so I did not
bother, but visibly your comment is that we could be more precise about
the ownership in this case?  An attempt is attached.

> For 0002, would adding vacuum_skip_rel() before and after
> try_relation_open() in vacuum_rel() be enough?  That way, we could
> avoid emitting an ERROR for only the VACUUM FULL case (and skip it
> with a WARNING like everything else).

Er, we need a lock on it while looking at its data in vacuum_rel(), no?
So the call to vacuum_skip_rel() needs to happen after
try_relation_open(), once we are sure that the relation is opened.
Having two set of checks is actually better as the operation can involve
multiple operations.

The error messages generated by vacuum_skip_rel are not especially smart
when elevel >= ERROR.  As we need a proper errcode for that case as well
just using a separate error message is less confusing.  I have
implemented my idea in the updated set attached.  Another issue I have
found is that when doing for example a system-wide analyze, we would
finish with spurious warnings, as toast relations need to be discarded
from the first set of relations built.

Anyway, I have done more work on the patches, mainly I have fixed the
calls to RangeVarGetRelidExtended using booleans.  I have added
isolation tests for cases which are cheap, aka those not involving a
system-wide operation.  Running those tests on HEAD, it is easy to see
that TRUNCATE or VACUUM complete after a session doing a catalog lookup
commits its transaction.  VACUUM skips a relation and VACUUM FULL issues
an error.

Regarding those patches, I am pretty happy how things turn out for
TRUNCATE and REINDEX, way less for VACUUM, so getting 0001 and 0003
committed first makes the most sense to me as their logic is rather
straight-forward (well way of speaking ;p ).
--
Michael