út 15. 12. 2020 v 15:06 odesílatel Konstantin Knizhnik < k.knizh...@postgrespro.ru> napsal:
> > > On 15.12.2020 16:18, Pavel Stehule wrote: > > > > út 15. 12. 2020 v 14:12 odesílatel Konstantin Knizhnik < > k.knizh...@postgrespro.ru> napsal: > >> >> >> On 11.12.2020 19:27, Pavel Stehule wrote: >> >> >> >> pá 11. 12. 2020 v 17:05 odesílatel Konstantin Knizhnik < >> k.knizh...@postgrespro.ru> napsal: >> >>> >>> >>> On 11.12.2020 18:40, Pavel Stehule wrote: >>> >>> >>> is not correct. It makes it not possible to superuser to disable >>>> triggers for all users. >>>> >>> >>> pg_database_ownercheck returns true for superuser always. >>> >>> >>> Sorry, but I consider different case: when normal user is connected to >>> the database. >>> In this case pg_database_ownercheck returns false and trigger is not >>> disabled, isn't it? >>> >> >> My idea was to reduce necessary rights to database owners. But you have >> a true, so only superuser can create event trigger, so this feature cannot >> be used in DBaaS environments, and then my original idea was wrong. >> >> >>> >>> 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. >>> >>> >>> >>> But only superusers can set login triggers, right? >>> So only superuser can make a mistake in this trigger. But he have enough >>> rights to recover this error. Normal users are not able to define on >>> connection triggers and >>> should not have rights to disable them. >>> >> >> yes, it is true >> >> Pavel >> >> >>> -- >>> Konstantin Knizhnik >>> Postgres Professional: http://www.postgrespro.com >>> The Russian Postgres Company >>> >>> >> So what's next? >> I see three options: >> >> 1. Do not introduce GUC for disabling all event triggers (especially >> taken in account that them are disabled by default). >> Return back to the patch >> on_connect_event_trigger_WITH_SUGGESTED_UPDATES.patch with >> "disable_client_connection_trigger" >> and make it true by default (to eliminate any overhead for users which >> are not using on logintriggers). >> >> 2. Have two GUCS: "disable_client_connection_trigger" and >> "disable_event_triggers". >> >> 3. Implement some mechanism for caching presence of event triggers in >> shared memory. >> > > @3 is the best design (the things should work well by default), @2 is a > little bit chaotic and @1 looks like a workaround. > > > Please notice that we still need GUC to disable on-login triggers: to make > it possible for superuser who did mistake and defined incorrect on-login > trigger to > login to the system. > Do we need GUC to disable all other event triggers? May be I am wrong, but > I do not see much need in such GUC: error in any of such event triggers is > non fatal > and can be easily reverted. > So the only question is whether "disable_client_connection_trigger" should > be true by default or not... > > I agree with you that @2 is a little bit chaotic and @1 looks like a > workaround. > But from my point of view @3 is not the best solution but overkill: > maintaining yet another shared hash just to save few milliseconds on login > seems to be too high price. > Actually there are many things which are loaded by new backend from the > database on start: for example - catalog. > This is why launch of new backend is an expensive operation. > Certainly if we execute "select 1", then system catalog is not needed... > But does anybody start new backend just to execute "select 1" and exit? > > > I understand so the implementation of a new shared cache can be a lot of work. The best way is enhancing pg_database about one column with information about the login triggers (dathaslogontriggers). In init time these data are in syscache, and can be easily checked. Some like pg_attribute have an atthasdef column. Same fields has pg_class - relhasrules, relhastriggers, ... Then the overhead of this design should be really zero. What do you think about it? Pavel > > > > > Regards > > Pavel > > >> >> -- >> Konstantin Knizhnik >> Postgres Professional: http://www.postgrespro.com >> The Russian Postgres Company >> >> > -- > Konstantin Knizhnik > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company > >
