Add context info to OAT_POST_CREATE security hook

... and have sepgsql use it to determine whether to check permissions
during certain operations.  Indexes that are being created as a result
of REINDEX, for instance, do not need to have their permissions checked;
they were already checked when the index was created.

Author: KaiGai Kohei, slightly revised by me

contrib/sepgsql/hooks.c

@@ -38,7 +38,6 @@ void		_PG_init(void);
 static object_access_hook_type next_object_access_hook = NULL;
 static ExecutorCheckPerms_hook_type next_exec_check_perms_hook = NULL;
 static ProcessUtility_hook_type next_ProcessUtility_hook = NULL;
-static ExecutorStart_hook_type next_ExecutorStart_hook = NULL;
 
 /*
  * Contextual information on DDL commands
@@ -97,14 +96,22 @@ sepgsql_object_access(ObjectAccessType access,
 	switch (access)
 	{
 		case OAT_POST_CREATE:
+			{
+				ObjectAccessPostCreate *pc_arg = arg;
+				bool	is_internal;
+
+				is_internal = pc_arg ? pc_arg->is_internal : false;
+
 				switch (classId)
 				{
 					case DatabaseRelationId:
+						Assert(!is_internal);
 						sepgsql_database_post_create(objectId,
 													 sepgsql_context_info.createdb_dtemplate);
 						break;
 
 					case NamespaceRelationId:
+						Assert(!is_internal);
 						sepgsql_schema_post_create(objectId);
 						break;
 
@@ -112,32 +119,25 @@ sepgsql_object_access(ObjectAccessType access,
 						if (subId == 0)
 						{
 							/*
-						 * All cases we want to apply permission checks on
-						 * creation of a new relation are invocation of the
-						 * heap_create_with_catalog via DefineRelation or
-						 * OpenIntoRel. Elsewhere, we need neither assignment
-						 * of security label nor permission checks.
+							 * The cases in which we want to apply permission
+							 * checks on creation of a new relation correspond
+							 * to direct user invocation.  For internal uses,
+							 * that is creation of toast tables, index rebuild
+							 * or ALTER TABLE commands, we need neither
+							 * assignment of security labels nor permission
+							 * checks.
 							 */
-						switch (sepgsql_context_info.cmdtype)
-						{
-							case T_CreateStmt:
-							case T_ViewStmt:
-							case T_CreateSeqStmt:
-							case T_CompositeTypeStmt:
-							case T_CreateForeignTableStmt:
-							case T_SelectStmt:
-								sepgsql_relation_post_create(objectId);
+							if (is_internal)
 								break;
-							default:
-								/* via make_new_heap() */
-								break;
-						}
+
+							sepgsql_relation_post_create(objectId);
 						}
 						else
 							sepgsql_attribute_post_create(objectId, subId);
 						break;
 
 					case ProcedureRelationId:
+						Assert(!is_internal);
 						sepgsql_proc_post_create(objectId);
 						break;
 
@@ -145,6 +145,7 @@ sepgsql_object_access(ObjectAccessType access,
 						/* Ignore unsupported object classes */
 						break;
 				}
+			}
 			break;
 
 		case OAT_DROP:
@@ -215,46 +216,6 @@ sepgsql_exec_check_perms(List *rangeTabls, bool abort)
 	return true;
 }
 
-/*
- * sepgsql_executor_start
- *
- * It saves contextual information during ExecutorStart to distinguish
- * a case with/without permission checks later.
- */
-static void
-sepgsql_executor_start(QueryDesc *queryDesc, int eflags)
-{
-	sepgsql_context_info_t saved_context_info = sepgsql_context_info;
-
-	PG_TRY();
-	{
-		if (queryDesc->operation == CMD_SELECT)
-			sepgsql_context_info.cmdtype = T_SelectStmt;
-		else if (queryDesc->operation == CMD_INSERT)
-			sepgsql_context_info.cmdtype = T_InsertStmt;
-		else if (queryDesc->operation == CMD_DELETE)
-			sepgsql_context_info.cmdtype = T_DeleteStmt;
-		else if (queryDesc->operation == CMD_UPDATE)
-			sepgsql_context_info.cmdtype = T_UpdateStmt;
-
-		/*
-		 * XXX - If queryDesc->operation is not above four cases, an error
-		 * shall be raised on the following executor stage soon.
-		 */
-		if (next_ExecutorStart_hook)
-			(*next_ExecutorStart_hook) (queryDesc, eflags);
-		else
-			standard_ExecutorStart(queryDesc, eflags);
-	}
-	PG_CATCH();
-	{
-		sepgsql_context_info = saved_context_info;
-		PG_RE_THROW();
-	}
-	PG_END_TRY();
-	sepgsql_context_info = saved_context_info;
-}
-
 /*
  * sepgsql_utility_command
  *
@@ -425,10 +386,6 @@ _PG_init(void)
 	next_ProcessUtility_hook = ProcessUtility_hook;
 	ProcessUtility_hook = sepgsql_utility_command;
 
-	/* ExecutorStart hook */
-	next_ExecutorStart_hook = ExecutorStart_hook;
-	ExecutorStart_hook = sepgsql_executor_start;
-
 	/* init contextual info */
 	memset(&sepgsql_context_info, 0, sizeof(sepgsql_context_info));
 }

contrib/sepgsql/relation.c

@@ -23,11 +23,14 @@
 #include "utils/fmgroids.h"
 #include "utils/catcache.h"
 #include "utils/lsyscache.h"
+#include "utils/rel.h"
 #include "utils/syscache.h"
 #include "utils/tqual.h"
 
 #include "sepgsql.h"
 
+static void		sepgsql_index_modify(Oid indexOid);
+
 /*
  * sepgsql_attribute_post_create
  *
@@ -229,6 +232,23 @@ sepgsql_relation_post_create(Oid relOid)
 
 	classForm = (Form_pg_class) GETSTRUCT(tuple);
 
+	/* ignore indexes on toast tables */
+	if (classForm->relkind == RELKIND_INDEX &&
+		classForm->relnamespace == PG_TOAST_NAMESPACE)
+		goto out;
+
+	/*
+	 * check db_schema:{add_name} permission of the namespace
+	 */
+	object.classId = NamespaceRelationId;
+	object.objectId = classForm->relnamespace;
+	object.objectSubId = 0;
+	sepgsql_avc_check_perms(&object,
+							SEPG_CLASS_DB_SCHEMA,
+							SEPG_DB_SCHEMA__ADD_NAME,
+							getObjectDescription(&object),
+							true);
+
 	switch (classForm->relkind)
 	{
 		case RELKIND_RELATION:
@@ -243,22 +263,15 @@ sepgsql_relation_post_create(Oid relOid)
 			tclass = SEPG_CLASS_DB_VIEW;
 			tclass_text = "view";
 			break;
+		case RELKIND_INDEX:
+			/* deal with indexes specially; no need for tclass */
+			sepgsql_index_modify(relOid);
+			goto out;
 		default:
+			/* ignore other relkinds */
 			goto out;
 	}
 
-	/*
-	 * check db_schema:{add_name} permission of the namespace
-	 */
-	object.classId = NamespaceRelationId;
-	object.objectId = classForm->relnamespace;
-	object.objectSubId = 0;
-	sepgsql_avc_check_perms(&object,
-							SEPG_CLASS_DB_SCHEMA,
-							SEPG_DB_SCHEMA__ADD_NAME,
-							getObjectDescription(&object),
-							true);
-
 	/*
 	 * Compute a default security label when we create a new relation object
 	 * under the specified namespace.
@@ -342,6 +355,7 @@ sepgsql_relation_post_create(Oid relOid)
 		heap_close(arel, AccessShareLock);
 	}
 	pfree(rcontext);
+
 out:
 	systable_endscan(sscan);
 	heap_close(rel, AccessShareLock);
@@ -357,18 +371,31 @@ sepgsql_relation_drop(Oid relOid)
 {
 	ObjectAddress object;
 	char	   *audit_name;
-	uint16_t	tclass = 0;
+	uint16_t	tclass;
 	char		relkind;
 
 	relkind = get_rel_relkind(relOid);
-	if (relkind == RELKIND_RELATION)
+	switch (relkind)
+	{
+		case RELKIND_RELATION:
 			tclass = SEPG_CLASS_DB_TABLE;
-	else if (relkind == RELKIND_SEQUENCE)
+			break;
+		case RELKIND_SEQUENCE:
 			tclass = SEPG_CLASS_DB_SEQUENCE;
-	else if (relkind == RELKIND_VIEW)
+			break;
+		case RELKIND_VIEW:
 			tclass = SEPG_CLASS_DB_VIEW;
-	else
+			break;
+		case RELKIND_INDEX:
+			/* ignore indexes on toast tables */
+			if (get_rel_namespace(relOid) == PG_TOAST_NAMESPACE)
+				return;
+			/* other indexes are handled specially below; no need for tclass */
+			break;
+		default:
+			/* ignore other relkinds */
 			return;
+	}
 
 	/*
 	 * check db_schema:{remove_name} permission
@@ -385,6 +412,13 @@ sepgsql_relation_drop(Oid relOid)
 							true);
 	pfree(audit_name);
 
+	/* deal with indexes specially */
+	if (relkind == RELKIND_INDEX)
+	{
+		sepgsql_index_modify(relOid);
+		return;
+	}
+
 	/*
 	 * check db_table/sequence/view:{drop} permission
 	 */
@@ -486,3 +520,121 @@ sepgsql_relation_relabel(Oid relOid, const char *seclabel)
 								  true);
 	pfree(audit_name);
 }
+
+/*
+ * sepgsql_relation_setattr
+ *
+ * It checks privileges to set attribute of the supplied relation
+ */
+void
+sepgsql_relation_setattr(Oid relOid)
+{
+	ObjectAddress object;
+	char	   *audit_name;
+	uint16_t	tclass;
+
+	switch (get_rel_relkind(relOid))
+	{
+		case RELKIND_RELATION:
+			tclass = SEPG_CLASS_DB_TABLE;
+			break;
+		case RELKIND_SEQUENCE:
+			tclass = SEPG_CLASS_DB_SEQUENCE;
+			break;
+		case RELKIND_VIEW:
+			tclass = SEPG_CLASS_DB_VIEW;
+			break;
+		case RELKIND_INDEX:
+			/* deal with indexes specially */
+			sepgsql_index_modify(relOid);
+			return;
+		default:
+			/* other relkinds don't need additional work */
+			return;
+	}
+
+	object.classId = RelationRelationId;
+	object.objectId = relOid;
+	object.objectSubId = 0;
+	audit_name = getObjectDescription(&object);
+
+	/*
+	 * XXX - we should add checks related to namespace stuff, when
+	 * object_access_hook get support for ALTER statement.  Right now, there is
+	 * no invocation path on ALTER ...  RENAME TO / SET SCHEMA.
+	 */
+
+	/*
+	 * check db_xxx:{setattr} permission
+	 */
+	sepgsql_avc_check_perms(&object,
+							tclass,
+							SEPG_DB_TABLE__SETATTR,
+							audit_name,
+							true);
+	pfree(audit_name);
+}
+
+/*
+ * sepgsql_relation_setattr_extra
+ *
+ * It checks permission of the relation being referenced by extra attributes,
+ * such as pg_index entries. Like core PostgreSQL, sepgsql also does not deal
+ * with such entries as individual "objects", thus, modification of these
+ * entries shall be considered as setting an attribute of the underlying
+ * relation.
+ */
+static void
+sepgsql_relation_setattr_extra(Relation catalog,
+							   Oid catindex_id,
+							   Oid extra_oid,
+							   AttrNumber anum_relation_id,
+							   AttrNumber anum_extra_id)
+{
+	ScanKeyData	skey;
+	SysScanDesc	sscan;
+	HeapTuple	tuple;
+	Datum		datum;
+	bool		isnull;
+
+	ScanKeyInit(&skey, anum_extra_id,
+				BTEqualStrategyNumber, F_OIDEQ,
+				ObjectIdGetDatum(extra_oid));
+
+	sscan = systable_beginscan(catalog, catindex_id, true,
+							   SnapshotSelf, 1, &skey);
+	tuple = systable_getnext(sscan);
+	if (!HeapTupleIsValid(tuple))
+		elog(ERROR, "catalog lookup failed for object %u in catalog \"%s\"",
+			 extra_oid, RelationGetRelationName(catalog));
+
+	datum = heap_getattr(tuple, anum_relation_id,
+						 RelationGetDescr(catalog), &isnull);
+	Assert(!isnull);
+
+	sepgsql_relation_setattr(DatumGetObjectId(datum));
+
+	systable_endscan(sscan);
+}
+
+/*
+ * sepgsql_index_modify
+ * 		Handle index create, update, drop
+ *
+ * Unlike other relation kinds, indexes do not have their own security labels,
+ * so instead of doing checks directly, treat them as extra attributes of their
+ * owning tables; so check 'setattr' permissions on the table.
+ */
+static void
+sepgsql_index_modify(Oid indexOid)
+{
+	Relation	catalog = heap_open(IndexRelationId, AccessShareLock);
+
+	/* check db_table:{setattr} permission of the table being indexed */
+	sepgsql_relation_setattr_extra(catalog,
+								   IndexRelidIndexId,
+								   indexOid,
+								   Anum_pg_index_indrelid,
+								   Anum_pg_index_indexrelid);
+	heap_close(catalog, AccessShareLock);
+}

contrib/sepgsql/sepgsql.h

@@ -145,7 +145,6 @@
 #define SEPG_DB_TABLE__INSERT				(1<<8)
 #define SEPG_DB_TABLE__DELETE				(1<<9)
 #define SEPG_DB_TABLE__LOCK					(1<<10)
-#define SEPG_DB_TABLE__INDEXON				(1<<11)
 
 #define SEPG_DB_SEQUENCE__CREATE			(SEPG_DB_DATABASE__CREATE)
 #define SEPG_DB_SEQUENCE__DROP				(SEPG_DB_DATABASE__DROP)
@@ -312,6 +311,7 @@ extern void sepgsql_attribute_relabel(Oid relOid, AttrNumber attnum,
 extern void sepgsql_relation_post_create(Oid relOid);
 extern void sepgsql_relation_drop(Oid relOid);
 extern void sepgsql_relation_relabel(Oid relOid, const char *seclabel);
+extern void sepgsql_relation_setattr(Oid relOid);
 
 /*
  * proc.c

contrib/sepgsql/sql/ddl.sql

@@ -59,6 +59,18 @@ CREATE FUNCTION regtest_func_2(int) RETURNS bool LANGUAGE plpgsql
 
 RESET SESSION AUTHORIZATION;
 
+--
+-- ALTER and CREATE/DROP extra attribute permissions
+--
+CREATE TABLE regtest_table_4 (x int primary key, y int, z int);
+CREATE INDEX regtest_index_tbl4_y ON regtest_table_4(y);
+CREATE INDEX regtest_index_tbl4_z ON regtest_table_4(z);
+ALTER TABLE regtest_table_4 ALTER COLUMN y TYPE float;
+DROP INDEX regtest_index_tbl4_y;
+ALTER TABLE regtest_table_4
+      ADD CONSTRAINT regtest_tbl4_con EXCLUDE USING btree (z WITH =);
+DROP TABLE regtest_table_4 CASCADE;
+
 --
 -- DROP Permission checks (with clean-up)
 --

doc/src/sgml/sepgsql.sgml

@@ -449,6 +449,12 @@ UPDATE t1 SET x = 2, y = md5sum(y) WHERE z = 100;
     <literal>remove_name</> on the schema.
    </para>
 
+   <para>
+    When objects that are subsidiary of other objects (such as a table's indexes
+    or triggers) are created or dropped, <literal>setattr</> permission will be
+    checked on the main object, instead of the subsidiary object itself.
+   </para>
+
    <para>
     When <xref linkend="sql-security-label"> is executed, <literal>setattr</>
     and <literal>relabelfrom</> will be checked on the object being relabeled

src/backend/bootstrap/bootparse.y

@@ -247,7 +247,8 @@ Boot_CreateStmt:
 													  ONCOMMIT_NOOP,
 													  (Datum) 0,
 													  false,
-													  true);
+													  true,
+													  false);
 						elog(DEBUG4, "relation created with OID %u", id);
 					}
 					do_end();

src/backend/catalog/heap.c

@@ -985,7 +985,8 @@ heap_create_with_catalog(const char *relname,
 						 OnCommitAction oncommit,
 						 Datum reloptions,
 						 bool use_user_acl,
-						 bool allow_system_table_mods)
+						 bool allow_system_table_mods,
+						 bool is_internal)
 {
 	Relation	pg_class_desc;
 	Relation	new_rel_desc;
@@ -1275,8 +1276,15 @@ heap_create_with_catalog(const char *relname,
 	}
 
 	/* Post creation hook for new relation */
-	InvokeObjectAccessHook(OAT_POST_CREATE,
-						   RelationRelationId, relid, 0, NULL);
+	if (object_access_hook)
+	{
+		ObjectAccessPostCreate	post_create_args;
+
+		memset(&post_create_args, 0, sizeof(ObjectAccessPostCreate));
+		post_create_args.is_internal = is_internal;
+		(*object_access_hook)(OAT_POST_CREATE, RelationRelationId,
+							  relid, 0, &post_create_args);
+    }
 
 	/*
 	 * Store any supplied constraints and defaults.

src/backend/catalog/index.c

@@ -33,6 +33,7 @@
 #include "catalog/dependency.h"
 #include "catalog/heap.h"
 #include "catalog/index.h"
+#include "catalog/objectaccess.h"
 #include "catalog/pg_collation.h"
 #include "catalog/pg_constraint.h"
 #include "catalog/pg_operator.h"
@@ -686,7 +687,8 @@ index_create(Relation heapRelation,
 			 bool initdeferred,
 			 bool allow_system_table_mods,
 			 bool skip_build,
-			 bool concurrent)
+			 bool concurrent,
+			 bool is_internal)
 {
 	Oid			heapRelationId = RelationGetRelid(heapRelation);
 	Relation	pg_class;
@@ -1018,6 +1020,17 @@ index_create(Relation heapRelation,
 		Assert(!initdeferred);
 	}
 
+	/* Post creation hook for new index */
+	if (object_access_hook)
+	{
+		ObjectAccessPostCreate	post_create_args;
+
+		memset(&post_create_args, 0, sizeof(ObjectAccessPostCreate));
+		post_create_args.is_internal = is_internal;
+		(*object_access_hook)(OAT_POST_CREATE, RelationRelationId,
+							  indexRelationId, 0, &post_create_args);
+	}
+
 	/*
 	 * Advance the command counter so that we can see the newly-entered
 	 * catalog tuples for the index.

src/backend/catalog/toasting.c

@@ -226,6 +226,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptio
 										   ONCOMMIT_NOOP,
 										   reloptions,
 										   false,
+										   true,
 										   true);
 	Assert(toast_relid != InvalidOid);
 
@@ -279,7 +280,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid, Datum reloptio
 				 rel->rd_rel->reltablespace,
 				 collationObjectId, classObjectId, coloptions, (Datum) 0,
 				 true, false, false, false,
-				 true, false, false);
+				 true, false, false, true);
 
 	heap_close(toast_rel, NoLock);
 

src/backend/commands/cluster.c

@@ -643,6 +643,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace)
 										  ONCOMMIT_NOOP,
 										  reloptions,
 										  false,
+										  true,
 										  true);
 	Assert(OIDNewHeap != InvalidOid);
 

src/backend/commands/indexcmds.c

@@ -596,7 +596,7 @@ DefineIndex(IndexStmt *stmt,
 					 stmt->isconstraint, stmt->deferrable, stmt->initdeferred,
 					 allowSystemTableMods,
 					 skip_build || stmt->concurrent,
-					 stmt->concurrent);
+					 stmt->concurrent, !check_rights);
 
 	/* Add any requested comment */
 	if (stmt->idxcomment != NULL)

src/backend/commands/tablecmds.c

@@ -630,7 +630,8 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
 										  stmt->oncommit,
 										  reloptions,
 										  true,
-										  allowSystemTableMods);
+										  allowSystemTableMods,
+										  false);
 
 	/* Store inheritance information for new rel. */
 	StoreCatalogInheritance(relationId, inheritOids);

src/include/catalog/heap.h

@@ -66,7 +66,8 @@ extern Oid heap_create_with_catalog(const char *relname,
 						 OnCommitAction oncommit,
 						 Datum reloptions,
 						 bool use_user_acl,
-						 bool allow_system_table_mods);
+						 bool allow_system_table_mods,
+						 bool is_internal);
 
 extern void heap_create_init_fork(Relation rel);
 

src/include/catalog/index.h

@@ -50,7 +50,8 @@ extern Oid index_create(Relation heapRelation,
 			 bool initdeferred,
 			 bool allow_system_table_mods,
 			 bool skip_build,
-			 bool concurrent);
+			 bool concurrent,
+			 bool is_internal);
 
 extern void index_constraint_create(Relation heapRelation,
 						Oid indexRelationId,

src/include/catalog/objectaccess.h

@@ -30,6 +30,19 @@ typedef enum ObjectAccessType
 	OAT_DROP,
 } ObjectAccessType;
 
+/*
+ * Arguments of OAT_POST_CREATE event
+ */
+typedef struct
+{
+	/*
+	 * This flag informs extensions whether the context of this creation
+	 * is invoked by user's operations, or not. E.g, it shall be dealt
+	 * as internal stuff on toast tables or indexes due to type changes.
+	 */
+	bool		is_internal;
+} ObjectAccessPostCreate;
+
 /*
  * Arguments of OAT_DROP event
  */