Re: On login trigger: take three

On 11.12.2020 19:27, Pavel Stehule wrote:
>
>
> pá 11. 12. 2020 v 17:05 odesílatel Konstantin Knizhnik
> <k(dot)knizhnik(at)postgrespro(dot)ru <mailto:k(dot)knizhnik(at)postgrespro(dot)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.

--
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company