Re: On login trigger: take three

On 02.09.2022 18:36, Daniel Gustafsson wrote:
> This had bitrotted a fair bit, attached is a rebase along with (mostly)
> documentation fixes. 0001 adds a generic GUC for ignoring event triggers and
> 0002 adds the login event with event trigger support, and hooks it up to the
> GUC such that broken triggers wont require single-user mode. Moving the CF
> entry back to Needs Review.

Hello!

There is a race around setting and clearing of dathasloginevt.

Steps to reproduce:

1. Create a trigger:

CREATE FUNCTION on_login_proc() RETURNS event_trigger AS $$
BEGIN
RAISE NOTICE 'You are welcome!';
END;
$$ LANGUAGE plpgsql;

CREATE EVENT TRIGGER on_login_trigger ON login EXECUTE PROCEDURE
on_login_proc();

2. Then drop it, but don't start new sessions:

DROP EVENT TRIGGER on_login_trigger;

3. Create another trigger, but don't commit yet:

BEGIN;
CREATE EVENT TRIGGER on_login_trigger ON login EXECUTE PROCEDURE
on_login_proc();

4. Start a new session. This clears dathasloginevt.

5. Commit the transaction:

COMMIT;

Now we have a trigger, but it doesn't fire, because dathasloginevt=false.

If two sessions create triggers concurrently, one of them will fail.

Steps:

1. In the first session, start a transaction and create a trigger:

BEGIN;
CREATE EVENT TRIGGER on_login_trigger1 ON login EXECUTE PROCEDURE
on_login_proc();

2. In the second session, create another trigger (this query blocks):

CREATE EVENT TRIGGER on_login_trigger2 ON login EXECUTE PROCEDURE
on_login_proc();

3. Commit in the first session:

COMMIT;

The second session fails:

postgres=# CREATE EVENT TRIGGER on_login_trigger2 ON login EXECUTE
PROCEDURE on_login_proc();
ERROR: tuple concurrently updated

What else bothers me is that login triggers execute in an environment
under user control and one has to be very careful. 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...

Best regards,

--
Sergey Shinderuk https://postgrespro.com/