On Wed, 2023-03-29 at 16:00 -0400, Robert Haas wrote:
> Here's a new patch set to show how this would work. 0001 is as
> before.
The following special case ("unless they can SET ROLE..."):
/*
* Try to prevent the user to which we're switching from assuming the
* privileges of the current user, unless they can SET ROLE to that
* user anyway.
*/
which turns SwitchToUntrustedUser() into a no-op, is conceptually
redundant with patch 0002. The only difference is that the former (in
0001) is implicit and the latter (in 0002) is declared.
I say just take the special case out of 0001. If the trigger doesn't
work as a SECURITY_RESTRICTED_OPERATION, and is also ENABLED ALWAYS,
then the user can just use the new option in 0002 to get the old
behavior. I don't see a reason to implicitly give them the old
behavior, as 0001 does.
> 0002 adds a run_as_owner option to subscriptions. This doesn't make
> the updated regression tests fail, because they don't use it. If you
> revert the changes to 027_nosuperuser.pl then you get failures (as
> one
> would hope) and if you then add WITH (run_as_owner = true) to the
> CREATE SUBSCRIPTION command in 027_nosuperuser.pl then it passes
> again. I need to spend some more time thinking about what the tests
> actually ought to look like if we go this route -- I haven't looked
> through what Jelte proposed yet -- and also the documentation would
> need a bunch of updating.
"run_as_owner" is ambiguous -- subscription owner or table owner?
I would prefer something like "trust_table_owners". That would
communicate that the user shouldn't choose it unless they know what
they're doing.
> And either way, at least PostgreSQL
> hacker who has taken the time to write about this topic is going to
> think we've made a poor call, or at best a dubious one, and either
> way,
If you are worried that *I* think 0002 would be a poor call, then no, I
don't. Initially I didn't like the idea of supporting two behaviors
(and who would?), but we probably can't avoid it at least for right
now.
Regards,
Jeff Davis
