pá 11. 12. 2020 v 16:07 odesílatel Konstantin Knizhnik < k.knizh...@postgrespro.ru> napsal:
> > > On 09.12.2020 15:24, Pavel Stehule wrote: > > > > st 9. 12. 2020 v 13:17 odesílatel Greg Nancarrow <gregn4...@gmail.com> > napsal: > >> On Tue, Dec 8, 2020 at 3:26 PM Pavel Stehule <pavel.steh...@gmail.com> >> wrote: >> > >> > >> > There are two maybe generic questions? >> > >> > 1. Maybe we can introduce more generic GUC for all event triggers like >> disable_event_triggers? This GUC can be checked only by the database owner >> or super user. It can be an alternative ALTER TABLE DISABLE TRIGGER ALL. It >> can be protection against necessity to restart to single mode to repair the >> event trigger. I think so more generic solution is better than special >> disable_client_connection_trigger GUC. >> > >> > 2. I have no objection against client_connection. It is probably better >> for the mentioned purpose - possibility to block connection to database. >> Can be interesting, and I am not sure how much work it is to introduce the >> second event - session_start. This event should be started after connecting >> - so the exception there doesn't block connect, and should be started also >> after the new statement "DISCARD SESSION", that will be started >> automatically after DISCARD ALL. This feature should not be implemented in >> first step, but it can be a plan for support pooled connections >> > >> > > PGC_SU_BACKEND is too strong, there should be PGC_BACKEND if this option > can be used by database owner > > Pavel > > >> I've created a separate patch to address question (1), rather than >> include it in the main patch, which I've adjusted accordingly. I'll >> leave question (2) until another time, as you suggest. >> See the attached patches. >> >> Regards, >> Greg Nancarrow >> Fujitsu Australia >> > > It seems to me that current implementation of EventTriggersDisabled: > > > /* > * Indicates whether all event triggers are currently disabled > * for the current user. > * Event triggers are disabled when configuration parameter > * "disable_event_triggers" is true, and the current user > * is the database owner or has superuser privileges. > */ > static inline bool > EventTriggersDisabled(void) > { > return (disable_event_triggers && pg_database_ownercheck(MyDatabaseId, > GetUserId())); > } > > > is not correct. It makes it not possible to superuser to disable triggers > for all users. > pg_database_ownercheck returns true for superuser always. Also GUCs are not associated with any database. So I do not understand why > this check of database ownership is relevant at all? > > What kind of protection violation we want to prevent? > > It seems to be obvious that normal user should not be able to prevent > trigger execution because this triggers may be used to enforce some > security policies. > If trigger was created by user itself, then it can drop or disable it > using ALTER statement. GUC is not needed for it. > when you cannot connect to the database, then you cannot do ALTER. In DBaaS environments lot of users has not superuser rights. > So I think that EventTriggersDisabled function is not needed and can be > replaced just with disable_event_triggers GUC check. > And this option should be defined with PGC_SU_BACKEND > > > > > -- > Konstantin Knizhnik > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company > >