Fw: Isn't pg_statistic a security hole - Solution Proposal

Started by Joe Conwayover 24 years ago41 messages
#1Joe Conway
joe@conway-family.com
1 attachment(s)

Hello all,

Attached is a patch to implement a new internal function, 'has_privilege'.
My proposal below explains the reasoning behind this submittal, although I
never did get any feedback -- positive or negative. If the patch is accepted
I'll be happy to do the work to create the system view as descibed.

The patch applies cleanly against cvs tip. One item I was not sure about was
the selection of the OID value for the new function. I chose 1920 for no
other reason that the highest OID in pg_proc.h was 1909, and this seemed
like a safe value. Is there somewhere I should have looked for guidance on
this?

Thanks,

-- Joe

The recent discussions on pg_statistic got me started thinking about how

to

implement a secure form of the view. Based on the list discussion, and a
suggestion from Tom, I did some research regarding how SQL92 and some of

the

larger commercial database systems allow access to system privilege
information.

I reviewed the ANSI SQL 92 specification, Oracle, MSSQL, and IBM DB2
(documentation only). Here's what I found:

ANSI SQL 92 does not have any functions defined for retrieving privilege
information. It does, however define an "information schema" and

"definition

schema" which among other things includes a TABLE_PRIVILEGES view.

With this view available, it is possible to discern what privileges the
current user has using a simple SQL statement. In Oracle, I found this

view,

and some other variations. According to the Oracle DBA I work with, there

is

no special function, and a SQL statement on the view is how he would

gather

this kind of information when needed.

MSSQL Server 7 also has this same view. Additionally, SQL7 has a T-SQL
function called PERMISSIONS with the following description:
"Returns a value containing a bitmap that indicates the statement, object,
or column permissions for the current user.
Syntax PERMISSIONS([objectid [, 'column']])".

I only looked briefly at the IBM DB2 documentation, but could find no
mention of TABLE_PRIVILEGES or any privilege specific function. I imagine
TABLE_PRIVILEGES might be there somewhere since it seems to be standard
SQL92.

Based on all of the above, I concluded that there is nothing compelling in
terms of a specific function to be compatible with. I do think that in the
longer term it makes sense to implement the SQL 92 information schema

views

in PostgreSQL.

So, now for the proposal. I created a function (attached) which will allow
any privilege type to be probed, called has_privilege. It is used like

this:

select relname from pg_class where has_privilege(current_user, relname,
'update');

or

select has_privilege('postgres', 'pg_shadow', 'select');

where
the first parameter is any valid user name
the second parameter can be a table, view, or sequence
the third parameter can be 'select', 'insert', 'update', 'delete', or
'rule'

The function is currently implemented as an external c function and

designed

to be built under contrib. This function should really be an internal
function. If the proposal is acceptable, I would like to take on the task

of

Show quoted text

turning the function into an internal one (with guidance, pointers,
suggestions greatly appreciated). This would allow a secure view to be
implemented over pg_statistic as:

create view pg_userstat as (
select
s.starelid
,s.staattnum
,s.staop
,s.stanullfrac
,s.stacommonfrac
,s.stacommonval
,s.staloval
,s.stahival
,c.relname
,a.attname
,sh.usename
from
pg_statistic as s
,pg_class as c
,pg_shadow as sh
,pg_attribute as a
where
has_privilege(current_user,c.relname,'select')
and sh.usesysid = c.relowner
and a.attrelid = c.oid
and c.oid = s.starelid
);

Then restrict pg_statistic from public viewing. This view would allow the
current user to view statistics only on relations for which they already
have 'select' granted.

Comments?

Regards,
-- Joe

installation:

place in contrib
tar -xzvf has_priv.tgz
cd has_priv
./install.sh
Note: installs the function into template1 by default. Edit install.sh to
change.

Attachments:

has_priv.diffapplication/octet-stream; name=has_priv.diffDownload
diff -Naur pgsql.virg/src/backend/utils/adt/Makefile pgsql/src/backend/utils/adt/Makefile
--- pgsql.virg/src/backend/utils/adt/Makefile	Thu May 31 04:21:26 2001
+++ pgsql/src/backend/utils/adt/Makefile	Thu May 31 05:51:18 2001
@@ -17,7 +17,7 @@
 
 OBJS = acl.o arrayfuncs.o arrayutils.o bool.o cash.o char.o \
 	date.o datetime.o datum.o float.o format_type.o \
-	geo_ops.o geo_selfuncs.o int.o int8.o like.o \
+	geo_ops.o geo_selfuncs.o has_priv.o int.o int8.o like.o \
 	misc.o nabstime.o name.o not_in.o numeric.o numutils.o \
 	oid.o oracle_compat.o \
 	regexp.o regproc.o ruleutils.o selfuncs.o sets.o \
diff -Naur pgsql.virg/src/backend/utils/adt/has_priv.c pgsql/src/backend/utils/adt/has_priv.c
--- pgsql.virg/src/backend/utils/adt/has_priv.c	Thu Jan  1 00:00:00 1970
+++ pgsql/src/backend/utils/adt/has_priv.c	Fri Jun  1 05:34:49 2001
@@ -0,0 +1,158 @@
+/*
+ * has_priv.c
+ *
+ * Check for user privileges
+ *
+ * Copyright (c) Joseph Conway <joe.conway@mail.com>, 2001;
+ * 
+ * Permission to use, copy, modify, and distribute this software and its
+ * documentation for any purpose, without fee, and without a written agreement
+ * is hereby granted, provided that the above copyright notice and this
+ * paragraph and the following two paragraphs appear in all copies.
+ * 
+ * IN NO EVENT SHALL THE AUTHOR OR DISTRIBUTORS BE LIABLE TO ANY PARTY FOR
+ * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING
+ * LOST PROFITS, ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS
+ * DOCUMENTATION, EVEN IF THE AUTHOR OR DISTRIBUTORS HAVE BEEN ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ * 
+ * THE AUTHOR AND DISTRIBUTORS SPECIFICALLY DISCLAIMS ANY WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE.  THE SOFTWARE PROVIDED HEREUNDER IS
+ * ON AN "AS IS" BASIS, AND THE AUTHOR AND DISTRIBUTORS HAS NO OBLIGATIONS TO
+ * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+ *
+ */
+
+
+#include "utils/has_priv.h"
+
+/*
+ * has_privilege
+ *
+ * Check user privileges on a relation
+ * Returns bool
+ *
+ */
+
+PG_FUNCTION_INFO_V1(has_privilege);
+Datum
+has_privilege(PG_FUNCTION_ARGS)
+{
+	text		*username_text;
+	text		*relname_text;
+	text		*priv_type_text;
+	char		*relname;
+	char		*username;
+	char		*priv_type;
+	AclMode		mode;
+	int32		result;
+	HeapTuple	tuple;
+	Oid			userid;
+
+	if (PG_ARGISNULL(0) || PG_ARGISNULL(1) || PG_ARGISNULL(2))
+	{
+		elog(ERROR, "has_privilege: NULL arguments are not permitted");
+	}
+
+	username_text = PG_GETARG_TEXT_P(0);
+	relname_text = PG_GETARG_TEXT_P(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert 'text' pattern to null-terminated string
+	 */
+
+	relname = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(relname_text)));
+	username = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(username_text)));
+	priv_type = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(priv_type_text)));
+
+	/*
+	 * Lookup userid based on username
+	 */
+
+	tuple = SearchSysCache(SHADOWNAME, NameGetDatum(username), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_privilege: invalid user name %s", (char *) username);
+	}
+
+	userid = ((Form_pg_shadow) GETSTRUCT(tuple))->usesysid;
+
+	ReleaseSysCache(tuple);
+
+	/*
+	 * Verify relname is valid
+	 */
+
+	tuple = SearchSysCache(RELNAME, NameGetDatum(relname), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_privilege: invalid relation name %s", (char *) relname);
+	}
+
+	ReleaseSysCache(tuple);
+
+
+	/*
+	 * select = ACL_SELECT
+	 * update = ACL_UPDATE
+	 * delete = ACL_DELETE
+	 * rule = ACL_RULE
+	 * insert = ACL_INSERT
+	 * references = ACL_REFERENCES
+	 * trigger = ACL_TRIGGER
+	 * 
+	 * OLD COMMENTS -- pre REL 7.2
+	 * select = r = (ACL_RD)
+	 * update = w (ACL_WR)
+	 * delete = w (ACL_WR)
+	 * rule = R (ACL_RU)
+	 * insert = a (ACL_AP) or w (ACL_WR)
+	 *
+	 */
+
+
+	if (strcasecmp(priv_type, PRIV_SELECT) == 0) {
+
+		mode = (AclMode) ACL_SELECT;
+
+	} else if (strcasecmp(priv_type, PRIV_INSERT) == 0) {
+
+		mode = (AclMode) ACL_INSERT;
+
+	} else if (strcasecmp(priv_type, PRIV_UPDATE) == 0) {
+
+		mode = (AclMode) ACL_UPDATE;
+
+	} else if (strcasecmp(priv_type, PRIV_DELETE) == 0) {
+
+		mode = (AclMode) ACL_DELETE;
+
+	} else if (strcasecmp(priv_type, PRIV_RULE) == 0) {
+
+		mode = (AclMode) ACL_RULE;
+
+	} else if (strcasecmp(priv_type, PRIV_REFERENCES) == 0) {
+
+		mode = (AclMode) ACL_REFERENCES;
+
+	} else if (strcasecmp(priv_type, PRIV_TRIGGER) == 0) {
+
+		mode = (AclMode) ACL_TRIGGER;
+
+	} else {
+
+		mode = (AclMode) ACL_NO;
+		elog(ERROR, "has_privilege: invalid privilege type %s", (char *) priv_type);
+
+	}
+
+	result = pg_aclcheck(relname, userid, mode);
+
+
+	if (result == 1) {
+		PG_RETURN_BOOL(FALSE);
+	} else {
+		PG_RETURN_BOOL(TRUE);
+	}
+}
+
diff -Naur pgsql.virg/src/include/catalog/pg_proc.h pgsql/src/include/catalog/pg_proc.h
--- pgsql.virg/src/include/catalog/pg_proc.h	Thu May 31 04:21:33 2001
+++ pgsql/src/include/catalog/pg_proc.h	Fri Jun  1 05:52:14 2001
@@ -2614,6 +2614,9 @@
 DATA(insert OID = 1909 (  int8shr		   PGUID 12 f t t t 2 f 20 "20 23" 100 0 0 100	int8shr - ));
 DESCR("binary shift right");
 
+DATA(insert OID = 1920 (  has_privilege	   PGUID 12 f t f f 3 f 16 "25 25 25" 100 0 0 100	has_privilege - ));
+DESCR("determine privileges");
+
 /*
  * prototypes for functions pg_proc.c
  */
diff -Naur pgsql.virg/src/include/utils/builtins.h pgsql/src/include/utils/builtins.h
--- pgsql.virg/src/include/utils/builtins.h	Thu May 31 04:21:34 2001
+++ pgsql/src/include/utils/builtins.h	Thu May 31 05:52:23 2001
@@ -608,4 +608,7 @@
 extern Datum quote_ident(PG_FUNCTION_ARGS);
 extern Datum quote_literal(PG_FUNCTION_ARGS);
 
+/* has_priv.c */
+extern Datum has_privilege(PG_FUNCTION_ARGS);
+
 #endif	 /* BUILTINS_H */
diff -Naur pgsql.virg/src/include/utils/has_priv.h pgsql/src/include/utils/has_priv.h
--- pgsql.virg/src/include/utils/has_priv.h	Thu Jan  1 00:00:00 1970
+++ pgsql/src/include/utils/has_priv.h	Fri Jun  1 05:19:49 2001
@@ -0,0 +1,55 @@
+/*
+ * has_priv.h
+ *
+ * Check for user privileges
+ *
+ * Copyright (c) Joseph Conway <joe.conway@mail.com>, 2001;
+ * 
+ * Permission to use, copy, modify, and distribute this software and its
+ * documentation for any purpose, without fee, and without a written agreement
+ * is hereby granted, provided that the above copyright notice and this
+ * paragraph and the following two paragraphs appear in all copies.
+ * 
+ * IN NO EVENT SHALL THE AUTHOR OR DISTRIBUTORS BE LIABLE TO ANY PARTY FOR
+ * DIRECT, INDIRECT, SPECIAL, INCIDENTAL, OR CONSEQUENTIAL DAMAGES, INCLUDING
+ * LOST PROFITS, ARISING OUT OF THE USE OF THIS SOFTWARE AND ITS
+ * DOCUMENTATION, EVEN IF THE AUTHOR OR DISTRIBUTORS HAVE BEEN ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ * 
+ * THE AUTHOR AND DISTRIBUTORS SPECIFICALLY DISCLAIMS ANY WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+ * AND FITNESS FOR A PARTICULAR PURPOSE.  THE SOFTWARE PROVIDED HEREUNDER IS
+ * ON AN "AS IS" BASIS, AND THE AUTHOR AND DISTRIBUTORS HAS NO OBLIGATIONS TO
+ * PROVIDE MAINTENANCE, SUPPORT, UPDATES, ENHANCEMENTS, OR MODIFICATIONS.
+ *
+ */
+
+#ifndef HAS_PRIV_H
+#define HAS_PRIV_H
+
+#include <string.h>
+#include <ctype.h>
+#include "postgres.h"
+#include "access/heapam.h"
+#include "fmgr.h"
+#include "utils/acl.h"
+#include "utils/builtins.h"
+#include "utils/syscache.h"
+#include "catalog/pg_shadow.h"
+
+#include "storage/proc.h"
+#include "catalog/catname.h"
+#include "utils/fmgroids.h"
+#include "catalog/pg_database.h"
+
+extern Datum has_privilege(PG_FUNCTION_ARGS);
+
+#define PRIV_INSERT			"INSERT\0"
+#define PRIV_SELECT			"SELECT\0"
+#define PRIV_UPDATE			"UPDATE\0"
+#define PRIV_DELETE			"DELETE\0"
+#define PRIV_RULE			"RULE\0"
+#define PRIV_REFERENCES		"REFERENCES\0"
+#define PRIV_TRIGGER		"TRIGGER\0"
+
+#endif	 /* HAS_PRIV_H */
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Joe Conway (#1)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Joe Conway writes:

The patch applies cleanly against cvs tip. One item I was not sure about was
the selection of the OID value for the new function. I chose 1920 for no
other reason that the highest OID in pg_proc.h was 1909, and this seemed
like a safe value. Is there somewhere I should have looked for guidance on
this?

~/pgsql/src/include/catalog$ ./unused_oids
3 - 11
90
143
352 - 353
1264
1713 - 1717
1813
1910 - 16383

ANSI SQL 92 does not have any functions defined for retrieving privilege
information. It does, however define an "information schema" and

"definition

schema" which among other things includes a TABLE_PRIVILEGES view.

Yes, that's what we pretty much want to do once we have schema support.
The function you propose, or one similar to it, will probably be needed to
make this work.

select has_privilege('postgres', 'pg_shadow', 'select');

where
the first parameter is any valid user name
the second parameter can be a table, view, or sequence
the third parameter can be 'select', 'insert', 'update', 'delete', or
'rule'

This is probably going to blow up when we have the said schema support.
Probably better to reference things by oid. Also, since things other than
relations might have privileges sometime, the function name should
probably imply this; maybe "has_table_privilege".

Implementation notes:

* This function should probably go into backend/utils/adt/acl.c.

* You don't need PG_FUNCTION_INFO_V1 for built-in functions.

* I'm not sure whether it's useful to handle NULL parameters explicitly.
The common approach is to return NULL, which would be semantically right
for this function.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#2)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Peter Eisentraut <peter_e@gmx.net> writes:

This is probably going to blow up when we have the said schema support.
Probably better to reference things by oid.

Two versions, one that takes an oid and one that takes a name, might be
convenient. The name version will probably have to accept qualified
names (schema.table) once we have schema support --- but I don't think
that needs to break the function definition. An unqualified name would
be looked up using whatever schema resolution rules would be in effect
for ordinary table references.

We might also want the user to be specified by usesysid rather than
name; and a two-parameter form that assumes user == current_user would
be a particularly useful shorthand.

Also, since things other than
relations might have privileges sometime, the function name should
probably imply this; maybe "has_table_privilege".

Agreed.

* I'm not sure whether it's useful to handle NULL parameters explicitly.
The common approach is to return NULL, which would be semantically right
for this function.

The standard approach for C-coded functions is to mark them
'proisstrict' in pg_proc, and then not waste any code checking for NULL;
the function manager takes care of it for you. The only reason not to
do it that way is if you actually want to return non-NULL for (some
cases with) NULL inputs. Offhand this looks like a strict function to
me...

regards, tom lane

#4Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#2)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

The standard approach for C-coded functions is to mark them
'proisstrict' in pg_proc, and then not waste any code checking for NULL;
the function manager takes care of it for you. The only reason not to
do it that way is if you actually want to return non-NULL for (some
cases with) NULL inputs. Offhand this looks like a strict function to
me...

Thanks for the feedback! To summarize the recommended changes:

- put function into backend/utils/adt/acl.c.
- remove PG_FUNCTION_INFO_V1
- mark 'proisstrict' in pg_proc
- rename to has_table_privilege()
- overload the function name for 6 versions (OIDs 1920 - 1925):
-> has_table_privilege(text username, text relname, text priv)
-> has_table_privilege(oid usesysid, text relname, text priv)
-> has_table_privilege(oid usesysid, oid reloid, text priv)
-> has_table_privilege(text username, oid reloid, text priv)
-> has_table_privilege(text relname, text priv) /* assumes
current_user */
-> has_table_privilege(oid reloid, text priv) /* assumes current_user
*/

New patch forthcoming . . .

-- Joe

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#3)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Tom Lane writes:

Two versions, one that takes an oid and one that takes a name, might be
convenient. The name version will probably have to accept qualified
names (schema.table) once we have schema support

Will you expect the function to do dequoting etc. as well? This might get
out of hand.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#5)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Peter Eisentraut <peter_e@gmx.net> writes:

Will you expect the function to do dequoting etc. as well? This might get
out of hand.

Hm. We already have such code available for nextval(), so I suppose
it might be appropriate to invoke that. Not sure. Might be better
to expect the given string to be the correct case already. Let's see
... if you expect the function to be applied to names extracted from
pg_class or other tables, then exact case would be better --- but it'd
be just as easy to invoke the OID form in such cases. For hand-entered
data the nextval convention is probably more convenient.

regards, tom lane

#7Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#2)
3 attachment(s)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Thanks for the feedback! To summarize the recommended changes:

- put function into backend/utils/adt/acl.c.
- remove PG_FUNCTION_INFO_V1
- mark 'proisstrict' in pg_proc
- rename to has_table_privilege()
- overload the function name for 6 versions (OIDs 1920 - 1925):
-> has_table_privilege(text username, text relname, text priv)
-> has_table_privilege(oid usesysid, text relname, text priv)
-> has_table_privilege(oid usesysid, oid reloid, text priv)
-> has_table_privilege(text username, oid reloid, text priv)
-> has_table_privilege(text relname, text priv) /* assumes
current_user */
-> has_table_privilege(oid reloid, text priv) /* assumes

current_user

*/

Here's a new patch for has_table_privilege( . . .). One change worthy of
note is that I added a definition to fmgr.h as follows:

#define PG_NARGS (fcinfo->nargs)

This allowed me to use two of the new functions to handle both 2 and 3
argument cases. Also different from the above, I used int instead of oid for
the usesysid type.

I'm also attaching a test script and expected output. I haven't yet looked
at how to properly include these into the normal regression testing -- any
pointers are much appreciated.

Thanks,

-- Joe

Attachments:

test_has_table_priv.sqlapplication/octet-stream; name=test_has_table_priv.sqlDownload
test_has_table_priv.outapplication/octet-stream; name=test_has_table_priv.outDownload
has_priv_r2.diffapplication/octet-stream; name=has_priv_r2.diffDownload
diff -Naur pgsql.virg/src/backend/utils/adt/acl.c pgsql.dev/src/backend/utils/adt/acl.c
--- pgsql.virg/src/backend/utils/adt/acl.c	Sun May 27 09:59:30 2001
+++ pgsql.dev/src/backend/utils/adt/acl.c	Sat Jun  2 17:29:41 2001
@@ -756,3 +756,343 @@
 	pfree(str.data);
 	return n;
 }
+
+
+/*
+ * text_text_has_table_privilege
+ *		Check user privileges on a relation given
+ *		usename, relname, and priv name.
+ *		If usename is omitted, current_user is assumed
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+text_text_has_table_privilege(PG_FUNCTION_ARGS)
+{
+	text		*username_text = NULL;
+	char		*username = NULL;
+	int			usesysid = -1;
+	text		*relname_text = NULL;
+	char		*relname = NULL;
+	text		*priv_type_text = NULL;
+	char		*priv_type = NULL;
+	HeapTuple	tuple;
+	bool		result;
+
+	if (PG_NARGS == 2) {
+
+		relname_text = PG_GETARG_TEXT_P(0);
+		priv_type_text = PG_GETARG_TEXT_P(1);
+
+		usesysid = GetUserId();
+
+	} else if (PG_NARGS == 3) {
+
+		username_text = PG_GETARG_TEXT_P(0);
+		relname_text = PG_GETARG_TEXT_P(1);
+		priv_type_text = PG_GETARG_TEXT_P(2);
+
+		/* 
+		 * Convert username 'text' pattern to null-terminated string
+		 */
+		username = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(username_text)));
+
+		/*
+		 * Lookup userid based on username
+		 */
+
+		tuple = SearchSysCache(SHADOWNAME, NameGetDatum(username), 0, 0, 0);
+		if (!HeapTupleIsValid(tuple)) {
+			elog(ERROR, "has_table_privilege: invalid user name %s", (char *) username);
+		}
+
+		usesysid = ((Form_pg_shadow) GETSTRUCT(tuple))->usesysid;
+
+		ReleaseSysCache(tuple);
+
+	} else {
+		/* 
+		 * should never get here!
+		 */
+
+		elog(ERROR, "has_table_privilege: wrong number of arguments %d", PG_NARGS);
+
+	}
+
+	/* 
+	 * Convert relname and priv_type 'text' pattern to null-terminated string
+	 */
+	relname = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(relname_text)));
+	priv_type = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(priv_type_text)));
+
+	result = has_table_privilege(usesysid, relname, priv_type);
+
+	PG_RETURN_BOOL(result);
+}
+
+
+/*
+ * text_oid_has_table_privilege
+ *		Check user privileges on a relation given
+ *		usename, rel oid, and priv name.
+ *		If usename is omitted, current_user is assumed
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+text_oid_has_table_privilege(PG_FUNCTION_ARGS)
+{
+	text		*username_text = NULL;
+	char		*username = NULL;
+	int			usesysid = -1;
+	Oid			reloid = 0;
+	char		*relname = NULL;
+	text		*priv_type_text = NULL;
+	char		*priv_type = NULL;
+	HeapTuple	tuple;
+	bool		result;
+
+	if (PG_NARGS == 2) {
+
+		reloid = PG_GETARG_OID(0);
+		priv_type_text = PG_GETARG_TEXT_P(1);
+
+		usesysid = GetUserId();
+
+	} else if (PG_NARGS == 3) {
+
+		username_text = PG_GETARG_TEXT_P(0);
+		reloid = PG_GETARG_OID(1);
+		priv_type_text = PG_GETARG_TEXT_P(2);
+
+		/* 
+		 * Convert username 'text' pattern to null-terminated string
+		 */
+		username = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(username_text)));
+
+		/*
+		 * Lookup userid based on username
+		 */
+
+		tuple = SearchSysCache(SHADOWNAME, NameGetDatum(username), 0, 0, 0);
+		if (!HeapTupleIsValid(tuple)) {
+			elog(ERROR, "has_table_privilege: invalid user name %s", (char *) username);
+		}
+
+		usesysid = ((Form_pg_shadow) GETSTRUCT(tuple))->usesysid;
+
+		ReleaseSysCache(tuple);
+
+	} else {
+		/* 
+		 * should never get here!
+		 */
+
+		elog(ERROR, "has_table_privilege: wrong number of arguments %d", PG_NARGS);
+
+	}
+
+	/* 
+	 * Convert priv_type 'text' pattern to null-terminated string
+	 */
+	priv_type = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(priv_type_text)));
+
+
+	/*
+	 * Lookup relname based on rel oid
+	 */
+	tuple = SearchSysCache(RELOID, ObjectIdGetDatum(reloid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation oid %d", (int) reloid);
+	}
+
+	relname = NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname);
+
+	ReleaseSysCache(tuple);
+
+	result = has_table_privilege(usesysid, relname, priv_type);
+
+	PG_RETURN_BOOL(result);
+}
+
+
+/*
+ * int_text_has_table_privilege
+ *		Check user privileges on a relation given
+ *		usesysid, relname, and priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+int_text_has_table_privilege(PG_FUNCTION_ARGS)
+{
+	int			usesysid;
+	text		*relname_text;
+	char		*relname;
+	text		*priv_type_text;
+	char		*priv_type;
+	bool		result;
+
+
+	usesysid = PG_GETARG_INT32(0);
+	relname_text = PG_GETARG_TEXT_P(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert relname and priv_type 'text' pattern to null-terminated string
+	 */
+	relname = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(relname_text)));
+	priv_type = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(priv_type_text)));
+
+	result = has_table_privilege(usesysid, relname, priv_type);
+
+	PG_RETURN_BOOL(result);
+}
+
+
+/*
+ * int_oid_has_table_privilege
+ *		Check user privileges on a relation given
+ *		usesysid, rel oid, and priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+int_oid_has_table_privilege(PG_FUNCTION_ARGS)
+{
+	int			usesysid;
+	Oid			reloid;
+	char		*relname;
+	text		*priv_type_text;
+	char		*priv_type;
+	HeapTuple	tuple;
+	bool		result;
+
+
+	usesysid = PG_GETARG_INT32(0);
+	reloid = PG_GETARG_OID(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert priv_type 'text' pattern to null-terminated string
+	 */
+	priv_type = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(priv_type_text)));
+
+	/*
+	 * Lookup relname based on rel oid
+	 */
+	tuple = SearchSysCache(RELOID, ObjectIdGetDatum(reloid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation oid %d", (int) reloid);
+	}
+
+	relname = NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname);
+
+	ReleaseSysCache(tuple);
+
+	result = has_table_privilege(usesysid, relname, priv_type);
+
+	PG_RETURN_BOOL(result);
+}
+
+
+/*
+ * has_table_privilege
+ *		Internal function.
+ *		Check user privileges on a relation given
+ *		usesysid, relname, and priv name
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+
+Datum
+has_table_privilege(int usesysid, char *relname, char *priv_type)
+{
+	HeapTuple	tuple;
+	AclMode		mode;
+	int32		result;
+
+	/*
+	 * Verify usesysid is valid
+	 */
+	tuple = SearchSysCache(SHADOWSYSID, Int32GetDatum(usesysid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid usesysid %d", usesysid);
+	}
+
+	ReleaseSysCache(tuple);
+
+	/*
+	 * Verify relname is valid
+	 */
+	tuple = SearchSysCache(RELNAME, NameGetDatum(relname), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation name %s", (char *) relname);
+	}
+
+	ReleaseSysCache(tuple);
+
+
+	/*
+	 * Determine mode from priv_type string
+	 */
+	if (strcasecmp(priv_type, PRIV_SELECT) == 0) {
+
+		mode = (AclMode) ACL_SELECT;
+
+	} else if (strcasecmp(priv_type, PRIV_INSERT) == 0) {
+
+		mode = (AclMode) ACL_INSERT;
+
+	} else if (strcasecmp(priv_type, PRIV_UPDATE) == 0) {
+
+		mode = (AclMode) ACL_UPDATE;
+
+	} else if (strcasecmp(priv_type, PRIV_DELETE) == 0) {
+
+		mode = (AclMode) ACL_DELETE;
+
+	} else if (strcasecmp(priv_type, PRIV_RULE) == 0) {
+
+		mode = (AclMode) ACL_RULE;
+
+	} else if (strcasecmp(priv_type, PRIV_REFERENCES) == 0) {
+
+		mode = (AclMode) ACL_REFERENCES;
+
+	} else if (strcasecmp(priv_type, PRIV_TRIGGER) == 0) {
+
+		mode = (AclMode) ACL_TRIGGER;
+
+	} else {
+
+		mode = (AclMode) ACL_NO;
+		elog(ERROR, "has_table_privilege: invalid privilege type %s", (char *) priv_type);
+
+	}
+
+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == 1) {
+		PG_RETURN_BOOL(FALSE);
+	} else {
+		PG_RETURN_BOOL(TRUE);
+	}
+}
+
+
diff -Naur pgsql.virg/src/include/fmgr.h pgsql.dev/src/include/fmgr.h
--- pgsql.virg/src/include/fmgr.h	Thu May 17 17:44:18 2001
+++ pgsql.dev/src/include/fmgr.h	Sat Jun  2 14:28:57 2001
@@ -144,6 +144,7 @@
 
 /* Macros for fetching arguments of standard types */
 
+#define PG_NARGS             (fcinfo->nargs)
 #define PG_GETARG_DATUM(n)	 (fcinfo->arg[n])
 #define PG_GETARG_INT32(n)	 DatumGetInt32(PG_GETARG_DATUM(n))
 #define PG_GETARG_UINT32(n)  DatumGetUInt32(PG_GETARG_DATUM(n))
diff -Naur pgsql.virg/src/include/utils/acl.h pgsql.dev/src/include/utils/acl.h
--- pgsql.virg/src/include/utils/acl.h	Sun May 27 09:59:30 2001
+++ pgsql.dev/src/include/utils/acl.h	Sat Jun  2 16:16:07 2001
@@ -164,6 +164,16 @@
 #define ACLCHECK_NO_CLASS		  2
 #define ACLCHECK_NOT_OWNER		  3
 
+
+/* Privilege names for oid_oid_has_table_privilege */
+#define PRIV_INSERT			"INSERT\0"
+#define PRIV_SELECT			"SELECT\0"
+#define PRIV_UPDATE			"UPDATE\0"
+#define PRIV_DELETE			"DELETE\0"
+#define PRIV_RULE			"RULE\0"
+#define PRIV_REFERENCES		"REFERENCES\0"
+#define PRIV_TRIGGER		"TRIGGER\0"
+
 /* warning messages.  set these in aclchk.c. */
 extern char *aclcheck_error_strings[];
 
@@ -174,6 +184,8 @@
 
 extern Acl *aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg);
 
+extern Datum has_table_privilege(int usesysid, char *relname, char *priv_type);
+
 /*
  * routines used by the parser
  */
@@ -192,6 +204,11 @@
 extern Datum aclremove(PG_FUNCTION_ARGS);
 extern Datum aclcontains(PG_FUNCTION_ARGS);
 extern void ExecuteChangeACLStmt(ChangeACLStmt *stmt);
+
+extern Datum int_text_has_table_privilege(PG_FUNCTION_ARGS);
+extern Datum text_text_has_table_privilege(PG_FUNCTION_ARGS);
+extern Datum text_oid_has_table_privilege(PG_FUNCTION_ARGS);
+extern Datum int_oid_has_table_privilege(PG_FUNCTION_ARGS);
 
 /*
  * prototypes for functions in aclchk.c
diff -Naur pgsql.virg/src/include/utils/builtins.h pgsql.dev/src/include/utils/builtins.h
--- pgsql.virg/src/include/utils/builtins.h	Thu Mar 22 04:01:11 2001
+++ pgsql.dev/src/include/utils/builtins.h	Sat Jun  2 16:42:29 2001
@@ -22,6 +22,13 @@
 /*
  *		Defined in adt/
  */
+
+/* acl.c */
+extern Datum text_text_has_table_privilege(PG_FUNCTION_ARGS);
+extern Datum text_oid_has_table_privilege(PG_FUNCTION_ARGS);
+extern Datum int_text_has_table_privilege(PG_FUNCTION_ARGS);
+extern Datum int_oid_has_table_privilege(PG_FUNCTION_ARGS);
+
 /* bool.c */
 extern Datum boolin(PG_FUNCTION_ARGS);
 extern Datum boolout(PG_FUNCTION_ARGS);
diff -Naur pgsql.virg/src/include/catalog/pg_proc.h pgsql.dev/src/include/catalog/pg_proc.h
--- pgsql.virg/src/include/catalog/pg_proc.h	Thu May 24 09:29:29 2001
+++ pgsql.dev/src/include/catalog/pg_proc.h	Sat Jun  2 17:06:33 2001
@@ -2614,6 +2614,20 @@
 DATA(insert OID = 1909 (  int8shr		   PGUID 12 f t t t 2 f 20 "20 23" 100 0 0 100	int8shr - ));
 DESCR("binary shift right");
 
+DATA(insert OID = 1920 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "25 25 25" 100 0 0 100	text_text_has_table_privilege - ));
+DESCR("user privilege on relation by username, relname");
+DATA(insert OID = 1921 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "25 26 25" 100 0 0 100	text_oid_has_table_privilege - ));
+DESCR("user privilege on relation by username, rel oid");
+DATA(insert OID = 1922 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "23 25 25" 100 0 0 100	int_text_has_table_privilege - ));
+DESCR("user privilege on relation by usesysid, relname");
+DATA(insert OID = 1923 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "23 26 25" 100 0 0 100	int_oid_has_table_privilege - ));
+DESCR("user privilege on relation by usesysid, rel oid");
+DATA(insert OID = 1924 (  has_table_privilege		   PGUID 12 f t f t 2 f 16 "25 25" 100 0 0 100	text_text_has_table_privilege - ));
+DESCR("current user privilege on relation by relname");
+DATA(insert OID = 1925 (  has_table_privilege		   PGUID 12 f t f t 2 f 16 "26 25" 100 0 0 100	text_oid_has_table_privilege - ));
+DESCR("current user privilege on relation by rel oid");
+
+
 /*
  * prototypes for functions pg_proc.c
  */
#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#7)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

"Joe Conway" <joe@conway-family.com> writes:

Here's a new patch for has_table_privilege

Looks like you're getting there. Herewith some miscellaneous comments
on minor matters like coding style:

I used int instead of oid for the usesysid type.

I am not sure if that's a good idea or not. Peter E. has proposed
changing usesysid to type OID or even eliminating it entirely
(identifying users by pg_shadow row OID alone). While this hasn't
happened yet, it'd be a good idea to minimize the dependencies of your
code on which type is used for user ID. In particular I'd suggest using
"name" and "id" in the names of your variant functions, not "text" and
"oid" and "int", so that they don't have to be renamed if the types
change.

I'm also attaching a test script and expected output.

The script doesn't seem to demonstrate that any attention is paid to the
mode input --- AFAICT all the tested cases are either all privileges
available or no privileges available. It could probably be a lot
shorter without being materially less effective, too; quasi-exhaustive
tests are usually not worth the cycles to run.

+Datum
+text_oid_has_table_privilege(PG_FUNCTION_ARGS)

This is just my personal preference, but I'd put the type identifiers at
the end (has_table_privilege_name_id) rather than giving them pride of
place at the start of the name.

+Datum
+has_table_privilege(int usesysid, char *relname, char *priv_type)

Since has_table_privilege is just an internal function and is not
fmgr-callable, there's no percentage in declaring it to return Datum;
it should just return bool and not use the PG_RETURN_ macros. You have
in fact called it as though it returned bool, which would be a type
violation if C were not so lax about type conversions.

Actually, though, I'm wondering why has_table_privilege is a function
at all. Its tests for valid usesysid and relname are a waste of cycles;
pg_aclcheck will do those for itself. The only actually useful code in
it is the conversion from a priv_type string to an AclMode code, which
would seem to be better handled as a separate function that just does
that part. The has_table_privilege_foo_bar routines could call
pg_aclcheck for themselves without any material loss of concision.

+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == 1) {

This is not only non-symbolic, but outright wrong. You should be
testing pg_aclcheck's result to see if it is ACLCHECK_OK or not.

+/* Privilege names for oid_oid_has_table_privilege */
+#define PRIV_INSERT			"INSERT\0"
+#define PRIV_SELECT			"SELECT\0"
+#define PRIV_UPDATE			"UPDATE\0"
+#define PRIV_DELETE			"DELETE\0"
+#define PRIV_RULE			"RULE\0"
+#define PRIV_REFERENCES		"REFERENCES\0"
+#define PRIV_TRIGGER		"TRIGGER\0"

You need not write these strings with those redundant null terminators.
For that matter, since they're only known in one function, it's not
clear that they should be exported to all and sundry in a header file
in the first place. I'd be inclined to just code

AclMode convert_priv_string (char * str)
{
if (strcasecmp(str, "SELECT") == 0)
return ACL_SELECT;
if (strcasecmp(str, "INSERT") == 0)
return ACL_INSERT;
... etc ...
elog(ERROR, ...);
}

and keep all the knowledge right there in that function. (Possibly it
should take a text* not char*, so as to avoid duplicated conversion code
in the callers, but this is minor.)

Despite all these gripes, it looks pretty good. One more round of
revisions ...

regards, tom lane

#9Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#2)
3 attachment(s)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Thanks for the detailed feedback, Tom. I really appreciate the pointers on
my style and otherwise. Attached is my next attempt. To summarize the
changes:

- changed usesysid back to Oid. I noticed that the Acl functions all treated
usesysid as an Oid anyway.

- changed function names to has_user_privilege_name_name,
has_user_privilege_name_id, etc

- trimmed down test script, added variety (some privs granted, not all), and
added bad input cases (this already paid off -- see below)

- replaced has_table_privilege(int usesysid, char *relname, char *priv_type)
with
AclMode convert_priv_string (text * priv_type_text)

- changed
if (result == 1) {
PG_RETURN_BOOL(FALSE);
. . .
to
if (result == ACLCHECK_OK) {
PG_RETURN_BOOL(TRUE);
. . .
- removed #define PRIV_INSERT "INSERT\0", etc from acl.h

One item of note -- while pg_aclcheck *does* validate relname for
non-superusers, it *does not* bother for superusers. Therefore I left the
relname check in the has_table_privilege_*_name() functions. Also note that
I skipped has_priv_r3.diff -- that one helped find the superuser/relname
issue.

I hope this version passes muster ;-)

-- Joe

Attachments:

has_priv_r4.diffapplication/octet-stream; name=has_priv_r4.diffDownload
diff -Naur pgsql.virg/src/backend/utils/adt/acl.c pgsql.dev/src/backend/utils/adt/acl.c
--- pgsql.virg/src/backend/utils/adt/acl.c	Sun Jun  3 02:29:13 2001
+++ pgsql.dev/src/backend/utils/adt/acl.c	Sun Jun  3 02:23:17 2001
@@ -756,3 +756,366 @@
 	pfree(str.data);
 	return n;
 }
+
+
+/*
+ * has_table_privilege_name_name
+ *		Check user privileges on a relation given
+ *		usename, relname, and priv name.
+ *		If usename is omitted, current_user is assumed
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_name_name(PG_FUNCTION_ARGS)
+{
+	text		*username_text = NULL;
+	char		*username = NULL;
+	Oid			usesysid = (Oid) -1;
+	text		*relname_text = NULL;
+	char		*relname = NULL;
+	text		*priv_type_text = NULL;
+	HeapTuple	tuple;
+	AclMode		mode;
+	int32		result;
+
+	if (PG_NARGS == 2) {
+
+		relname_text = PG_GETARG_TEXT_P(0);
+		priv_type_text = PG_GETARG_TEXT_P(1);
+
+		usesysid = GetUserId();
+
+	} else if (PG_NARGS == 3) {
+
+		username_text = PG_GETARG_TEXT_P(0);
+		relname_text = PG_GETARG_TEXT_P(1);
+		priv_type_text = PG_GETARG_TEXT_P(2);
+
+		/* 
+		 * Convert username 'text' pattern to null-terminated string
+		 */
+		username = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(username_text)));
+
+		/*
+		 * Lookup userid based on username
+		 */
+
+		tuple = SearchSysCache(SHADOWNAME, NameGetDatum(username), 0, 0, 0);
+		if (!HeapTupleIsValid(tuple)) {
+			elog(ERROR, "has_table_privilege: invalid user name %s", (char *) username);
+		}
+
+		usesysid = (Oid) ((Form_pg_shadow) GETSTRUCT(tuple))->usesysid;
+
+		ReleaseSysCache(tuple);
+
+	} else {
+		/* 
+		 * should never get here!
+		 */
+
+		elog(ERROR, "has_table_privilege: wrong number of arguments %d", PG_NARGS);
+
+	}
+
+	/* 
+	 * Convert relname 'text' pattern to null-terminated string
+	 */
+	relname = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(relname_text)));
+
+	/*
+	 * Check relname is valid.
+	 * This is needed to deal with the case when usename is a superuser
+	 * in which case pg_aclcheck simply returns ACLCHECK_OK
+	 * without validating relname
+	 */
+	tuple = SearchSysCache(RELNAME, PointerGetDatum(relname), 0, 0, 0);
+
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relname %s", relname);
+	}
+	ReleaseSysCache(tuple);
+
+	/* 
+	 * Convert priv_type_text to an AclMode
+	 */
+	mode = convert_priv_string(priv_type_text);
+
+	/* 
+	 * Finally, check for the privilege
+	 */
+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == ACLCHECK_OK) {
+		PG_RETURN_BOOL(TRUE);
+	} else {
+		PG_RETURN_BOOL(FALSE);
+	}
+
+}
+
+
+/*
+ * has_table_privilege_name_id
+ *		Check user privileges on a relation given
+ *		usename, rel oid, and priv name.
+ *		If usename is omitted, current_user is assumed
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_name_id(PG_FUNCTION_ARGS)
+{
+	text		*username_text = NULL;
+	char		*username = NULL;
+	Oid			usesysid = (Oid) -1;
+	Oid			reloid = 0;
+	char		*relname = NULL;
+	text		*priv_type_text = NULL;
+	HeapTuple	tuple;
+	AclMode		mode;
+	int32		result;
+
+	if (PG_NARGS == 2) {
+
+		reloid = PG_GETARG_OID(0);
+		priv_type_text = PG_GETARG_TEXT_P(1);
+
+		usesysid = GetUserId();
+
+	} else if (PG_NARGS == 3) {
+
+		username_text = PG_GETARG_TEXT_P(0);
+		reloid = PG_GETARG_OID(1);
+		priv_type_text = PG_GETARG_TEXT_P(2);
+
+		/* 
+		 * Convert username 'text' pattern to null-terminated string
+		 */
+		username = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(username_text)));
+
+		/*
+		 * Lookup userid based on username
+		 */
+
+		tuple = SearchSysCache(SHADOWNAME, NameGetDatum(username), 0, 0, 0);
+		if (!HeapTupleIsValid(tuple)) {
+			elog(ERROR, "has_table_privilege: invalid user name %s", (char *) username);
+		}
+
+		usesysid = (Oid) ((Form_pg_shadow) GETSTRUCT(tuple))->usesysid;
+
+		ReleaseSysCache(tuple);
+
+	} else {
+		/* 
+		 * should never get here!
+		 */
+
+		elog(ERROR, "has_table_privilege: wrong number of arguments %d", PG_NARGS);
+
+	}
+
+	/*
+	 * Lookup relname based on rel oid
+	 */
+	tuple = SearchSysCache(RELOID, ObjectIdGetDatum(reloid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation oid %d", (int) reloid);
+	}
+
+	relname = NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname);
+
+	ReleaseSysCache(tuple);
+
+	/* 
+	 * Convert priv_type_text to an AclMode
+	 */
+	mode = convert_priv_string(priv_type_text);
+
+	/* 
+	 * Finally, check for the privilege
+	 */
+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == ACLCHECK_OK) {
+		PG_RETURN_BOOL(TRUE);
+	} else {
+		PG_RETURN_BOOL(FALSE);
+	}
+
+}
+
+
+/*
+ * has_table_privilege_id_name
+ *		Check user privileges on a relation given
+ *		usesysid, relname, and priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_id_name(PG_FUNCTION_ARGS)
+{
+	Oid			usesysid;
+	text		*relname_text;
+	char		*relname;
+	text		*priv_type_text;
+	HeapTuple	tuple;
+	AclMode		mode;
+	int32		result;
+
+	usesysid = PG_GETARG_OID(0);
+	relname_text = PG_GETARG_TEXT_P(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert relname 'text' pattern to null-terminated string
+	 */
+	relname = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(relname_text)));
+
+	/*
+	 * Check relname is valid.
+	 * This is needed to deal with the case when usename is a superuser
+	 * in which case pg_aclcheck simply returns ACLCHECK_OK
+	 * without validating relname
+	 */
+	tuple = SearchSysCache(RELNAME, PointerGetDatum(relname), 0, 0, 0);
+
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relname %s", relname);
+	}
+	ReleaseSysCache(tuple);
+
+	/* 
+	 * Convert priv_type_text to an AclMode
+	 */
+	mode = convert_priv_string(priv_type_text);
+
+	/* 
+	 * Finally, check for the privilege
+	 */
+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == ACLCHECK_OK) {
+		PG_RETURN_BOOL(TRUE);
+	} else {
+		PG_RETURN_BOOL(FALSE);
+	}
+
+}
+
+
+/*
+ * has_table_privilege_id_id
+ *		Check user privileges on a relation given
+ *		usesysid, rel oid, and priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_id_id(PG_FUNCTION_ARGS)
+{
+	Oid			usesysid;
+	Oid			reloid;
+	char		*relname;
+	text		*priv_type_text;
+	HeapTuple	tuple;
+	AclMode		mode;
+	int32		result;
+
+
+	usesysid = PG_GETARG_OID(0);
+	reloid = PG_GETARG_OID(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/*
+	 * Lookup relname based on rel oid
+	 */
+	tuple = SearchSysCache(RELOID, ObjectIdGetDatum(reloid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation oid %d", (int) reloid);
+	}
+
+	relname = NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname);
+
+	ReleaseSysCache(tuple);
+
+	/* 
+	 * Convert priv_type_text to an AclMode
+	 */
+	mode = convert_priv_string(priv_type_text);
+
+	/* 
+	 * Finally, check for the privilege
+	 */
+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == ACLCHECK_OK) {
+		PG_RETURN_BOOL(TRUE);
+	} else {
+		PG_RETURN_BOOL(FALSE);
+	}
+
+}
+
+/*
+ * convert_priv_string
+ *		Internal function.
+ *		Return mode from priv_type string
+ *
+ * RETURNS
+ *		AclMode
+ */
+
+AclMode
+convert_priv_string(text *priv_type_text)
+{
+	char	*priv_type = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(priv_type_text)));
+
+	/*
+	 * Return mode from priv_type string
+	 */
+	if (strcasecmp(priv_type, "SELECT") == 0)
+		return ACL_SELECT;
+
+	if (strcasecmp(priv_type, "INSERT") == 0)
+		return ACL_INSERT;
+
+	if (strcasecmp(priv_type, "UPDATE") == 0)
+		return ACL_UPDATE;
+
+	if (strcasecmp(priv_type, "DELETE") == 0)
+		return ACL_DELETE;
+
+	if (strcasecmp(priv_type, "RULE") == 0)
+		return ACL_RULE;
+
+	if (strcasecmp(priv_type, "REFERENCES") == 0)
+		return ACL_REFERENCES;
+
+	if (strcasecmp(priv_type, "TRIGGER") == 0)
+		return ACL_TRIGGER;
+
+	elog(ERROR, "has_table_privilege: invalid privilege type %s", priv_type);
+	/*
+	 * We should never get here, but stop the compiler from complaining
+	 */
+	return ACL_NO;
+
+}
+
+
diff -Naur pgsql.virg/src/include/catalog/pg_proc.h pgsql.dev/src/include/catalog/pg_proc.h
--- pgsql.virg/src/include/catalog/pg_proc.h	Sun Jun  3 02:29:15 2001
+++ pgsql.dev/src/include/catalog/pg_proc.h	Sun Jun  3 00:52:19 2001
@@ -2614,6 +2614,20 @@
 DATA(insert OID = 1909 (  int8shr		   PGUID 12 f t t t 2 f 20 "20 23" 100 0 0 100	int8shr - ));
 DESCR("binary shift right");
 
+DATA(insert OID = 1920 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "25 25 25" 100 0 0 100	has_table_privilege_name_name - ));
+DESCR("user privilege on relation by username, relname");
+DATA(insert OID = 1921 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "25 26 25" 100 0 0 100	has_table_privilege_name_id - ));
+DESCR("user privilege on relation by username, rel oid");
+DATA(insert OID = 1922 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "26 25 25" 100 0 0 100	has_table_privilege_id_name - ));
+DESCR("user privilege on relation by usesysid, relname");
+DATA(insert OID = 1923 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "26 26 25" 100 0 0 100	has_table_privilege_id_id - ));
+DESCR("user privilege on relation by usesysid, rel oid");
+DATA(insert OID = 1924 (  has_table_privilege		   PGUID 12 f t f t 2 f 16 "25 25" 100 0 0 100	has_table_privilege_name_name - ));
+DESCR("current user privilege on relation by relname");
+DATA(insert OID = 1925 (  has_table_privilege		   PGUID 12 f t f t 2 f 16 "26 25" 100 0 0 100	has_table_privilege_name_id - ));
+DESCR("current user privilege on relation by rel oid");
+
+
 /*
  * prototypes for functions pg_proc.c
  */
diff -Naur pgsql.virg/src/include/fmgr.h pgsql.dev/src/include/fmgr.h
--- pgsql.virg/src/include/fmgr.h	Sun Jun  3 02:29:17 2001
+++ pgsql.dev/src/include/fmgr.h	Sat Jun  2 23:56:01 2001
@@ -144,6 +144,7 @@
 
 /* Macros for fetching arguments of standard types */
 
+#define PG_NARGS             (fcinfo->nargs)
 #define PG_GETARG_DATUM(n)	 (fcinfo->arg[n])
 #define PG_GETARG_INT32(n)	 DatumGetInt32(PG_GETARG_DATUM(n))
 #define PG_GETARG_UINT32(n)  DatumGetUInt32(PG_GETARG_DATUM(n))
diff -Naur pgsql.virg/src/include/utils/acl.h pgsql.dev/src/include/utils/acl.h
--- pgsql.virg/src/include/utils/acl.h	Sun Jun  3 02:29:17 2001
+++ pgsql.dev/src/include/utils/acl.h	Sun Jun  3 00:37:19 2001
@@ -164,6 +164,7 @@
 #define ACLCHECK_NO_CLASS		  2
 #define ACLCHECK_NOT_OWNER		  3
 
+
 /* warning messages.  set these in aclchk.c. */
 extern char *aclcheck_error_strings[];
 
@@ -174,6 +175,8 @@
 
 extern Acl *aclinsert3(Acl *old_acl, AclItem *mod_aip, unsigned modechg);
 
+extern AclMode convert_priv_string(text *priv_type_text);
+
 /*
  * routines used by the parser
  */
@@ -192,6 +195,11 @@
 extern Datum aclremove(PG_FUNCTION_ARGS);
 extern Datum aclcontains(PG_FUNCTION_ARGS);
 extern void ExecuteChangeACLStmt(ChangeACLStmt *stmt);
+
+extern Datum has_table_privilege_name_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_id(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id_id(PG_FUNCTION_ARGS);
 
 /*
  * prototypes for functions in aclchk.c
diff -Naur pgsql.virg/src/include/utils/builtins.h pgsql.dev/src/include/utils/builtins.h
--- pgsql.virg/src/include/utils/builtins.h	Sun Jun  3 02:29:19 2001
+++ pgsql.dev/src/include/utils/builtins.h	Sat Jun  2 23:59:45 2001
@@ -22,6 +22,13 @@
 /*
  *		Defined in adt/
  */
+
+/* acl.c */
+extern Datum has_table_privilege_name_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_id(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id_id(PG_FUNCTION_ARGS);
+
 /* bool.c */
 extern Datum boolin(PG_FUNCTION_ARGS);
 extern Datum boolout(PG_FUNCTION_ARGS);
test_has_table_priv.sqlapplication/octet-stream; name=test_has_table_priv.sqlDownload
test_has_table_priv.outapplication/octet-stream; name=test_has_table_priv.outDownload
#10Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#6)
Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

[ -> hackers ]

Tom Lane writes:

Will you expect the function to do dequoting etc. as well? This might get
out of hand.

Hm. We already have such code available for nextval(),

IMHO, nextval() isn't the greatest interface in the world. I do like the
alternative (deprecated?) syntax sequence.nextval() because of the
notational resemblence to OO. (We might even be able to turn this into
something like an SQL99 "class" feature.)

As I understand it, currently

relation.function(a, b, c)

ends up as being a function call

function(relation, a, b, c)

where the first argument is "text". This is probably an unnecessary
fragility, since the oid of the relation should already be known by that
time. So perhaps we could change this that the first argument gets passed
in an Oid. Then we'd really only need the Oid version of Joe's
has_*_privilege functions.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#10)
Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Peter Eisentraut <peter_e@gmx.net> writes:

IMHO, nextval() isn't the greatest interface in the world. I do like the
alternative (deprecated?) syntax sequence.nextval() because of the
notational resemblence to OO.

Try "nonexistent". I too would like a notation like that, because it
would be more transparent to the user w.r.t. case folding and such.
But it doesn't exist now.

Observe, however, that such a notation would work well only for queries
in which the sequence/table name is fixed and known when the query is
written. I don't see a way to use it in the case where the name is
being computed at runtime (eg, taken from a table column). So it
doesn't really solve the problem posed by has_table_privilege.

As I understand it, currently
relation.function(a, b, c)
ends up as being a function call
function(relation, a, b, c)
where the first argument is "text".

Sorry, that has nothing to do with reality. What we actually have is
an equivalence between the two notations
rel.func
func(rel)
where the semantics are that an entire tuple of the relation "rel" is
passed to the function. This doesn't really gain us anything for the
problem at hand (and we'll quite likely have to give it up anyway when
we implement schemas, since SQL has very different ideas about what
a.b.c means than our current parser does).

regards, tom lane

#12Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#10)
Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

where the semantics are that an entire tuple of the relation "rel" is
passed to the function. This doesn't really gain us anything for the
problem at hand (and we'll quite likely have to give it up anyway when
we implement schemas, since SQL has very different ideas about what
a.b.c means than our current parser does).

I wasn't quite sure if there are changes I can/should make to
has_table_privilege based on this discussion. Is there any action for me on
this (other than finishing the regression test and creating documentation
patches)?

Thanks,

-- Joe

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#12)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

"Joe Conway" <joe@conway-family.com> writes:

I wasn't quite sure if there are changes I can/should make to
has_table_privilege based on this discussion.

My feeling is that the name-based variants of has_table_privilege should
perform downcasing and truncation of the supplied strings before trying
to use them as tablename or username; see get_seq_name in
backend/commands/sequence.c for a model. (BTW, I only just now added
truncation code to that routine, so look at current CVS. Perhaps the
routine should be renamed and placed somewhere else, so that sequence.c
and has_table_privilege can share it.)

Peter's argument seemed to be that there shouldn't be name-based
variants at all, with which I do not agree; but perhaps that's not
what he meant.

regards, tom lane

#14Joe Conway
joe.conway@mail.com
In reply to: Peter Eisentraut (#10)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

My feeling is that the name-based variants of has_table_privilege should
perform downcasing and truncation of the supplied strings before trying
to use them as tablename or username; see get_seq_name in
backend/commands/sequence.c for a model. (BTW, I only just now added
truncation code to that routine, so look at current CVS. Perhaps the
routine should be renamed and placed somewhere else, so that sequence.c
and has_table_privilege can share it.)

Looking at get_seq_name, it does seem like it should be called something
like get_object_name (or just get_name?) and moved to a common location. Am
I correct in thinking that this function could/should be called by any other
function (internal, C, plpgsql, or otherwise) which accepts a text
representation of a system object name?

What if I rename the get_seq_name function and move it to
backend/utils/adt/name.c (and of course change the references to it in
sequence.c)? Actually, now I'm wondering why nameout doesn't downcase and
truncate.

-- Joe

#15Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#10)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

My feeling is that the name-based variants of has_table_privilege should
perform downcasing and truncation of the supplied strings before trying
to use them as tablename or username; see get_seq_name in
backend/commands/sequence.c for a model. (BTW, I only just now added
truncation code to that routine, so look at current CVS. Perhaps the
routine should be renamed and placed somewhere else, so that sequence.c
and has_table_privilege can share it.)

Looking at get_seq_name, it does seem like it should be called something
like get_object_name (or just get_name?) and moved to a common location. Am
I correct in thinking that this function could/should be called by any other
function (internal, C, plpgsql, or otherwise) which accepts a text
representation of a system object name?

What if I rename the get_seq_name function and move it to
backend/utils/adt/name.c (and of course change the references to it in
sequence.c)? Actually, now I'm wondering why nameout doesn't downcase and
truncate.

-- Joe

#16Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#10)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

My feeling is that the name-based variants of has_table_privilege should
perform downcasing and truncation of the supplied strings before trying
to use them as tablename or username; see get_seq_name in
backend/commands/sequence.c for a model. (BTW, I only just now added
truncation code to that routine, so look at current CVS. Perhaps the
routine should be renamed and placed somewhere else, so that sequence.c
and has_table_privilege can share it.)

Looking at get_seq_name, it does seem like it should be called something
like get_object_name (or just get_name?) and moved to a common location. Am
I correct in thinking that this function could/should be called by any other
function (internal, C, plpgsql, or otherwise) which accepts a text
representation of a system object name?

What if I rename the get_seq_name function and move it to
backend/utils/adt/name.c (and of course change the references to it in
sequence.c)? Actually, now I'm wondering why nameout doesn't downcase and
truncate.

-- Joe

#17Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#10)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

My feeling is that the name-based variants of has_table_privilege should
perform downcasing and truncation of the supplied strings before trying
to use them as tablename or username; see get_seq_name in
backend/commands/sequence.c for a model. (BTW, I only just now added
truncation code to that routine, so look at current CVS. Perhaps the
routine should be renamed and placed somewhere else, so that sequence.c
and has_table_privilege can share it.)

Looking at get_seq_name, it does seem like it should be called something
like get_object_name (or just get_name?) and moved to a common location. Am
I correct in thinking that this function could/should be called by any other
function (internal, C, plpgsql, or otherwise) which accepts a text
representation of a system object name?

What if I rename the get_seq_name function and move it to
backend/utils/adt/name.c (and of course change the references to it in
sequence.c)? Actually, now I'm wondering why nameout doesn't downcase and
truncate.

-- Joe

#18Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#10)
sorry for the repeats - no spam intended :-)

representation of a system object name?

What if I rename the get_seq_name function and move it to
backend/utils/adt/name.c (and of course change the references to it in
sequence.c)? Actually, now I'm wondering why nameout doesn't downcase and
truncate.

-- Joe

Yikes! Sorry about sending that last message 3 times -- I guess that's what
I get for using an evil mail client ;-)

Joe

#19Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#13)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Tom Lane writes:

My feeling is that the name-based variants of has_table_privilege should
perform downcasing and truncation of the supplied strings before trying
to use them as tablename or username; see get_seq_name in
backend/commands/sequence.c for a model.

I don't like this approach. It's ugly, non-intuitive, and inconvenient.

Since these functions will primarily be used in building a sort of
information schema and for querying system catalogs, we should use the
approach that is or will be used there: character type values contain the
table name already case-adjusted. Imagine the pain we would have to go
through to *re-quote* the names we get from the system catalogs and
information schema components before passing them to this function.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#20Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#19)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Peter Eisentraut <peter_e@gmx.net> writes:

Since these functions will primarily be used in building a sort of
information schema and for querying system catalogs, we should use the
approach that is or will be used there: character type values contain the
table name already case-adjusted.

Weren't you just arguing that such cases could/should use the OID, not
the name at all? ISTM the name-based variants will primarily be used
for user-entered names, and in that case the user can reasonably expect
that a name will be interpreted the same way as if he'd written it out
in a query.

The nextval approach is ugly, I'll grant you, but it's also functional.
We got complaints about nextval before we put that in; we get lots
fewer now.

regards, tom lane

#21Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#20)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Tom Lane writes:

Weren't you just arguing that such cases could/should use the OID, not
the name at all?

Yes, but if we're going to have name arguments, we should have sane ones.

ISTM the name-based variants will primarily be used for user-entered
names, and in that case the user can reasonably expect that a name
will be interpreted the same way as if he'd written it out in a query.

That would be correct if the user were actually entering the name in the
same way, i.e., unquoted or double-quoted.

The nextval approach is ugly, I'll grant you, but it's also functional.

But it's incompatible with the SQL conventions.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#22Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#21)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

ISTM the name-based variants will primarily be used for user-entered
names, and in that case the user can reasonably expect that a name
will be interpreted the same way as if he'd written it out in a query.

That would be correct if the user were actually entering the name in the
same way, i.e., unquoted or double-quoted.

The nextval approach is ugly, I'll grant you, but it's also functional.

But it's incompatible with the SQL conventions.

Is the concern that the name-based variants of the function should be called
like:

select has_table_privilege(current_user, pg_class, 'insert');
or
select has_table_privilege(current_user, "My Quoted Relname", 'insert');

instead of

select has_table_privilege(current_user, 'pg_class', 'insert');
or
select has_table_privilege(current_user, '"My Quoted Relname"',
'insert');

?

If so, what would be involved in fixing it?

From an end user's perspective, I wouldn't mind the latter syntax, although
the former is clearly more intuitive. But I'd rather have the second form
than nothing (just MHO).

-- Joe

#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#22)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

"Joe Conway" <joe@conway-family.com> writes:

Is the concern that the name-based variants of the function should be called
like:

select has_table_privilege(current_user, pg_class, 'insert');
or
select has_table_privilege(current_user, "My Quoted Relname", 'insert');

It'd be really nice to do that, but I don't see any reasonable way to do
it. The problem is that the unquoted relation name will probably be
resolved (to the wrong thing) before we discover that the function wants
it to be resolved as a relation OID. Remember that the arguments of a
function have to be resolved before we can even start to look up the
function, since function lookup depends on the types of the arguments.

I have just thought of a possible compromise. Peter is right that we
don't want case conversion on table names that are extracted from
catalogs. But I think we do want it on table names expressed as string
literals. Could we make the assumption that table names in catalogs
will be of type 'name'? If so, it'd work to make two versions of the
has_table_privilege function, one taking type "name" and the other
taking type "text". The "name" version would take its input as-is,
the "text" version would do case folding and truncation. This would
work transparently for queries selecting relation names from the system
catalogs, and it'd also work transparently for queries using unmarked
string literals (which will be preferentially resolved as type "text").
Worst case if the system makes the wrong choice is you throw in an
explicit coercion to name or text. Comments?

regards, tom lane

#24Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Joe Conway (#9)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

Thanks for the detailed feedback, Tom. I really appreciate the pointers on
my style and otherwise. Attached is my next attempt. To summarize the
changes:

- changed usesysid back to Oid. I noticed that the Acl functions all treated
usesysid as an Oid anyway.

- changed function names to has_user_privilege_name_name,
has_user_privilege_name_id, etc

- trimmed down test script, added variety (some privs granted, not all), and
added bad input cases (this already paid off -- see below)

- replaced has_table_privilege(int usesysid, char *relname, char *priv_type)
with
AclMode convert_priv_string (text * priv_type_text)

- changed
if (result == 1) {
PG_RETURN_BOOL(FALSE);
. . .
to
if (result == ACLCHECK_OK) {
PG_RETURN_BOOL(TRUE);
. . .
- removed #define PRIV_INSERT "INSERT\0", etc from acl.h

One item of note -- while pg_aclcheck *does* validate relname for
non-superusers, it *does not* bother for superusers. Therefore I left the
relname check in the has_table_privilege_*_name() functions. Also note that
I skipped has_priv_r3.diff -- that one helped find the superuser/relname
issue.

I hope this version passes muster ;-)

-- Joe

[ Attachment, skipping... ]

[ Attachment, skipping... ]

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to majordomo@postgresql.org)

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#25Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#24)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.

It's not approved yet ...

regards, tom lane

#26Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#25)
Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Your patch has been added to the PostgreSQL unapplied patches list at:
http://candle.pha.pa.us/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.

It's not approved yet ...

OK.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#27Joe Conway
joe@conway-family.com
In reply to: Peter Eisentraut (#21)
1 attachment(s)
Re: [HACKERS] Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

I have just thought of a possible compromise. Peter is right that we
don't want case conversion on table names that are extracted from
catalogs. But I think we do want it on table names expressed as string
literals. Could we make the assumption that table names in catalogs
will be of type 'name'? If so, it'd work to make two versions of the
has_table_privilege function, one taking type "name" and the other
taking type "text". The "name" version would take its input as-is,
the "text" version would do case folding and truncation. This would
work transparently for queries selecting relation names from the system
catalogs, and it'd also work transparently for queries using unmarked
string literals (which will be preferentially resolved as type "text").
Worst case if the system makes the wrong choice is you throw in an
explicit coercion to name or text. Comments?

OK -- here's take #5.

It "make"s and "make check"s clean against current cvs tip.

There are now both Text and Name variants, and the regression test support
is rolled into the patch. Note that to be complete wrt Name based variants,
there are now 12 user visible versions of has_table_privilege:

has_table_privilege(Text usename, Text relname, Text priv_type)
has_table_privilege(Text usename, Name relname, Text priv_type)
has_table_privilege(Name usename, Text relname, Text priv_type)
has_table_privilege(Name usename, Name relname, Text priv_type)
has_table_privilege(Text relname, Text priv_type) /* assumes current_user */
has_table_privilege(Name relname, Text priv_type) /* assumes current_user */
has_table_privilege(Text usename, Oid reloid, Text priv_type)
has_table_privilege(Name usename, Oid reloid, Text priv_type)
has_table_privilege(Oid reloid, Text priv_type) /* assumes current_user */
has_table_privilege(Oid usesysid, Text relname, Text priv_type)
has_table_privilege(Oid usesysid, Name relname, Text priv_type)
has_table_privilege(Oid usesysid, Oid reloid, Text priv_type)

For the Text based inputs, a new internal function, get_Name is used
(shamelessly copied from get_seq_name in sequence.c) to downcase if not
quoted, or remove quotes if quoted, and truncate. I also added a few test
cases for the downcasing, quote removal, and Name based variants to the
regression test.

Only thing left (I hope!) is documentation. I'm sure I either have or can
get the DocBook tools, but I've never used them. Would it be simpler to
clone and hand edit one of the existing docs? Any suggestions to get me
started?

Thanks,

-- Joe

Attachments:

has_priv_r5.diffapplication/octet-stream; name=has_priv_r5.diffDownload
diff -Naur pgsql.virg/src/backend/utils/adt/acl.c pgsql.dev/src/backend/utils/adt/acl.c
--- pgsql.virg/src/backend/utils/adt/acl.c	Sat Jun  9 23:21:55 2001
+++ pgsql.dev/src/backend/utils/adt/acl.c	Sun Jun 10 02:10:34 2001
@@ -31,6 +31,12 @@
 static bool aclitemeq(const AclItem *a1, const AclItem *a2);
 static bool aclitemgt(const AclItem *a1, const AclItem *a2);
 
+AclMode convert_priv_string(text *priv_type_text);
+bool has_table_privilege_cname_cname(char *username, char *relname, text *priv_type_text);
+bool has_table_privilege_id_cname(Oid usesysid, char *relname, text *priv_type_text);
+bool has_table_privilege_cname_id(char *username, Oid reloid, text *priv_type_text);
+static char *get_Name(text *relin);
+
 #define ACL_IDTYPE_GID_KEYWORD	"group"
 #define ACL_IDTYPE_UID_KEYWORD	"user"
 
@@ -709,4 +715,753 @@
 	ret = pstrdup(str.data);
 	pfree(str.data);
 	return ret;
+}
+
+
+/*
+ * has_table_privilege_tname_tname
+ *		Check user privileges on a relation given
+ *		text usename, text relname, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_tname_tname(PG_FUNCTION_ARGS)
+{
+	text		*username_text;
+	char		*username;
+	text		*relname_text;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	username_text = PG_GETARG_TEXT_P(0);
+	relname_text = PG_GETARG_TEXT_P(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert username and relname 'text' pattern to null-terminated string
+	 */
+	username = get_Name(username_text);
+	relname = get_Name(relname_text);
+
+	/*
+	 * Make use of has_table_privilege_cname_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_cname_cname(username, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_tname_name
+ *		Check user privileges on a relation given
+ *		text usename, name relname, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_tname_name(PG_FUNCTION_ARGS)
+{
+	text		*username_text;
+	char		*username;
+	Name		relname_name;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	username_text = PG_GETARG_TEXT_P(0);
+	relname_name = PG_GETARG_NAME(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert username 'text' pattern to null-terminated string
+	 */
+	username = get_Name(username_text);
+
+	/* 
+	 * Convert relname 'name' pattern to null-terminated string
+	 */
+	relname = DatumGetCString(DirectFunctionCall1(nameout, PointerGetDatum(relname_name)));
+
+	/*
+	 * Make use of has_table_privilege_cname_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_cname_cname(username, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_name_tname
+ *		Check user privileges on a relation given
+ *		name usename, text relname, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_name_tname(PG_FUNCTION_ARGS)
+{
+	Name		username_name;
+	char		*username;
+	text		*relname_text;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	username_name = PG_GETARG_NAME(0);
+	relname_text = PG_GETARG_TEXT_P(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert username 'name' pattern to null-terminated string
+	 */
+	username = DatumGetCString(DirectFunctionCall1(nameout, PointerGetDatum(username_name)));
+
+	/* 
+	 * Convert relname 'text' pattern to null-terminated string
+	 */
+	relname = get_Name(relname_text);
+
+	/*
+	 * Make use of has_table_privilege_cname_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_cname_cname(username, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_name_name
+ *		Check user privileges on a relation given
+ *		name usename, name relname, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_name_name(PG_FUNCTION_ARGS)
+{
+	Name		username_name;
+	char		*username;
+	Name		relname_name;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	username_name = PG_GETARG_NAME(0);
+	relname_name = PG_GETARG_NAME(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert username and relname 'name' pattern to null-terminated string
+	 */
+	username = DatumGetCString(DirectFunctionCall1(nameout, PointerGetDatum(username_name)));
+	relname = DatumGetCString(DirectFunctionCall1(nameout, PointerGetDatum(relname_name)));
+
+	/*
+	 * Make use of has_table_privilege_cname_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_cname_cname(username, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_tname
+ *		Check user privileges on a relation given
+ *		text relname and text priv name.
+ *		current_user is assumed
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_tname(PG_FUNCTION_ARGS)
+{
+	Oid			usesysid = (Oid) -1;
+	text		*relname_text;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	relname_text = PG_GETARG_TEXT_P(0);
+	priv_type_text = PG_GETARG_TEXT_P(1);
+
+	usesysid = GetUserId();
+
+	/* 
+	 * Convert relname 'text' pattern to null-terminated string
+	 */
+	relname = get_Name(relname_text);
+
+	/*
+	 * Make use of has_table_privilege_id_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_id_cname(usesysid, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+/*
+ * has_table_privilege_name
+ *		Check user privileges on a relation given
+ *		name relname and text priv name.
+ *		current_user is assumed
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_name(PG_FUNCTION_ARGS)
+{
+	Oid			usesysid = (Oid) -1;
+	Name		relname_name;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	relname_name = PG_GETARG_NAME(0);
+	priv_type_text = PG_GETARG_TEXT_P(1);
+
+	usesysid = GetUserId();
+
+	/* 
+	 * Convert relname 'Name' pattern to null-terminated string
+	 */
+	relname = DatumGetCString(DirectFunctionCall1(nameout, PointerGetDatum(relname_name)));
+
+	/*
+	 * Make use of has_table_privilege_id_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_id_cname(usesysid, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_tname_id
+ *		Check user privileges on a relation given
+ *		text usename, rel oid, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_tname_id(PG_FUNCTION_ARGS)
+{
+	text		*username_text;
+	char		*username;
+	Oid			reloid = 0;
+	text		*priv_type_text;
+	bool		result;
+
+	username_text = PG_GETARG_TEXT_P(0);
+	reloid = PG_GETARG_OID(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert username 'text' pattern to null-terminated string
+	 */
+	username = get_Name(username_text);
+
+	/*
+	 * Make use of has_table_privilege_cname_id.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_cname_id(username, reloid, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_name_id
+ *		Check user privileges on a relation given
+ *		name usename, rel oid, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_name_id(PG_FUNCTION_ARGS)
+{
+	Name		username_name;
+	char		*username;
+	Oid			reloid = 0;
+	text		*priv_type_text = NULL;
+	bool		result;
+
+	username_name = PG_GETARG_NAME(0);
+	reloid = PG_GETARG_OID(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert username 'name' pattern to null-terminated string
+	 */
+	username = DatumGetCString(DirectFunctionCall1(nameout, PointerGetDatum(username_name)));
+
+	/*
+	 * Make use of has_table_privilege_cname_id.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_cname_id(username, reloid, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_id
+ *		Check user privileges on a relation given
+ *		rel oid, and text priv name.
+ *		current_user is assumed
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_id(PG_FUNCTION_ARGS)
+{
+	char		*username;
+	Oid			reloid = 0;
+	text		*priv_type_text;
+	bool		result;
+
+	reloid = PG_GETARG_OID(0);
+	priv_type_text = PG_GETARG_TEXT_P(1);
+	username = GetUserName(GetUserId());
+
+	/*
+	 * Make use of has_table_privilege_cname_id.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_cname_id(username, reloid, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_id_tname
+ *		Check user privileges on a relation given
+ *		usesysid, text relname, and priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_id_tname(PG_FUNCTION_ARGS)
+{
+	Oid			usesysid;
+	text		*relname_text;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	usesysid = PG_GETARG_OID(0);
+	relname_text = PG_GETARG_TEXT_P(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert relname 'text' pattern to null-terminated string
+	 */
+	relname = get_Name(relname_text);
+
+	/*
+	 * Make use of has_table_privilege_id_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_id_cname(usesysid, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_id_name
+ *		Check user privileges on a relation given
+ *		usesysid, name relname, and priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_id_name(PG_FUNCTION_ARGS)
+{
+	Oid			usesysid;
+	Name		relname_name;
+	char		*relname;
+	text		*priv_type_text;
+	bool		result;
+
+	usesysid = PG_GETARG_OID(0);
+	relname_name = PG_GETARG_NAME(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/* 
+	 * Convert relname 'name' pattern to null-terminated string
+	 */
+	relname = DatumGetCString(DirectFunctionCall1(nameout, PointerGetDatum(relname_name)));
+
+	/*
+	 * Make use of has_table_privilege_id_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_id_cname(usesysid, relname, priv_type_text);
+
+	PG_RETURN_BOOL(result);
+
+}
+
+
+/*
+ * has_table_privilege_id_id
+ *		Check user privileges on a relation given
+ *		usesysid, rel oid, and priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+Datum
+has_table_privilege_id_id(PG_FUNCTION_ARGS)
+{
+	Oid			usesysid;
+	Oid			reloid;
+	char		*relname;
+	text		*priv_type_text;
+	HeapTuple	tuple;
+	AclMode		mode;
+	int32		result;
+
+
+	usesysid = PG_GETARG_OID(0);
+	reloid = PG_GETARG_OID(1);
+	priv_type_text = PG_GETARG_TEXT_P(2);
+
+	/*
+	 * Lookup relname based on rel oid
+	 */
+	tuple = SearchSysCache(RELOID, ObjectIdGetDatum(reloid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation oid %d", (int) reloid);
+	}
+
+	relname = NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname);
+
+	ReleaseSysCache(tuple);
+
+	/* 
+	 * Convert priv_type_text to an AclMode
+	 */
+	mode = convert_priv_string(priv_type_text);
+
+	/* 
+	 * Finally, check for the privilege
+	 */
+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == ACLCHECK_OK) {
+		PG_RETURN_BOOL(TRUE);
+	} else {
+		PG_RETURN_BOOL(FALSE);
+	}
+
+}
+
+/*
+ *		Internal functions.
+ */
+
+/*
+ * convert_priv_string
+ *		Internal function.
+ *		Return mode from priv_type string
+ *
+ * RETURNS
+ *		AclMode
+ */
+
+AclMode
+convert_priv_string(text *priv_type_text)
+{
+	char	*priv_type = DatumGetCString(DirectFunctionCall1(textout, PointerGetDatum(priv_type_text)));
+
+	/*
+	 * Return mode from priv_type string
+	 */
+	if (strcasecmp(priv_type, "SELECT") == 0)
+		return ACL_SELECT;
+
+	if (strcasecmp(priv_type, "INSERT") == 0)
+		return ACL_INSERT;
+
+	if (strcasecmp(priv_type, "UPDATE") == 0)
+		return ACL_UPDATE;
+
+	if (strcasecmp(priv_type, "DELETE") == 0)
+		return ACL_DELETE;
+
+	if (strcasecmp(priv_type, "RULE") == 0)
+		return ACL_RULE;
+
+	if (strcasecmp(priv_type, "REFERENCES") == 0)
+		return ACL_REFERENCES;
+
+	if (strcasecmp(priv_type, "TRIGGER") == 0)
+		return ACL_TRIGGER;
+
+	elog(ERROR, "has_table_privilege: invalid privilege type %s", priv_type);
+	/*
+	 * We should never get here, but stop the compiler from complaining
+	 */
+	return ACL_NO;
+
+}
+
+/*
+ * has_table_privilege_cname_cname
+ *		Check user privileges on a relation given
+ *		char *usename, char *relname, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+bool
+has_table_privilege_cname_cname(char *username, char *relname, text *priv_type_text)
+{
+
+	Oid			usesysid = (Oid) -1;
+	HeapTuple	tuple;
+	bool		result;
+
+	/*
+	 * Lookup userid based on username
+	 */
+
+	tuple = SearchSysCache(SHADOWNAME, NameGetDatum(username), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid user name %s", (char *) username);
+	}
+
+	usesysid = (Oid) ((Form_pg_shadow) GETSTRUCT(tuple))->usesysid;
+	ReleaseSysCache(tuple);
+
+	/*
+	 * Make use of has_table_privilege_id_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_id_cname(usesysid, relname, priv_type_text);
+
+	return result;
+
+}
+
+
+/*
+ * has_table_privilege_cname_id
+ *		Check user privileges on a relation given
+ *		char *usename, rel oid, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+bool
+has_table_privilege_cname_id(char *username, Oid reloid, text *priv_type_text)
+{
+	Oid			usesysid = (Oid) -1;
+	char		*relname = NULL;
+	HeapTuple	tuple;
+	bool		result;
+
+	/*
+	 * Lookup userid based on username
+	 */
+
+	tuple = SearchSysCache(SHADOWNAME, NameGetDatum(username), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid user name %s", (char *) username);
+	}
+
+	usesysid = (Oid) ((Form_pg_shadow) GETSTRUCT(tuple))->usesysid;
+
+	ReleaseSysCache(tuple);
+
+	/*
+	 * Lookup relname based on rel oid
+	 */
+	tuple = SearchSysCache(RELOID, ObjectIdGetDatum(reloid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation oid %d", (int) reloid);
+	}
+
+	relname = NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname);
+
+	ReleaseSysCache(tuple);
+
+	/*
+	 * Make use of has_table_privilege_id_cname.
+	 * It accepts the arguments we now have.
+	 */
+	result = has_table_privilege_id_cname(usesysid, relname, priv_type_text);
+
+	return result;
+
+}
+
+
+/*
+ * has_table_privilege_id_cname
+ *		Check user privileges on a relation given
+ *		usesysid, char *relname, and text priv name.
+ *
+ * RETURNS
+ *		a boolean value
+ *		't' indicating user has the privilege
+ *		'f' indicating user does not have the privilege
+ */
+bool
+has_table_privilege_id_cname(Oid usesysid, char *relname, text *priv_type_text)
+{
+
+	HeapTuple	tuple;
+	AclMode		mode;
+	int32		result;
+
+	/*
+	 * Check relname is valid.
+	 * This is needed to deal with the case when usename is a superuser
+	 * in which case pg_aclcheck simply returns ACLCHECK_OK
+	 * without validating relname
+	 */
+	tuple = SearchSysCache(RELNAME, PointerGetDatum(relname), 0, 0, 0);
+
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relname %s", relname);
+	}
+	ReleaseSysCache(tuple);
+
+	/* 
+	 * Convert priv_type_text to an AclMode
+	 */
+	mode = convert_priv_string(priv_type_text);
+
+	/* 
+	 * Finally, check for the privilege
+	 */
+	result = pg_aclcheck(relname, usesysid, mode);
+
+	if (result == ACLCHECK_OK) {
+		return TRUE;
+	} else {
+		return FALSE;
+	}
+
+}
+
+
+/*
+ * Given a 'text' relname parameter to a function, extract the actual
+ * relname.  We downcase the name if it's not double-quoted,
+ * and truncate it if it's too long.
+ *
+ * This is a kluge, really --- should be able to write, e.g. nextval(seqrel).
+ */
+static char *
+get_Name(text *relin)
+{
+	char	   *rawname = DatumGetCString(DirectFunctionCall1(textout,
+												PointerGetDatum(relin)));
+	int			rawlen = strlen(rawname);
+	char	   *relname;
+
+	if (rawlen >= 2 &&
+		rawname[0] == '\"' && rawname[rawlen - 1] == '\"')
+	{
+		/* strip off quotes, keep case */
+		rawname[rawlen - 1] = '\0';
+		relname = pstrdup(rawname + 1);
+		pfree(rawname);
+	}
+	else
+	{
+		relname = rawname;
+
+		/*
+		 * It's important that this match the identifier downcasing code
+		 * used by backend/parser/scan.l.
+		 */
+		for (; *rawname; rawname++)
+		{
+			if (isupper((unsigned char) *rawname))
+				*rawname = tolower((unsigned char) *rawname);
+		}
+	}
+
+	/* Truncate name if it's overlength; again, should match scan.l */
+	if (strlen(relname) >= NAMEDATALEN)
+	{
+#ifdef MULTIBYTE
+		int len;
+
+		len = pg_mbcliplen(relname, i, NAMEDATALEN-1);
+		relname[len] = '\0';
+#else
+		relname[NAMEDATALEN-1] = '\0';
+#endif
+	}
+
+	return relname;
 }
diff -Naur pgsql.virg/src/include/catalog/pg_proc.h pgsql.dev/src/include/catalog/pg_proc.h
--- pgsql.virg/src/include/catalog/pg_proc.h	Sun Jun 10 01:59:22 2001
+++ pgsql.dev/src/include/catalog/pg_proc.h	Sat Jun  9 20:44:39 2001
@@ -2627,6 +2627,35 @@
 DATA(insert OID = 1915 (  numeric_uplus	   PGUID 12 f t t t 1 f 1700 "1700" 100 0 0 100  numeric_uplus - ));
 DESCR("unary plus");
 
+DATA(insert OID = 1920 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "25 25 25" 100 0 0 100	has_table_privilege_tname_tname - ));
+DESCR("user privilege on relation by text username, text relname");
+DATA(insert OID = 1921 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "25 19 25" 100 0 0 100	has_table_privilege_tname_name - ));
+DESCR("user privilege on relation by text username, name relname");
+DATA(insert OID = 1922 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "19 25 25" 100 0 0 100	has_table_privilege_name_tname - ));
+DESCR("user privilege on relation by name username, text relname");
+DATA(insert OID = 1923 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "19 19 25" 100 0 0 100	has_table_privilege_name_name - ));
+DESCR("user privilege on relation by name username, name relname");
+DATA(insert OID = 1924 (  has_table_privilege		   PGUID 12 f t f t 2 f 16 "25 25" 100 0 0 100	has_table_privilege_tname - ));
+DESCR("current user privilege on relation by text relname");
+DATA(insert OID = 1925 (  has_table_privilege		   PGUID 12 f t f t 2 f 16 "19 25" 100 0 0 100	has_table_privilege_name - ));
+DESCR("current user privilege on relation by name relname");
+
+DATA(insert OID = 1926 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "25 26 25" 100 0 0 100	has_table_privilege_tname_id - ));
+DESCR("user privilege on relation by text username, rel oid");
+DATA(insert OID = 1927 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "19 26 25" 100 0 0 100	has_table_privilege_name_id - ));
+DESCR("user privilege on relation by text username, rel oid");
+DATA(insert OID = 1928 (  has_table_privilege		   PGUID 12 f t f t 2 f 16 "26 25" 100 0 0 100	has_table_privilege_id - ));
+DESCR("current user privilege on relation by rel oid");
+
+DATA(insert OID = 1929 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "26 25 25" 100 0 0 100	has_table_privilege_id_tname - ));
+DESCR("user privilege on relation by usesysid, relname");
+DATA(insert OID = 1930 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "26 19 25" 100 0 0 100	has_table_privilege_id_name - ));
+DESCR("user privilege on relation by usesysid, relname");
+
+DATA(insert OID = 1931 (  has_table_privilege		   PGUID 12 f t f t 3 f 16 "26 26 25" 100 0 0 100	has_table_privilege_id_id - ));
+DESCR("user privilege on relation by usesysid, rel oid");
+
+
 /*
  * prototypes for functions pg_proc.c
  */
diff -Naur pgsql.virg/src/include/utils/acl.h pgsql.dev/src/include/utils/acl.h
--- pgsql.virg/src/include/utils/acl.h	Sun Jun 10 01:59:24 2001
+++ pgsql.dev/src/include/utils/acl.h	Sun Jun 10 00:47:42 2001
@@ -193,6 +193,22 @@
 extern Datum aclcontains(PG_FUNCTION_ARGS);
 extern void ExecuteChangeACLStmt(ChangeACLStmt *stmt);
 
+extern Datum has_table_privilege_tname_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_tname_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name(PG_FUNCTION_ARGS);
+
+extern Datum has_table_privilege_tname_id(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_id(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id(PG_FUNCTION_ARGS);
+
+extern Datum has_table_privilege_id_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id_name(PG_FUNCTION_ARGS);
+
+extern Datum has_table_privilege_id_id(PG_FUNCTION_ARGS);
+
 /*
  * prototypes for functions in aclchk.c
  */
diff -Naur pgsql.virg/src/include/utils/builtins.h pgsql.dev/src/include/utils/builtins.h
--- pgsql.virg/src/include/utils/builtins.h	Sun Jun 10 01:59:26 2001
+++ pgsql.dev/src/include/utils/builtins.h	Sat Jun  9 20:34:02 2001
@@ -22,6 +22,24 @@
 /*
  *		Defined in adt/
  */
+
+/* acl.c */
+extern Datum has_table_privilege_tname_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_tname_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_name(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name(PG_FUNCTION_ARGS);
+
+extern Datum has_table_privilege_tname_id(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_name_id(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id(PG_FUNCTION_ARGS);
+
+extern Datum has_table_privilege_id_tname(PG_FUNCTION_ARGS);
+extern Datum has_table_privilege_id_name(PG_FUNCTION_ARGS);
+
+extern Datum has_table_privilege_id_id(PG_FUNCTION_ARGS);
+
 /* bool.c */
 extern Datum boolin(PG_FUNCTION_ARGS);
 extern Datum boolout(PG_FUNCTION_ARGS);
diff -Naur pgsql.virg/src/test/regress/expected/has_table_priv.out pgsql.dev/src/test/regress/expected/has_table_priv.out
--- pgsql.virg/src/test/regress/expected/has_table_priv.out	Thu Jan  1 00:00:00 1970
+++ pgsql.dev/src/test/regress/expected/has_table_priv.out	Sun Jun 10 00:27:38 2001
@@ -0,0 +1,479 @@
+\connect - postgres
+
+--
+-- user postgres, NULL input
+--
+
+select has_table_privilege(NULL,'pg_shadow','select');
+ has_table_privilege 
+---------------------
+ 
+(1 row)
+
+
+--
+-- user postgres, bad relname
+--
+
+select has_table_privilege('postgres','pg_shad','select');
+ERROR:  has_table_privilege: invalid relname pg_shad
+
+--
+-- user postgres, bad usename
+--
+
+select has_table_privilege('post','pg_shadow','select');
+ERROR:  has_table_privilege: invalid user name post
+
+--
+-- user postgres, bad priv_type
+--
+
+select has_table_privilege('postgres','pg_shadow','sel');
+ERROR:  has_table_privilege: invalid privilege type sel
+
+--
+-- user postgres, bad usesysid
+--
+
+select has_table_privilege(-999999,'pg_shadow','update');
+ERROR:  pg_aclcheck: invalid user id 4293967297
+
+--
+-- user postgres, bad rel oid
+--
+
+select has_table_privilege('postgres',-999999,'rule');
+ERROR:  has_table_privilege: invalid relation oid -999999
+
+--
+-- user postgres, rel pg_shadow
+--
+
+select has_table_privilege('postgres','pg_shadow','select');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('postgres','pg_shadow','insert');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,'pg_shadow','update') from (select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t2.usesysid,'pg_shadow','delete') from (select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege('postgres',t1.oid,'rule') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('postgres',t1.oid,'references') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'pg_shadow') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'pg_shadow') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege('pg_shadow','update');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('pg_shadow','delete');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege(t1.oid,'select') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+--
+-- create rel testtable
+--
+
+create table testtable(f1 int, f2 text);
+
+--
+-- user postgres, rel testtable
+--
+
+select has_table_privilege('postgres','TESTtable','select');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('postgres','TESTtable','insert');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,'testtable','update') from (select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t2.usesysid,'testtable','delete') from (select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege('postgres',t1.oid,'rule') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('postgres',t1.oid,'references') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege('testtable','update');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('testtable','delete');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege(t1.oid,'rule') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+--
+-- create new user, grant on testtable, and connect
+--
+
+create user foo;
+grant select on testtable to foo;
+grant update on testtable to foo;
+grant rule on testtable to foo;
+\connect - foo
+
+--
+-- user foo, rel pg_shadow
+--
+
+select has_table_privilege('foo','pg_class','select');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('foo','pg_class','insert');
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,'pg_class','update') from (select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+select has_table_privilege(t2.usesysid,'pg_class','delete') from (select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege('foo',t1.oid,'rule') from (select oid from pg_class where relname = 'pg_class') as t1;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+select has_table_privilege('foo',t1.oid,'references') from (select oid from pg_class where relname = 'pg_class') as t1;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'pg_class') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'pg_class') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege('pg_class','update');
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+select has_table_privilege('pg_class','delete');
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege(t1.oid,'select') from (select oid from pg_class where relname = 'pg_class') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'pg_class') as t1;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+--
+-- user foo, rel testtable
+--
+
+select has_table_privilege('foo','testtable','select');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('foo','testtable','insert');
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,'testtable','update') from (select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t2.usesysid,'testtable','delete') from (select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege('foo',t1.oid,'rule') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('foo',t1.oid,'references') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege('testtable','update');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('testtable','delete');
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+select has_table_privilege(t1.oid,'select') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'testtable') as t1;
+ has_table_privilege 
+---------------------
+ f
+(1 row)
+
+
+--
+-- create rel "Quoted RelName"
+--
+
+\connect - postgres
+create table "Quoted RelName" (f1 int, f2 text);
+
+--
+-- user postgres, rel "Quoted RelName", name based variants
+--
+
+select has_table_privilege('postgres','"Quoted RelName"','select');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('postgres',t1.relname,'select') from (select relname from pg_class where relname = 'Quoted RelName') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.usename,'"Quoted RelName"','select') from (select usename from pg_user where usename = 'postgres') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.usename,t2.relname,'select') from (select usename from pg_user where usename = 'postgres') as t1, (select relname from pg_class where relname = 'Quoted RelName') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege('"Quoted RelName"','select');
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.relname,'select') from (select relname from pg_class where relname = 'Quoted RelName') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege('postgres',t1.oid,'rule') from (select oid from pg_class where relname = 'Quoted RelName') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.usename,t2.oid,'rule') from (select usename from pg_user where usename = 'postgres') as t1, (select oid from pg_class where relname = 'Quoted RelName') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.oid,'rule') from (select oid from pg_class where relname = 'Quoted RelName') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+select has_table_privilege(t1.usesysid,'"Quoted RelName"','select') from (select usesysid from pg_user where usename = 'postgres') as t1;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+select has_table_privilege(t1.usesysid,t2.relname,'select') from (select usesysid from pg_user where usename = 'postgres') as t1, (select relname from pg_class where relname = 'Quoted RelName') as t2;
+ has_table_privilege 
+---------------------
+ t
+(1 row)
+
+
+--
+-- Clean up
+--
+
+drop user foo;
+drop table testtable;
+drop table "Quoted RelName";
diff -Naur pgsql.virg/src/test/regress/parallel_schedule pgsql.dev/src/test/regress/parallel_schedule
--- pgsql.virg/src/test/regress/parallel_schedule	Sun Jun 10 01:59:42 2001
+++ pgsql.dev/src/test/regress/parallel_schedule	Sat Jun  9 20:08:18 2001
@@ -62,6 +62,7 @@
 test: select_into select_distinct select_distinct_on select_implicit select_having subselect union case join aggregates transactions random portals arrays btree_index hash_index
 
 test: privileges
+test: has_table_priv
 test: misc
 
 # ----------
diff -Naur pgsql.virg/src/test/regress/serial_schedule pgsql.dev/src/test/regress/serial_schedule
--- pgsql.virg/src/test/regress/serial_schedule	Sun Jun 10 01:59:43 2001
+++ pgsql.dev/src/test/regress/serial_schedule	Sat Jun  9 19:50:29 2001
@@ -69,6 +69,7 @@
 test: btree_index
 test: hash_index
 test: privileges
+test: has_table_priv
 test: misc
 test: select_views
 test: alter_table
diff -Naur pgsql.virg/src/test/regress/sql/has_table_priv.sql pgsql.dev/src/test/regress/sql/has_table_priv.sql
--- pgsql.virg/src/test/regress/sql/has_table_priv.sql	Thu Jan  1 00:00:00 1970
+++ pgsql.dev/src/test/regress/sql/has_table_priv.sql	Sun Jun 10 00:59:52 2001
@@ -0,0 +1,174 @@
+\connect - postgres
+
+--
+-- user postgres, NULL input
+--
+
+select has_table_privilege(NULL,'pg_shadow','select');
+
+--
+-- user postgres, bad relname
+--
+
+select has_table_privilege('postgres','pg_shad','select');
+
+--
+-- user postgres, bad usename
+--
+
+select has_table_privilege('post','pg_shadow','select');
+
+--
+-- user postgres, bad priv_type
+--
+
+select has_table_privilege('postgres','pg_shadow','sel');
+
+--
+-- user postgres, bad usesysid
+--
+
+select has_table_privilege(-999999,'pg_shadow','update');
+
+--
+-- user postgres, bad rel oid
+--
+
+select has_table_privilege('postgres',-999999,'rule');
+
+--
+-- user postgres, rel pg_shadow
+--
+
+select has_table_privilege('postgres','pg_shadow','select');
+select has_table_privilege('postgres','pg_shadow','insert');
+
+select has_table_privilege(t2.usesysid,'pg_shadow','update') from (select usesysid from pg_user where usename = 'postgres') as t2;
+select has_table_privilege(t2.usesysid,'pg_shadow','delete') from (select usesysid from pg_user where usename = 'postgres') as t2;
+
+select has_table_privilege('postgres',t1.oid,'rule') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+select has_table_privilege('postgres',t1.oid,'references') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'pg_shadow') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'pg_shadow') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+
+select has_table_privilege('pg_shadow','update');
+select has_table_privilege('pg_shadow','delete');
+
+select has_table_privilege(t1.oid,'select') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'pg_shadow') as t1;
+
+--
+-- create rel testtable
+--
+
+create table testtable(f1 int, f2 text);
+
+--
+-- user postgres, rel testtable
+--
+
+select has_table_privilege('postgres','TESTtable','select');
+select has_table_privilege('postgres','TESTtable','insert');
+
+select has_table_privilege(t2.usesysid,'testtable','update') from (select usesysid from pg_user where usename = 'postgres') as t2;
+select has_table_privilege(t2.usesysid,'testtable','delete') from (select usesysid from pg_user where usename = 'postgres') as t2;
+
+select has_table_privilege('postgres',t1.oid,'rule') from (select oid from pg_class where relname = 'testtable') as t1;
+select has_table_privilege('postgres',t1.oid,'references') from (select oid from pg_class where relname = 'testtable') as t1;
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'postgres') as t2;
+
+select has_table_privilege('testtable','update');
+select has_table_privilege('testtable','delete');
+
+select has_table_privilege(t1.oid,'rule') from (select oid from pg_class where relname = 'testtable') as t1;
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'testtable') as t1;
+
+--
+-- create new user, grant on testtable, and connect
+--
+
+create user foo;
+grant select on testtable to foo;
+grant update on testtable to foo;
+grant rule on testtable to foo;
+\connect - foo
+
+--
+-- user foo, rel pg_shadow
+--
+
+select has_table_privilege('foo','pg_class','select');
+select has_table_privilege('foo','pg_class','insert');
+
+select has_table_privilege(t2.usesysid,'pg_class','update') from (select usesysid from pg_user where usename = 'foo') as t2;
+select has_table_privilege(t2.usesysid,'pg_class','delete') from (select usesysid from pg_user where usename = 'foo') as t2;
+
+select has_table_privilege('foo',t1.oid,'rule') from (select oid from pg_class where relname = 'pg_class') as t1;
+select has_table_privilege('foo',t1.oid,'references') from (select oid from pg_class where relname = 'pg_class') as t1;
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'pg_class') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'pg_class') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+
+select has_table_privilege('pg_class','update');
+select has_table_privilege('pg_class','delete');
+
+select has_table_privilege(t1.oid,'select') from (select oid from pg_class where relname = 'pg_class') as t1;
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'pg_class') as t1;
+
+--
+-- user foo, rel testtable
+--
+
+select has_table_privilege('foo','testtable','select');
+select has_table_privilege('foo','testtable','insert');
+
+select has_table_privilege(t2.usesysid,'testtable','update') from (select usesysid from pg_user where usename = 'foo') as t2;
+select has_table_privilege(t2.usesysid,'testtable','delete') from (select usesysid from pg_user where usename = 'foo') as t2;
+
+select has_table_privilege('foo',t1.oid,'rule') from (select oid from pg_class where relname = 'testtable') as t1;
+select has_table_privilege('foo',t1.oid,'references') from (select oid from pg_class where relname = 'testtable') as t1;
+
+select has_table_privilege(t2.usesysid,t1.oid,'select') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+select has_table_privilege(t2.usesysid,t1.oid,'insert') from (select oid from pg_class where relname = 'testtable') as t1,(select usesysid from pg_user where usename = 'foo') as t2;
+
+select has_table_privilege('testtable','update');
+select has_table_privilege('testtable','delete');
+
+select has_table_privilege(t1.oid,'select') from (select oid from pg_class where relname = 'testtable') as t1;
+select has_table_privilege(t1.oid,'trigger') from (select oid from pg_class where relname = 'testtable') as t1;
+
+--
+-- create rel "Quoted RelName"
+--
+
+\connect - postgres
+create table "Quoted RelName" (f1 int, f2 text);
+
+--
+-- user postgres, rel "Quoted RelName", name based variants
+--
+
+select has_table_privilege('postgres','"Quoted RelName"','select');
+select has_table_privilege('postgres',t1.relname,'select') from (select relname from pg_class where relname = 'Quoted RelName') as t1;
+select has_table_privilege(t1.usename,'"Quoted RelName"','select') from (select usename from pg_user where usename = 'postgres') as t1;
+select has_table_privilege(t1.usename,t2.relname,'select') from (select usename from pg_user where usename = 'postgres') as t1, (select relname from pg_class where relname = 'Quoted RelName') as t2;
+select has_table_privilege('"Quoted RelName"','select');
+select has_table_privilege(t1.relname,'select') from (select relname from pg_class where relname = 'Quoted RelName') as t1;
+
+select has_table_privilege('postgres',t1.oid,'rule') from (select oid from pg_class where relname = 'Quoted RelName') as t1;
+select has_table_privilege(t1.usename,t2.oid,'rule') from (select usename from pg_user where usename = 'postgres') as t1, (select oid from pg_class where relname = 'Quoted RelName') as t2;
+select has_table_privilege(t1.oid,'rule') from (select oid from pg_class where relname = 'Quoted RelName') as t1;
+
+select has_table_privilege(t1.usesysid,'"Quoted RelName"','select') from (select usesysid from pg_user where usename = 'postgres') as t1;
+select has_table_privilege(t1.usesysid,t2.relname,'select') from (select usesysid from pg_user where usename = 'postgres') as t1, (select relname from pg_class where relname = 'Quoted RelName') as t2;
+
+--
+-- Clean up
+--
+
+drop user foo;
+drop table testtable;
+drop table "Quoted RelName";
#28Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Tom Lane (#23)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

I have just thought of a possible compromise. Peter is right that we
don't want case conversion on table names that are extracted from
catalogs. But I think we do want it on table names expressed as string
literals. Could we make the assumption that table names in catalogs
will be of type 'name'? If so, it'd work to make two versions of the
has_table_privilege function, one taking type "name" and the other
taking type "text". The "name" version would take its input as-is,
the "text" version would do case folding and truncation. This would
work transparently for queries selecting relation names from the system
catalogs, and it'd also work transparently for queries using unmarked
string literals (which will be preferentially resolved as type "text").
Worst case if the system makes the wrong choice is you throw in an
explicit coercion to name or text. Comments?

Seems you are adding a distinction between name and text that we never
had before. Is it worth it to fix this case?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#29Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Joe Conway (#27)
Re: [HACKERS] Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Your patch has been added to the PostgreSQL unapplied patches list at:

http://candle.pha.pa.us/cgi-bin/pgpatches

I will try to apply it within the next 48 hours.

I have just thought of a possible compromise. Peter is right that we
don't want case conversion on table names that are extracted from
catalogs. But I think we do want it on table names expressed as string
literals. Could we make the assumption that table names in catalogs
will be of type 'name'? If so, it'd work to make two versions of the
has_table_privilege function, one taking type "name" and the other
taking type "text". The "name" version would take its input as-is,
the "text" version would do case folding and truncation. This would
work transparently for queries selecting relation names from the system
catalogs, and it'd also work transparently for queries using unmarked
string literals (which will be preferentially resolved as type "text").
Worst case if the system makes the wrong choice is you throw in an
explicit coercion to name or text. Comments?

OK -- here's take #5.

It "make"s and "make check"s clean against current cvs tip.

There are now both Text and Name variants, and the regression test support
is rolled into the patch. Note that to be complete wrt Name based variants,
there are now 12 user visible versions of has_table_privilege:

has_table_privilege(Text usename, Text relname, Text priv_type)
has_table_privilege(Text usename, Name relname, Text priv_type)
has_table_privilege(Name usename, Text relname, Text priv_type)
has_table_privilege(Name usename, Name relname, Text priv_type)
has_table_privilege(Text relname, Text priv_type) /* assumes current_user */
has_table_privilege(Name relname, Text priv_type) /* assumes current_user */
has_table_privilege(Text usename, Oid reloid, Text priv_type)
has_table_privilege(Name usename, Oid reloid, Text priv_type)
has_table_privilege(Oid reloid, Text priv_type) /* assumes current_user */
has_table_privilege(Oid usesysid, Text relname, Text priv_type)
has_table_privilege(Oid usesysid, Name relname, Text priv_type)
has_table_privilege(Oid usesysid, Oid reloid, Text priv_type)

For the Text based inputs, a new internal function, get_Name is used
(shamelessly copied from get_seq_name in sequence.c) to downcase if not
quoted, or remove quotes if quoted, and truncate. I also added a few test
cases for the downcasing, quote removal, and Name based variants to the
regression test.

Only thing left (I hope!) is documentation. I'm sure I either have or can
get the DocBook tools, but I've never used them. Would it be simpler to
clone and hand edit one of the existing docs? Any suggestions to get me
started?

Thanks,

-- Joe

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#30Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Joe Conway (#27)
Re: [HACKERS] Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

I don't know about other people but the 48 hours notice email and the
web page of outstanding patches seems to be working well for me.

I have just thought of a possible compromise. Peter is right that we
don't want case conversion on table names that are extracted from
catalogs. But I think we do want it on table names expressed as string
literals. Could we make the assumption that table names in catalogs
will be of type 'name'? If so, it'd work to make two versions of the
has_table_privilege function, one taking type "name" and the other
taking type "text". The "name" version would take its input as-is,
the "text" version would do case folding and truncation. This would
work transparently for queries selecting relation names from the system
catalogs, and it'd also work transparently for queries using unmarked
string literals (which will be preferentially resolved as type "text").
Worst case if the system makes the wrong choice is you throw in an
explicit coercion to name or text. Comments?

OK -- here's take #5.

It "make"s and "make check"s clean against current cvs tip.

There are now both Text and Name variants, and the regression test support
is rolled into the patch. Note that to be complete wrt Name based variants,
there are now 12 user visible versions of has_table_privilege:

has_table_privilege(Text usename, Text relname, Text priv_type)
has_table_privilege(Text usename, Name relname, Text priv_type)
has_table_privilege(Name usename, Text relname, Text priv_type)
has_table_privilege(Name usename, Name relname, Text priv_type)
has_table_privilege(Text relname, Text priv_type) /* assumes current_user */
has_table_privilege(Name relname, Text priv_type) /* assumes current_user */
has_table_privilege(Text usename, Oid reloid, Text priv_type)
has_table_privilege(Name usename, Oid reloid, Text priv_type)
has_table_privilege(Oid reloid, Text priv_type) /* assumes current_user */
has_table_privilege(Oid usesysid, Text relname, Text priv_type)
has_table_privilege(Oid usesysid, Name relname, Text priv_type)
has_table_privilege(Oid usesysid, Oid reloid, Text priv_type)

For the Text based inputs, a new internal function, get_Name is used
(shamelessly copied from get_seq_name in sequence.c) to downcase if not
quoted, or remove quotes if quoted, and truncate. I also added a few test
cases for the downcasing, quote removal, and Name based variants to the
regression test.

Only thing left (I hope!) is documentation. I'm sure I either have or can
get the DocBook tools, but I've never used them. Would it be simpler to
clone and hand edit one of the existing docs? Any suggestions to get me
started?

Thanks,

-- Joe

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#31Joe Conway
joseph.conway@home.com
In reply to: Bruce Momjian (#30)
Re: [HACKERS] Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

I don't know about other people but the 48 hours notice email and the
web page of outstanding patches seems to be working well for me.

Sorry -- my "into the ether" comment wasn't griping about you :-)

I've been having serious problems with my "mail.com" account. I originally
sent this post Saturday afternoon, but it didn't even make it to the
pgsql-patches list
until Monday morning (and I only know that because I started reading the
comp.databases.postgresql.patches news feed).
:(

Joe

#32Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Joe Conway (#31)
Re: [HACKERS] Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

I don't know about other people but the 48 hours notice email and the
web page of outstanding patches seems to be working well for me.

Sorry -- my "into the ether" comment wasn't griping about you :-)

I've been having serious problems with my "mail.com" account. I originally
sent this post Saturday afternoon, but it didn't even make it to the
pgsql-patches list
until Monday morning (and I only know that because I started reading the
comp.databases.postgresql.patches news feed).
:(

No, I didn't see any gripe. I was just saying I like making the
announcement, having it in the queue on a web page everyone can see, and
later applying it. Seems like a good procedure, and added because
people were complaining I was not consistenly waiting for comments
before applying patches.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#33Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#23)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Tom Lane writes:

Could we make the assumption that table names in catalogs
will be of type 'name'?

I wouldn't want to guarantee it for the information schema.

If so, it'd work to make two versions of the has_table_privilege
function, one taking type "name" and the other taking type "text".
The "name" version would take its input as-is, the "text" version
would do case folding and truncation.

Target typing is ugly.

I've tried to look up the supposed \paraphrase{we had enough problems
before we added the existing behaviour to setval, etc.} discussion but
couldn't find it. My experience on the mailing list is that it goes the
other way.

The identifier quoting rules are already surprising enough for the
uninitiated, but it's even more surprising that they even apply when
syntactically no identifier is present. Between the behaviour of "what
you see is what you get" and "this language is kind of confusing so you
have to quote your strings twice with two different quoting characters"
the choice is obvious to me.

I'm also arguing for consistency with the standard. According to that,
users will be able to do

SELECT * FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 'SoMeThInG';

and a load of similar queries. You can't change the case folding rules
here unless you really want to go out of your way, and then you have
really confused the heck out of users.

We could make relname.func() work without breaking compatibility, ISTM,
and then we only need the Oid version. For computing the relation name at
execution time, the "plain" version is going to be more useful anyway.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#34Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Peter Eisentraut (#33)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Tom Lane writes:

Could we make the assumption that table names in catalogs
will be of type 'name'?

I wouldn't want to guarantee it for the information schema.

If so, it'd work to make two versions of the has_table_privilege
function, one taking type "name" and the other taking type "text".
The "name" version would take its input as-is, the "text" version
would do case folding and truncation.

Target typing is ugly.

I've tried to look up the supposed \paraphrase{we had enough problems
before we added the existing behaviour to setval, etc.} discussion but
couldn't find it. My experience on the mailing list is that it goes the
other way.

I am confused. What are you suggesting as far as having a name and text
version of the functions?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
#35Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#33)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Peter Eisentraut <peter_e@gmx.net> writes:

Could we make the assumption that table names in catalogs
will be of type 'name'?

I wouldn't want to guarantee it for the information schema.

Your objections are not without merit, and in the interest of bringing
this thing to closure I'll concede for now. I want to get on with this
so that I can wrap up the pg_statistic view that started the whole
thread.

What I suggest we do is apply the portions of Joe's latest patch that
support has_table_privilege with OID inputs and with NAME inputs,
omitting the combinations that take TEXT inputs and do casefolding.
We can add that part later if it proves that people do indeed want it.

I have specific reasons for wanting to keep the functions accepting
NAME rather than TEXT: that will save a run-time type conversion in the
common case where one is reading the input from a system catalog, and
it will at least provide automatic truncation of overlength names when
one is accepting a literal. (I trust Peter won't object to that ;-).)

We will probably have to revisit this territory when we implement
schemas: there will need to be a way to input qualified table names
like foo.bar, and a way to input NON qualified names like "foo.bar".
But we can cross that bridge when we come to it.

Comments, objections?

regards, tom lane

#36Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#35)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Tom Lane writes:

What I suggest we do is apply the portions of Joe's latest patch that
support has_table_privilege with OID inputs and with NAME inputs,
omitting the combinations that take TEXT inputs and do casefolding.
We can add that part later if it proves that people do indeed want it.

Okay.

We will probably have to revisit this territory when we implement
schemas: there will need to be a way to input qualified table names
like foo.bar, and a way to input NON qualified names like "foo.bar".
But we can cross that bridge when we come to it.

I figured we would add another argument to the function.

--
Peter Eisentraut peter_e@gmx.net http://funkturm.homeip.net/~peter

#37Joe Conway
joseph.conway@home.com
In reply to: Peter Eisentraut (#33)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

What I suggest we do is apply the portions of Joe's latest patch that
support has_table_privilege with OID inputs and with NAME inputs,
omitting the combinations that take TEXT inputs and do casefolding.
We can add that part later if it proves that people do indeed want it.

I have specific reasons for wanting to keep the functions accepting
NAME rather than TEXT: that will save a run-time type conversion in the
common case where one is reading the input from a system catalog, and
it will at least provide automatic truncation of overlength names when
one is accepting a literal. (I trust Peter won't object to that ;-).)

I'll rework the patch per the above and resend.

Thanks,

-- Joe

#38Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#37)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

"Joe Conway" <joseph.conway@home.com> writes:

I'll rework the patch per the above and resend.

Too late ;-). I just finished ripping out the unneeded parts and
applying.

I made a few minor changes too, mostly removing unnecessary code
(you don't need to call nameout, everyone else just uses NameStr)
and trying to line up stylistically with other code. One actual
bug noted: you were doing this in a couple of places:

+	tuple = SearchSysCache(RELOID, ObjectIdGetDatum(reloid), 0, 0, 0);
+	if (!HeapTupleIsValid(tuple)) {
+		elog(ERROR, "has_table_privilege: invalid relation oid %d", (int) reloid);
+	}
+
+	relname = NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname);
+
+	ReleaseSysCache(tuple);

Since relname is just a pointer into the tuple, expecting it to still
be valid after you release the syscache entry is not kosher. There are
several ways to deal with this, but what I actually did was to make use
of lsyscache.c's get_rel_name, which pstrdup()s its result to avoid this
trap.

regards, tom lane

#39Joe Conway
joseph.conway@home.com
In reply to: Peter Eisentraut (#33)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

Too late ;-). I just finished ripping out the unneeded parts and
applying.

Thanks! I take it I still need to do the documentation though ;)

I made a few minor changes too, mostly removing unnecessary code
(you don't need to call nameout, everyone else just uses NameStr)
and trying to line up stylistically with other code. One actual
bug noted: you were doing this in a couple of places:

Once again, thanks for the "important safety tips". I saw references to this
trap in the comments, and therefore should have known better. I guess only
practice makes perfect (hopefully!).

-- Joe

#40Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joe Conway (#39)
Re: Re: [PATCHES] Fw: Isn't pg_statistic a security hole - Solution Proposal

"Joe Conway" <joseph.conway@home.com> writes:

Too late ;-). I just finished ripping out the unneeded parts and
applying.

Thanks! I take it I still need to do the documentation though ;)

I put in a few words in func.sgml, but feel free to improve on it.

regards, tom lane

#41Bruce Momjian
pgman@candle.pha.pa.us
In reply to: Joe Conway (#27)
Re: [HACKERS] Re: Fw: Isn't pg_statistic a security hole - Solution Proposal

Patch applied by Tom for oid and Name versions.

I have just thought of a possible compromise. Peter is right that we
don't want case conversion on table names that are extracted from
catalogs. But I think we do want it on table names expressed as string
literals. Could we make the assumption that table names in catalogs
will be of type 'name'? If so, it'd work to make two versions of the
has_table_privilege function, one taking type "name" and the other
taking type "text". The "name" version would take its input as-is,
the "text" version would do case folding and truncation. This would
work transparently for queries selecting relation names from the system
catalogs, and it'd also work transparently for queries using unmarked
string literals (which will be preferentially resolved as type "text").
Worst case if the system makes the wrong choice is you throw in an
explicit coercion to name or text. Comments?

OK -- here's take #5.

It "make"s and "make check"s clean against current cvs tip.

There are now both Text and Name variants, and the regression test support
is rolled into the patch. Note that to be complete wrt Name based variants,
there are now 12 user visible versions of has_table_privilege:

has_table_privilege(Text usename, Text relname, Text priv_type)
has_table_privilege(Text usename, Name relname, Text priv_type)
has_table_privilege(Name usename, Text relname, Text priv_type)
has_table_privilege(Name usename, Name relname, Text priv_type)
has_table_privilege(Text relname, Text priv_type) /* assumes current_user */
has_table_privilege(Name relname, Text priv_type) /* assumes current_user */
has_table_privilege(Text usename, Oid reloid, Text priv_type)
has_table_privilege(Name usename, Oid reloid, Text priv_type)
has_table_privilege(Oid reloid, Text priv_type) /* assumes current_user */
has_table_privilege(Oid usesysid, Text relname, Text priv_type)
has_table_privilege(Oid usesysid, Name relname, Text priv_type)
has_table_privilege(Oid usesysid, Oid reloid, Text priv_type)

For the Text based inputs, a new internal function, get_Name is used
(shamelessly copied from get_seq_name in sequence.c) to downcase if not
quoted, or remove quotes if quoted, and truncate. I also added a few test
cases for the downcasing, quote removal, and Name based variants to the
regression test.

Only thing left (I hope!) is documentation. I'm sure I either have or can
get the DocBook tools, but I've never used them. Would it be simpler to
clone and hand edit one of the existing docs? Any suggestions to get me
started?

Thanks,

-- Joe

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 3: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026