minor feature request: Secure defaults during function creation

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:

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/

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]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. 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.
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/