Re: [PATCHES] [pgsql-patches] [HACKERS] less privileged pl install
Jeremy Drake [EMAIL PROTECTED] writes: This version of the patch includes documentation changes. Please review... Applied with minor revisions --- in particular, I thought the initial owner of a language should be its creator, full stop, rather than the rather strange (and undocumented) behavior you had. Also you missed updating pg_dump. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [pgsql-patches] [HACKERS] less privileged pl install
On Mon, 26 Mar 2007, Tom Lane wrote: Jeremy Drake [EMAIL PROTECTED] writes: This version of the patch includes documentation changes. Please review... Applied with minor revisions --- in particular, I thought the initial owner of a language should be its creator, full stop, rather than the rather strange (and undocumented) behavior you had. The reason I did it like that was this email from you: http://archives.postgresql.org/pgsql-hackers/2007-01/msg01186.php Also you missed updating pg_dump. Indeed. Now, all I need is the 'tsearch2 in core' patch to go in, and 8.3 will solve almost all of my problems :) Then I just need to nag my hosting provider to upgrade once it is released. I have been refraining from nagging about them still running 8.1.3 in anticipation of this ;) -- Five is a sufficiently close approximation to infinity. -- Robert Firth ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] [pgsql-patches] [HACKERS] less privileged pl install
Jeremy Drake [EMAIL PROTECTED] writes: On Mon, 26 Mar 2007, Tom Lane wrote: Applied with minor revisions --- in particular, I thought the initial owner of a language should be its creator, full stop, rather than the rather strange (and undocumented) behavior you had. The reason I did it like that was this email from you: http://archives.postgresql.org/pgsql-hackers/2007-01/msg01186.php Yeah, but that idea predated the addition of ALTER LANGUAGE OWNER to the patch. Given that, a superuser can give away the language to someone else if he wants, and so there's no need for us to try to be fancy about guessing what he wants (which was more or less what that rule was). regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [pgsql-patches] [HACKERS] less privileged pl install
The most recent version of this patch has been added. Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. --- Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: I think that an ALTER LANGUAGE OWNER TO is the proper response to these things, and unless I hear otherwise I will attempt to add this to my patch. Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME TO for the owner, which I missed before. I would appreciate someone with more knowledge of the permissions infrastructure to take a look at it since I am fairly new to it and may not fully understand its intricacies. I have refactored the owner checking of languages in the same manner as it is for other owned objects. I have changed to using standard permissions error messages (aclcheck_error) for the language permissions errors. I consider this patch ready for review, assuming the permissions rules outlined by Tom Lane on -hackers are valid. For reference, here are the rules that this patch is intended to implement: 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). The only difference from this is, that when superuser is required, the owner of the language is not the superuser who created it, but BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the same behavior as currently took precedence. The current behavior in cvs is that languages have no owner, and for purposes where one would be needed it is assumed to be BOOTSTRAP_SUPERUSERID. Is this valid, or should I instead set the owner to GetUserId() in those cases? -- Academic politics is the most vicious and bitter form of politics, because the stakes are so low. -- Wallace Sayre Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 6: explain analyze is your friend -- Bruce Momjian [EMAIL PROTECTED] http://momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] [pgsql-patches] [HACKERS] less privileged pl install
On Tue, 20 Feb 2007, Bruce Momjian wrote: The most recent version of this patch has been added. Your patch has been added to the PostgreSQL unapplied patches list at: http://momjian.postgresql.org/cgi-bin/pgpatches It will be applied as soon as one of the PostgreSQL committers reviews and approves it. Cool, I was going to bring this up again once the regexp patch got in. There is one thing in this patch I was not sure on, and that is in AlterLanguageOwner what should the second parameter of heap_close be? I have RowExclusiveLock in the patch, but I am not sure that is correct. It would be good if someone more knowledgeable about such things checked on this when applying it... The latest version of the patch is currently at http://momjian.us/mhonarc/patches/msg00014.html --- Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: I think that an ALTER LANGUAGE OWNER TO is the proper response to these things, and unless I hear otherwise I will attempt to add this to my patch. Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME TO for the owner, which I missed before. I would appreciate someone with more knowledge of the permissions infrastructure to take a look at it since I am fairly new to it and may not fully understand its intricacies. I have refactored the owner checking of languages in the same manner as it is for other owned objects. I have changed to using standard permissions error messages (aclcheck_error) for the language permissions errors. I consider this patch ready for review, assuming the permissions rules outlined by Tom Lane on -hackers are valid. For reference, here are the rules that this patch is intended to implement: 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). The only difference from this is, that when superuser is required, the owner of the language is not the superuser who created it, but BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the same behavior as currently took precedence. The current behavior in cvs is that languages have no owner, and for purposes where one would be needed it is assumed to be BOOTSTRAP_SUPERUSERID. Is this valid, or should I instead set the owner to GetUserId() in those cases? -- Academic politics is the most vicious and bitter form of politics, because the stakes are so low. -- Wallace Sayre Content-Description: [ Attachment, skipping... ] ---(end of broadcast)--- TIP 6: explain analyze is your friend -- A UNIX saleslady, Lenore, Enjoys work, but she likes the beach more. She found a good way To combine work and play: She sells C shells by the seashore. ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [pgsql-patches] [HACKERS] less privileged pl install
This version of the patch includes documentation changes. Please review... -- The more things change, the more they stay insane.Index: doc/src/sgml/catalogs.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/catalogs.sgml,v retrieving revision 2.143 diff -c -r2.143 catalogs.sgml *** doc/src/sgml/catalogs.sgml 22 Jan 2007 01:35:19 - 2.143 --- doc/src/sgml/catalogs.sgml 27 Jan 2007 23:05:33 - *** *** 2635,2640 --- 2635,2647 /row row + entrystructfieldlanowner/structfield/entry + entrytypeoid/type/entry + entryliterallink linkend=catalog-pg-authidstructnamepg_authid/structname/link.oid/literal/entry + entryOwner of the language, usually either the owner of the database, or the superuser who created it/entry + /row + + row entrystructfieldlanispl/structfield/entry entrytypebool/type/entry entry/entry *** *** 3267,3272 --- 3274,3285 /row row + entrystructfieldtmpldbaallowed/structfield/entry + entrytypeboolean/type/entry + entryTrue if language can be created by a database owner if it is trusted/entry + /row + + row entrystructfieldtmplhandler/structfield/entry entrytypetext/type/entry entryName of call handler function/entry Index: doc/src/sgml/ref/alter_language.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/alter_language.sgml,v retrieving revision 1.6 diff -c -r1.6 alter_language.sgml *** doc/src/sgml/ref/alter_language.sgml16 Sep 2006 00:30:16 - 1.6 --- doc/src/sgml/ref/alter_language.sgml26 Jan 2007 01:01:40 - *** *** 21,26 --- 21,28 refsynopsisdiv synopsis ALTER LANGUAGE replaceablename/replaceable RENAME TO replaceablenewname/replaceable + + ALTER LANGUAGE replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable /synopsis /refsynopsisdiv *** *** 48,53 --- 50,64 /varlistentry varlistentry + termreplaceablenew_owner/replaceable/term + listitem + para + The new owner of the language. + /para + /listitem +/varlistentry + +varlistentry termreplaceablenewname/replaceable/term listitem para Index: doc/src/sgml/ref/create_language.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/create_language.sgml,v retrieving revision 1.42 diff -c -r1.42 create_language.sgml *** doc/src/sgml/ref/create_language.sgml 16 Sep 2006 00:30:17 - 1.42 --- doc/src/sgml/ref/create_language.sgml 28 Jan 2007 00:04:10 - *** *** 36,42 database. Subsequently, functions and trigger procedures can be defined in this new language. The user must have the productnamePostgreSQL/productname superuser privilege to !register a new language. /para para --- 36,47 database. Subsequently, functions and trigger procedures can be defined in this new language. The user must have the productnamePostgreSQL/productname superuser privilege to !register a new language, unless the language is present in the !link linkend=catalog-pg-pltemplatestructnamepg_pltemplate/structname/link !system catalog, the language template is marked as trusted, and the !structfieldtmpldbaallowed/structfield column for the template is !true, in which case the database owner is also allowed to register !the language. /para para 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.c26 Jan 2007 23:53:03 - *** *** 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
Re: [pgsql-patches] [HACKERS] less privileged pl install
On Thu, 25 Jan 2007, Jeremy Drake wrote: On Thu, 25 Jan 2007, Jeremy Drake wrote: I think that an ALTER LANGUAGE OWNER TO is the proper response to these things, and unless I hear otherwise I will attempt to add this to my patch. Here is the patch which adds this. It also allows ALTER LANGUAGE RENAME TO for the owner, which I missed before. I would appreciate someone with more knowledge of the permissions infrastructure to take a look at it since I am fairly new to it and may not fully understand its intricacies. I have refactored the owner checking of languages in the same manner as it is for other owned objects. I have changed to using standard permissions error messages (aclcheck_error) for the language permissions errors. I consider this patch ready for review, assuming the permissions rules outlined by Tom Lane on -hackers are valid. For reference, here are the rules that this patch is intended to implement: 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). The only difference from this is, that when superuser is required, the owner of the language is not the superuser who created it, but BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the same behavior as currently took precedence. The current behavior in cvs is that languages have no owner, and for purposes where one would be needed it is assumed to be BOOTSTRAP_SUPERUSERID. Is this valid, or should I instead set the owner to GetUserId() in those cases? -- Academic politics is the most vicious and bitter form of politics, because the stakes are so low. -- Wallace SayreIndex: doc/src/sgml/ref/alter_language.sgml === RCS file: /data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/alter_language.sgml,v retrieving revision 1.6 diff -c -r1.6 alter_language.sgml *** doc/src/sgml/ref/alter_language.sgml16 Sep 2006 00:30:16 - 1.6 --- doc/src/sgml/ref/alter_language.sgml26 Jan 2007 01:01:40 - *** *** 21,26 --- 21,28 refsynopsisdiv synopsis ALTER LANGUAGE replaceablename/replaceable RENAME TO replaceablenewname/replaceable + + ALTER LANGUAGE replaceablename/replaceable OWNER TO replaceablenew_owner/replaceable /synopsis /refsynopsisdiv *** *** 48,53 --- 50,64 /varlistentry varlistentry + termreplaceablenew_owner/replaceable/term + listitem + para + The new owner of the language. + /para + /listitem +/varlistentry + +varlistentry termreplaceablenewname/replaceable/term listitem para 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.c26 Jan 2007 23:53:03 - *** *** 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),
Re: [pgsql-patches] [HACKERS] less privileged pl install
Jeremy Drake [EMAIL PROTECTED] writes: The only difference from this is, that when superuser is required, the owner of the language is not the superuser who created it, but BOOTSTRAP_SUPERUSERID. This is because my interpretation was that the same behavior as currently took precedence. The current behavior in cvs is that languages have no owner, and for purposes where one would be needed it is assumed to be BOOTSTRAP_SUPERUSERID. Is this valid, or should I instead set the owner to GetUserId() in those cases? I'd go with GetUserId() in the cases where you're not explicitly assigning ownership to the datdba role. AFAIR the assumption that languages are owned by BOOTSTRAP_SUPERUSERID was just a kluge to use in some bits of code that had to have a notion of a specific owner. Now in reality every superuser has the same privileges as every other one, and so it doesn't matter much which one you use, but we might as well record who actually did the deed. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [pgsql-patches] [HACKERS] less privileged pl install
On Sat, 27 Jan 2007, Tom Lane wrote:
I'd go with GetUserId() in the cases where you're not explicitly
assigning ownership to the datdba role. AFAIR the assumption that
languages are owned by BOOTSTRAP_SUPERUSERID was just a kluge to use in
some bits of code that had to have a notion of a specific owner. Now
in reality every superuser has the same privileges as every other one,
and so it doesn't matter much which one you use, but we might as well
record who actually did the deed.
Here is a version of the patch which does this. Very minor change from
the last version...
--
Begathon, n.:
A multi-day event on public television, used to raise money so
you won't have to watch commercials.Index: doc/src/sgml/ref/alter_language.sgml
===
RCS file:
/data/local/jeremyd/postgres/cvsuproot/pgsql/doc/src/sgml/ref/alter_language.sgml,v
retrieving revision 1.6
diff -c -r1.6 alter_language.sgml
*** doc/src/sgml/ref/alter_language.sgml16 Sep 2006 00:30:16 -
1.6
--- doc/src/sgml/ref/alter_language.sgml26 Jan 2007 01:01:40 -
***
*** 21,26
--- 21,28
refsynopsisdiv
synopsis
ALTER LANGUAGE replaceablename/replaceable RENAME TO
replaceablenewname/replaceable
+
+ ALTER LANGUAGE replaceablename/replaceable OWNER TO
replaceablenew_owner/replaceable
/synopsis
/refsynopsisdiv
***
*** 48,53
--- 50,64
/varlistentry
varlistentry
+ termreplaceablenew_owner/replaceable/term
+ listitem
+ para
+ The new owner of the language.
+ /para
+ /listitem
+/varlistentry
+
+varlistentry
termreplaceablenewname/replaceable/term
listitem
para
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.c26 Jan 2007 23:53:03 -
***
*** 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);
***
*** 2148,2153
--- 2144,2177
}
/*
+ * Ownership check for a procedural language (specified by OID)
+ */
+ bool
+ pg_language_ownercheck(Oid lan_oid, Oid roleid)
+ {
+ HeapTuple tuple;
+ Oid ownerId;
+
+ /* Superusers bypass all permission checking. */
+ if (superuser_arg(roleid))
+ return true;
+
+ tuple = SearchSysCache(LANGOID,
+ ObjectIdGetDatum(lan_oid),
+ 0, 0, 0);
+ if (!HeapTupleIsValid(tuple))
+ ereport(ERROR,
+ (errcode(ERRCODE_UNDEFINED_FUNCTION),
+errmsg(language with OID %u does not exist,
lan_oid)));
+
+ ownerId = ((Form_pg_language) GETSTRUCT(tuple))-lanowner;
+
+ ReleaseSysCache(tuple);
+
+
