> 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.


Attachment: v8.tar.bz2
Description: BZip2 compressed data


—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Reply via email to