Thread: minor feature request: Secure defaults during function creation
First, I asked about this on #postgresql, and I realize that this request would be a low priority item. Yet, it would be an improvement for security reasons. When creating a function using EXTERNAL SECURITY DEFINER, by default PUBLIC has execute privileges on it. That's unexpected given that when I create a new table, PUBLIC doesn't have any privileges on it. It's also not a secure default. My request is to allow changing default permissions for function creation, a la "umask", or at least not give PUBLIC execute permissions by default. I am aware that it is possible to wrap the create function statement with the necessary grants/revokes inside a transaction, as a work-around, but it is not obvious and makes things unnecessarily inconvenient. This increases the chances of beginner and even medium-skill admins to get their security wrong. Thanks, Pascal Meunier Purdue University CERIAS
On Thu, Sep 14, 2006 at 10:24:43AM -0400, Pascal Meunier wrote: > First, I asked about this on #postgresql, and I realize that this request > would be a low priority item. Yet, it would be an improvement for security > reasons. > > When creating a function using EXTERNAL SECURITY DEFINER, by default PUBLIC > has execute privileges on it. That's unexpected given that when I create a > new table, PUBLIC doesn't have any privileges on it. It's also not a secure > default. > > My request is to allow changing default permissions for function creation, a > la "umask", or at least not give PUBLIC execute permissions by default. I > am aware that it is possible to wrap the create function statement with the > necessary grants/revokes inside a transaction, as a work-around, but it is > not obvious and makes things unnecessarily inconvenient. This increases the > chances of beginner and even medium-skill admins to get their security > wrong. Hrm... do we have any other objects that default to granting permissions on creation? ISTM all objects should be created with no permissions. -- Jim Nasby jim@nasby.net EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
"Jim C. Nasby" <jimn@enterprisedb.com> writes:
> On Thu, Sep 14, 2006 at 10:24:43AM -0400, Pascal Meunier wrote:
>> My request is to allow changing default permissions for function creation, a
>> la "umask", or at least not give PUBLIC execute permissions by default.
> Hrm... do we have any other objects that default to granting permissions
> on creation?
Yes; see the GRANT reference page.
I'm disinclined to change it. We've had the current behavior since we
introduced ACLs for functions at all, and there have been very few
complaints. I think we'd get a lot more complaints if we denied public
EXECUTE by default. One reason is that given the way pg_dump and
default permissions work, any such change would break existing
applications, because an existing schema loaded into a new backend
would suddenly have different permissions behavior.
regards, tom lane
Thanks for answering; I appreciate it, as well as the efforts of all the
people who contributed to this database that I now use in my projects.
However, I feel that making a decision based on the number of prior and
possible future complaints is a poor excuse to not do the right thing. A
low number of prior complaints simply suggests lax security audits of
default behaviors.
I believe that the default object creation permissions described in the
GRANT page are reasonable ("The default is no public access for tables,
schemas, and tablespaces; TEMP table creation privilege for databases;
EXECUTE privilege for functions; and USAGE privilege for languages."),
except when the EXTERNAL SECURITY DEFINER clause is used. That clause makes
the functions take on setuid-like properties, so they should be handled
cautiously. They shouldn't be given PUBLIC access by default.
I asked MITRE to provide a CCE number for this issue (the CCE is a new
effort like the CVE, but for configuration issues instead of
vulnerabilities). I'll let you know if it happens.
Regards,
Pascal Meunier
Purdue University CERIAS
On 9/16/06 8:57 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote:
> "Jim C. Nasby" <jimn@enterprisedb.com> writes:
>> On Thu, Sep 14, 2006 at 10:24:43AM -0400, Pascal Meunier wrote:
>>> My request is to allow changing default permissions for function creation, a
>>> la "umask", or at least not give PUBLIC execute permissions by default.
>
>> Hrm... do we have any other objects that default to granting permissions
>> on creation?
>
> Yes; see the GRANT reference page.
>
> I'm disinclined to change it. We've had the current behavior since we
> introduced ACLs for functions at all, and there have been very few
> complaints. I think we'd get a lot more complaints if we denied public
> EXECUTE by default. One reason is that given the way pg_dump and
> default permissions work, any such change would break existing
> applications, because an existing schema loaded into a new backend
> would suddenly have different permissions behavior.
>
> regards, tom lane
>
Pascal Meunier wrote: > Thanks for answering; I appreciate it, as well as the efforts of all the > people who contributed to this database that I now use in my projects. > > However, I feel that making a decision based on the number of prior and > possible future complaints is a poor excuse to not do the right thing. A > low number of prior complaints simply suggests lax security audits of > default behaviors. > At the very least we would need a way of getting the current behaviour, if we are not to break existing applications. People have a reasonable expectation that a dump and reload will work, and that can't be dismissed as cavalierly as this. Maybe a config file option would do the trick, or maybe an option to pg_dump / pg_dumpall to make it generate the extra GRANT statement that would be required. cheers andrew
Pascal Meunier <pmeunier@cerias.net> writes:
> I asked MITRE to provide a CCE number for this issue (the CCE is a new
> effort like the CVE, but for configuration issues instead of
> vulnerabilities). I'll let you know if it happens.
Trying to force us to change things by getting Mitre involved is a
really really good way to get pushback. I think you just killed any
chance of getting this idea adopted.
regards, tom lane
On 9/18/06 2:00 PM, "Tom Lane" <tgl@sss.pgh.pa.us> wrote: > Pascal Meunier <pmeunier@cerias.net> writes: >> I asked MITRE to provide a CCE number for this issue (the CCE is a new >> effort like the CVE, but for configuration issues instead of >> vulnerabilities). I'll let you know if it happens. > > Trying to force us to change things by getting Mitre involved is a > really really good way to get pushback. I think you just killed any > chance of getting this idea adopted. > > regards, tom lane > Please forgive my chronic lack of tact, which is evident in my previous email; it is one of my flaws. I've been involved in the CVE for a long time, where the original idea was to give a number to every issue under discussion (including ones that aren't confirmed -- those were candidates), so getting a CCE number seemed a normal process to me. I also read your previous email as a likely dismissal, and did not want you to be surprised by seeing a CCE assigned to it. I'm sorry it offended you so much, regardless of the outcome. Moreover, I'd rather be a carpet to the PostgreSQL developers than be cited as the cause for a security improvement not being made, due to having antagonized so much the developers. Please, consider the issue and not the silly messenger. Sincerely, Pascal Meunier
On Mon, Sep 18, 2006 at 02:49:23PM -0400, Pascal Meunier wrote: > regardless of the outcome. Moreover, I'd rather be a carpet to the > PostgreSQL developers than be cited as the cause for a security improvement > not being made, due to having antagonized so much the developers. Please, > consider the issue and not the silly messenger. The problem is that the issue is rather more complicated than you let on. Backward compatability is a big deal. The principle of least surprise also dictates that whatever default permissions are chosen should be the same for every function and not depend on various attributes. By your reasoning we should also have different default permissions if the function is in an untrusted language, or if the language doesn't have a validator. Where do you draw the line? Someone writing SECURITY DEFINER in their function definition has to be understood to know what they're doing. After all, "chmod +s" doesn't reset global execute permissions either, because that would be far too confusing. The same applies here IMHO. The whole point is to be executed by other users. We need much stronger arguments than what's been given so far. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
On Mon, Sep 18, 2006 at 01:59:00PM -0400, Andrew Dunstan wrote: > > Pascal Meunier wrote: > >Thanks for answering; I appreciate it, as well as the efforts of all the > >people who contributed to this database that I now use in my projects. > > > >However, I feel that making a decision based on the number of prior and > >possible future complaints is a poor excuse to not do the right thing. A > >low number of prior complaints simply suggests lax security audits of > >default behaviors. > > > > > At the very least we would need a way of getting the current behaviour, > if we are not to break existing applications. > > People have a reasonable expectation that a dump and reload will work, > and that can't be dismissed as cavalierly as this. > > Maybe a config file option would do the trick, or maybe an option to > pg_dump / pg_dumpall to make it generate the extra GRANT statement that > would be required. This pg_dump issue keeps biting us in the rear... I think at the very least we should have a means for a dump file to tell the backend that it's about to process a dump file generated by version XYZ. That at least gives us the ability to handle prior version incompatibilites. -- Jim Nasby jimn@enterprisedb.com EnterpriseDB http://enterprisedb.com 512.569.9461 (cell)
Jim C. Nasby wrote: > On Mon, Sep 18, 2006 at 01:59:00PM -0400, Andrew Dunstan wrote: > > > > Pascal Meunier wrote: > > >Thanks for answering; I appreciate it, as well as the efforts of all the > > >people who contributed to this database that I now use in my projects. > > > > > >However, I feel that making a decision based on the number of prior and > > >possible future complaints is a poor excuse to not do the right thing. A > > >low number of prior complaints simply suggests lax security audits of > > >default behaviors. > > > > > > > > > At the very least we would need a way of getting the current behaviour, > > if we are not to break existing applications. > > > > People have a reasonable expectation that a dump and reload will work, > > and that can't be dismissed as cavalierly as this. > > > > Maybe a config file option would do the trick, or maybe an option to > > pg_dump / pg_dumpall to make it generate the extra GRANT statement that > > would be required. > > This pg_dump issue keeps biting us in the rear... I think at the very > least we should have a means for a dump file to tell the backend that > it's about to process a dump file generated by version XYZ. That at > least gives us the ability to handle prior version incompatibilites. I have always agreed with that. I would just have a GUC that pg_dump would set that could be used in the future for conditional behavior. -- Bruce Momjian bruce@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Jim C. Nasby wrote: > > This pg_dump issue keeps biting us in the rear... I think at the very > least we should have a means for a dump file to tell the backend that > it's about to process a dump file generated by version XYZ. That at > least gives us the ability to handle prior version incompatibilites. > Jim, please review the long, not to say drawn out, discussion on this point from a year or two back. cheers andrew
Hi, Martijn, Martijn van Oosterhout wrote: > Someone writing SECURITY DEFINER in their function definition has to be > understood to know what they're doing. After all, "chmod +s" doesn't > reset global execute permissions either, because that would be far too > confusing. The same applies here IMHO. The whole point is to be > executed by other users. But I have the possibility to "chmod a-x" before "chmod +s" the file. Maybe we should add "[NOT] PUBLICLY EXCUTABLE"[1] keywords to CREATE FUNCTION, with the default being the current behaviour for now (possibly configurable). Add an appropriate note in the docs for CREATE FUNCTION, so users are informed about the security implications. [1] alternative spelling proposals: "[NOT] PUBLIC" or "PUBLIC | PRIVATE" Thinking about it, "CREATE [OR REPLACE] [PUBLIC|PRIVATE] FUNCTION ..." seems the "most sexy" variant in my eyes. HTH, Markus -- Markus Schaber | Logical Tracking&Tracing International AG Dipl. Inf. | Software Development GIS Fight against software patents in Europe! www.ffii.org www.nosoftwarepatents.org
On Wed, Sep 20, 2006 at 11:59:52AM +0200, Markus Schaber wrote: > But I have the possibility to "chmod a-x" before "chmod +s" the file. > > Maybe we should add "[NOT] PUBLICLY EXCUTABLE"[1] keywords to CREATE > FUNCTION, with the default being the current behaviour for now (possibly > configurable). Add an appropriate note in the docs for CREATE FUNCTION, > so users are informed about the security implications. If you're that paranoid, start a transaction, create the function, revoke the permissions and then commit. Then no-one else will see the function before you've set the permissions the way you want. I agree that maybe being able to specify it during function creation would be nice, but it's not like it's impossible now. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.