Re: On login trigger: take three

From: Nikita Malakhov <hukutoc(at)gmail(dot)com>
To: Mikhail Gribkov <youzhick(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Ian Lawrence Barwick <barwick(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Sergey Shinderuk <s(dot)shinderuk(at)postgrespro(dot)ru>, Ivan Panchenko <wao(at)mail(dot)ru>, Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, Teodor Sigaev <teodor(at)sigaev(dot)ru>, Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Subject: Re: On login trigger: take three
Date: 2022-12-16 20:17:45
Message-ID: CAN-LCVNSGnDn-9Oy=71oR=pL_ZhviUD+dLVrh8EZ3LuSzxRJpw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Mikhail, I've checked the patch and previous discussion,
the condition mentioned earlier is still actual:
>The example trigger from the documentation

+DECLARE

+ hour integer = EXTRACT('hour' FROM current_time);

+ rec boolean;

+BEGIN

+-- 1. Forbid logging in between 2AM and 4AM.

+IF hour BETWEEN 2 AND 4 THEN

+ RAISE EXCEPTION 'Login forbidden';

+END IF;

>can be bypassed with PGOPTIONS='-c timezone=...'. Probably this is
>nothing new and concerns any SECURITY DEFINER function, but still...

along with
+IF hour BETWEEN 8 AND 20 THEN
It seems to be a minor security issue, so just in case you haven't noticed
it.

On Fri, Dec 16, 2022 at 9:14 PM Mikhail Gribkov <youzhick(at)gmail(dot)com> wrote:

> Hi hackers,
> Attached v33 is a new rebase of the flagless version of the patch. As
> there were no objections at first glance, I’ll try to post it to the
> upcoming commitfest, thus the brief recap of all the patch details is below.
>
> v33-On_client_login_event_trigger
> The patch introduces a trigger on login event, allowing to fire some
> actions right on the user connection. This can be useful for logging or
> connection check purposes as well as for some personalization of the
> environment. Usage details are described in the documentation included, but
> shortly usage is the same as for other triggers: create function returning
> event_trigger and then create event trigger on login event.
>
> The patch is prepared to be applied to the master branch and tested when
> applied after e52f8b301ed54aac5162b185b43f5f1e44b6b17e commit by Thomas
> Munro (Date: Fri Dec 16 17:36:22 2022 +1300).
> Regression tests and documentation included.
>
> A couple of words about what and why I changed compared to the previous
> author's version.
> First, the (en/dis)abling GUC was removed from the patch because
> ideologically it is a separate feature and nowadays it’s discussed and
> supported in a separate thread by Daniel Gustaffson:
>
> https://www.postgresql.org/message-id/flat/9140106E-F9BF-4D85-8FC8-F2D3C094A6D9%40yesql.se
> Second, I have removed the dathasloginevt flag. The flag was initially
> added to the patch for performance reasons and it did the job, although it
> was quite a clumsy construct causing tons of bugs and could potentially
> lead to tons more during further PostgreSQL evolution. I have removed the
> flag and found that the performance drop is not that significant.
>
> And this is an aspect I should describe in more detail.
> The possible performance drop is expected as an increased time used to
> login a user NOT using a login trigger.
> First of all, the method of performance check:
> echo 'select 1;' > ./tst.sql
> pgbench -n -C -T3 -f tst.sql -U postgres postgres
> The output value "average connection time" is the one I use to compare
> performance.
> Now, what are the results.
> * master branch: 0.641 ms
> * patched version: 0.644 ms
> No significant difference here and it is just what was expected. Based on
> the patch design the performance drop can be expected when there are lots
> of event triggers created, but not the login one. Thus I have created 1000
> drop triggers (the script for creating triggers is attached too) in the
> database and repeated the test:
> * master branch: 0.646 ms
> * patched version: 0.754 ms
> For 2000 triggers the patched version connection time is further increased
> to 0.862. Thus we have a login time rise in about 16.5% per 1000 event
> triggers in the database. It is a statistically noticeable value, still I
> don’t think it’s a critical one we should be afraid of.
> N.B. The exact values of the login times slightly differ from the ones I
> posted in the previous email. Well, that’s the repeatability level we have.
> This convinces me even more that the observed level of performance drop is
> acceptable.
> --
> best regards,
> Mikhail A. Gribkov
>
> e-mail: youzhick(at)gmail(dot)com
> *http://www.flickr.com/photos/youzhick/albums
> <http://www.flickr.com/photos/youzhick/albums>*
> http://www.strava.com/athletes/5085772
> phone: +7(916)604-71-12
> Telegram: @youzhick
>

--
Regards,
Nikita Malakhov
Postgres Professional
https://postgrespro.ru/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-12-16 20:33:33 Re: BUG #17717: Regression in vacuumdb (15 is slower than 10/11 and possible memory issue)
Previous Message Peter Geoghegan 2022-12-16 18:47:07 Re: BUG #17717: Regression in vacuumdb (15 is slower than 10/11 and possible memory issue)