Re: Wrong security context for deferred triggers?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Wrong security context for deferred triggers?
Date: 2025-01-22 18:25:55
Message-ID: 3944969.1737570355@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> writes:
> [ v4-0001-Make-AFTER-triggers-run-with-the-correct-user.patch ]

I started to look at this and got distracted by wondering why
afterTriggerAddEvent's scanning loop wasn't checking ats_modifiedcols.
That led to ea68ea632, which means this now needs a minor rebase.

I have a couple other thoughts:

* It's kind of sad that we have to add yet another field to
AfterTriggerSharedData, especially since this one will cost 8 bytes
thanks to alignment considerations. I'm not sure if there's another
way, but it seems like ats_relid, ats_rolid, and ats_modifiedcols
are all going to be extremely redundant in typical scenarios.
Maybe it's worth rethinking that data structure a bit? Or maybe
I'm worried over nothing; perhaps normal situations have only a few
AfterTriggerSharedDatas anyway. It might be worth trying to gather
some statistics about that.

* I don't buy the bit about "We have to check if the role still
exists"; I think that's just a waste of cycles. There is no check
that prevents dropping the role a session is currently running as,
so why do we need one here? Moreover, the role could still get
dropped immediately after you look. Or for that matter, save_rolid
could be invalid by the time you restore it. None of this bothers me,
because if a session is running as a dropped role it will still
function pretty normally. It will find that it has no privileges
beyond those granted to PUBLIC, and if it tries to inspect
current_user or related functions those will fail, but there's
no compromise to system integrity. So I'd just remove that test.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeff Davis 2025-01-22 18:32:43 Re: Update Unicode data to Unicode 16.0.0
Previous Message Corey Huinker 2025-01-22 18:17:27 Extended Statistics set/restore/clear functions.