From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Non-superuser subscription owners |
Date: | 2023-01-27 22:00:35 |
Message-ID: | 20230127220035.q6igt2ylwu2p3qjo@awork3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2023-01-27 16:35:11 -0500, Robert Haas wrote:
> > Maybe a daft question:
> >
> > Have we considered using a "normal grant", e.g. on the database, instead of a
> > role? Could it e.g. be useful to grant a user the permission to create a
> > subscription in one database, but not in another?
>
> Potentially, but I didn't think we'd want to burn through permissions
> bits that fast, even given 7b378237aa805711353075de142021b1d40ff3b0.
> Still, if the consensus is otherwise, I can change it.
I don't really have an opinion on what's better. I looked briefly whether
there was discussion around ithis but I didn't see anything.
pg_create_subcription feels a bit different than most of the other pg_*
roles. For most of those there is no schema object to tie permissions to. But
here there is.
But I think there's good arguments against a GRANT approach, too. GRANT ALL ON
DATABASE would suddenly be dangerous. How does it interact with database
ownership? Etc.
> Then I guess we'd end up with GRANT CREATE ON DATABASE and GRANT CREATE
> SUBSCRIPTION ON DATABASE, which I'm sure wouldn't be confusing at all.
Heh. I guess it could just be GRANT SUBSCRIBE.
> Or, another thought, maybe this should be checking for CREATE on the
> current database + also pg_create_subscription. That seems like it
> might be the right idea, actually.
Yes, that seems like a good idea.
> > The subscription code already does ownership checks before locking and now
> > there's also the passwordrequired before. Is it possible that this could open
> > up some sort of race? Could e.g. the user change the ownership to the
> > superuser in one session, do an ALTER in the other?
> >
> > It looks like your change won't increase the danger of that, as the
> > superuser() check just checks the current users permissions.
>
> I'm not entirely clear whether there's a hazard there.
I'm not at all either. It's just a code pattern that makes me anxious - I
suspect there's a few places it makes us more vulnerable.
> If there is, I think we could fix it by moving the LockSharedObject call up
> higher, above object_ownercheck. The only problem with that is it lets you
> lock an object on which you have no permissions: see
> 2ad36c4e44c8b513f6155656e1b7a8d26715bb94. To really fix that, we'd need an
> analogue of RangeVarGetRelidExtended.
Yea, we really should have something like RangeVarGetRelidExtended() for other
kinds of objects. It'd take a fair bit of work / time to use it widely, but
it'll take even longer if we start in 5 years ;)
Perhaps the bulk of RangeVarGetRelidExtended() could be generalized by having
a separate name->oid lookup callback?
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-01-27 22:05:16 | Re: heapgettup() with NoMovementScanDirection unused in core? |
Previous Message | Melanie Plageman | 2023-01-27 21:40:01 | Re: heapgettup() with NoMovementScanDirection unused in core? |