Greetings, * Mark Dilger (mark.dil...@enterprisedb.com) wrote: > > On Aug 23, 2021, at 11:13 AM, Stephen Frost <sfr...@snowman.net> wrote: > > This I have to object to pretty strongly- we have got to get away from > > the idea that just because X isn't a superuser or isn't owned by a > > superuser that it's fine to allow some non-superuser to mess with it. > > I thought we were trying to create a set of roles which could cumulatively do > everything *inside the sandbox* that superuser can do, but which cannot > escape the sandbox. I would think this pg_manage_database_objects role would > be reasonable in the context of that effort.
I wasn't objecting to the general concept of trying to have a role that can do lots of things inside the sandbox but aren't allowed to escape it. I was specifically objecting to the idea that just checking if an object is directly owned by a superuser is not sufficient to prevent a role from being able to escape the sandbox. > > In particlar, just because a role isn't explicitly marked as a superuser > > doesn't mean that the role can't *become* a superuser, or that it hasn't > > got privileged access to the system in other ways, such as by being a > > member of other predefined roles that perhaps the role who is a member > > of pg_manage_database_objects doesn't have. > > The implementation does not allow pg_manage_database_objects to mess with > objects that are owned by a role which satisfies superuser_arg(). If you are > renting out a database to a tenant and change the ownership of stuff to a > non-superuser, then you get what you get. But why would you do that? Simply using superuser_arg() isn't sufficient is exactly the point that I'm trying to make. As a 'landlord', I might very well want to have some kind of 'landlord' role that isn't directly a superuser but which could *become* a superuser by having been GRANT'd a superuser role- but I certainly don't want that role's objects to be able to be messed with by the tenant. > > Such a check against > > modifying of "superuser owned" objects implies that it's providing some > > kind of protection against the role being able to become a superuser > > when it doesn't actually provide that protection in any kind of reliable > > fashion and instead ends up fooling the user. > > Please provide steps to reproduce this issue. Assume that a database is > initialized and that everything is owned by the system. A "tenant" role is > created and granted pg_manage_database_objects, and other non-superuser roles > are created. Now, what exactly can "tenant" do that you find objectionable? If one of those other non-superuser roles is, itself, a role that can become a superuser and it decides to create some functions for its own purposes, then the tenant role would be able to modify those functions, allowing the tenant to gain access to the non-superuser role, and from there being able to gain access to superuser. Something along these lines, basically: CREATE USER tenant; GRANT pg_manage_database_objects TO tenant; CREATE USER landlord; GRANT postgres TO landlord; SET ROLE landlord; CREATE FUNCTION do_stuff(); put call to do_stuff() into a cronjob SET ROLE tenant; CREATE OR REPLACE do_stuff(); -- with code to take over landlord poof- tenant has ability to be landlord and then further to become postgres. All of the above applies beyond just superuser too- consider a non-superuser role which has been grant'd pg_execute_server_program. That won't trip up superuser_arg() but it sure would allow a role to break out of the sandbox. > > This is the issue with CREATEROLE and we definitely shouldn't be > > doubling-down on that mistake, and also brings up the point that I, at > > least, had certainly hoped that part of this effort would include a way > > for roles to be created by a user with an appropriate predefined role, > > and w/o CREATEROLE (which would then be deprecated or, ideally, just > > outright removed). > > Well, pg_manage_database_objects has no special ability to create or drop > roles. I thought separating those powers made more sense than grouping them > together. We can have a new role for doing what you say, but that seems > redundant with CREATEROLE. I didn't want this patch set to be bogged down > waiting for a consensus on how to change the CREATEROLE privilege. CREATEROLE doesn't work to give to folks generally because of the issues above- its check is, similarly, too simple and always has been. This isn't news either, it's been discussed in various places from time to time and is part of why people who run cloud providers end up either not giving out that role attribute and providing another way, or they hack up the PG core code to handle the way that attribute works differently. > > I get that this doesn't have to be in the first > > patch or even patch set going down this road but the lack of discussion > > or of any coordination between this effort and the other one that is > > trying to address the CREATEROLE issue seems likely to land us in a bad > > place with two distinct approaches being used. > > I'm confused. This patch set doesn't come within a country mile of > CREATEROLE. Why should this patch set have to coordinate with that one? I'm > not arguing with you -- merely asking what I'm misunderstanding? Perhaps it's just because I'm looking at the exact same issues cropping up here that do with the CREATEROLE situation and I'd really like to find a solution that gets us away from putting out a half-solution that won't actually be directly usable by the folks who care about making sure people don't break out of the sandbox because of the issues outlined above. Thanks, Stephen
signature.asc
Description: PGP signature
