On Wed, Oct 20, 2021 at 11:27:11AM -0700, Jeff Davis wrote: > On Wed, 2021-10-20 at 10:32 -0700, Mark Dilger wrote: > > I'd like to have a much clearer understanding of Noah's complaint > > first. There are multiple things to consider: (1) the role which > > owns the trigger, (2) the role which is performing an action which > > would cause the trigger to fire, (3) the set of roles role #1 belongs > > to, (4) the set of roles role #1 has ADMIN privilege on, (5) the set > > of roles that role #2 belongs to, and (6) the set of roles that role > > #2 has ADMIN privilege on. Maybe more?
> > I'd like to know precisely which combinations of these six things are > > objectionable, and why. There may be a way around the objections > > without needing to create new user options or new privileged roles. > > I can't speak for Noah, but my interpretation is that it would be > surprising if GRANT/REVOKE or membership in an ordinary role had > effects other than "permission denied" errors. It might make sense for > event trigger firing in all the cases we can think of, but it would > certainly be strange if we started accumulating a collection of > behaviors that implicitly change when you move in or out of a role. > > That's pretty general, so to answer your question: it seems like a > problem to use #3-6 in the calculation about whether to fire an event > trigger. Exactly. That's the main point. Also, it's currently a best practice for only non-LOGIN roles to have members. The proposed approach invites folks to abandon that best practice. I take the two smells as a sign that we'll regret adopting the proposal, despite not knowing how it will go seriously wrong. On Wed, Oct 20, 2021 at 12:09:08PM -0700, Jeff Davis wrote: > On Thu, 2021-05-27 at 23:06 -0700, Noah Misch wrote: > > pg_logical_replication would not be safe to delegate that way: > > > https://postgr.es/m/flat/CACqFVBbx6PDq%2B%3DvHM0n78kHzn8tvOM-kGO_2q_q0zNAMT%2BTzdA%40mail.gmail.com > > What do you mean "that way"? Do you mean it's not safe to delegate > subscription creation to non-superusers at all? I meant "pg_logical_replication would not be safe to delegate to the tenant of a database provided as a service." It's not safe today, but it can be made safe: > From the thread above, I don't see anything so dangerous that it can't > be delegated: > > * persistent background workers on subscriber > - still seems reasonable to delegate to a privileged user Agreed, I don't have a problem with pg_logical_replication implying that ability. If you can create this worker, you can bypass ADMIN OPTION by running the GRANT/REVOKE inside a subscription. That's probably fine if documented, or else is_admin_of_role() could prevent it. > * arbitrary code execution by the apply worker on subscriber > - apply worker runs as subscription owner, so doesn't seem > like a problem? Sounds right. I think Mark Dilger drafted a patch to add ACL checks and a TAP test confirming that the worker does get permission denied. That change has no disadvantage, so this problem is on the way to getting solved. > * connection info may be visible to non-superusers > - seems either solvable or not necessarily a problem Yes. The other matter from the thread you linked is "the connection to the publisher must enforce the equivalent of dblink_security_check()". I think Mark Dilger drafted a patch for that, too.
