Re: Non-superuser subscription owners - Mailing list pgsql-hackers

On Thu, Dec 2, 2021 at 12:51 AM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
>
>
> > On Dec 1, 2021, at 5:36 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Dec 1, 2021 at 2:12 AM Jeff Davis <pgsql@j-davis.com> wrote:
> >>
> >> On Tue, 2021-11-30 at 17:25 +0530, Amit Kapila wrote:
> >>> I think it would be better to do it before we allow subscription
> >>> owners to be non-superusers.
> >>
> >> There are a couple other things to consider before allowing non-
> >> superusers to create subscriptions anyway. For instance, a non-
> >> superuser shouldn't be able to use a connection string that reads the
> >> certificate file from the server unless they also have
> >> pg_read_server_files privs.
> >>
> >
> > Isn't allowing to create subscriptions via non-superusers and allowing
> > to change the owner two different things? I am under the impression
> > that the latter one is more towards allowing the workers to apply
> > changes with a non-superuser role.
>
> The short-term goal is to have logical replication workers respect the privileges of the role which owns the
subscription.
>
> The long-term work probably includes creating a predefined role with permission to create subscriptions, and the
abilityto transfer those subscriptions to roles who might be neither superuser nor members of any particular predefined
role;the idea being that logical replication subscriptions can be established without any superuser involvement, and
maythereafter run without any special privilege. 
>
> The more recent patches on this thread are not as ambitious as the earlier patch-sets.  We are no longer trying to
supporttransferring subscriptions to non-superusers. 
>
> Right now, on HEAD, if a subscription owner has superuser revoked, the subscription can continue to operate as
superuserin so far as its replication actions are concerned.  That seems like a pretty big security hole. 
>
> This patch mostly plugs that hole by adding permissions checks, so that a subscription owned by a role who has
privilegesrevoked cannot (for the most part) continue to act under the old privileges. 
>

If we want to maintain the property that subscriptions can only be
owned by superuser for your first version then isn't a simple check
like ((!superuser()) for each of the operations is sufficient?

> There are two problematic edge cases that can occur after transfer of ownership.  Remember, the new owner is required
tobe superuser for the transfer of ownership to occur. 
>
> 1) A subscription is transferred to a new owner, and the new owner then has privilege revoked.
>
> 2) A subscription is transferred to a new owner, and then the old owner has privileges increased.
>

In (2), I am not clear what do you mean by "the old owner has
privileges increased"? If the owners can only be superusers then what
does it mean to increase the privileges.

> In both cases, a currently running logical replication worker may finish a transaction in progress acting with the
currentprivileges of the old owner.  The clearest solution is, as you suggest, to refuse transfer of ownership of
subscriptionsthat are enabled. 
>
> Doing so will create a failure case for REASSIGN OWNED BY.  Will that be ok?
>

I think so. Do we see any problem with that? I think we have some
failure cases currently as well like "All Tables Publication" can only
be owned by superusers whereas ownership for others can be to
non-superusers and similarly we can't change ownership for pinned
objects. I think the case being discussed is not exactly the same but
I am not able to see a problem with it.

--
With Regards,
Amit Kapila.