Thread: Allowing to create LEAKPROOF functions to non-superuser
Hi hackers! This thread continues discussion of allowing something to non-superuser, AFAIK previous was [0]. Currently only superuser is allowed to create LEAKPROOF functions because leakproof functions can see tuples which have notyet been filtered out by security barrier views or row level security policies. But managed cloud services typically do not provide superuser roles. I'm thinking about allowing the database owner or someonewith BYPASSRLS flag to create these functions. Or, perhaps, pg_read_all_data. And I'm trying to figure out if there are any security implications. Consider a user who already has access to all user datain a DB and the ability to create LEAKPROOF functions. Can they gain a superuser role or access something else that isavailable only to a superuser? Is it possible to relax requirements for the creator of LEAKPROOF functions in upstream Postgres? I'll appreciate any comments. Thanks! Best regards, Andrey Borodin. [0] https://www.postgresql.org/message-id/flat/CACqFVBbx6PDq%2B%3DvHM0n78kHzn8tvOM-kGO_2q_q0zNAMT%2BTzdA%40mail.gmail.com
Andrey Borodin <x4mmm@yandex-team.ru> writes:
> Currently only superuser is allowed to create LEAKPROOF functions because leakproof functions can see tuples which
havenot yet been filtered out by security barrier views or row level security policies.
Yeah.
> But managed cloud services typically do not provide superuser roles.
This is not a good argument for relaxing superuser requirements.
regards, tom lane
On 4/12/21 10:37 PM, Tom Lane wrote: > Andrey Borodin <x4mmm@yandex-team.ru> writes: >> Currently only superuser is allowed to create LEAKPROOF functions >> because leakproof functions can see tuples which have not yet been >> filtered out by security barrier views or row level security >> policies. > > Yeah. > >> But managed cloud services typically do not provide superuser >> roles. > > This is not a good argument for relaxing superuser requirements. > I guess for the cloud services it's not an issue - they're mostly concerned about manageability and restricting access to the OS. It's unfortunate that we tie the this capability to being superuser, so maybe the right solution would be to introduce a separate role with this privilege? regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Thanks for so quick response, Tom! > 12 апр. 2021 г., в 23:37, Tom Lane <tgl@sss.pgh.pa.us> написал(а): > >> But managed cloud services typically do not provide superuser roles. > > This is not a good argument for relaxing superuser requirements. Ok, let's put aside question about relaxing requirements in upstream. Do I risk having some extra superusers in my installation if I allow everyone to create LEAKPROOF functions? Thanks! Best regards, Andrey Borodin.
Hi, On 2021-04-12 16:37:01 -0400, Tom Lane wrote: > Andrey Borodin <x4mmm@yandex-team.ru> writes: > > Currently only superuser is allowed to create LEAKPROOF functions > > because leakproof functions can see tuples which have not yet been > > filtered out by security barrier views or row level security > > policies. > > Yeah. > > > But managed cloud services typically do not provide superuser roles. > > This is not a good argument for relaxing superuser requirements. IDK. I may have been adjacent to people operating database-as-a-service for too long, but ISTM there's decent reasons for (and also against) not providing full superuser access. Even outside of managed services it seems like a decent idea to split the "can execute native code" role from the "administers an application" role. That reduces the impact a bug in the application can incur. There's certain things that are pretty intrinsically "can execute native code", like defining new 'C' functions, arbitrary ALTER SYSTEM, arbitrary file reads/writes, etc. Splitting them off from superuser is a fools errand. But it's not at all clear why adding LEAKPROOF to functions falls into that category? Greetings, Andres Freund
Hi, On 2021-04-12 22:42:03 +0200, Tomas Vondra wrote: > It's unfortunate that we tie the this capability to being superuser, > so maybe the right solution would be to introduce a separate role with > this privilege? Perhaps DB owner + BYPASSRLS would be enough? Greetings, Andres Freund
Thanks, Tomas! > 12 апр. 2021 г., в 23:42, Tomas Vondra <tomas.vondra@enterprisedb.com> написал(а): > > I guess for the cloud services it's not an issue - they're mostly > concerned about manageability and restricting access to the OS. In fact, we would happily give a client access to an OS too. It's a client's VM after all and all software is open source.But it opens a way to attack control plane. Which in turn opens a way for clients to attack each other. And we reallydo not want it. Thanks! Best regards, Andrey Borodin.
Hi, On 2021-04-12 23:51:02 +0300, Andrey Borodin wrote: > Do I risk having some extra superusers in my installation if I allow > everyone to create LEAKPROOF functions? I think that depends on what you define "superuser" to exactly be. Defining it as "has a path to executing arbitrary native code", I don't think, if implemented sensibly, allowing to set LEAKPROOF on new functions would equate superuser permissions. But you soon after might hit further limitations where lifting them would have such a risk, e.g. defining new types with in/out functions. Greetings, Andres Freund
> 13 апр. 2021 г., в 00:01, Andres Freund <andres@anarazel.de> написал(а): > > Hi, > > On 2021-04-12 23:51:02 +0300, Andrey Borodin wrote: >> Do I risk having some extra superusers in my installation if I allow >> everyone to create LEAKPROOF functions? > > I think that depends on what you define "superuser" to exactly > be. Defining it as "has a path to executing arbitrary native code", I > don't think, if implemented sensibly, allowing to set LEAKPROOF on new > functions would equate superuser permissions. Thanks! > But you soon after might > hit further limitations where lifting them would have such a risk, > e.g. defining new types with in/out functions. I think, real extensibility of a managed DB service is a very distant challenge. Currently we just allow-list extensions. Best regards, Andrey Borodin.
Andres Freund <andres@anarazel.de> writes:
> On 2021-04-12 23:51:02 +0300, Andrey Borodin wrote:
>> Do I risk having some extra superusers in my installation if I allow
>> everyone to create LEAKPROOF functions?
> I think that depends on what you define "superuser" to exactly
> be. Defining it as "has a path to executing arbitrary native code", I
> don't think, if implemented sensibly, allowing to set LEAKPROOF on new
> functions would equate superuser permissions. But you soon after might
> hit further limitations where lifting them would have such a risk,
> e.g. defining new types with in/out functions.
I think the issue here is more that superuser = "able to break the
security guarantees of the database". I doubt that falsely labeling
a function LEAKPROOF can get you more than the ability to read data
you're not supposed to be able to read ... but that ability is then
available to all users, or at least all users who can execute the
function in question. So it definitely is a fairly serious security
hazard, and one that's not well modeled by role labels. If you
give somebody e.g. pg_read_all_data privileges, you don't expect
that that means they can give it to other users.
regards, tom lane
Hi, On 2021-04-12 17:14:20 -0400, Tom Lane wrote: > I doubt that falsely labeling a function LEAKPROOF can get you more > than the ability to read data you're not supposed to be able to read > ... but that ability is then available to all users, or at least all > users who can execute the function in question. So it definitely is a > fairly serious security hazard, and one that's not well modeled by > role labels. If you give somebody e.g. pg_read_all_data privileges, > you don't expect that that means they can give it to other users. A user with BYPASSRLS can create public security definer functions returning data. If the concern is a BYPASSRLS user intentionally exposing data, then there's not a meaningful increase to allow defining LEAKPROOF functions. To me the more relevant concern is that it's hard to determine LEAKPROOF-ness and that many use-cases for BYPASSRLS do not require the target to have the technical chops to determine if a function actually is leakproof. But that seems more an argument for providing a separate control over allowing to specify LEAKPROOF than against separating it from superuser. Greetings, Andres Freund
On Mon, Apr 12, 2021 at 02:35:27PM -0700, Andres Freund wrote: > On 2021-04-12 17:14:20 -0400, Tom Lane wrote: > > I doubt that falsely labeling a function LEAKPROOF can get you more > > than the ability to read data you're not supposed to be able to read > > ... but that ability is then available to all users, or at least all > > users who can execute the function in question. So it definitely is a > > fairly serious security hazard, and one that's not well modeled by > > role labels. If you give somebody e.g. pg_read_all_data privileges, > > you don't expect that that means they can give it to other users. I do expect that, essentially. Like Andres describes for BYPASSRLS, they can create and GRANT a SECURITY DEFINER function that performs an arbitrary query and returns a refcursor (or stores the data to a table of the caller's choosing, etc.). Unlike BYPASSRLS, they can even make pg_read_all_data own the function, making the situation persist after one drops the actor's role and that role's objects. > A user with BYPASSRLS can create public security definer functions > returning data. If the concern is a BYPASSRLS user intentionally > exposing data, then there's not a meaningful increase to allow defining > LEAKPROOF functions. Hence, I do find it reasonable to let pg_read_all_data be sufficient for setting LEAKPROOF. I would not consult datdba, because datdba currently has no special read abilities. It feels too weird to let BYPASSRLS start affecting non-RLS access controls. A reasonable person may assume that BYPASSRLS has no consequences until someone uses CREATE POLICY. That said, I wouldn't be horrified if BYPASSRLS played a part. BYPASSRLS, like pg_read_all_data, clearly isn't something to grant lightly.
On Fri, Apr 16, 2021 at 3:57 AM Noah Misch <noah@leadboat.com> wrote: > On Mon, Apr 12, 2021 at 02:35:27PM -0700, Andres Freund wrote: > > On 2021-04-12 17:14:20 -0400, Tom Lane wrote: > > > I doubt that falsely labeling a function LEAKPROOF can get you more > > > than the ability to read data you're not supposed to be able to read > > > ... but that ability is then available to all users, or at least all > > > users who can execute the function in question. So it definitely is a > > > fairly serious security hazard, and one that's not well modeled by > > > role labels. If you give somebody e.g. pg_read_all_data privileges, > > > you don't expect that that means they can give it to other users. > > I do expect that, essentially. Like Andres describes for BYPASSRLS, they can > create and GRANT a SECURITY DEFINER function that performs an arbitrary query > and returns a refcursor (or stores the data to a table of the caller's > choosing, etc.). Unlike BYPASSRLS, they can even make pg_read_all_data own > the function, making the situation persist after one drops the actor's role > and that role's objects. Yes. I think that if someone can read all the data, it's unworkable to suppose that they can't find a way to delegate that ability to others. If nothing else, a station wagon full of tapes has a lot of bandwidth. > > A user with BYPASSRLS can create public security definer functions > > returning data. If the concern is a BYPASSRLS user intentionally > > exposing data, then there's not a meaningful increase to allow defining > > LEAKPROOF functions. > > Hence, I do find it reasonable to let pg_read_all_data be sufficient for > setting LEAKPROOF. I would not consult datdba, because datdba currently has > no special read abilities. It feels too weird to let BYPASSRLS start > affecting non-RLS access controls. A reasonable person may assume that > BYPASSRLS has no consequences until someone uses CREATE POLICY. That said, I > wouldn't be horrified if BYPASSRLS played a part. BYPASSRLS, like > pg_read_all_data, clearly isn't something to grant lightly. I agree that datdba doesn't seem like quite the right thing, but I'm not sure I agree with the rest. How can we say that leakproof is a non-RLS access control? Its only purpose is to keep RLS secure, so I guess I'd be inclined to think that of the two, BYPASSRLS is more closely related to the topic at hand. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Apr 16, 2021 at 3:57 AM Noah Misch <noah@leadboat.com> wrote:
>> Hence, I do find it reasonable to let pg_read_all_data be sufficient for
>> setting LEAKPROOF. I would not consult datdba, because datdba currently has
>> no special read abilities. It feels too weird to let BYPASSRLS start
>> affecting non-RLS access controls. A reasonable person may assume that
>> BYPASSRLS has no consequences until someone uses CREATE POLICY. That said, I
>> wouldn't be horrified if BYPASSRLS played a part. BYPASSRLS, like
>> pg_read_all_data, clearly isn't something to grant lightly.
> I agree that datdba doesn't seem like quite the right thing, but I'm
> not sure I agree with the rest. How can we say that leakproof is a
> non-RLS access control? Its only purpose is to keep RLS secure, so I
> guess I'd be inclined to think that of the two, BYPASSRLS is more
> closely related to the topic at hand.
Umm ... I'm pretty sure LEAKPROOF also affects optimization around
"security barrier" views, which I wouldn't call RLS. Out of these
options, I'd prefer granting the ability to pg_read_all_data.
regards, tom lane
On Mon, Apr 19, 2021 at 4:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Fri, Apr 16, 2021 at 3:57 AM Noah Misch <noah@leadboat.com> wrote: > >> Hence, I do find it reasonable to let pg_read_all_data be sufficient for > >> setting LEAKPROOF. I would not consult datdba, because datdba currently has > >> no special read abilities. It feels too weird to let BYPASSRLS start > >> affecting non-RLS access controls. A reasonable person may assume that > >> BYPASSRLS has no consequences until someone uses CREATE POLICY. That said, I > >> wouldn't be horrified if BYPASSRLS played a part. BYPASSRLS, like > >> pg_read_all_data, clearly isn't something to grant lightly. > > > I agree that datdba doesn't seem like quite the right thing, but I'm > > not sure I agree with the rest. How can we say that leakproof is a > > non-RLS access control? Its only purpose is to keep RLS secure, so I > > guess I'd be inclined to think that of the two, BYPASSRLS is more > > closely related to the topic at hand. > > Umm ... I'm pretty sure LEAKPROOF also affects optimization around > "security barrier" views, which I wouldn't call RLS. Out of these > options, I'd prefer granting the ability to pg_read_all_data. Oops, I forgot about security_barrier views, which is rather embarrassing since I committed them. So, yeah, I agree: pg_read_all_data is better. -- Robert Haas EDB: http://www.enterprisedb.com
Greetings, * Robert Haas (robertmhaas@gmail.com) wrote: > On Mon, Apr 19, 2021 at 4:32 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Robert Haas <robertmhaas@gmail.com> writes: > > > On Fri, Apr 16, 2021 at 3:57 AM Noah Misch <noah@leadboat.com> wrote: > > >> Hence, I do find it reasonable to let pg_read_all_data be sufficient for > > >> setting LEAKPROOF. I would not consult datdba, because datdba currently has > > >> no special read abilities. It feels too weird to let BYPASSRLS start > > >> affecting non-RLS access controls. A reasonable person may assume that > > >> BYPASSRLS has no consequences until someone uses CREATE POLICY. That said, I > > >> wouldn't be horrified if BYPASSRLS played a part. BYPASSRLS, like > > >> pg_read_all_data, clearly isn't something to grant lightly. > > > > > I agree that datdba doesn't seem like quite the right thing, but I'm > > > not sure I agree with the rest. How can we say that leakproof is a > > > non-RLS access control? Its only purpose is to keep RLS secure, so I > > > guess I'd be inclined to think that of the two, BYPASSRLS is more > > > closely related to the topic at hand. > > > > Umm ... I'm pretty sure LEAKPROOF also affects optimization around > > "security barrier" views, which I wouldn't call RLS. Out of these > > options, I'd prefer granting the ability to pg_read_all_data. > > Oops, I forgot about security_barrier views, which is rather > embarrassing since I committed them. So, yeah, I agree: > pg_read_all_data is better. I'm not really sure that attaching it to pg_read_all_data makes sense.. In general, I've been frustrated by the places where we lump privileges together rather than having a way to distinctly GRANT capabilities independently- that's more-or-less exactly what lead me to work on implementing the role system in the first place, and later the predefined roles. I do think it's good to reduce the number of places that require superuser, in general, but I'm not sure that marking functions as leakproof as a non-superuser makes sense. Here's what I'd ask Andrey- what's the actual use-case here? Are these cases where users are actually adding new functions which they believe are leakproof where those functions don't require superuser already to be created? Clearly if they're in a untrusted language and you have to be a superuser to install them in the first place then they should just mark the function as leakproof when they install it. If these are trusted language functions, I'd be curious to actually see them as I have doubts about if they're actually leakproof.. Or is the actual use-case here that they just want to mark functions we know aren't leakproof as leakproof anyway because they aren't getting the performance they want? If it's the latter, as I suspect it is, then I don't really think the use-case justifies any change on our part- the right answer is to make those functions actually leakproof, or write ones which are. Thanks, Stephen
Attachment
On Mon, Apr 19, 2021 at 05:38:43PM -0400, Stephen Frost wrote: > > > > On Fri, Apr 16, 2021 at 3:57 AM Noah Misch <noah@leadboat.com> wrote: > > > >> Hence, I do find it reasonable to let pg_read_all_data be sufficient for > > > >> setting LEAKPROOF. > I'm not really sure that attaching it to pg_read_all_data makes sense.. > > In general, I've been frustrated by the places where we lump privileges > together rather than having a way to distinctly GRANT capabilities > independently- that's more-or-less exactly what lead me to work on > implementing the role system in the first place, and later the > predefined roles. This would be no more lumpy than e.g. pg_read_all_stats. However, I could live with a separate pg_change_leakproof (or whatever name). > Here's what I'd ask Andrey- what's the actual use-case here? Are these > cases where users are actually adding new functions which they believe > are leakproof where those functions don't require superuser already to > be created? Clearly if they're in a untrusted language and you have to > be a superuser to install them in the first place then they should just > mark the function as leakproof when they install it. If these are > trusted language functions, I'd be curious to actually see them as I > have doubts about if they're actually leakproof.. > > Or is the actual use-case here that they just want to mark functions we > know aren't leakproof as leakproof anyway because they aren't getting > the performance they want? Hearing those answers would be interesting, but it shouldn't change decisions. A reasonable person can write an actually-leakproof function and wish to mark it LEAKPROOF, independent of whether that happened in the case that prompted this thread.
> 20 апр. 2021 г., в 02:38, Stephen Frost <sfrost@snowman.net> написал(а): > > Here's what I'd ask Andrey- what's the actual use-case here? Are these > cases where users are actually adding new functions which they believe > are leakproof where those functions don't require superuser already to > be created? Clearly if they're in a untrusted language and you have to > be a superuser to install them in the first place then they should just > mark the function as leakproof when they install it. If these are > trusted language functions, I'd be curious to actually see them as I > have doubts about if they're actually leakproof.. > > Or is the actual use-case here that they just want to mark functions we > know aren't leakproof as leakproof anyway because they aren't getting > the performance they want? > > If it's the latter, as I suspect it is, then I don't really think the > use-case justifies any change on our part- the right answer is to make > those functions actually leakproof, or write ones which are. Customer was restoring pg_dump of on-premise ERP known as 1C (something like TurboTax) with this add-on [0] CREATE FUNCTION simple1c.date_from_guid(varchar(36)) RETURNS timestamp LANGUAGE plpgsql IMMUTABLE LEAKPROOF STRICT I'm not 1C-expert (programmed it a bit to get few bucks when I was a student), but seems like this function simple1c.date_from_guid()can be used in DSL queries. It have no obvious side effects. Maybe we could hack it by exploitingtimestamp overflow, but I doubt it's practically usable. Thanks! Best regards, Andrey Borodin. [0] https://github.com/ivan816/simple-1c/blob/f2e5ce78b98f70f30039fd3de79308a59d432fc2/Simple1C/Impl/Sql/SchemaMapping/Simple1cSchemaCreator.cs#L74
Greetings, * Andrey Borodin (x4mmm@yandex-team.ru) wrote: > > 20 апр. 2021 г., в 02:38, Stephen Frost <sfrost@snowman.net> написал(а): > > Here's what I'd ask Andrey- what's the actual use-case here? Are these > > cases where users are actually adding new functions which they believe > > are leakproof where those functions don't require superuser already to > > be created? Clearly if they're in a untrusted language and you have to > > be a superuser to install them in the first place then they should just > > mark the function as leakproof when they install it. If these are > > trusted language functions, I'd be curious to actually see them as I > > have doubts about if they're actually leakproof.. > > > > Or is the actual use-case here that they just want to mark functions we > > know aren't leakproof as leakproof anyway because they aren't getting > > the performance they want? > > > > If it's the latter, as I suspect it is, then I don't really think the > > use-case justifies any change on our part- the right answer is to make > > those functions actually leakproof, or write ones which are. > > Customer was restoring pg_dump of on-premise ERP known as 1C (something like TurboTax) with this add-on [0] > > CREATE FUNCTION simple1c.date_from_guid(varchar(36)) RETURNS timestamp LANGUAGE plpgsql IMMUTABLE LEAKPROOF STRICT > > I'm not 1C-expert (programmed it a bit to get few bucks when I was a student), but seems like this function simple1c.date_from_guid()can be used in DSL queries. It have no obvious side effects. Maybe we could hack it by exploitingtimestamp overflow, but I doubt it's practically usable. Erm, it's very clearly not leakproof and will happily return information about the value passed in during some error cases... Thanks, Stephen
Attachment
Greetings, * Noah Misch (noah@leadboat.com) wrote: > On Mon, Apr 19, 2021 at 05:38:43PM -0400, Stephen Frost wrote: > > > > > On Fri, Apr 16, 2021 at 3:57 AM Noah Misch <noah@leadboat.com> wrote: > > > > >> Hence, I do find it reasonable to let pg_read_all_data be sufficient for > > > > >> setting LEAKPROOF. > > > I'm not really sure that attaching it to pg_read_all_data makes sense.. > > > > In general, I've been frustrated by the places where we lump privileges > > together rather than having a way to distinctly GRANT capabilities > > independently- that's more-or-less exactly what lead me to work on > > implementing the role system in the first place, and later the > > predefined roles. > > This would be no more lumpy than e.g. pg_read_all_stats. However, I could > live with a separate pg_change_leakproof (or whatever name). There's been already some disagreements about pg_read_all_stats, so I don't think that is actually a good model to look at. I have doubts about users generally being able to write actually leakproof functions (this case being an example of someone writing a function that certainly wasn't leakproof but marking it as such anyway...), though I suppose it's unlikely that it's any worse than the cases of people writing SECURITY DEFINER functions that aren't careful enough, of which I've seen plenty of. I would think the role/capability would be 'pg_mark_function_leakproof' or similar though, and allow a user who had that role to either create leakproof functions (provided they have access to create the function in the first place) or to mark an existing function as leakproof (but requiring them to be a member of the role which owns the function). Thanks, Stephen
Attachment
Stephen Frost <sfrost@snowman.net> writes:
> * Andrey Borodin (x4mmm@yandex-team.ru) wrote:
>> Customer was restoring pg_dump of on-premise ERP known as 1C (something like TurboTax) with this add-on [0]
> Erm, it's very clearly not leakproof and will happily return information
> about the value passed in during some error cases...
Yeah, that's pretty much a poster-child example for NOT letting
random users fool with leakproofness settings.
regards, tom lane
On Sun, Apr 25, 2021 at 02:40:54PM -0400, Stephen Frost wrote: > * Noah Misch (noah@leadboat.com) wrote: > > On Mon, Apr 19, 2021 at 05:38:43PM -0400, Stephen Frost wrote: > > > > > > On Fri, Apr 16, 2021 at 3:57 AM Noah Misch <noah@leadboat.com> wrote: > > > > > >> Hence, I do find it reasonable to let pg_read_all_data be sufficient for > > > > > >> setting LEAKPROOF. > > > > > I'm not really sure that attaching it to pg_read_all_data makes sense.. > > > > > > In general, I've been frustrated by the places where we lump privileges > > > together rather than having a way to distinctly GRANT capabilities > > > independently- that's more-or-less exactly what lead me to work on > > > implementing the role system in the first place, and later the > > > predefined roles. > > > > This would be no more lumpy than e.g. pg_read_all_stats. However, I could > > live with a separate pg_change_leakproof (or whatever name). > > There's been already some disagreements about pg_read_all_stats, so I > don't think that is actually a good model to look at. > > I have doubts about users generally being able to write actually > leakproof functions (this case being an example of someone writing a > function that certainly wasn't leakproof but marking it as such > anyway...), though I suppose it's unlikely that it's any worse than the > cases of people writing SECURITY DEFINER functions that aren't careful > enough, of which I've seen plenty of. Making "it's hard to do well" imply "only superusers get to try" doesn't mitigate a risk; it multiplies risks. > I would think the role/capability would be 'pg_mark_function_leakproof' > or similar though, and allow a user who had that role to either create > leakproof functions (provided they have access to create the function in > the first place) or to mark an existing function as leakproof (but > requiring them to be a member of the role which owns the function). That's fine.