Thread: allow building trusted languages without the untrusted versions
Hi hackers, Presently, if you want to only build trusted PL/Perl and PL/Tcl, you need to make a couple of code changes to compile out the untrusted parts. I suspect many users (e.g., anyone who wants to disallow file system access) would benefit from a better supported way to do this. Thus, I've attached some patches that introduce an optional argument for the --with-perl and --with-tcl configuration options. This new argument can be used to build only the trusted or untrusted version of the language. If the argument is not provided, both the trusted and untrusted versions are built, so this change is backward compatible. The PL/Tcl patch (0003) is relatively straightforward, as there are already separate handler functions for the trusted and untrusted versions of the language. PL/Perl, however, is slightly more complicated. 0001 first modifies PL/Perl to use separate handle/validator functions for the trusted and untrusted versions. 0002 then adds support for building only trusted or untrusted PL/Perl in a similar fashion to 0003. Since a few contrib modules depend on PL/Perl, 0002 also modifies some modules' Makefiles to handle whether trusted and/or untrusted PL/Perl is built. I haven't made the required changes (if any) for MSVC, as I do not currently have a way to test it. For now, I am parking these patches in the July commitfest while I gauge interest in this feature and await any feedback on the proposed approach. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Attachment
Nathan Bossart <nathandbossart@gmail.com> writes: > Presently, if you want to only build trusted PL/Perl and PL/Tcl, you need > to make a couple of code changes to compile out the untrusted parts. I > suspect many users (e.g., anyone who wants to disallow file system access) > would benefit from a better supported way to do this. Thus, I've attached > some patches that introduce an optional argument for the --with-perl and > --with-tcl configuration options. This new argument can be used to build > only the trusted or untrusted version of the language. If the argument is > not provided, both the trusted and untrusted versions are built, so this > change is backward compatible. I do not believe that this is worth the extra complication. Nobody has ever asked for it before, so I estimate the number of people who would use it as near zero, and those folk are entirely capable of removing the relevant extension files from their installations. Moreover, if we accept this as a useful configure option, what other things will we be called on to change? It surely makes no sense to install contrib/adminpack, for instance, if you're afraid of having plperlu installed. Lastly, you've offered no reason to think this would provide any real security improvement. Someone who's gained the ability to issue CREATE EXTENSION on untrusted extensions has already got all the privileges he needs; leaving out a few extension files is at most going to slow him down a bit on the way to full filesystem access. (See, eg, COPY TO PROGRAM.) regards, tom lane
On Fri, May 20, 2022 at 08:20:11PM -0400, Tom Lane wrote: > Nathan Bossart <nathandbossart@gmail.com> writes: >> Presently, if you want to only build trusted PL/Perl and PL/Tcl, you need >> to make a couple of code changes to compile out the untrusted parts. I >> suspect many users (e.g., anyone who wants to disallow file system access) >> would benefit from a better supported way to do this. Thus, I've attached >> some patches that introduce an optional argument for the --with-perl and >> --with-tcl configuration options. This new argument can be used to build >> only the trusted or untrusted version of the language. If the argument is >> not provided, both the trusted and untrusted versions are built, so this >> change is backward compatible. > > I do not believe that this is worth the extra complication. Nobody has > ever asked for it before, so I estimate the number of people who would > use it as near zero, and those folk are entirely capable of removing > the relevant extension files from their installations. Of course, if there is no interest in this feature, I'll withdraw the patch from consideration. However, I will note that moving the extension files aside is not sufficient for blocking all use of untrusted languages, since the symbols for their handler/validator functions will still be present. > Moreover, if we accept this as a useful configure option, what other > things will we be called on to change? It surely makes no sense to > install contrib/adminpack, for instance, if you're afraid of having > plperlu installed. > > Lastly, you've offered no reason to think this would provide any real > security improvement. Someone who's gained the ability to issue CREATE > EXTENSION on untrusted extensions has already got all the privileges he > needs; leaving out a few extension files is at most going to slow him > down a bit on the way to full filesystem access. (See, eg, COPY TO > PROGRAM.) I'd like to provide the ability to disallow these other things, too. This is intended to be a first step in that direction. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes: > On Fri, May 20, 2022 at 08:20:11PM -0400, Tom Lane wrote: >> Lastly, you've offered no reason to think this would provide any real >> security improvement. Someone who's gained the ability to issue CREATE >> EXTENSION on untrusted extensions has already got all the privileges he >> needs; leaving out a few extension files is at most going to slow him >> down a bit on the way to full filesystem access. (See, eg, COPY TO >> PROGRAM.) > I'd like to provide the ability to disallow these other things, too. This > is intended to be a first step in that direction. There would probably be some interest in a "--disable-disk-access" configure option that did all of this stuff (and some more things too), with the aim of locking down *all* known paths to filesystem access. I don't see much value in retail options that do some of that. In fact, what they might mostly accomplish is to give people a false sense of security. regards, tom lane
On Mon, May 23, 2022 at 01:17:08PM -0400, Tom Lane wrote: > There would probably be some interest in a "--disable-disk-access" > configure option that did all of this stuff (and some more things > too), with the aim of locking down *all* known paths to filesystem > access. I don't see much value in retail options that do some of that. > In fact, what they might mostly accomplish is to give people a false > sense of security. That's a reasonable point. I'll go ahead an explore some options for something along those lines. A couple of questions immediately come to mind. For example, should this configuration option just cause these functions to ERROR, or should it compile them out? -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes: > That's a reasonable point. I'll go ahead an explore some options for > something along those lines. A couple of questions immediately come to > mind. For example, should this configuration option just cause these > functions to ERROR, or should it compile them out? Letting them be present but throw error is likely to be far less painful than the other way, because then you don't need a separate set of SQL-visible object definitions. You could, in fact, imagine jacking up an existing database and driving a set of locked-down binaries under it --- or vice versa. If there have to be different versions of the extension SQL files for the two cases then everything gets way hairier, both for developers and users. regards, tom lane
On Mon, May 23, 2022 at 02:20:02PM -0400, Tom Lane wrote: > Nathan Bossart <nathandbossart@gmail.com> writes: >> That's a reasonable point. I'll go ahead an explore some options for >> something along those lines. A couple of questions immediately come to >> mind. For example, should this configuration option just cause these >> functions to ERROR, or should it compile them out? > > Letting them be present but throw error is likely to be far less > painful than the other way, because then you don't need a separate > set of SQL-visible object definitions. You could, in fact, imagine > jacking up an existing database and driving a set of locked-down > binaries under it --- or vice versa. If there have to be different > versions of the extension SQL files for the two cases then everything > gets way hairier, both for developers and users. Agreed. I'll do it that way. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Mon, May 23, 2022 at 1:17 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > There would probably be some interest in a "--disable-disk-access" > configure option that did all of this stuff (and some more things > too), with the aim of locking down *all* known paths to filesystem > access. I don't see much value in retail options that do some of that. > In fact, what they might mostly accomplish is to give people a false > sense of security. I definitely think there's a need for a user who can manipulate objects in the database much like a superuser (i.e. access all objects, grant and revoke all privileges, etc.) but who can't break out into the OS user account and assume it's privileges. I'm not sure whether it's best to try to get there by creating a mode where the superuser's privileges are trimmed back, or to get there by still having a super-user who is just as super as at present but then also have the ability to create other users who are not superusers but have many of the same privileges with respect to in-database objects. It seems to me that you've got to think not only about vectors for executing arbitrary C code and/or shell commands, but also the superuser's power to mess with the catalogs. If you can UPDATE pg_proc, you can certainly hack the system, I think. But that isn't really implied by --disable-disk-access, which makes me think that's not really the right way of thinking about it. In my mind, it's reasonable as a matter of security policy to decide that you don't ever want plperlu on your system, only plperl. And it's reasonable to decide whether or not you also need some kind of restricted super-user facility. They're just two different issues. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes: > I definitely think there's a need for a user who can manipulate > objects in the database much like a superuser (i.e. access all > objects, grant and revoke all privileges, etc.) but who can't break > out into the OS user account and assume it's privileges. I'm not sure > whether it's best to try to get there by creating a mode where the > superuser's privileges are trimmed back, or to get there by still > having a super-user who is just as super as at present but then also > have the ability to create other users who are not superusers but have > many of the same privileges with respect to in-database objects. Maybe I shouldn't be putting words into Nathan's mouth, but I think what he is after is a mode intended for use by cloud service providers, who would like to offer locked-down database services where there's just no way to get to the disk from inside the DB, superuser or no. What you're talking about is perhaps interesting to a different set of people, but it doesn't offer any guarantees because it's always possible that $attacker manages to hack his way into access to a superuser role. > It seems to me that you've got to think not only about vectors for > executing arbitrary C code and/or shell commands, but also the > superuser's power to mess with the catalogs. If you can UPDATE > pg_proc, you can certainly hack the system, I think. I think if all the functions that would let you get to the disk are disabled at the C-code level, it doesn't much matter what the catalogs say about them. The main flaw I'm aware of in that argument is that it used to be possible for superusers to create C-language pg_proc entries pointing at random C entry point symbols, eg open(2) or write(2), and then invoke those functions from SQL --- maybe with only restricted possibilities for the arguments, but you just need to find one combination that works. When we got rid of v0 function call support, that became at least far more difficult to exploit, but I'm not sure if it's entirely impossible. A component of this exercise would need to be making sure that that's bulletproof, ie you can't make a usable pg_proc entry that points at something that wasn't meant to be a SQL-callable function. > In my mind, it's > reasonable as a matter of security policy to decide that you don't > ever want plperlu on your system, only plperl. Absolutely, but for that you can just not install plperlu's extension support files. If you're concerned about whether that decision is un-hackable, then you soon realize that you need a bulletproof no-disk-access restriction. regards, tom lane
On Mon, May 23, 2022 at 4:46 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > Maybe I shouldn't be putting words into Nathan's mouth, but I think > what he is after is a mode intended for use by cloud service providers, > who would like to offer locked-down database services where there's > just no way to get to the disk from inside the DB, superuser or no. The cloud service provider use case is also what I was thinking about. > What you're talking about is perhaps interesting to a different set of > people, but it doesn't offer any guarantees because it's always possible > that $attacker manages to hack his way into access to a superuser role. I mean, you can hypothesize that any sort of restriction can be bypassed, regardless of how they're implemented. I don't think this is a valid way of discriminating among possible solutions. > The main flaw I'm aware of in that argument is that it used to be possible > for superusers to create C-language pg_proc entries pointing at random C > entry point symbols, eg open(2) or write(2), and then invoke those > functions from SQL --- maybe with only restricted possibilities for the > arguments, but you just need to find one combination that works. > When we got rid of v0 function call support, that became at least far > more difficult to exploit, but I'm not sure if it's entirely impossible. > A component of this exercise would need to be making sure that that's > bulletproof, ie you can't make a usable pg_proc entry that points at > something that wasn't meant to be a SQL-callable function. It's not just a question of whether it was meant to be SQL-callable -- it's also a question of what arguments it was expecting to be called with. At the very least, you can cause the server to core dump if you pass something that isn't a valid pointer to a function that is expecting a pointer, which is something a CSP very likely does not want a customer to be able to do. I think, however, that there's every possibility that you can create more havoc than that. You can basically call a function that's expecting a pointer with a pointer to anything you can find or guess the memory address of. Maybe that's not enough control to cause anything worse than a server crash, but I sure wouldn't bet on it. There's a lot of functions floating around, and if none of them can be tricked into doing filesystem access today, well someone might add a new one tomorrow. -- Robert Haas EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes: > It's not just a question of whether it was meant to be SQL-callable -- > it's also a question of what arguments it was expecting to be called > with. At the very least, you can cause the server to core dump if you > pass something that isn't a valid pointer to a function that is > expecting a pointer, which is something a CSP very likely does not > want a customer to be able to do. I think, however, that there's every > possibility that you can create more havoc than that. You can > basically call a function that's expecting a pointer with a pointer to > anything you can find or guess the memory address of. Maybe that's not > enough control to cause anything worse than a server crash, but I sure > wouldn't bet on it. There's a lot of functions floating around, and if > none of them can be tricked into doing filesystem access today, well > someone might add a new one tomorrow. [ shrug... ] So is your point that we shouldn't bother to do anything? I don't personally have a problem with leaving things where they stand in this area. However, if we're going to do something, I think at minimum it should involve blocking off everything we can identify as straightforward reproducible methods to get disk access. regards, tom lane
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > It's not just a question of whether it was meant to be SQL-callable -- > > it's also a question of what arguments it was expecting to be called > > with. At the very least, you can cause the server to core dump if you > > pass something that isn't a valid pointer to a function that is > > expecting a pointer, which is something a CSP very likely does not > > want a customer to be able to do. I think, however, that there's every > > possibility that you can create more havoc than that. You can > > basically call a function that's expecting a pointer with a pointer to > > anything you can find or guess the memory address of. Maybe that's not > > enough control to cause anything worse than a server crash, but I sure > > wouldn't bet on it. There's a lot of functions floating around, and if > > none of them can be tricked into doing filesystem access today, well > > someone might add a new one tomorrow. > > [ shrug... ] So is your point that we shouldn't bother to do anything? > I don't personally have a problem with leaving things where they stand > in this area. However, if we're going to do something, I think at > minimum it should involve blocking off everything we can identify as > straightforward reproducible methods to get disk access. I have a hard time seeing the value in allowing catalog hacking, even for a cloud provider, and making sure to cut off all possible ways that could be abused strikes me as unlikely to be successful. Instead, I'd argue that we should be continuing to work in the direction of splitting up what can only be done by a superuser today using predefined roles and other methods along those lines. How that lines up with this latest ask around untrusted languages is something I'm not exactly sure about, but a magic configure option that is "--don't-allow-what-AWS-doesn't-want-to-allow" certainly doesn't seem like it's going in the right direction (and, no, not every cloud provider is going to want the exact same thing when it comes to whatever this option is that we're talking about, so we'd end up having to have configure options for each if we start going down this road...). I agree with the general idea of "has all of today's superuser rights except the ability to hack catalogs or do disk access" being one use-case we should be thinking about, along with "also can't do network access" and "allowed to do network or disk access but can't directly hack up the catalog", but I don't see us growing configure options for all these things and would much rather we have a way to let folks configure their systems along these different lines, ideally without having to make that decision at build time. Thanks, Stephen
Attachment
On Mon, May 23, 2022 at 07:09:03PM -0400, Stephen Frost wrote: > Instead, I'd argue that we should be continuing to work in the direction > of splitting up what can only be done by a superuser today using > predefined roles and other methods along those lines. How that lines up > with this latest ask around untrusted languages is something I'm not > exactly sure about, but a magic configure option that is > "--don't-allow-what-AWS-doesn't-want-to-allow" certainly doesn't seem > like it's going in the right direction (and, no, not every cloud > provider is going to want the exact same thing when it comes to whatever > this option is that we're talking about, so we'd end up having to have > configure options for each if we start going down this road...). I guess I'd like to do both. I agree with continuing the work with predefined roles, etc., but I also think there is value in being able to compile out things that allow arbitrary disk/network access. My intent with this thread is the latter, and I'm trying to tackle this in a way that is generically useful even beyond the cloud provider use case. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On 5/23/22 8:04 PM, Nathan Bossart wrote: > On Mon, May 23, 2022 at 07:09:03PM -0400, Stephen Frost wrote: >> Instead, I'd argue that we should be continuing to work in the direction >> of splitting up what can only be done by a superuser today using >> predefined roles and other methods along those lines. How that lines up >> with this latest ask around untrusted languages is something I'm not >> exactly sure about, but a magic configure option that is >> "--don't-allow-what-AWS-doesn't-want-to-allow" certainly doesn't seem >> like it's going in the right direction (and, no, not every cloud >> provider is going to want the exact same thing when it comes to whatever >> this option is that we're talking about, so we'd end up having to have >> configure options for each if we start going down this road...). > > I guess I'd like to do both. I agree with continuing the work with > predefined roles, etc., but I also think there is value in being able to > compile out things that allow arbitrary disk/network access. My intent > with this thread is the latter, and I'm trying to tackle this in a way that > is generically useful even beyond the cloud provider use case. (+1 on continuing to split up superuser into other predefined roles and other privs) For other use cases, I suggest considering PostgreSQL deployments in environments that run on restricted filesystems, e.g. containers. When configured properly, a containerized filesystem will disallow writes outside of the data directory. However, a) they typically only restrict writes (which is often good enough) and b) this model holds so long as there are no exploits in the container itself. The latter may not be our problem, but we can provide an additional risk mitigation for folks who deploy PostgreSQL in containers or other restricted environments the option to compile out features that do allow arbitrary disk access. I agree with a bunch of the upthread sentiment, but I would ask if the current proposal provides acceptable risk mitigation for PostgreSQL deployments who want to restrict users having access to the filesystem? Jonathan
Attachment
On Mon, May 23, 2022 at 6:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > [ shrug... ] So is your point that we shouldn't bother to do anything? > I don't personally have a problem with leaving things where they stand > in this area. However, if we're going to do something, I think at > minimum it should involve blocking off everything we can identify as > straightforward reproducible methods to get disk access. No, my point is that one size doesn't fit all. Bundling everything together that could result in a disk access is going to suck too many marginally-related into the same bucket. It's much better to have individual switches controlling individual behaviors, so that people can opt into or out of the behavior that they want. I would argue that Stephen's proposal (that is, using predefined roles more) and Nathan's proposal (that is, making it possible to build only the trusted version of some PL) are tackling this problem are far superior to your idea (that is, a flag to disable all disk access) precisely because they are more granular. Your idea appears to presuppose that there is exactly one thing in this area that anybody wants and that we know what that thing is. I think people want a bunch of slightly different things and that we're probably unaware of many of them. Letting them pick which behaviors they want seems to me to make a lot of sense. -- Robert Haas EDB: http://www.enterprisedb.com
On Tue, May 24, 2022 at 12:39:16PM -0400, Robert Haas wrote: > On Mon, May 23, 2022 at 6:42 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> [ shrug... ] So is your point that we shouldn't bother to do anything? >> I don't personally have a problem with leaving things where they stand >> in this area. However, if we're going to do something, I think at >> minimum it should involve blocking off everything we can identify as >> straightforward reproducible methods to get disk access. > > No, my point is that one size doesn't fit all. Bundling everything > together that could result in a disk access is going to suck too many > marginally-related into the same bucket. It's much better to have > individual switches controlling individual behaviors, so that people > can opt into or out of the behavior that they want. > > I would argue that Stephen's proposal (that is, using predefined roles > more) and Nathan's proposal (that is, making it possible to build only > the trusted version of some PL) are tackling this problem are far > superior to your idea (that is, a flag to disable all disk access) > precisely because they are more granular. Your idea appears to > presuppose that there is exactly one thing in this area that anybody > wants and that we know what that thing is. I think people want a bunch > of slightly different things and that we're probably unaware of many > of them. Letting them pick which behaviors they want seems to me to > make a lot of sense. Can we do both? That is, can we add retail options for untrusted languages, generic file access functions, etc., and then also introduce a --disable-disk-access configuration option? The latter might even just be a combination of retail options. This would allow for more granular configurations, but it also could help address Tom's concerns. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes: > On Tue, May 24, 2022 at 12:39:16PM -0400, Robert Haas wrote: >> No, my point is that one size doesn't fit all. Bundling everything >> together that could result in a disk access is going to suck too many >> marginally-related into the same bucket. It's much better to have >> individual switches controlling individual behaviors, so that people >> can opt into or out of the behavior that they want. > Can we do both? That is, can we add retail options for untrusted > languages, generic file access functions, etc., and then also introduce a > --disable-disk-access configuration option? The latter might even just be > a combination of retail options. This would allow for more granular > configurations, but it also could help address Tom's concerns. Don't see why not. I'm a bit skeptical of Robert's position, mainly because I don't think he's offered any credible threat model that would justify disabling individual features of this sort but not all of them. However, if what it takes to have consensus is some individual knobs in addition to an "easy button", let's do it that way. regards, tom lane
On Tue, May 24, 2022 at 1:28 PM Nathan Bossart <nathandbossart@gmail.com> wrote: > Can we do both? That is, can we add retail options for untrusted > languages, generic file access functions, etc., and then also introduce a > --disable-disk-access configuration option? The latter might even just be > a combination of retail options. This would allow for more granular > configurations, but it also could help address Tom's concerns. Oh, sure. We're in charge around here. We can do whatever we want! The only trick is agreeing with each other to a sufficient degree to get something done ... and of course the small matter of writing the code. I guess one question is at what level we want to disable these various things. Your original proposal seemed reasonable to me because I feel like users who are compiling PostgreSQL ought to have control over which things they compile. If you can turn plperl and plperlu off together, and you can, then why shouldn't you be able to turn them on and off separately? I can't think of a good reason why we shouldn't make that possible if people want it, and evidently at least one person does: you. I'm even willing to assume that you represent the interests of some larger group of people. :-) But it's not evident to me that it's useful to disable everything specifically at compile time. I have long thought that it's pretty bizarre that we permit DML on system catalogs even with allow_system_table_mods=off, and if I were going to provide a way to lock that down, I would think of doing it via a new GUC, or a modification to the existing GUC, or something like that, rather than a compile-time option -- because we might easily discover a problem in a future release that requires catalog DML to fix, and you wouldn't want to have to replace the binaries or even bounce the server to do that. And similarly, is it really want we want here to categorically disable all functions that permit file access for all users under all circumstances? Or do we maybe want that to be something that can be reconfigured at runtime? Or even just make it a privilege extended to some users but not others? What about COPY TO/FROM PROGRAM? Anyway, I'm not desperately opposed to the idea of having a PostgreSQL mode that locks a whole lotta crap down at configure time, but I bet it's going to be (1) hard to get agreement that all of the relevant stuff is actually worth including and (2) kinda inconvenient not to be able to change any of that behavior without replacing the binaries. I do agree that there are SOME things where people are going to explicitly want that stuff to be unchangeable without replacing the binaries, and that's fine. I'm just not sure that's what people are going to want in all cases. -- Robert Haas EDB: http://www.enterprisedb.com
On Tue, May 24, 2022 at 02:10:19PM -0400, Robert Haas wrote: > I guess one question is at what level we want to disable these various > things. Your original proposal seemed reasonable to me because I feel > like users who are compiling PostgreSQL ought to have control over > which things they compile. If you can turn plperl and plperlu off > together, and you can, then why shouldn't you be able to turn them on > and off separately? I can't think of a good reason why we shouldn't > make that possible if people want it, and evidently at least one > person does: you. I'm even willing to assume that you represent the > interests of some larger group of people. :-) :) FWIW this was my original thinking. I can choose to build/install extensions separately, but when it comes to PL/Tcl and PL/Perl, you've got to build the trusted and untrusted stuff at the same time, and the untrusted symbols remain even if you remove the control file and installation scripts. Of course, this isn't a complete solution for removing the ability to do any sort of random file system access, though. > But it's not evident to me that it's useful to disable everything > specifically at compile time. I have long thought that it's pretty > bizarre that we permit DML on system catalogs even with > allow_system_table_mods=off, and if I were going to provide a way to > lock that down, I would think of doing it via a new GUC, or a > modification to the existing GUC, or something like that, rather than > a compile-time option -- because we might easily discover a problem in > a future release that requires catalog DML to fix, and you wouldn't > want to have to replace the binaries or even bounce the server to do > that. Yeah, for certain things, a GUC probably makes more sense. > And similarly, is it really want we want here to categorically disable > all functions that permit file access for all users under all > circumstances? Or do we maybe want that to be something that can be > reconfigured at runtime? Or even just make it a privilege extended to > some users but not others? What about COPY TO/FROM PROGRAM? I guess I'd ask again whether we can do both... We've got predefined roles like pg_execute_server_program that allow access to COPY TO/FROM PROGRAM, but I have no way to categorically disable that ѕort of thing if I wanted to really lock things down, even for superusers. I'm not suggesting that every predefined role needs a corresponding configure option, but basic things like arbitrary disk/network/program access seem like reasonable proposals. I have about 50% of a generic --disable-disk-access patch coded up which I'll share soon to help inform the discussion. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
On Tue, May 24, 2022 at 02:10:19PM -0400, Robert Haas wrote: > I guess one question is at what level we want to disable these various > things. Your original proposal seemed reasonable to me because I feel > like users who are compiling PostgreSQL ought to have control over > which things they compile. If you can turn plperl and plperlu off > together, and you can, then why shouldn't you be able to turn them on > and off separately? I can't think of a good reason why we shouldn't > make that possible if people want it, and evidently at least one > person does: you. I'm even willing to assume that you represent the > interests of some larger group of people. :-) I always thought if pg_proc is able to call an arbitrary function in an arbitrary library, it could access to the file system, and if that is true, locking the super-user from file system access seems impossible and unwise to try because it would give a false sense of security. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Indecision is a decision. Inaction is an action. Mark Batterson
Bruce Momjian <bruce@momjian.us> writes: > I always thought if pg_proc is able to call an arbitrary function in an > arbitrary library, it could access to the file system, and if that is > true, locking the super-user from file system access seems impossible > and unwise to try because it would give a false sense of security. That was the situation when we had v0 function call semantics. ISTM we are at least a lot closer now to being able to say it's locked down: "internal" functions can only reach things that are in the fmgrtab table, and "C" functions can only reach things that have associated PG_FUNCTION_INFO_V1 symbols. Plus we won't load shared libraries that don't have PG_MODULE_MAGIC blocks. Maybe there's still a way around all that, but it's sure a lot less obvious than it once was, and there are probably things we could do to make it even harder. I think would-be hackers are now reduced to doing what Robert suggested, which is trying to find a way to subvert a validly SQL-callable function by passing it bogus arguments. Maybe there's a way to gain filesystem access by doing that, but it's not going to be easy if the function is not one that intended to allow such operations. regards, tom lane
On Tue, May 24, 2022 at 09:19:40PM -0400, Tom Lane wrote: > Bruce Momjian <bruce@momjian.us> writes: > > I always thought if pg_proc is able to call an arbitrary function in an > > arbitrary library, it could access to the file system, and if that is > > true, locking the super-user from file system access seems impossible > > and unwise to try because it would give a false sense of security. > > That was the situation when we had v0 function call semantics. ISTM > we are at least a lot closer now to being able to say it's locked down: > "internal" functions can only reach things that are in the fmgrtab > table, and "C" functions can only reach things that have associated > PG_FUNCTION_INFO_V1 symbols. Plus we won't load shared libraries > that don't have PG_MODULE_MAGIC blocks. Maybe there's still a way > around all that, but it's sure a lot less obvious than it once was, > and there are probably things we could do to make it even harder. Okay, good to know. > I think would-be hackers are now reduced to doing what Robert > suggested, which is trying to find a way to subvert a validly > SQL-callable function by passing it bogus arguments. Maybe there's > a way to gain filesystem access by doing that, but it's not going > to be easy if the function is not one that intended to allow such > operations. Yes, I think if we can say we are safe in standard superuser-changeable things like modifying the system tables, we might have a chance. Are settings like archive_command safe? -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Indecision is a decision. Inaction is an action. Mark Batterson
Greetings, * Nathan Bossart (nathandbossart@gmail.com) wrote: > I guess I'd ask again whether we can do both... We've got predefined roles > like pg_execute_server_program that allow access to COPY TO/FROM PROGRAM, > but I have no way to categorically disable that ѕort of thing if I wanted > to really lock things down, even for superusers. I'm not suggesting that > every predefined role needs a corresponding configure option, but basic > things like arbitrary disk/network/program access seem like reasonable > proposals. Locking things down "even for superuser" is something we've very explicitly said we're not going to try and do. Even with v1 functions, the ability to hack around with pg_proc strikes me as almost certainly going to provide a way for someone to gain enough control of execution to be able to 'break out', not to mention obvious other things like ALTER SYSTEM to change archive_command to run whatever shell commands an attacker with superuser wants to.. > I have about 50% of a generic --disable-disk-access patch coded up which > I'll share soon to help inform the discussion. Do you disable the ability of superusers to use ALTER SYSTEM with this? I really don't think this is going to be anywhere near as straight-forward as it might appear to be to prevent a superuser from being able to break out of PG. Instead, we should be moving in the direction of making it so that there doesn't need to be a superuser that's ever logged into except under serious emergency situations where the system is built to require multi-person access to do so. Thanks, Stephen
Attachment
Stephen Frost <sfrost@snowman.net> writes: > I really don't think this is going to be anywhere near as > straight-forward as it might appear to be to prevent a superuser from > being able to break out of PG. This gets back to the point I made before about it not being worthwhile to implement half-measures. There is a whole lot of history and code details associated with the presumption that superuser gives you OS access, and I'm certainly prepared to believe that turning that off is a fool's errand. Perhaps a better answer for providers who need something like this is to sandbox the Postgres server using OS-provided facilities. > Instead, we should be moving in the > direction of making it so that there doesn't need to be a superuser > that's ever logged into except under serious emergency situations where > the system is built to require multi-person access to do so. I'm a little skeptical that our present design direction really moves the needle very far in this area. We've sliced and diced superuser aplenty, but that doesn't make individual capabilities such as pg_write_all_data or ALTER SYSTEM any less dangerous from the standpoint of someone trying to prevent breaking out. regards, tom lane
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > I really don't think this is going to be anywhere near as > > straight-forward as it might appear to be to prevent a superuser from > > being able to break out of PG. > > This gets back to the point I made before about it not being worthwhile > to implement half-measures. There is a whole lot of history and code > details associated with the presumption that superuser gives you OS > access, and I'm certainly prepared to believe that turning that off > is a fool's errand. Right. > Perhaps a better answer for providers who need something like this > is to sandbox the Postgres server using OS-provided facilities. I'm guessing they wouldn't feel that to be a very satisfactory answer but if they want to give people PG superuser access then that does seem like an approach which at least might be able to work. > > Instead, we should be moving in the > > direction of making it so that there doesn't need to be a superuser > > that's ever logged into except under serious emergency situations where > > the system is built to require multi-person access to do so. > > I'm a little skeptical that our present design direction really moves > the needle very far in this area. We've sliced and diced superuser > aplenty, but that doesn't make individual capabilities such as > pg_write_all_data or ALTER SYSTEM any less dangerous from the standpoint > of someone trying to prevent breaking out. I'm guessing you're referring to pg_write_server_files here, not pg_write_all_data (as the latter should generally be 'safe' in these terms? If not, would be good to understand the concern there). I don't think that what they're actually looking for is a way to give a user access to pg_write_server_files or to ALTER SYSTEM though- and what we have today explicitly allows them to GRANT out lots of rights to non-superusers without also giving those users access to pg_write_all_data and ALTER SYSTEM and that's basically the point. Allowing non-superusers to create extensions which have C functions is one example of moving in this direction of allowing the 'typical DBA' things to be done by non-superusers. There's certainly a lot more that we can do in that direction. Allowing users to create other users without being a superuser or effectively being able to gain superuser access strikes me as the next big step in that same direction of splitting up what only superusers are able to do today. That's what the recent discussion about CREATEROLE was really all about, just figuring out how to allow CREATEROLE and some level of control over those roles after they've been created (and by whom). What isn't terribly clear to me is how what started this particular thread is moving things in that direction though, instead it seems to be trying to go in the direction of having a system where superuser could be "safely" given out and I am concerned about the project trying to provide a way to guarantee that based on some configure switches. That strikes me as unlikely to end up being successful and might also make it so that even a superuser isn't able to do what a superuser needs to be able to to do- and then do we need a super superuser..? The very specific "it'd be nice to build PG w/o having untrusted languages compiled in" is at least reasonably clearly contained and reasonable to see if we are, in fact, doing what we claim we're doing with such a switch. A switch that's "--disable-disk-access" seems to be basically impossible for it to *really* do what a simple reading of the option implies (clearly we're going to access the disk..) and even if we try to say "well, not direct disk access" then does that need to disable ALTER SYSTEM (or just for certain GUCs..?) along with things like pg_write_server_files and pg_execute_server_programs, and probably modifying pg_proc and maybe modification of the other PG catalogs? But then, what if you actually need to modify pg_proc due to what we say to do in release notes or for other reasons? Would you have to replace the PG binaries to do so? That doesn't strike me as particularly reasonable. Thanks, Stephen
Attachment
On Wed, May 25, 2022 at 2:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote: > I'm a little skeptical that our present design direction really moves > the needle very far in this area. We've sliced and diced superuser > aplenty, but that doesn't make individual capabilities such as > pg_write_all_data or ALTER SYSTEM any less dangerous from the standpoint > of someone trying to prevent breaking out. We have really not sliced and diced superuser in any serious way. I'm not here to say that the existing predefined roles are useless, especially the more powerful ones like pg_read_all_data, but I don't think "primitive" would be an unfair characterization. The problem is twofold. On the one hand, you can't delegate all of the things that the server can do - in particular, and I think this is the really important thing, the superuser's unique ability to administer objects inside the database. On the other hand, you can only delegate privileges on an all-or-nothing basis. You either have the predefined role or you don't. In the case of something like pg_read_all_data, that's fine, because it's equivalent to SELECT privileges on every table, which are separately grantable if you prefer. But it's a little less obviously sufficient for things like pg_read_server_files where, we must hope, you're OK with granting access to all or none of them, and it's clearly insufficient for administration of objects in the database. Hence the whole "CREATEROLE and role ownership hierarchies" thread, which strikes me as as way to make some really meaningful progress toward a future in which you don't have to be superuser to do a useful amount of database administration. Unfortunately that discussion was less productive than I think it could have been. -- Robert Haas EDB: http://www.enterprisedb.com
On Wed, May 25, 2022 at 4:07 PM Stephen Frost <sfrost@snowman.net> wrote: > The very specific "it'd be nice to build PG w/o having untrusted > languages compiled in" is at least reasonably clearly contained and > reasonable to see if we are, in fact, doing what we claim we're doing > with such a switch. A switch that's "--disable-disk-access" seems to > be basically impossible for it to *really* do what a simple reading of > the option implies (clearly we're going to access the disk..) and even > if we try to say "well, not direct disk access" then does that need to > disable ALTER SYSTEM (or just for certain GUCs..?) along with things > like pg_write_server_files and pg_execute_server_programs, and probably > modifying pg_proc and maybe modification of the other PG catalogs? But > then, what if you actually need to modify pg_proc due to what we say to > do in release notes or for other reasons? Would you have to replace the > PG binaries to do so? That doesn't strike me as particularly > reasonable. +1 to all that. The original proposal was self-contained and reasonable on its face. Blowing it up into a general --disable-disk-access feature makes it both a lot more difficult and a lot less well-defined. -- Robert Haas EDB: http://www.enterprisedb.com
Stephen Frost <sfrost@snowman.net> writes: > The very specific "it'd be nice to build PG w/o having untrusted > languages compiled in" is at least reasonably clearly contained and > reasonable to see if we are, in fact, doing what we claim we're doing > with such a switch. I agree that it's specific and easily measured. What I don't get is why it's worth troubling over, if we acknowledge that keeping superusers from breaking out to OS access is infeasible. At most, not having access to plpythonu means you've got to kluge something up involving COPY TO PROGRAM 'python'. If somebody else is excited enough about it to do the legwork, I won't stand in the way particularly. But it strikes me as a waste of effort, not only for the patch author but for everyone who has to read about or maintain the resulting configure options etc. regards, tom lane
Greetings, * Tom Lane (tgl@sss.pgh.pa.us) wrote: > Stephen Frost <sfrost@snowman.net> writes: > > The very specific "it'd be nice to build PG w/o having untrusted > > languages compiled in" is at least reasonably clearly contained and > > reasonable to see if we are, in fact, doing what we claim we're doing > > with such a switch. > > I agree that it's specific and easily measured. What I don't get is why > it's worth troubling over, if we acknowledge that keeping superusers from > breaking out to OS access is infeasible. At most, not having access to > plpythonu means you've got to kluge something up involving COPY TO > PROGRAM 'python'. I agree that this seems to need more discussion and explanation as it isn't actually sufficient by itself for "anyone who wants to disallow file system access" as the initial post claimed. If there isn't sufficient explanation coming forward to support this change by itself then we can reject it, but I don't think it makes sense to try and morph it into something a lot more generic and a lot harder to actually get right and document and guarantee. > If somebody else is excited enough about it to do the legwork, I won't > stand in the way particularly. But it strikes me as a waste of effort, > not only for the patch author but for everyone who has to read about > or maintain the resulting configure options etc. I agree that we need to be judicious in what configure options we add as new options introduce additional maintenance effort. Thanks, Stephen
Attachment
Stephen Frost <sfrost@snowman.net> writes: > I agree that this seems to need more discussion and explanation as it > isn't actually sufficient by itself for "anyone who wants to disallow > file system access" as the initial post claimed. If there isn't > sufficient explanation coming forward to support this change by itself > then we can reject it, but I don't think it makes sense to try and morph > it into something a lot more generic and a lot harder to actually get > right and document and guarantee. The reason I pushed the discussion in that direction was that I was curious to see if --disable-disk-access could actually be a thing. If it could, it'd have clear utility for at least some service providers. But it seems the (preliminary?) conclusion is "no, we still can't do that in any way that's credibly bulletproof". So yeah, that justification for the currently-proposed patch doesn't seem to hold water. regards, tom lane
On 24.05.22 22:58, Nathan Bossart wrote: > FWIW this was my original thinking. I can choose to build/install > extensions separately, but when it comes to PL/Tcl and PL/Perl, you've > got to build the trusted and untrusted stuff at the same time, and the > untrusted symbols remain even if you remove the control file and > installation scripts. Of course, this isn't a complete solution for > removing the ability to do any sort of random file system access, though. This only makes sense to me if you install directly from the source tree to your production installation. Presumably, there is usually a packaging step in between. And you can decide at that point which files to install or not to install.
Given the discussion in this thread, I intend to mark the commitfest entry as Withdrawn shortly. Before I do, I thought I'd first check whether 0001 [0] might be worthwhile independent of $SUBJECT. This change separates the [un]trusted handler and validator functions for PL/Perl so that we no longer need to inspect pg_language to determine whether to use the trusted or untrusted code path. I was surprised to learn that you can end up with PL/PerlU even if you've specified the trusted handler/validator functions. Besides bringing things more in line with how PL/Tcl does things, this change simplifies function lookup in plperl_proc_hash. I suppose such a change might introduce a compatibility break for users who are depending on this behavior, but I don't know if that's worth worrying about. [0] https://www.postgresql.org/message-id/attachment/133940/v1-0001-Do-not-use-pg_language-to-determine-whether-PL-Pe.patch -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Nathan Bossart <nathandbossart@gmail.com> writes: > Given the discussion in this thread, I intend to mark the commitfest entry > as Withdrawn shortly. Before I do, I thought I'd first check whether 0001 > [0] might be worthwhile independent of $SUBJECT. This change separates the > [un]trusted handler and validator functions for PL/Perl so that we no > longer need to inspect pg_language to determine whether to use the trusted > or untrusted code path. I was surprised to learn that you can end up with > PL/PerlU even if you've specified the trusted handler/validator functions. > Besides bringing things more in line with how PL/Tcl does things, this > change simplifies function lookup in plperl_proc_hash. I suppose such a > change might introduce a compatibility break for users who are depending on > this behavior, but I don't know if that's worth worrying about. Meh. Avoiding the potential repeat hashtable lookup is worth something, but I'm not sure I buy that this is a semantic improvement. ISTM that lanpltrusted *should* be the ultimate source of truth on this point. My feelings about it are not terribly strong either way, though. regards, tom lane
On 7/13/22 12:49, Tom Lane wrote: > Nathan Bossart <nathandbossart@gmail.com> writes: >> Given the discussion in this thread, I intend to mark the commitfest entry >> as Withdrawn shortly. I'll mark this RwF rather than bring it forward; if you'd prefer a different status please feel free (or let me know). Thanks, --Jacob
On Mon, Aug 01, 2022 at 02:41:21PM -0700, Jacob Champion wrote: > On 7/13/22 12:49, Tom Lane wrote: >> Nathan Bossart <nathandbossart@gmail.com> writes: >>> Given the discussion in this thread, I intend to mark the commitfest entry >>> as Withdrawn shortly. > > I'll mark this RwF rather than bring it forward; if you'd prefer a > different status please feel free (or let me know). Thanks. I think 0001 might still be worth considering, but that needn't be tracked with this commitfest entry. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com