Get rid of the separate RULE privilege for tables: now only a table's owner

can create or modify rules for the table.  Do setRuleCheckAsUser() while
loading rules into the relcache, rather than when defining a rule.  This
ensures that permission checks for tables referenced in a rule are done with
respect to the current owner of the rule's table, whereas formerly ALTER TABLE
OWNER would fail to update the permission checking for associated rules.
Removal of separate RULE privilege is needed to prevent various scenarios
in which a grantee of RULE privilege could effectively have any privilege
of the table owner.  For backwards compatibility, GRANT/REVOKE RULE is still
accepted, but it doesn't do anything.  Per discussion here:
http://archives.postgresql.org/pgsql-hackers/2006-04/msg01138.php

7 files changed, 66 insertions, 83 deletions

diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
index 73b9ef6e9d9..e74c9b4410e 100644
--- a/src/backend/catalog/aclchk.c
+++ b/src/backend/catalog/aclchk.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/aclchk.c,v 1.130 2006/07/14 14:52:17 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/aclchk.c,v 1.131 2006/09/05 21:08:35 tgl Exp $
  *
  * NOTES
  *	  See acl.h.
@@ -1328,8 +1328,6 @@ string_to_privilege(const char *privname)
 		return ACL_UPDATE;
 	if (strcmp(privname, "delete") == 0)
 		return ACL_DELETE;
-	if (strcmp(privname, "rule") == 0)
-		return ACL_RULE;
 	if (strcmp(privname, "references") == 0)
 		return ACL_REFERENCES;
 	if (strcmp(privname, "trigger") == 0)
@@ -1346,6 +1344,8 @@ string_to_privilege(const char *privname)
 		return ACL_CREATE_TEMP;
 	if (strcmp(privname, "connect") == 0)
 		return ACL_CONNECT;
+	if (strcmp(privname, "rule") == 0)
+		return 0;				/* ignore old RULE privileges */
 	ereport(ERROR,
 			(errcode(ERRCODE_SYNTAX_ERROR),
 			 errmsg("unrecognized privilege type \"%s\"", privname)));
@@ -1365,8 +1365,6 @@ privilege_to_string(AclMode privilege)
 			return "UPDATE";
 		case ACL_DELETE:
 			return "DELETE";
-		case ACL_RULE:
-			return "RULE";
 		case ACL_REFERENCES:
 			return "REFERENCES";
 		case ACL_TRIGGER:
diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
index a30cddd2878..a735fa3081f 100644
--- a/src/backend/catalog/information_schema.sql
+++ b/src/backend/catalog/information_schema.sql
@@ -4,7 +4,7 @@
  *
  * Copyright (c) 2003-2006, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/catalog/information_schema.sql,v 1.35 2006/09/04 23:13:01 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/catalog/information_schema.sql,v 1.36 2006/09/05 21:08:35 tgl Exp $
  */
 
 /*
@@ -1194,7 +1194,6 @@ CREATE VIEW role_table_grants AS
           SELECT 'INSERT' UNION ALL
           SELECT 'UPDATE' UNION ALL
           SELECT 'REFERENCES' UNION ALL
-          SELECT 'RULE' UNION ALL
           SELECT 'TRIGGER') AS pr (type)
 
     WHERE c.relnamespace = nc.oid
@@ -1705,7 +1704,6 @@ CREATE VIEW table_constraints AS
                OR has_table_privilege(r.oid, 'INSERT')
                OR has_table_privilege(r.oid, 'UPDATE')
                OR has_table_privilege(r.oid, 'DELETE')
-               OR has_table_privilege(r.oid, 'RULE')
                OR has_table_privilege(r.oid, 'REFERENCES')
                OR has_table_privilege(r.oid, 'TRIGGER') )
 
@@ -1739,7 +1737,6 @@ CREATE VIEW table_constraints AS
                OR has_table_privilege(r.oid, 'INSERT')
                OR has_table_privilege(r.oid, 'UPDATE')
                OR has_table_privilege(r.oid, 'DELETE')
-               OR has_table_privilege(r.oid, 'RULE')
                OR has_table_privilege(r.oid, 'REFERENCES')
                OR has_table_privilege(r.oid, 'TRIGGER') );
 
@@ -1785,7 +1782,6 @@ CREATE VIEW table_privileges AS
           SELECT 'INSERT' UNION ALL
           SELECT 'UPDATE' UNION ALL
           SELECT 'REFERENCES' UNION ALL
-          SELECT 'RULE' UNION ALL
           SELECT 'TRIGGER') AS pr (type)
 
     WHERE c.relnamespace = nc.oid
@@ -1841,7 +1837,6 @@ CREATE VIEW tables AS
                OR has_table_privilege(c.oid, 'INSERT')
                OR has_table_privilege(c.oid, 'UPDATE')
                OR has_table_privilege(c.oid, 'DELETE')
-               OR has_table_privilege(c.oid, 'RULE')
                OR has_table_privilege(c.oid, 'REFERENCES')
                OR has_table_privilege(c.oid, 'TRIGGER') );
 
@@ -1963,7 +1958,6 @@ CREATE VIEW triggers AS
                OR has_table_privilege(c.oid, 'INSERT')
                OR has_table_privilege(c.oid, 'UPDATE')
                OR has_table_privilege(c.oid, 'DELETE')
-               OR has_table_privilege(c.oid, 'RULE')
                OR has_table_privilege(c.oid, 'REFERENCES')
                OR has_table_privilege(c.oid, 'TRIGGER') );
 
@@ -2162,7 +2156,6 @@ CREATE VIEW views AS
                OR has_table_privilege(c.oid, 'INSERT')
                OR has_table_privilege(c.oid, 'UPDATE')
                OR has_table_privilege(c.oid, 'DELETE')
-               OR has_table_privilege(c.oid, 'RULE')
                OR has_table_privilege(c.oid, 'REFERENCES')
                OR has_table_privilege(c.oid, 'TRIGGER') );
 
diff --git a/src/backend/commands/comment.c b/src/backend/commands/comment.c
index 2e29535c4e7..f54243a4955 100644
--- a/src/backend/commands/comment.c
+++ b/src/backend/commands/comment.c
@@ -7,7 +7,7 @@
  * Copyright (c) 1996-2006, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/comment.c,v 1.90 2006/07/14 14:52:18 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/comment.c,v 1.91 2006/09/05 21:08:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -741,7 +741,6 @@ CommentRule(List *qualname, char *comment)
 	HeapTuple	tuple;
 	Oid			reloid;
 	Oid			ruleoid;
-	AclResult	aclcheck;
 
 	/* Separate relname and trig name */
 	nnames = list_length(qualname);
@@ -819,9 +818,8 @@ CommentRule(List *qualname, char *comment)
 	}
 
 	/* Check object security */
-	aclcheck = pg_class_aclcheck(reloid, GetUserId(), ACL_RULE);
-	if (aclcheck != ACLCHECK_OK)
-		aclcheck_error(aclcheck, ACL_KIND_CLASS,
+	if (!pg_class_ownercheck(reloid, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
 					   get_rel_name(reloid));
 
 	/* Call CreateComments() to create/drop the comments */
diff --git a/src/backend/rewrite/rewriteDefine.c b/src/backend/rewrite/rewriteDefine.c
index a961f49d9b1..17f437c3855 100644
--- a/src/backend/rewrite/rewriteDefine.c
+++ b/src/backend/rewrite/rewriteDefine.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.113 2006/09/02 17:06:52 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteDefine.c,v 1.114 2006/09/05 21:08:35 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -33,9 +33,8 @@
 
 static void checkRuleResultList(List *targetList, TupleDesc resultDesc,
 								bool isSelect);
-static void setRuleCheckAsUser_Query(Query *qry, Oid userid);
-static void setRuleCheckAsUser_Expr(Node *node, Oid userid);
 static bool setRuleCheckAsUser_walker(Node *node, Oid *context);
+static void setRuleCheckAsUser_Query(Query *qry, Oid userid);
 
 
 /*
@@ -193,7 +192,6 @@ DefineQueryRewrite(RuleStmt *stmt)
 	int			event_attno;
 	ListCell   *l;
 	Query	   *query;
-	AclResult	aclresult;
 	bool		RelisBecomingView = false;
 
 	/*
@@ -209,9 +207,8 @@ DefineQueryRewrite(RuleStmt *stmt)
 	/*
 	 * Check user has permission to apply rules to this relation.
 	 */
-	aclresult = pg_class_aclcheck(ev_relid, GetUserId(), ACL_RULE);
-	if (aclresult != ACLCHECK_OK)
-		aclcheck_error(aclresult, ACL_KIND_CLASS,
+	if (!pg_class_ownercheck(ev_relid, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
 					   RelationGetRelationName(event_relation));
 
 	/*
@@ -411,19 +408,6 @@ DefineQueryRewrite(RuleStmt *stmt)
 	 */
 	event_attno = -1;
 
-	/*
-	 * We want the rule's table references to be checked as though by the rule
-	 * owner, not the user referencing the rule.  Therefore, scan through the
-	 * rule's rtables and set the checkAsUser field on all rtable entries.  We
-	 * have to look at event_qual as well, in case it contains sublinks.
-	 */
-	foreach(l, action)
-	{
-		query = (Query *) lfirst(l);
-		setRuleCheckAsUser_Query(query, GetUserId());
-	}
-	setRuleCheckAsUser_Expr(event_qual, GetUserId());
-
 	/* discard rule if it's null action and not INSTEAD; it's a no-op */
 	if (action != NIL || is_instead)
 	{
@@ -554,9 +538,9 @@ checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect)
 }
 
 /*
- * setRuleCheckAsUser_Query
- *		Recursively scan a query and set the checkAsUser field to the
- *		given userid in all rtable entries.
+ * setRuleCheckAsUser
+ *		Recursively scan a query or expression tree and set the checkAsUser
+ *		field to the given userid in all rtable entries.
  *
  * Note: for a view (ON SELECT rule), the checkAsUser field of the *OLD*
  * RTE entry will be overridden when the view rule is expanded, and the
@@ -565,6 +549,26 @@ checkRuleResultList(List *targetList, TupleDesc resultDesc, bool isSelect)
  * it's important to set these fields to match the rule owner.  So we just set
  * them always.
  */
+void
+setRuleCheckAsUser(Node *node, Oid userid)
+{
+	(void) setRuleCheckAsUser_walker(node, &userid);
+}
+
+static bool
+setRuleCheckAsUser_walker(Node *node, Oid *context)
+{
+	if (node == NULL)
+		return false;
+	if (IsA(node, Query))
+	{
+		setRuleCheckAsUser_Query((Query *) node, *context);
+		return false;
+	}
+	return expression_tree_walker(node, setRuleCheckAsUser_walker,
+								  (void *) context);
+}
+
 static void
 setRuleCheckAsUser_Query(Query *qry, Oid userid)
 {
@@ -591,31 +595,6 @@ setRuleCheckAsUser_Query(Query *qry, Oid userid)
 						  QTW_IGNORE_RT_SUBQUERIES);
 }
 
-/*
- * Expression-tree walker to find sublink queries
- */
-static void
-setRuleCheckAsUser_Expr(Node *node, Oid userid)
-{
-	(void) setRuleCheckAsUser_walker(node, &userid);
-}
-
-static bool
-setRuleCheckAsUser_walker(Node *node, Oid *context)
-{
-	if (node == NULL)
-		return false;
-	if (IsA(node, Query))
-	{
-		Query	   *qry = (Query *) node;
-
-		setRuleCheckAsUser_Query(qry, *context);
-		return false;
-	}
-	return expression_tree_walker(node, setRuleCheckAsUser_walker,
-								  (void *) context);
-}
-
 
 /*
  * Rename an existing rewrite rule.
diff --git a/src/backend/rewrite/rewriteRemove.c b/src/backend/rewrite/rewriteRemove.c
index b0acc01f827..e57724af4d7 100644
--- a/src/backend/rewrite/rewriteRemove.c
+++ b/src/backend/rewrite/rewriteRemove.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteRemove.c,v 1.65 2006/06/16 20:23:44 adunstan Exp $
+ *	  $PostgreSQL: pgsql/src/backend/rewrite/rewriteRemove.c,v 1.66 2006/09/05 21:08:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -39,7 +39,6 @@ RemoveRewriteRule(Oid owningRel, const char *ruleName, DropBehavior behavior,
 {
 	HeapTuple	tuple;
 	Oid			eventRelationOid;
-	AclResult	aclresult;
 	ObjectAddress object;
 
 	/*
@@ -72,9 +71,8 @@ RemoveRewriteRule(Oid owningRel, const char *ruleName, DropBehavior behavior,
 	 */
 	eventRelationOid = ((Form_pg_rewrite) GETSTRUCT(tuple))->ev_class;
 	Assert(eventRelationOid == owningRel);
-	aclresult = pg_class_aclcheck(eventRelationOid, GetUserId(), ACL_RULE);
-	if (aclresult != ACLCHECK_OK)
-		aclcheck_error(aclresult, ACL_KIND_CLASS,
+	if (!pg_class_ownercheck(eventRelationOid, GetUserId()))
+		aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
 					   get_rel_name(eventRelationOid));
 
 	/*
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index c3ac26dce4a..509c6dea92a 100644
--- a/src/backend/utils/adt/acl.c
+++ b/src/backend/utils/adt/acl.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.134 2006/07/14 14:52:23 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/adt/acl.c,v 1.135 2006/09/05 21:08:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -265,9 +265,6 @@ aclparse(const char *s, AclItem *aip)
 			case ACL_DELETE_CHR:
 				read = ACL_DELETE;
 				break;
-			case ACL_RULE_CHR:
-				read = ACL_RULE;
-				break;
 			case ACL_REFERENCES_CHR:
 				read = ACL_REFERENCES;
 				break;
@@ -289,6 +286,9 @@ aclparse(const char *s, AclItem *aip)
 			case ACL_CONNECT_CHR:
 				read = ACL_CONNECT;
 				break;
+			case 'R':			/* ignore old RULE privileges */
+				read = 0;
+				break;
 			default:
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_TEXT_REPRESENTATION),
@@ -1325,8 +1325,6 @@ convert_priv_string(text *priv_type_text)
 		return ACL_UPDATE;
 	if (pg_strcasecmp(priv_type, "DELETE") == 0)
 		return ACL_DELETE;
-	if (pg_strcasecmp(priv_type, "RULE") == 0)
-		return ACL_RULE;
 	if (pg_strcasecmp(priv_type, "REFERENCES") == 0)
 		return ACL_REFERENCES;
 	if (pg_strcasecmp(priv_type, "TRIGGER") == 0)
@@ -1343,6 +1341,8 @@ convert_priv_string(text *priv_type_text)
 		return ACL_CREATE_TEMP;
 	if (pg_strcasecmp(priv_type, "CONNECT") == 0)
 		return ACL_CONNECT;
+	if (pg_strcasecmp(priv_type, "RULE") == 0)
+		return 0;				/* ignore old RULE privileges */
 
 	ereport(ERROR,
 			(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -1553,11 +1553,6 @@ convert_table_priv_string(text *priv_type_text)
 	if (pg_strcasecmp(priv_type, "DELETE WITH GRANT OPTION") == 0)
 		return ACL_GRANT_OPTION_FOR(ACL_DELETE);
 
-	if (pg_strcasecmp(priv_type, "RULE") == 0)
-		return ACL_RULE;
-	if (pg_strcasecmp(priv_type, "RULE WITH GRANT OPTION") == 0)
-		return ACL_GRANT_OPTION_FOR(ACL_RULE);
-
 	if (pg_strcasecmp(priv_type, "REFERENCES") == 0)
 		return ACL_REFERENCES;
 	if (pg_strcasecmp(priv_type, "REFERENCES WITH GRANT OPTION") == 0)
@@ -1568,6 +1563,11 @@ convert_table_priv_string(text *priv_type_text)
 	if (pg_strcasecmp(priv_type, "TRIGGER WITH GRANT OPTION") == 0)
 		return ACL_GRANT_OPTION_FOR(ACL_TRIGGER);
 
+	if (pg_strcasecmp(priv_type, "RULE") == 0)
+		return 0;				/* ignore old RULE privileges */
+	if (pg_strcasecmp(priv_type, "RULE WITH GRANT OPTION") == 0)
+		return 0;
+
 	ereport(ERROR,
 			(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
 			 errmsg("unrecognized privilege type: \"%s\"", priv_type)));
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 08697d50366..190543e2bd7 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.247 2006/07/31 20:09:05 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/cache/relcache.c,v 1.248 2006/09/05 21:08:36 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -51,6 +51,7 @@
 #include "optimizer/clauses.h"
 #include "optimizer/planmain.h"
 #include "optimizer/prep.h"
+#include "rewrite/rewriteDefine.h"
 #include "storage/fd.h"
 #include "storage/smgr.h"
 #include "utils/builtins.h"
@@ -683,6 +684,22 @@ RelationBuildRuleLock(Relation relation)
 		if ((Pointer) rule_text != DatumGetPointer(rule_datum))
 			pfree(rule_text);
 
+		/*
+		 * We want the rule's table references to be checked as though by the
+		 * table owner, not the user referencing the rule.  Therefore, scan
+		 * through the rule's actions and set the checkAsUser field on all
+		 * rtable entries.  We have to look at the qual as well, in case it
+		 * contains sublinks.
+		 *
+		 * The reason for doing this when the rule is loaded, rather than
+		 * when it is stored, is that otherwise ALTER TABLE OWNER would have
+		 * to grovel through stored rules to update checkAsUser fields.
+		 * Scanning the rule tree during load is relatively cheap (compared
+		 * to constructing it in the first place), so we do it here.
+		 */
+		setRuleCheckAsUser((Node *) rule->actions, relation->rd_rel->relowner);
+		setRuleCheckAsUser(rule->qual, relation->rd_rel->relowner);
+
 		if (numlocks >= maxlocks)
 		{
 			maxlocks *= 2;