pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Laurenz Albe <laurenz.albe@cybertec.at>
Cc: Pavel Stehule <pavel.stehule@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Wrong security context for deferred triggers?
Date: Wed, 22 Jan 2025 13:25:55 -0500
Message-ID: <3944969.1737570355@sss.pgh.pa.us> (raw)
In-Reply-To: <51386e72c47d922ef68eaa67451a52dc3f569843.camel@cybertec.at>
References: <77ee784cf248e842f74588418f55c2931e47bd78.camel@cybertec.at>
	<dede297a-3d24-7029-dfd9-06aeef8b9766@enterprisedb.com>
	<6f27f51bc98354d7254bbde1c171fa13aca03f28.camel@cybertec.at>
	<77b89e609f21380785865542609fbc14010021c8.camel@cybertec.at>
	<CAFj8pRDcE3+9EM+PSSEW1bHA-EWMAYgVog41q0c47N=Pnq_qiQ@mail.gmail.com>
	<3f0a08f3e3d445137ae287125bc8c1a007045db8.camel@cybertec.at>
	<CAFj8pRCCdL_vB-42161CWxemBVqF75WOq53cMnmRCrzjvKpLOQ@mail.gmail.com>
	<66021ca9d81c4e1a22b6c417659b05424e130f27.camel@cybertec.at>
	<CAFj8pRB1UVUuEh_T7j7XFo8yYDuEYxoUgMhPXbny6dK1eCvgRg@mail.gmail.com>
	<6bca669972ef0d4a665fa83c5b751ab116324f8f.camel@cybertec.at>
	<CAFj8pRDcy2OaPS7OtceFRhTRq6VKWbdnuawgaX3EXMQVwTR4VA@mail.gmail.com>
	<2653121.1730737337@sss.pgh.pa.us>
	<51386e72c47d922ef68eaa67451a52dc3f569843.camel@cybertec.at>

Laurenz Albe <laurenz.albe@cybertec.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





view thread (44+ messages)  latest in thread

Message-ID: <3944969.1737570355@sss.pgh.pa.us>
Permalink:  ../3944969.1737570355@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/3944969.1737570355@sss.pgh.pa.us

 · 

reply
Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, laurenz.albe@cybertec.at, pavel.stehule@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: Wrong security context for deferred triggers?
  In-Reply-To: <3944969.1737570355@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox