Add support for extensions with an owned schema

Writing the sql migration scripts that are run by CREATE EXTENSION and
ALTER EXTENSION UPDATE are security minefields for extension authors.
One big reason for this is that search_path is set to the schema of the
extension while running these scripts, and thus if a user with lower
privileges can create functions or operators in that schema they can do
all kinds of search_path confusion attacks if not every function and
operator that is used in the script is schema qualified. While doing
such schema qualification is possible, it relies on the author to never
make a mistake in any of the sql files. And sadly humans have a tendency
to make mistakes.

This patch adds a new "owned_schema" option to the extension control
file that can be set to true to indicate that this extension wants to
own the schema in which it is installed. What that means is that the
schema should not exist before creating the extension, and will be
created during extension creation. This thus gives the extension author
an easy way to use a safe search_path, while still allowing all objects
to be grouped together in a schema. The implementation also has the
pleasant side effect that the schema will be automatically dropped when
the extension is dropped.

Author: JelteF

doc/src/sgml/extend.sgml

@@ -814,6 +814,40 @@ RETURNS anycompatible AS ...
       </listitem>
      </varlistentry>
 
+     <varlistentry id="extend-extensions-files-owned-schema">
+      <term><varname>owned_schema</varname> (<type>boolean</type>)</term>
+      <listitem>
+       <para>
+        An extension should set <firstterm>owned_schema</firstterm> to
+        <literal>true</literal> in its control file if the extension wants a
+        dedicated schema for its objects. Such a schema should not exist yet at
+        the time of extension creation, and will be created automatically by
+        <literal>CREATE EXTENSION</literal>. The default is
+        <literal>false</literal>, i.e., the extension can be installed into an
+        existing schema.
+       </para>
+       <para>
+        Having a schema owned by the extension can make it much easier to
+        reason about possible <literal>search_path</literal> injection attacks.
+        For instance with an owned schema, it is generally safe to set the
+        <literal>search_path</literal> of a <literal>SECURITY DEFINER</literal>
+        function to the schema of the extension. While without an owned schema
+        it might not be safe to do so, because a malicious user could insert
+        objects in that schema and thus <link
+        linkend="sql-createfunction-security"> cause malicious to be executed
+        as superuser</link>. Similarly, having an owned schema can make it safe
+        by default to execute general-purpose SQL in the extension script,
+        because the search_path now only contains trusted schemas. Without an
+        owned schema it's <link linkend="extend-extensions-security-scripts">
+        recommended to manually change the search_path</link>.
+       </para>
+       <para>
+        Apart from the security considerations, having an owned schema can help
+        prevent naming conflicts between objects of different extensions.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="extend-extensions-files-schema">
       <term><varname>schema</varname> (<type>string</type>)</term>
       <listitem>

doc/src/sgml/ref/create_extension.sgml

@@ -104,7 +104,9 @@ CREATE EXTENSION [ IF NOT EXISTS ] <replaceable class="parameter">extension_name
        <para>
         The name of the schema in which to install the extension's
         objects, given that the extension allows its contents to be
-        relocated.  The named schema must already exist.
+        relocated.  The named schema must already exist, unless
+        <literal>owned_schema</literal> is set to <literal>true</literal> in
+        the control file, then the schema must not exist.
         If not specified, and the extension's control file does not specify a
         schema either, the current default object creation schema is used.
        </para>

src/backend/commands/extension.c

@@ -90,6 +90,8 @@ typedef struct ExtensionControlFile
 									 * MODULE_PATHNAME */
 	char	   *comment;		/* comment, if any */
 	char	   *schema;			/* target schema (allowed if !relocatable) */
+	bool		owned_schema;	/* if the schema should be owned by the
+								 * extension */
 	bool		relocatable;	/* is ALTER EXTENSION SET SCHEMA supported? */
 	bool		superuser;		/* must be superuser to install? */
 	bool		trusted;		/* allow becoming superuser on the fly? */
@@ -611,6 +613,14 @@ parse_extension_control_file(ExtensionControlFile *control,
 		{
 			control->schema = pstrdup(item->value);
 		}
+		else if (strcmp(item->name, "owned_schema") == 0)
+		{
+			if (!parse_bool(item->value, &control->owned_schema))
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+						 errmsg("parameter \"%s\" requires a Boolean value",
+								item->name)));
+		}
 		else if (strcmp(item->name, "relocatable") == 0)
 		{
 			if (!parse_bool(item->value, &control->relocatable))
@@ -1744,8 +1754,11 @@ CreateExtensionInternal(char *extensionName,
 	 */
 	if (schemaName)
 	{
-		/* If the user is giving us the schema name, it must exist already. */
-		schemaOid = get_namespace_oid(schemaName, false);
+		/*
+		 * If the user is giving us the schema name, it must exist already if
+		 * the extension does not want to own the schema
+		 */
+		schemaOid = get_namespace_oid(schemaName, control->owned_schema);
 	}
 
 	if (control->schema != NULL)
@@ -1767,7 +1780,10 @@ CreateExtensionInternal(char *extensionName,
 
 		/* Always use the schema from control file for current extension. */
 		schemaName = control->schema;
+	}
 
+	if (schemaName)
+	{
 		/* Find or create the schema in case it does not exist. */
 		schemaOid = get_namespace_oid(schemaName, true);
 
@@ -1788,8 +1804,22 @@ CreateExtensionInternal(char *extensionName,
 			 */
 			schemaOid = get_namespace_oid(schemaName, false);
 		}
+		else if (control->owned_schema)
+		{
+			ereport(ERROR,
+					(errcode(ERRCODE_DUPLICATE_SCHEMA),
+					 errmsg("schema \"%s\" already exists",
+							schemaName)));
+		}
+
 	}
-	else if (!OidIsValid(schemaOid))
+	else if (control->owned_schema)
+	{
+		ereport(ERROR,
+				(errcode(ERRCODE_UNDEFINED_SCHEMA),
+				 errmsg("no schema has been selected to create in")));
+	}
+	else
 	{
 		/*
 		 * Neither user nor author of the extension specified schema; use the
@@ -1856,6 +1886,7 @@ CreateExtensionInternal(char *extensionName,
 	 */
 	address = InsertExtensionTuple(control->name, extowner,
 								   schemaOid, control->relocatable,
+								   control->owned_schema,
 								   versionName,
 								   PointerGetDatum(NULL),
 								   PointerGetDatum(NULL),
@@ -2061,7 +2092,8 @@ CreateExtension(ParseState *pstate, CreateExtensionStmt *stmt)
  */
 ObjectAddress
 InsertExtensionTuple(const char *extName, Oid extOwner,
-					 Oid schemaOid, bool relocatable, const char *extVersion,
+					 Oid schemaOid, bool relocatable, bool ownedSchema,
+					 const char *extVersion,
 					 Datum extConfig, Datum extCondition,
 					 List *requiredExtensions)
 {
@@ -2091,6 +2123,7 @@ InsertExtensionTuple(const char *extName, Oid extOwner,
 	values[Anum_pg_extension_extowner - 1] = ObjectIdGetDatum(extOwner);
 	values[Anum_pg_extension_extnamespace - 1] = ObjectIdGetDatum(schemaOid);
 	values[Anum_pg_extension_extrelocatable - 1] = BoolGetDatum(relocatable);
+	values[Anum_pg_extension_extownedschema - 1] = BoolGetDatum(ownedSchema);
 	values[Anum_pg_extension_extversion - 1] = CStringGetTextDatum(extVersion);
 
 	if (extConfig == PointerGetDatum(NULL))
@@ -2135,6 +2168,17 @@ InsertExtensionTuple(const char *extName, Oid extOwner,
 	record_object_address_dependencies(&myself, refobjs, DEPENDENCY_NORMAL);
 	free_object_addresses(refobjs);
 
+	if (ownedSchema)
+	{
+		ObjectAddress schemaAddress = {
+			.classId = NamespaceRelationId,
+			.objectId = schemaOid,
+		};
+
+		recordDependencyOn(&schemaAddress, &myself, DEPENDENCY_EXTENSION);
+	}
+
+
 	/* Post creation hook for new extension */
 	InvokeObjectPostCreateHook(ExtensionRelationId, extensionOid, 0);
 
@@ -3053,11 +3097,10 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
 	HeapTuple	depTup;
 	ObjectAddresses *objsMoved;
 	ObjectAddress extAddr;
+	bool		ownedSchema;
 
 	extensionOid = get_extension_oid(extensionName, false);
 
-	nspOid = LookupCreationNamespace(newschema);
-
 	/*
 	 * Permission check: must own extension.  Note that we don't bother to
 	 * check ownership of the individual member objects ...
@@ -3066,22 +3109,6 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
 		aclcheck_error(ACLCHECK_NOT_OWNER, OBJECT_EXTENSION,
 					   extensionName);
 
-	/* Permission check: must have creation rights in target namespace */
-	aclresult = object_aclcheck(NamespaceRelationId, nspOid, GetUserId(), ACL_CREATE);
-	if (aclresult != ACLCHECK_OK)
-		aclcheck_error(aclresult, OBJECT_SCHEMA, newschema);
-
-	/*
-	 * If the schema is currently a member of the extension, disallow moving
-	 * the extension into the schema.  That would create a dependency loop.
-	 */
-	if (getExtensionOfObject(NamespaceRelationId, nspOid) == extensionOid)
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("cannot move extension \"%s\" into schema \"%s\" "
-						"because the extension contains the schema",
-						extensionName, newschema)));
-
 	/* Locate the pg_extension tuple */
 	extRel = table_open(ExtensionRelationId, RowExclusiveLock);
 
@@ -3105,14 +3132,38 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
 
 	systable_endscan(extScan);
 
-	/*
-	 * If the extension is already in the target schema, just silently do
-	 * nothing.
-	 */
-	if (extForm->extnamespace == nspOid)
+	ownedSchema = extForm->extownedschema;
+
+	if (!ownedSchema)
 	{
-		table_close(extRel, RowExclusiveLock);
-		return InvalidObjectAddress;
+		nspOid = LookupCreationNamespace(newschema);
+
+		/* Permission check: must have creation rights in target namespace */
+		aclresult = object_aclcheck(NamespaceRelationId, nspOid, GetUserId(), ACL_CREATE);
+		if (aclresult != ACLCHECK_OK)
+			aclcheck_error(aclresult, OBJECT_SCHEMA, newschema);
+
+		/*
+		 * If the schema is currently a member of the extension, disallow
+		 * moving the extension into the schema.  That would create a
+		 * dependency loop.
+		 */
+		if (getExtensionOfObject(NamespaceRelationId, nspOid) == extensionOid)
+			ereport(ERROR,
+					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+					 errmsg("cannot move extension \"%s\" into schema \"%s\" "
+							"because the extension contains the schema",
+							extensionName, newschema)));
+
+		/*
+		 * If the extension is already in the target schema, just silently do
+		 * nothing.
+		 */
+		if (extForm->extnamespace == nspOid)
+		{
+			table_close(extRel, RowExclusiveLock);
+			return InvalidObjectAddress;
+		}
 	}
 
 	/* Check extension is supposed to be relocatable */
@@ -3185,6 +3236,13 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
 			}
 		}
 
+		/*
+		 * We don't actually have to move any objects anything for owned
+		 * schemas, because we simply rename the schema.
+		 */
+		if (ownedSchema)
+			continue;
+
 		/*
 		 * Otherwise, ignore non-membership dependencies.  (Currently, the
 		 * only other case we could see here is a normal dependency from
@@ -3228,18 +3286,26 @@ AlterExtensionNamespace(const char *extensionName, const char *newschema, Oid *o
 
 	relation_close(depRel, AccessShareLock);
 
-	/* Now adjust pg_extension.extnamespace */
-	extForm->extnamespace = nspOid;
+	if (ownedSchema)
+	{
+		RenameSchema(get_namespace_name(oldNspOid), newschema);
+		table_close(extRel, RowExclusiveLock);
+	}
+	else
+	{
+		/* Now adjust pg_extension.extnamespace */
+		extForm->extnamespace = nspOid;
 
-	CatalogTupleUpdate(extRel, &extTup->t_self, extTup);
+		CatalogTupleUpdate(extRel, &extTup->t_self, extTup);
 
-	table_close(extRel, RowExclusiveLock);
+		table_close(extRel, RowExclusiveLock);
 
-	/* update dependency to point to the new schema */
-	if (changeDependencyFor(ExtensionRelationId, extensionOid,
-							NamespaceRelationId, oldNspOid, nspOid) != 1)
-		elog(ERROR, "could not change schema dependency for extension %s",
-			 NameStr(extForm->extname));
+		/* update dependency to point to the new schema */
+		if (changeDependencyFor(ExtensionRelationId, extensionOid,
+								NamespaceRelationId, oldNspOid, nspOid) != 1)
+			elog(ERROR, "could not change schema dependency for extension %s",
+				 NameStr(extForm->extname));
+	}
 
 	InvokeObjectPostAlterHook(ExtensionRelationId, extensionOid, 0);
 

src/backend/utils/adt/pg_upgrade_support.c

@@ -19,6 +19,7 @@
 #include "catalog/pg_subscription_rel.h"
 #include "catalog/pg_type.h"
 #include "commands/extension.h"
+#include "commands/schemacmds.h"
 #include "miscadmin.h"
 #include "replication/logical.h"
 #include "replication/origin.h"
@@ -184,41 +185,45 @@ Datum
 binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
 {
 	text	   *extName;
-	text	   *schemaName;
+	char	   *schemaName;
 	bool		relocatable;
+	bool		ownedschema;
 	text	   *extVersion;
 	Datum		extConfig;
 	Datum		extCondition;
 	List	   *requiredExtensions;
+	Oid			schemaOid;
 
 	CHECK_IS_BINARY_UPGRADE;
 
 	/* We must check these things before dereferencing the arguments */
 	if (PG_ARGISNULL(0) ||
 		PG_ARGISNULL(1) ||
 		PG_ARGISNULL(2) ||
-		PG_ARGISNULL(3))
+		PG_ARGISNULL(3) ||
+		PG_ARGISNULL(4))
 		elog(ERROR, "null argument to binary_upgrade_create_empty_extension is not allowed");
 
 	extName = PG_GETARG_TEXT_PP(0);
-	schemaName = PG_GETARG_TEXT_PP(1);
+	schemaName = text_to_cstring(PG_GETARG_TEXT_PP(1));
 	relocatable = PG_GETARG_BOOL(2);
-	extVersion = PG_GETARG_TEXT_PP(3);
+	ownedschema = PG_GETARG_BOOL(3);
+	extVersion = PG_GETARG_TEXT_PP(4);
 
-	if (PG_ARGISNULL(4))
+	if (PG_ARGISNULL(5))
 		extConfig = PointerGetDatum(NULL);
 	else
-		extConfig = PG_GETARG_DATUM(4);
+		extConfig = PG_GETARG_DATUM(5);
 
-	if (PG_ARGISNULL(5))
+	if (PG_ARGISNULL(6))
 		extCondition = PointerGetDatum(NULL);
 	else
-		extCondition = PG_GETARG_DATUM(5);
+		extCondition = PG_GETARG_DATUM(6);
 
 	requiredExtensions = NIL;
-	if (!PG_ARGISNULL(6))
+	if (!PG_ARGISNULL(7))
 	{
-		ArrayType  *textArray = PG_GETARG_ARRAYTYPE_P(6);
+		ArrayType  *textArray = PG_GETARG_ARRAYTYPE_P(7);
 		Datum	   *textDatums;
 		int			ndatums;
 		int			i;
@@ -233,10 +238,28 @@ binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
 		}
 	}
 
+	if (ownedschema)
+	{
+		CreateSchemaStmt *csstmt = makeNode(CreateSchemaStmt);
+
+		csstmt->schemaname = schemaName;
+		csstmt->authrole = NULL;	/* will be created by current user */
+		csstmt->schemaElts = NIL;
+		csstmt->if_not_exists = false;
+		schemaOid = CreateSchemaCommand(csstmt, "(generated CREATE SCHEMA command)",
+										-1, -1);
+
+	}
+	else
+	{
+		schemaOid = get_namespace_oid(schemaName, false);
+	}
+
 	InsertExtensionTuple(text_to_cstring(extName),
 						 GetUserId(),
-						 get_namespace_oid(text_to_cstring(schemaName), false),
+						 schemaOid,
 						 relocatable,
+						 ownedschema,
 						 text_to_cstring(extVersion),
 						 extConfig,
 						 extCondition,