Re: Non-superuser subscription owners
Robert Haas <robertmhaas@gmail.com>
From: Robert Haas <robertmhaas@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Mark Dilger <mark.dilger@enterprisedb.com>,
Jeff Davis <pgsql@j-davis.com>, Amit Kapila <amit.kapila16@gmail.com>, Andrew Dunstan <andrew@dunslane.net>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2023-01-20T16:08:54Z
Lists: pgsql-hackers
Attachments
- v1-0001-Add-new-predefined-role-pg_create_subscriptions.patch (application/octet-stream) patch v1-0001
On Fri, Jan 20, 2023 at 8:25 AM Robert Haas <robertmhaas@gmail.com> wrote: > I still think you're talking about a different problem here. I'm > talking about the problem of knowing whether local files are going to > be accessed by the connection string. So here's a dumb patch for this. At least in my mind, the connection string sanitization/validation is the major design problem here, and I'm not at all sure that what I did in the attached patch is right. But let's talk about that. This approach is inspired by Jeff's comments about local file access upthread, but as Andres pointed out, that's a completely different set of things than we worry about in other places. I'm not quite sure what the right model is here. This patch incidentally allows ALTER SUBSCRIPTION .. SKIP for any subscription owner, removing the existing check that limits that operation to superusers and replacing it with nothing. I can't really see why this needs to be any more restricted than that, and regrettably neither the check in the existing code nor the commit that added it have any comments explaining the logic behind that check. If, for example, skipping a subscription could lead to a server crash, that would be a reason to restrict the feature to superusers (or revert it). If it's just a case of the operation being maybe not the right thing to do, that's not a sufficient reason to restrict it to superusers. This change is really independent of the rest of the patch and, if we want to do this, I will separate it into its own patch. But since this is just for discussion, I didn't worry about that right now. Aside from the above, I don't yet see a problem here that I would consider to be serious enough that we couldn't proceed. I'll try to avoid too much repetition of what's already been said on this topic, but I do want to add that I think that creating subscriptions is properly viewed as a *slightly* scary operation, not a *very* scary operation. It lets you do two things that you couldn't otherwise. One is get background processes running that take up process slots and consume resources -- but note that your ability to consume resources with however many normal database connections you can make is virtually unlimited. The other thing it lets you do is poke around the network, maybe figure out whether some ports are open or closed, and try to replicate data from any accessible servers you can find, which could include ports or servers that you can't access directly. I think that the superuser will be in a good position to evaluate whether that is a risk in a certain environment or not, and I think many superusers will conclude that it isn't a big risk. I think that the main motivation for NOT handing out pg_create_subscription will turn out to be administrative rather than security-related i.e. they'll want to be something that falls under their authority rather than someone else's. -- Robert Haas EDB: http://www.enterprisedb.com
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Fix possible crash in tablesync worker.
- b5c517379a40 16.0 landed
-
Display 'password_required' option for \dRs+ command.
- 19e65dff38bd 16.0 landed
-
Restart the apply worker if the 'password_required' option is changed.
- c1cc4e688b60 16.0 landed
-
Fix possible logical replication crash.
- e7e7da2f8d57 16.0 landed
-
Add new predefined role pg_create_subscription.
- c3afe8cf5a1e 16.0 landed
-
Expand AclMode to 64 bits
- 7b378237aa80 16.0 cited
-
More cleanup of a2ab9c06ea.
- 96a6f11c0625 15.0 landed
-
Respect permissions within logical replication.
- a2ab9c06ea15 15.0 landed
-
Improve table locking behavior in the face of current DDL.
- 2ad36c4e44c8 9.2.0 cited