On 15.12.2020 16:18, Pavel Stehule wrote:


út 15. 12. 2020 v 14:12 odesílatel Konstantin Knizhnik <k.knizh...@postgrespro.ru <mailto: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 <mailto: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?






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

Reply via email to