Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Jeremy Drake wrote:
> On Wed, 24 Jan 2007, Tom Lane wrote:
>
> > In detail, it'd look something like:
> >
> > * For an untrusted language: must be superuser to either create or use
> > the language (no change from current rules). Ownership of the
> > pg_language entry is really irrelevant, as is its ACL.
> >
> > * For a trusted language:
> >
> > * if pg_pltemplate.something is ON: either a superuser or the current
> > DB's owner can CREATE the language. In either case the pg_language
> > entry will be marked as owned by the DB owner (pg_database.datdba),
> > which means that subsequently he (or a superuser) can grant or deny
> > USAGE within his DB.
> >
> > * if pg_pltemplate.something is OFF: must be superuser to CREATE the
> > language; subsequently it will be owned by you, so only you or another
> > superuser can grant or deny USAGE (same behavior as currently).
>
> I think I have what is described here implemented in this patch, so that
> it can be better understood. Thoughts?
This version of the patch creates a shared dependency on the language
owner.
I have thought of some other questions about the owner stuff which I will
send on -hackers...
--
Afternoon, n.:
That part of the day we spend worrying about how we wasted the
morning.Index: src/backend/catalog/aclchk.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.135
diff -c -r1.135 aclchk.c
*** src/backend/catalog/aclchk.c23 Jan 2007 05:07:17 - 1.135
--- src/backend/catalog/aclchk.c25 Jan 2007 06:35:21 -
***
*** 1003,1013
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
-*
-* Note: for now, languages are treated as owned by the
bootstrap
-* user. We should add an owner column to pg_language instead.
*/
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
&isNull);
if (isNull)
--- 1003,1010
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
*/
! ownerId = pg_language_tuple->lanowner;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
&isNull);
if (isNull)
***
*** 1770,1777
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("language with OID %u does not exist",
lang_oid)));
! /* XXX pg_language should have an owner column, but doesn't */
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
&isNull);
--- 1767,1773
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("language with OID %u does not exist",
lang_oid)));
! ownerId = ((Form_pg_language) GETSTRUCT(tuple))->lanowner;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
&isNull);
Index: src/backend/commands/proclang.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/commands/proclang.c,v
retrieving revision 1.71
diff -c -r1.71 proclang.c
*** src/backend/commands/proclang.c 22 Jan 2007 01:35:20 - 1.71
--- src/backend/commands/proclang.c 25 Jan 2007 23:15:45 -
***
*** 17,22
--- 17,24
#include "access/heapam.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
+ #include "catalog/pg_authid.h"
+ #include "catalog/pg_database.h"
#include "catalog/pg_language.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_pltemplate.h"
***
*** 27,32
--- 29,35
#include "miscadmin.h"
#include "parser/gramparse.h"
#include "parser/parse_func.h"
+ #include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
***
*** 36,50
typedef struct
{
booltmpltrusted;/* trusted? */
char *tmplhandler;/* name of handler function */
char *tmplvalidator; /* name of validator function, or NULL
*/
char *tmpllibrary;/* path of shared library */
} PLTemplate;
static void create_proc_lang(const char *languageName,
!
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Tom Lane wrote:
> In detail, it'd look something like:
>
> * For an untrusted language: must be superuser to either create or use
> the language (no change from current rules). Ownership of the
> pg_language entry is really irrelevant, as is its ACL.
>
> * For a trusted language:
>
> * if pg_pltemplate.something is ON: either a superuser or the current
> DB's owner can CREATE the language. In either case the pg_language
> entry will be marked as owned by the DB owner (pg_database.datdba),
> which means that subsequently he (or a superuser) can grant or deny
> USAGE within his DB.
>
> * if pg_pltemplate.something is OFF: must be superuser to CREATE the
> language; subsequently it will be owned by you, so only you or another
> superuser can grant or deny USAGE (same behavior as currently).
I think I have what is described here implemented in this patch, so that
it can be better understood. Thoughts?
--
Nobody said computers were going to be polite.Index: src/backend/catalog/aclchk.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/catalog/aclchk.c,v
retrieving revision 1.135
diff -c -r1.135 aclchk.c
*** src/backend/catalog/aclchk.c23 Jan 2007 05:07:17 - 1.135
--- src/backend/catalog/aclchk.c25 Jan 2007 06:35:21 -
***
*** 1003,1013
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
-*
-* Note: for now, languages are treated as owned by the
bootstrap
-* user. We should add an owner column to pg_language instead.
*/
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
&isNull);
if (isNull)
--- 1003,1010
/*
* Get owner ID and working copy of existing ACL. If there's no
ACL,
* substitute the proper default.
*/
! ownerId = pg_language_tuple->lanowner;
aclDatum = SysCacheGetAttr(LANGNAME, tuple,
Anum_pg_language_lanacl,
&isNull);
if (isNull)
***
*** 1770,1777
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("language with OID %u does not exist",
lang_oid)));
! /* XXX pg_language should have an owner column, but doesn't */
! ownerId = BOOTSTRAP_SUPERUSERID;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
&isNull);
--- 1767,1773
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("language with OID %u does not exist",
lang_oid)));
! ownerId = ((Form_pg_language) GETSTRUCT(tuple))->lanowner;
aclDatum = SysCacheGetAttr(LANGOID, tuple, Anum_pg_language_lanacl,
&isNull);
Index: src/backend/commands/proclang.c
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/src/backend/commands/proclang.c,v
retrieving revision 1.71
diff -c -r1.71 proclang.c
*** src/backend/commands/proclang.c 22 Jan 2007 01:35:20 - 1.71
--- src/backend/commands/proclang.c 25 Jan 2007 06:30:12 -
***
*** 17,22
--- 17,24
#include "access/heapam.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
+ #include "catalog/pg_authid.h"
+ #include "catalog/pg_database.h"
#include "catalog/pg_language.h"
#include "catalog/pg_namespace.h"
#include "catalog/pg_pltemplate.h"
***
*** 27,32
--- 29,35
#include "miscadmin.h"
#include "parser/gramparse.h"
#include "parser/parse_func.h"
+ #include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
***
*** 36,50
typedef struct
{
booltmpltrusted;/* trusted? */
char *tmplhandler;/* name of handler function */
char *tmplvalidator; /* name of validator function, or NULL
*/
char *tmpllibrary;/* path of shared library */
} PLTemplate;
static void create_proc_lang(const char *languageName,
!Oid handlerOid, Oid valOid, bool trusted);
static PLTemplate *find_language_template(const char *languageName);
/* -
* CREATE PROCEDURAL LANGUAGE
--- 39,56
typedef struct
{
booltmpltrus
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Tom Lane wrote: > Not the DB owner. If you are worried about whether to allow use of PLs > it's almost certainly an installation-wide security concern, so I'd say > that the privilege has to flow from a superuser. > > GRANT CREATE ON LANGUAGE feeding into a flag bit in pltemplate would > work, if restricted to superusers, but I suspect people would find this > confusing because it'd work completely differently from GRANT USAGE ON > LANGUAGE (eg, because the latter has only database-local effects). > Might be better to use a different syntax. I had thought that it would be database-local, but I understand now that it makes more sense to be global. > > Note I'm not arguing against allowing it to be "on" by default, I just > want to be sure there is a way for paranoid DBAs to turn it off. Maybe > it'd be sufficient if the flag bit was there but "UPDATE pg_pltemplate" > was the only way to manipulate it --- we've gotten along with treating > datistemplate and datallowconn that way. That sounds reasonable to me. I'll try to put together a patch like this (adding a boolean column to pg_pltemplate) and see if this is acceptable. I assume that only superusers can modify pg_pltemplate already ;) > Or we could go the full nine yards and add ACLs to pltemplate, but > that's probably overkill. Agreed. -- He thought he saw an albatross That fluttered 'round the lamp. He looked again and saw it was A penny postage stamp. "You'd best be getting home," he said, "The nights are rather damp." ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install (formerly tsearch
Jeremy Drake <[EMAIL PROTECTED]> writes: > On Wed, 24 Jan 2007, Tom Lane wrote: >> that there really needs to be *some* sort of privilege check here. >> What that is and how to implement it are the hard parts. > So I guess it depends on what you mean by "DBA". Perhaps the database > owner? Or some new privilege type (GRANT CREATE ON LANGUAGE ...? Or GRANT > CREATE LANGUAGE ON DATABASE...?) that the db owner has by default? Not the DB owner. If you are worried about whether to allow use of PLs it's almost certainly an installation-wide security concern, so I'd say that the privilege has to flow from a superuser. GRANT CREATE ON LANGUAGE feeding into a flag bit in pltemplate would work, if restricted to superusers, but I suspect people would find this confusing because it'd work completely differently from GRANT USAGE ON LANGUAGE (eg, because the latter has only database-local effects). Might be better to use a different syntax. Note I'm not arguing against allowing it to be "on" by default, I just want to be sure there is a way for paranoid DBAs to turn it off. Maybe it'd be sufficient if the flag bit was there but "UPDATE pg_pltemplate" was the only way to manipulate it --- we've gotten along with treating datistemplate and datallowconn that way. Or we could go the full nine yards and add ACLs to pltemplate, but that's probably overkill. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install
On Wed, 24 Jan 2007, Tom Lane wrote: > Jeremy Drake <[EMAIL PROTECTED]> writes: > > On Wed, 24 Jan 2007, Jeremy Drake wrote: > >> That would be great, and also it would be great to be able to CREATE > >> LANGUAGE as a regular user for a trusted pl that is already > >> compiled/installed. > > > Something like the attached (simple) change to allow CREATE LANGUAGE by > > unprivileged users for trusted languages already present in pg_pltemplate. > > If it were merely a matter of removing an error check I think we would > have done it already. However, pltemplate will have all the languages > in it whether the DBA wants to allow them to be used or not; so I'd say > that there really needs to be *some* sort of privilege check here. > What that is and how to implement it are the hard parts. So I guess it depends on what you mean by "DBA". Perhaps the database owner? Or some new privilege type (GRANT CREATE ON LANGUAGE ...? Or GRANT CREATE LANGUAGE ON DATABASE...?) that the db owner has by default? -- 7:30, Channel 5: The Bionic Dog (Action/Adventure) The Bionic Dog drinks too much and kicks over the National Redwood Forest. ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install (formerly tsearch
Jeremy Drake <[EMAIL PROTECTED]> writes: > On Wed, 24 Jan 2007, Jeremy Drake wrote: >> That would be great, and also it would be great to be able to CREATE >> LANGUAGE as a regular user for a trusted pl that is already >> compiled/installed. > Something like the attached (simple) change to allow CREATE LANGUAGE by > unprivileged users for trusted languages already present in pg_pltemplate. If it were merely a matter of removing an error check I think we would have done it already. However, pltemplate will have all the languages in it whether the DBA wants to allow them to be used or not; so I'd say that there really needs to be *some* sort of privilege check here. What that is and how to implement it are the hard parts. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [pgsql-patches] [HACKERS] unprivileged contrib and pl install (formerly tsearch
On Wed, 24 Jan 2007, Jeremy Drake wrote:
> On Wed, 24 Jan 2007, Martijn van Oosterhout wrote:
>
> > Something I've wondered about before is the concept of having installed
> > Modules in the system. Let's say for example that while compiling
> > postgres it compiled the modules in contrib also and installed them in
> > a modules directory.
> >
> > Once installed there, unpriviledged users could say "INSTALL foo" and
> > it would install the module, even if they do not have the permissions
> > to create them themselves.
>
> That would be great, and also it would be great to be able to CREATE
> LANGUAGE as a regular user for a trusted pl that is already
> compiled/installed.
Something like the attached (simple) change to allow CREATE LANGUAGE by
unprivileged users for trusted languages already present in pg_pltemplate.
I'm not quite sure how one would go about doing the module thing, I think
that would be more complex. Something simple like allowing creation of C
language functions in libraries in $libdir would probably not be
sufficient, because an unprivileged user could create functions that have
the wrong paramters or return values and crash things pretty good that
way. Any ideas how this would work? Perhaps a sql script in
sharedir could be run by the backend as though by a superuser...
--
Ed Sullivan will be around as long as someone else has talent.
-- Fred AllenIndex: src/backend/commands/proclang.c
===
RCS file:
/home/jeremyd/local/postgres/cvsuproot/pgsql/src/backend/commands/proclang.c,v
retrieving revision 1.71
diff -c -r1.71 proclang.c
*** src/backend/commands/proclang.c 22 Jan 2007 01:35:20 - 1.71
--- src/backend/commands/proclang.c 24 Jan 2007 23:50:49 -
***
*** 61,74
Oid funcargtypes[1];
/*
-* Check permission
-*/
- if (!superuser())
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
-errmsg("must be superuser to create procedural
language")));
-
- /*
* Translate the language name and check that this language doesn't
* already exist
*/
--- 61,66
***
*** 97,102
--- 89,103
(errmsg("using pg_pltemplate
information instead of CREATE LANGUAGE parameters")));
/*
+* Check permission
+*/
+ if (!pltemplate->tmpltrusted && !superuser())
+ ereport(ERROR,
+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+errmsg("must be superuser to create
untrusted procedural language")));
+
+
+ /*
* Find or create the handler function, which we force to be in
the
* pg_catalog schema. If already present, it must have the
correct
* return type.
***
*** 189,194
--- 190,203
errhint("The supported languages are
listed in the pg_pltemplate system catalog.")));
/*
+* Check permission
+*/
+ if (!superuser())
+ ereport(ERROR,
+
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+errmsg("must be superuser to create
custom procedural language")));
+
+ /*
* Lookup the PL handler function and check that it is of the
expected
* return type
*/
---(end of broadcast)---
TIP 6: explain analyze is your friend
