Re: Delegating superuser tasks to new security roles (Was: Granting control of SUSET gucs to non-superusers)

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Robert Haas <robertmhaas(at)gmail(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Jacob Champion <pchampion(at)vmware(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "tgl(at)sss(dot)pgh(dot)pa(dot)us" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "chap(at)anastigmatix(dot)net" <chap(at)anastigmatix(dot)net>, torikoshia <torikoshia(at)oss(dot)nttdata(dot)com>
Subject: Re: Delegating superuser tasks to new security roles (Was: Granting control of SUSET gucs to non-superusers)
Date: 2021-09-27 18:15:05
Message-ID: AE20103F-2DBD-44FC-8C79-EBBCE9D3534A@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


> On Sep 21, 2021, at 12:58 PM, Andrew Dunstan <andrew(at)dunslane(dot)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 Content-Type Size
v8.tar.bz2 application/x-bzip2 56.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Melanie Plageman 2021-09-27 18:58:53 Re: pg_stat_bgwriter.buffers_backend is pretty meaningless (and more?)
Previous Message Christophe Pettus 2021-09-27 18:10:19 Re: statement_timeout vs DECLARE CURSOR