Re: Non-superuser subscription owners

> On Nov 17, 2021, at 1:06 PM, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>
> On Wed, 2021-11-17 at 10:25 -0800, Mark Dilger wrote:
>> We may eventually allow non-superusers to create subscriptions, but
>> there are lots of details to work out.
>
> I am setting aside the idea of subscriptions created by non-superusers.

Ok, fair enough. I think eventually we'll want that, but I'm also setting that aside for this patch.

> My comments were about your idea for "low-power users" that can still
> do things with subscriptions. And for that, GRANT seems like a better
> fit than ownership.

This patch has basically no value beyond the fact that it allows the replication to be *applied* as a user other than superuser. Throw that out, and there isn't any point. Everything else is window dressing. The real security problem with subscriptions is that they act with superuser power. That naturally means that they must be owned and operated by superuser, too, otherwise they serve as a privilege escalation attack vector. It really doesn't make any sense to think of subscriptions as operating under the permissions of multiple non-superusers. You must choose a single role you want the subscription to run under. What purpose would be served by GRANTing privileges on a subscription to more than one non-superuser? It still operates as just the one user. I agree you *could* give multiple users privileges to mess with it, but you'd still need to assign a single role as the one whose privileges matter for the purpose of applying replication changes. I'm using "owner" for that purpose, and I think that is consistent with how security definer functions work. They run as the owner, too. It's perfectly well-precedented to use "owner" for this.

I think the longer term plan is that non-superusers who have some privileged role will be allowed to create subscriptions, and naturally they will own the subscriptions that they create, at least until an ALTER SUBSCRIPTION..OWNER TO is successfully executed to transfer ownership. Once that longer term plan is complete, non-superusers will be able to create publications of their tables on one database, and subscriptions to those publications on another database, all without needing the help of a superuser. This patch doesn't get us all the way there, but it heads directly toward that goal.

> With v2-0001, there are several things that seem weird to me:
>
> * Why can there be only one low-power user per subscription?

Because the apply workers run as only one user. Currently it is always superuser. After this patch, it is always the owner, which amounts to the same thing for legacy subscriptions created and owned by superuser prior to upgrading to v15, but not necessarily for new ones or ones that have ownership transferred after upgrade.

We could think about subscriptions that act under multiple roles, perhaps taking role information as part of the data-stream from the publisher, but that's a pretty complicated proposal, and it is far from clear that we want it anyway. There is a security case to be made for *not* allowing the publisher to call all the shots, so such a proposal would at best be an alternate mode of operation, not the one and only mode.

> * Why is RENAME a separate capability from CREATE/DROP?

I don't care enough to argue this point. If you want me to remove RENAME privilege from the owner, I can resubmit with it removed. It just doesn't seem like it's dangerous to allow a non-superuser to rename their subscriptions, so I saw no compelling reason to disallow it.

CREATE clearly must be disallowed since it gives the creator the ability to form network connections, set fsync modes, etc., and there is no reason to assume arbitrary non-superusers should be able to do that.

The argument against DROP is a bit weaker. It doesn't seem like a user who cannot bring subscriptions into existence should be able to drop them either. I was expecting to visit that issue in a follow-on patch which deals with non-superuser predefined roles that have some power to create and drop subscriptions. What that patch will propose to do is not obvious, since some of what you can do with subscriptions is so powerful we may not want non-superusers doing it, even with a privileged role. If you can't picture what I mean, consider that you might use a connection parameter that connects outside and embeds data into the connection string, with a server listening on the other end, not really to publish data, but to harvest the secret data that you are embedding in the network connection attempt.

> * What if you want to make the privileges more fine-grained, or make
> changes in the future? Ownership is a single bit, so it requires that
> everyone agree.

We can modify the patch to have the subscription owner have zero privileges on the subscription, not even the ability to see how it is defined, and just have "owner" mean the role under whose privileges the logical replication workers apply changes. Would that be better? I would expect people to find that odd.

The problem is that we want a setuid/setgid type behavior. Actual setuid/setgid programs act as the user/group of the executable. There's no reason that user/group needs to be one that any real human uses to log into the system. Likewise, we need the subscription to act under a role, and we're establishing which role that is by having that role own the subscription. That is like how setuid/setgid programs work by executing as the user/group that owns the executable, except that postgres doesn't have separate user/group concepts, just roles. Isn't this design pattern completely commonplace?

> Maybe some people want RENAME to be a part of that, and
> others don't.

Fair enough. Should I remove RENAME from what the patch allows the owner to do? On this particular point, I genuinely don't care. I think it can be reasonably argued either way.

> GRANT seems to provide better answers here.

No, because we don't have infinite privilege bits to burn.

>> Since we're trying to make subscriptions into things that non-
>> superusers can use, we have to deal with some things in those
>> functions.
>
> I understand the use case where a superuser isn't required anywhere in
> the process, and some special users can create and own subscriptions. I
> also understand that's not what these patches are trying to accomplish
> (though v2-0003 seems like a good step in that direction).

There is a cart-before-the-horse problem here. If I propose a patch with a privileged role for creating and owning subscriptions *before* I tighten down how non-superuser-owned subscriptions work, that patch would surely be rejected. So I either propose this first, and only if/when it gets accepted, propose the other, or I propose them together. That's a damned-if-you-do--damned-if-you-dont situation, because if I propose them together, I'll get arguments that they are clearly separable and should be proposed separately, and if I do them one before the other, I'll get the argument that you are making now. I fully expect the privileged role proposal to be made (possibly by me), though it is unclear if there will be time left to do it in v15.

> I don't understand the use case as well where a non-superuser can
> merely "use" a subscription. I'm sure such use cases exist and I'm fine
> to go along with that idea, but I'd like to understand why ownership
> (partial ownership?) is the right way to do this and GRANT is the wrong
> way.

Even if we had the privilege bits to burn, no spelling of that GRANT idea sounds all that great:

GRANT RUN AS ON subscription TO role;
GRANT RUN AS ON role TO subscription;
GRANT SUDO ON subscription TO role;
GRANT SETUID ON role TO subscription;
...

I just don't see how that really works. I'm not inclined to spend time being more clever, since I already know that privilege bits are in short supply, but if you want to propose something, go ahead. Elsewhere you proposed GRANT REFRESH or something, not looking at that email just now, but that's not the same thing as GRANT RUN AS, and burns another privilege bit, and still doesn't get us all the way there, because you presumably also want GRANT RENAME, GRANT ALTER CONNECTION SETTING, GRANT ALTER FSYNC SETTING, ..., and we're out of privilege bits before we're done.

>> For example, ALTER SUBSCRIPTION can change the database connection
>> parameter, or the publication subscribed, or whether
>> synchronous_commit is used. I don't see that a subscription owner
>> should necessarily be allowed to mess with that, at least not without
>> some other privilege checks beyond mere ownership.
>
> That violates my expectations of what "ownership" means.

I think that's because you're thinking of these settings as properties of the subscription. You may *own* the subscription, but the subscription doesn't *own* the right to make connections to arbitrary databases, nor *own* the right to change buffer cache settings, nor *own* the right to bring data from a publication on some other server which, if it existed on the local server, would violate site policy and possibly constitute a civil or criminal violation of data privacy laws. I may own my house, and the land it sits on, and my driveway, but that doesn't mean I own the ability to make my driveway go across my neighbor's field, down through town, and to the waterfront. But that's the kind of ownership definition you seem to be defending.

Some of what I perceive as the screwiness of your argument I must admin is not your fault. The properties of subscriptions are defined in ways that don't make sense to me. It would be far more sensible if connection strings were objects in their own right, and you could grant USAGE on a connection string to a role, and USAGE on a subscription to a role, and only if the subscription worker's role had privileges on the connection string could they use it as part of fulfilling their task of replicating the data, and otherwise they'd error out in the attempt. Likewise, fsync modes could be proper objects, and only if the subscription's role had privileges on the fsync mode they wanted to use would they be able to use it. But we don't have these things as proper objects, with acl lists on them, so we're stuck trying to design around that. To my mind, that means subscription owners *do not own* properties associated with the subscription. To your mind, that's not what "ownership" means. What to do?

>> I think this is pretty analogous to how security definer functions
>> work.
>
> The analogy to SECURITY DEFINER functions seems to support my
> suggestion for GRANT at least as much as your modified definition of
> ownership.

I don't see how. Can you please explain?

>>> This would not address the weirdness of the existing code where a
>>> superuser loses their superuser privileges but still owns a
>>> subscription. But perhaps we can solve that a different way, like
>>> just
>>> performing a check when someone loses their superuser privileges
>>> that
>>> they don't own any subscriptions.
>>
>> I gave that a slight amount of thought during the design of this
>> patch, but didn't think we could refuse to revoke superuser on such a
>> basis,
>
> I don't necessarily see a problem there, but I could be missing
> something.

Close your eyes and imagine that I have superuser on your database... really picture it in your mind. Now, do you want the revoke command you are issuing to work?

>> and didn't see what we should do with the subscription other than
>> have it continue to be owned by the recently-non-superuser. If you
>> have a better idea, we can discuss it, but to some degree I think
>> that is also orthogonal to the purpose of this patch. The only sense
>> in which this patch depends on that issue is that this patch proposes
>> that non-superuser subscription owners are already an issue, and
>> therefore that this patch isn't creating a new issue, but rather
>> making more sane something that already can happen.
>
> By introducing and documenting a way to get non-superusers to own a
> subscription, it makes it more likely that people will do it, and
> harder for us to change. That means the standard should be "this is
> what we really want", rather than just "more sane than before".

Ok, I'll wait to hear back from you on the points above.

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