Le lundi 27 septembre 2021, 20:15:05 CEST Mark Dilger a écrit : > > On Sep 21, 2021, at 12:58 PM, Andrew Dunstan <and...@dunslane.net> wrote: > > > > This patch set is failing to apply for me - it fails on patch 2. > > Thanks for looking! I have pulled together a new patch set which applies > cleanly against current master. > > I haven't dug terribly deeply into it yet, but I notice that there is a > > very large increase in test volume, which appears to account for much of > > the 44635 lines of the patch set. I think we're probably going to want > > to reduce that. We've had complaints in the past from prominent hackers > > about adding too much volume to the regression tests. > > The v8 patch set is much smaller, with the reduction being in the size of > regression tests covering which roles can perform SET, RESET, ALTER SYSTEM > SET, and ALTER SYSTEM RESET and on which GUCs. The v7 patch set did > exhaustive testing on this, which is why it was so big. The v8 set does > just a sampling of GUCs per role. The total number of lines for the patch > set drops from 44635 to 13026, with only 1960 lines total between the .sql > and .out tests of GUC privileges. > > I do like the basic thrust of reducing the power of CREATEROLE. There's > > an old legal maxim I learned in my distant youth that says "nemo dat > > quod non habet" - Nobody can give something they don't own. This seems > > to be in that spirit, and I approve :-) > > Great! I'm glad to hear the approach has some support. > > > Other changes in v8: > > Add a new test for subscriptions owned by non-superusers to verify that the > tablesync and apply workers replicating their subscription won't replicate > into schemas and tables that the subscription owner lacks privilege to > touch. The logic to prevent that existed in the v7 patch, but I overlooked > adding tests for it. Fixed. > > Allow non-superusers to create event triggers. The logic already existed > and is unchanged to handle event triggers owned by non-superusers and > conditioning those triggers firing on the (trigger-owner, > role-performing-event) pair. The v7 patch set didn't go quite so far as > allowing non-superusers to create event triggers, but that undercuts much > of the benefit of the changes for no obvious purpose. > > > Not changed in v8, but worth discussing: > > Non-superusers are still prohibited from creating subscriptions, despite > improvements to the security around that circumstance. Improvements to the > security model around event triggers does not have to also include > permission for non-superuser to create event triggers, but v8 does both. > These could be viewed as inconsistent choices, but I struck the balance > this way because roles creating event triggers does not entail them doing > anything that they can't already do, whereas allowing arbitrary users to > create subscriptions would entail an ordinary user causing external network > connections being initiated. We likely need to create another privileged > role and require a non-superuser to be part of that role before they can > create subscriptions. That seems, however, like something to do as a > follow-on patch, since tightening up the security on subscriptions as done > in this patch doesn't depend on that.
The changes proposed around subscription management make a lot of sense, especially considering that now that we don't allow to run ALTER SUBSCRIPTION REFRESH in a function anymore, there is no way to delegate this to a non superuser (using a security definer function). Since it doesn't involve the rest of the patchset, patches 16, 17 and 18 could be separated in another thread / patchset. -- Ronan Dunklau
