Nathan Bossart <nathandbossart@gmail.com> writes:
> On Thu, Apr 27, 2023 at 03:22:04PM -0400, Tom Lane wrote:
>> The right way to do this was not to add some
>> poorly-explained option to ALTER ROLE, but to record the role OID that
>> issued the ALTER ROLE, and then to check when loading the ALTER ROLE
>> setting whether that role (still) has the right to change the
>> specified setting. As implemented, this can't possibly track changes
>> in GRANT/REVOKE SET privileges correctly, and I wonder if it's not
>> introducing outright security holes like the one fixed by 13d838815.
> I generally agree. At least, I think it would be nice to avoid adding a
> new option if possible. It's not clear to me why we'd need to also check
> privileges at login time as opposed to only checking them at ALTER ROLE SET
> time.
Perhaps there's room to argue about that. But ISTM that if someone
does ALTER ROLE SET on the strength of some privilege you granted
them, and then you regret that and revoke the privilege, then the
ALTER ROLE setting should not continue to work. So I would regard
the session-start-time check as the primary one. Checking when
ALTER ROLE is done is just a user-friendliness detail.
Also, in the case of an extension-defined GUC, we don't necessarily
know its privilege level at either ALTER ROLE time or session start,
since the extension might not yet be loaded at either point.
I've forgotten exactly what restrictive hack we use to get around
that, but the *only* way to do that fully correctly is to record
the role that did the ALTER and then check its privileges at extension
load time. I think that 096dd80f3 has made it harder not easier
to get to the point of doing that correctly, because it's added
a feature that we'll have to figure out how to make interoperate
with a correct implementation.
regards, tom lane