Re: BUG #15182: Canceling authentication due to timeout aka Denial ofService Attack
От | Bossart, Nathan |
---|---|
Тема | Re: BUG #15182: Canceling authentication due to timeout aka Denial ofService Attack |
Дата | |
Msg-id | 62728D88-85A0-4A9A-A4F8-67D96C1179E0@amazon.com обсуждение исходный текст |
Ответ на | Re: BUG #15182: Canceling authentication due to timeout aka Denialof Service Attack (Michael Paquier <michael@paquier.xyz>) |
Ответы |
Re: BUG #15182: Canceling authentication due to timeout aka Denialof Service Attack
(Michael Paquier <michael@paquier.xyz>)
Re: BUG #15182: Canceling authentication due to timeout aka Denialof Service Attack (Michael Paquier <michael@paquier.xyz>) |
Список | pgsql-hackers |
On 7/26/18, 11:16 PM, "Michael Paquier" <michael@paquier.xyz> wrote: > On Fri, Jul 27, 2018 at 12:37:02PM +0900, Kyotaro HORIGUCHI wrote: >> I intended the same as Bossart said. It is not reasonable that >> VACUUM and VACUUM FULL behaves differently on rejection for the >> same reason. The objective of the first (or early) check is >> rejecting (non-superuser's) unprivileged VACUUM FULL. Any >> possible modifications on a syscache tuple before taking a lock >> doesn't seem to harm in the point of view. Anyway the lock >> acquired in expand_vacuum_rel is not held until vacuum_rel. > > Thinking about it harder, it is possible to pass down a status pointer > that the callback of RangeVarGetRelidExtended could fill if we pass it > down using the argument list. This would need special handling for the > case where expand_vacuum_rel() is NIL but that would be workable. If > vacuum_check_rel() does not need to work with elevel >= ERROR, then the > set of log messages remains the same. Perhaps I am misunderstanding the goal of the patch. The original issue reported above is for a lock pile-up that blocks connection authentication. Specifically, VACUUM FULL blocks waiting for an AccessExclusiveLock on pg_authid in vacuum_rel(), even if the user does not have the right permissions. IIUC 0002 is adding a callback to the OID lookup logic in expand_vacuum_rel() that aims to prevent users from taking an AccessShareLock when they don't have permissions to VACUUM the relation. However, ISTM that this AccessShareLock is not the issue. If we allowed all users to get the AccessShareLock but we ensured we called vacuum_skip_rel() prior to waiting for the AccessExclusiveLock, I think we are good. Of course, there's a chance that a concurrent change would cause the role to lose permissions in between expand_vacuum_rel() and vacuum_rel(), but that seems like a risk we are taking regardless. I think I'm essentially suggesting what you have in 0002 but without the new RangeVarGetRelidExtended() callback. I've attached a modified version of 0002 that seems to fix the originally reported issue. (I haven't looked into any extra handling needed for ANALYZE or partitioned tables.) Running the same checks for all VACUUMs would keep things simple and provide a more uniform user experience. > 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. Sorry, I should have been clearer. But yes, your update is what I was thinking. Nathan
Вложения
В списке pgsql-hackers по дате отправления: