public inbox for [email protected]
help / color / mirror / Atom feedFrom: Jelte Fennema-Nio <[email protected]>
To: Tomas Vondra <[email protected]>
Cc: David G. Johnston <[email protected]>
Cc: Jeff Davis <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Extension security improvement: Add support for extensions with an owned schema
Date: Fri, 4 Oct 2024 23:05:51 +0200
Message-ID: <CAGECzQS02M6YPDXemo36tShO-ZYObjqnyTJyVttua1PGyN4xRw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAGECzQQzDqDzakBkR71ZkQ1N1ffTjAaruRSqppQAKu3WF+6rNQ@mail.gmail.com>
<[email protected]>
<CAGECzQTe3y6xwP9aMbFBCxKnQEEn3G3=cEDqoD5xaOwPwypd_A@mail.gmail.com>
<CAGECzQTAgtTp3G=Em3xEY=T=uiKfyu5xLYri9By=sfNBS5C_9A@mail.gmail.com>
<CAKFQuwaT4_n=e0YKBZAyox1CQUra2ka0cySs+3pGZR5p50pn-g@mail.gmail.com>
<CAGECzQTOJrnnJkmMe9nems0jouiKUbFcEb1rb9kE_svsAZiGQg@mail.gmail.com>
<[email protected]>
On Fri, 27 Sept 2024 at 14:00, Tomas Vondra <[email protected]> wrote:
> One thing that's not quite clear to me is what's the correct way for
> existing extensions to switch to an "owned schema". Let's say you have
> an extension. How do you transition to this? Can you just add it to the
> control file and then some magic happens?
Sadly no. As currently implemented this feature can only really be
used by new extensions. There is no upgrade path to it for existing
extensions. This is fairly common btw, the only field in the control
file that ever gets used after the taken from the control file for
ALTER EXTENSION is the version field. e.g. if you change the schema
field in the control file of an already installed extension, the
extension will not be moved to the new schema unless you DROP + CREATE
EXTENSION.
After this message I tried messing around a bit with changing an
existing extension to become an owned schema (either with a new
command or as part of ALTER EXTENSION UPDATE). But it's not as trivial
as I hoped for a few reasons:
1. There are multiple states that extensions are currently in all of
which need somewhat different conversion strategies. Specifically:
relocatable/non-relocatable &
schema=pg_catalog/public/actual_extension_schema.
2. There are two important assumptions on the schema for an
owned_schema, which we would need to check on an existing schema
converting it:
a. The owner of the extension should be the owner of the schema
b. There are no other objects in the schema appart from the extension.
I'll probably continue some efforts to allow for migration, because I
agree it's useful. But I don't think it's critical for this feature to
be committable. Even if this can only be used by new extensions (that
target PG18+ only), I think that would already be very useful. i.e. if
in 5 years time I don't have to worry about these security pitfalls
for any new extensions that I write then I'll be very happy. Also if
an extension really wants to upgrade to an owned schema, then that
should be possible by doing some manual catalog hackery in its
extension update script, because that's effectively what any automatic
conversion would also do.
> A couple minor comments:
> Doesn't "extension is owned_schema" sound a bit weird?
Updated the docs to address all of your feedback I think.
> Also, perhaps "dedicated_schema" would be better than "owned_schema"? I
> mean, the point is not that it's "owned" by the extension, but that
> there's nothing else in it. But that's nitpicking.
I agree that's probably a better name. Given the amount of effort
necessary to update everything I haven't done that yet. I'll think a
bit if there's a name I like even better in the meantime, and I'm
definitely open to other suggestions.
> 3) src/backend/commands/extension.c
>
> I'm not sure why AlterExtensionNamespace moves the privilege check. Why
> should it not check the privilege for owned schema too?
Because for an owned schema we're not creating objects in an existing
schema. We're only renaming the schema that the extension is already
in using RenameSchema, so there's no point in checking if the user can
create objects in that schema, since the objects are already there.
(RenameSchema also checks internally if the current user is the owner
of the schema)
> Shouldn't binary_upgrade_extension_member still set ext=NULL in the for
> loop, the way the original code does that?
I don't think that's necessary. It was necessary before to set extobj
to NULL, because that variable was set every loop. But now extobj is
scoped to the loop, and ext is only set right before the break. So
either we set ext in the loop and break out of the loop right away, or
ext is still set to the NULL value that's was assigned at the top of
the function.
> The long if conditions might need some indentation, I guess. pgindent
> leaves them like this, but 100 columns seems a bit too much. I'd do a
> line break after each condition, I guess.
Done
Attachments:
[application/octet-stream] v4-0001-Add-support-for-extensions-with-an-owned-schema.patch (32.1K, 2-v4-0001-Add-support-for-extensions-with-an-owned-schema.patch)
download | inline diff:
From 5bd7357f0c8cca63f3a26aa2d78adddc671a4757 Mon Sep 17 00:00:00 2001
From: Jelte Fennema-Nio <[email protected]>
Date: Fri, 4 Oct 2024 22:34:51 +0200
Subject: [PATCH v4] 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.
---
doc/src/sgml/extend.sgml | 34 +++++
doc/src/sgml/ref/create_extension.sgml | 4 +-
src/backend/commands/extension.c | 142 +++++++++++++-----
src/backend/utils/adt/pg_upgrade_support.c | 45 ++++--
src/bin/pg_dump/pg_dump.c | 54 ++++++-
src/bin/pg_dump/pg_dump.h | 1 +
src/include/catalog/pg_extension.h | 1 +
src/include/catalog/pg_proc.dat | 2 +-
src/include/commands/extension.h | 4 +-
src/test/modules/test_extensions/Makefile | 7 +-
.../expected/test_extensions.out | 50 ++++++
src/test/modules/test_extensions/meson.build | 4 +
.../test_extensions/sql/test_extensions.sql | 27 ++++
.../test_ext_owned_schema--1.0.sql | 2 +
.../test_ext_owned_schema.control | 5 +
...test_ext_owned_schema_relocatable--1.0.sql | 2 +
.../test_ext_owned_schema_relocatable.control | 4 +
17 files changed, 326 insertions(+), 62 deletions(-)
create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema.control
create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema_relocatable--1.0.sql
create mode 100644 src/test/modules/test_extensions/test_ext_owned_schema_relocatable.control
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index 218940ee5ce..654799b2714 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -809,6 +809,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>
diff --git a/doc/src/sgml/ref/create_extension.sgml b/doc/src/sgml/ref/create_extension.sgml
index ca2b80d669c..ffbe759f84e 100644
--- a/doc/src/sgml/ref/create_extension.sgml
+++ b/doc/src/sgml/ref/create_extension.sgml
@@ -102,7 +102,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>
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index fab59ad5f66..06d8658c61f 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -84,6 +84,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? */
@@ -505,6 +507,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))
@@ -1491,8 +1501,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)
@@ -1514,7 +1527,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);
@@ -1535,8 +1551,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
@@ -1603,6 +1633,7 @@ CreateExtensionInternal(char *extensionName,
*/
address = InsertExtensionTuple(control->name, extowner,
schemaOid, control->relocatable,
+ control->owned_schema,
versionName,
PointerGetDatum(NULL),
PointerGetDatum(NULL),
@@ -1808,7 +1839,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)
{
@@ -1838,6 +1870,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))
@@ -1882,6 +1915,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);
@@ -2729,11 +2773,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 ...
@@ -2742,22 +2785,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);
@@ -2781,14 +2808,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 */
@@ -2861,6 +2912,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
@@ -2904,18 +2962,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);
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index c54b08fe180..05205cfb7f4 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/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"
@@ -185,12 +186,14 @@ 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;
@@ -198,28 +201,30 @@ binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
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;
@@ -234,10 +239,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,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1b47c388ced..395cc85285c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1776,6 +1776,19 @@ checkExtensionMembership(DumpableObject *dobj, Archive *fout)
if (ext == NULL)
return false;
+ /*
+ * If this is the "owned_schema" of the extension, then we don't want to
+ * create it manually, because it gets created together with the
+ * extension.
+ */
+ if (dobj->objType == DO_NAMESPACE &&
+ ext->ownedschema && strcmp(ext->namespace, dobj->name) == 0)
+ {
+ NamespaceInfo *nsinfo = (NamespaceInfo *) dobj;
+
+ nsinfo->create = false;
+ }
+
dobj->ext_member = true;
/* Record dependency so that getDependencies needn't deal with that */
@@ -5645,7 +5658,7 @@ binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
const char *objname,
const char *objnamespace)
{
- DumpableObject *extobj = NULL;
+ ExtensionInfo *ext = NULL;
int i;
if (!dobj->ext_member)
@@ -5659,19 +5672,33 @@ binary_upgrade_extension_member(PQExpBuffer upgrade_buffer,
*/
for (i = 0; i < dobj->nDeps; i++)
{
- extobj = findObjectByDumpId(dobj->dependencies[i]);
+ DumpableObject *extobj = findObjectByDumpId(dobj->dependencies[i]);
+
if (extobj && extobj->objType == DO_EXTENSION)
+ {
+ ext = (ExtensionInfo *) extobj;
break;
- extobj = NULL;
+ }
}
- if (extobj == NULL)
+ if (ext == NULL)
pg_fatal("could not find parent extension for %s %s",
objtype, objname);
+ /*
+ * If the object is the "owned_schema" of the extension, we don't need to
+ * add it to the extension because it was already made a member of the
+ * extension when the extension was created.
+ */
+ if (dobj->objType == DO_NAMESPACE &&
+ ext->ownedschema && strcmp(ext->namespace, dobj->name) == 0)
+ {
+ return;
+ }
+
appendPQExpBufferStr(upgrade_buffer,
"\n-- For binary upgrade, handle extension membership the hard way\n");
appendPQExpBuffer(upgrade_buffer, "ALTER EXTENSION %s ADD %s ",
- fmtId(extobj->name),
+ fmtId(ext->dobj.name),
objtype);
if (objnamespace && *objnamespace)
appendPQExpBuffer(upgrade_buffer, "%s.", fmtId(objnamespace));
@@ -5828,6 +5855,7 @@ getExtensions(Archive *fout, int *numExtensions)
int i_extname;
int i_nspname;
int i_extrelocatable;
+ int i_extownedschema;
int i_extversion;
int i_extconfig;
int i_extcondition;
@@ -5836,7 +5864,14 @@ getExtensions(Archive *fout, int *numExtensions)
appendPQExpBufferStr(query, "SELECT x.tableoid, x.oid, "
"x.extname, n.nspname, x.extrelocatable, x.extversion, x.extconfig, x.extcondition "
- "FROM pg_extension x "
+ );
+
+ if (fout->remoteVersion >= 180000)
+ appendPQExpBufferStr(query, ", x.extownedschema ");
+ else
+ appendPQExpBufferStr(query, ", false AS extownedschema ");
+
+ appendPQExpBufferStr(query, "FROM pg_extension x "
"JOIN pg_namespace n ON n.oid = x.extnamespace");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -5852,6 +5887,7 @@ getExtensions(Archive *fout, int *numExtensions)
i_extname = PQfnumber(res, "extname");
i_nspname = PQfnumber(res, "nspname");
i_extrelocatable = PQfnumber(res, "extrelocatable");
+ i_extownedschema = PQfnumber(res, "extownedschema");
i_extversion = PQfnumber(res, "extversion");
i_extconfig = PQfnumber(res, "extconfig");
i_extcondition = PQfnumber(res, "extcondition");
@@ -5865,6 +5901,7 @@ getExtensions(Archive *fout, int *numExtensions)
extinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_extname));
extinfo[i].namespace = pg_strdup(PQgetvalue(res, i, i_nspname));
extinfo[i].relocatable = *(PQgetvalue(res, i, i_extrelocatable)) == 't';
+ extinfo[i].ownedschema = *(PQgetvalue(res, i, i_extownedschema)) == 't';
extinfo[i].extversion = pg_strdup(PQgetvalue(res, i, i_extversion));
extinfo[i].extconfig = pg_strdup(PQgetvalue(res, i, i_extconfig));
extinfo[i].extcondition = pg_strdup(PQgetvalue(res, i, i_extcondition));
@@ -10613,9 +10650,9 @@ dumpNamespace(Archive *fout, const NamespaceInfo *nspinfo)
{
/* see selectDumpableNamespace() */
appendPQExpBufferStr(delq,
- "-- *not* dropping schema, since initdb creates it\n");
+ "-- *not* dropping schema, since initdb or CREATE EXTENSION creates it\n");
appendPQExpBufferStr(q,
- "-- *not* creating schema, since initdb creates it\n");
+ "-- *not* creating schema, since initdb or CREATE EXTENSION creates it\n");
}
if (dopt->binary_upgrade)
@@ -10727,6 +10764,7 @@ dumpExtension(Archive *fout, const ExtensionInfo *extinfo)
appendStringLiteralAH(q, extinfo->namespace, fout);
appendPQExpBufferStr(q, ", ");
appendPQExpBuffer(q, "%s, ", extinfo->relocatable ? "true" : "false");
+ appendPQExpBuffer(q, "%s, ", extinfo->ownedschema ? "true" : "false");
appendStringLiteralAH(q, extinfo->extversion, fout);
appendPQExpBufferStr(q, ", ");
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 9f907ed5ad4..349e5219925 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -181,6 +181,7 @@ typedef struct _extensionInfo
DumpableObject dobj;
char *namespace; /* schema containing extension's objects */
bool relocatable;
+ bool ownedschema;
char *extversion;
char *extconfig; /* info about configuration tables */
char *extcondition;
diff --git a/src/include/catalog/pg_extension.h b/src/include/catalog/pg_extension.h
index 673181b39ae..6a595fae53c 100644
--- a/src/include/catalog/pg_extension.h
+++ b/src/include/catalog/pg_extension.h
@@ -34,6 +34,7 @@ CATALOG(pg_extension,3079,ExtensionRelationId)
Oid extnamespace BKI_LOOKUP(pg_namespace); /* namespace of
* contained objects */
bool extrelocatable; /* if true, allow ALTER EXTENSION SET SCHEMA */
+ bool extownedschema; /* if true, schema is owned by extension */
#ifdef CATALOG_VARLEN /* variable-length fields start here */
/* extversion may never be null, but the others can be. */
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 77f54a79e6a..3e294327a39 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11558,7 +11558,7 @@
{ oid => '3591', descr => 'for use by pg_upgrade',
proname => 'binary_upgrade_create_empty_extension', proisstrict => 'f',
provolatile => 'v', proparallel => 'u', prorettype => 'void',
- proargtypes => 'text text bool text _oid _text _text',
+ proargtypes => 'text text bool bool text _oid _text _text',
prosrc => 'binary_upgrade_create_empty_extension' },
{ oid => '4083', descr => 'for use by pg_upgrade',
proname => 'binary_upgrade_set_record_init_privs', provolatile => 'v',
diff --git a/src/include/commands/extension.h b/src/include/commands/extension.h
index c6f3f867eb7..8e7fa574032 100644
--- a/src/include/commands/extension.h
+++ b/src/include/commands/extension.h
@@ -36,7 +36,9 @@ extern ObjectAddress CreateExtension(ParseState *pstate, CreateExtensionStmt *st
extern void RemoveExtensionById(Oid extId);
extern 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);
diff --git a/src/test/modules/test_extensions/Makefile b/src/test/modules/test_extensions/Makefile
index 05272e6a40b..28f20290190 100644
--- a/src/test/modules/test_extensions/Makefile
+++ b/src/test/modules/test_extensions/Makefile
@@ -9,7 +9,8 @@ EXTENSION = test_ext1 test_ext2 test_ext3 test_ext4 test_ext5 test_ext6 \
test_ext_extschema \
test_ext_evttrig \
test_ext_set_schema \
- test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3
+ test_ext_req_schema1 test_ext_req_schema2 test_ext_req_schema3 \
+ test_ext_owned_schema test_ext_owned_schema_relocatable
DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext4--1.0.sql test_ext5--1.0.sql test_ext6--1.0.sql \
@@ -23,7 +24,9 @@ DATA = test_ext1--1.0.sql test_ext2--1.0.sql test_ext3--1.0.sql \
test_ext_set_schema--1.0.sql \
test_ext_req_schema1--1.0.sql \
test_ext_req_schema2--1.0.sql \
- test_ext_req_schema3--1.0.sql
+ test_ext_req_schema3--1.0.sql \
+ test_ext_owned_schema--1.0.sql \
+ test_ext_owned_schema_relocatable--1.0.sql
REGRESS = test_extensions test_extdepend
diff --git a/src/test/modules/test_extensions/expected/test_extensions.out b/src/test/modules/test_extensions/expected/test_extensions.out
index f357cc21aaa..c0a2b7b315e 100644
--- a/src/test/modules/test_extensions/expected/test_extensions.out
+++ b/src/test/modules/test_extensions/expected/test_extensions.out
@@ -626,3 +626,53 @@ SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
NOTICE: drop cascades to extension test_ext_req_schema2
+--
+-- Test owned schema extensions
+--
+CREATE SCHEMA test_ext_owned_schema;
+-- Fails for an already existing schema to be provided
+CREATE EXTENSION test_ext_owned_schema SCHEMA test_ext_owned_schema;
+ERROR: schema "test_ext_owned_schema" already exists
+-- Fails because a different schema is set in control file
+CREATE EXTENSION test_ext_owned_schema SCHEMA test_schema;
+ERROR: extension "test_ext_owned_schema" must be installed in schema "test_ext_owned_schema"
+DROP SCHEMA test_ext_owned_schema;
+CREATE EXTENSION test_ext_owned_schema;
+\dx+ test_ext_owned_schema;
+Objects in extension "test_ext_owned_schema"
+ Object description
+-----------------------------------------
+ function test_ext_owned_schema.owned1()
+ schema test_ext_owned_schema
+(2 rows)
+
+DROP EXTENSION test_ext_owned_schema;
+CREATE SCHEMA already_existing;
+-- Fails for an already existing schema to be provided
+CREATE EXTENSION test_ext_owned_schema_relocatable SCHEMA already_existing;
+ERROR: schema "already_existing" already exists
+-- Fails because no schema is set in control file
+CREATE EXTENSION test_ext_owned_schema_relocatable;
+ERROR: no schema has been selected to create in
+CREATE EXTENSION test_ext_owned_schema_relocatable SCHEMA test_schema;
+\dx+ test_ext_owned_schema_relocatable
+Objects in extension "test_ext_owned_schema_relocatable"
+ Object description
+-------------------------------
+ function test_schema.owned2()
+ schema test_schema
+(2 rows)
+
+-- Fails because schema already exists
+ALTER EXTENSION test_ext_owned_schema_relocatable SET SCHEMA already_existing;
+ERROR: schema "already_existing" already exists
+ALTER EXTENSION test_ext_owned_schema_relocatable SET SCHEMA some_other_name;
+\dx+ test_ext_owned_schema_relocatable
+Objects in extension "test_ext_owned_schema_relocatable"
+ Object description
+-----------------------------------
+ function some_other_name.owned2()
+ schema some_other_name
+(2 rows)
+
+DROP EXTENSION test_ext_owned_schema_relocatable;
diff --git a/src/test/modules/test_extensions/meson.build b/src/test/modules/test_extensions/meson.build
index c5f3424da51..52e8841480b 100644
--- a/src/test/modules/test_extensions/meson.build
+++ b/src/test/modules/test_extensions/meson.build
@@ -42,6 +42,10 @@ test_install_data += files(
'test_ext_req_schema3.control',
'test_ext_set_schema--1.0.sql',
'test_ext_set_schema.control',
+ 'test_ext_owned_schema--1.0.sql',
+ 'test_ext_owned_schema.control',
+ 'test_ext_owned_schema_relocatable--1.0.sql',
+ 'test_ext_owned_schema_relocatable.control',
)
tests += {
diff --git a/src/test/modules/test_extensions/sql/test_extensions.sql b/src/test/modules/test_extensions/sql/test_extensions.sql
index 642c82ff5d3..136967db395 100644
--- a/src/test/modules/test_extensions/sql/test_extensions.sql
+++ b/src/test/modules/test_extensions/sql/test_extensions.sql
@@ -299,3 +299,30 @@ ALTER EXTENSION test_ext_req_schema1 SET SCHEMA test_s_dep2; -- now ok
SELECT test_s_dep2.dep_req1();
SELECT test_s_dep.dep_req2();
DROP EXTENSION test_ext_req_schema1 CASCADE;
+
+--
+-- Test owned schema extensions
+--
+
+CREATE SCHEMA test_ext_owned_schema;
+-- Fails for an already existing schema to be provided
+CREATE EXTENSION test_ext_owned_schema SCHEMA test_ext_owned_schema;
+-- Fails because a different schema is set in control file
+CREATE EXTENSION test_ext_owned_schema SCHEMA test_schema;
+DROP SCHEMA test_ext_owned_schema;
+CREATE EXTENSION test_ext_owned_schema;
+\dx+ test_ext_owned_schema;
+DROP EXTENSION test_ext_owned_schema;
+
+CREATE SCHEMA already_existing;
+-- Fails for an already existing schema to be provided
+CREATE EXTENSION test_ext_owned_schema_relocatable SCHEMA already_existing;
+-- Fails because no schema is set in control file
+CREATE EXTENSION test_ext_owned_schema_relocatable;
+CREATE EXTENSION test_ext_owned_schema_relocatable SCHEMA test_schema;
+\dx+ test_ext_owned_schema_relocatable
+-- Fails because schema already exists
+ALTER EXTENSION test_ext_owned_schema_relocatable SET SCHEMA already_existing;
+ALTER EXTENSION test_ext_owned_schema_relocatable SET SCHEMA some_other_name;
+\dx+ test_ext_owned_schema_relocatable
+DROP EXTENSION test_ext_owned_schema_relocatable;
diff --git a/src/test/modules/test_extensions/test_ext_owned_schema--1.0.sql b/src/test/modules/test_extensions/test_ext_owned_schema--1.0.sql
new file mode 100644
index 00000000000..672ab8e607f
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_owned_schema--1.0.sql
@@ -0,0 +1,2 @@
+CREATE FUNCTION owned1() RETURNS text
+LANGUAGE SQL AS $$ SELECT 1 $$;
diff --git a/src/test/modules/test_extensions/test_ext_owned_schema.control b/src/test/modules/test_extensions/test_ext_owned_schema.control
new file mode 100644
index 00000000000..531c38daefd
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_owned_schema.control
@@ -0,0 +1,5 @@
+comment = 'Test extension with an owned schema'
+default_version = '1.0'
+relocatable = false
+schema = test_ext_owned_schema
+owned_schema = true
diff --git a/src/test/modules/test_extensions/test_ext_owned_schema_relocatable--1.0.sql b/src/test/modules/test_extensions/test_ext_owned_schema_relocatable--1.0.sql
new file mode 100644
index 00000000000..bfccaf4af82
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_owned_schema_relocatable--1.0.sql
@@ -0,0 +1,2 @@
+CREATE FUNCTION owned2() RETURNS text
+LANGUAGE SQL AS $$ SELECT 1 $$;
diff --git a/src/test/modules/test_extensions/test_ext_owned_schema_relocatable.control b/src/test/modules/test_extensions/test_ext_owned_schema_relocatable.control
new file mode 100644
index 00000000000..3cda1e12341
--- /dev/null
+++ b/src/test/modules/test_extensions/test_ext_owned_schema_relocatable.control
@@ -0,0 +1,4 @@
+comment = 'Test extension with an owned schema'
+default_version = '1.0'
+relocatable = true
+owned_schema = true
base-commit: 259a0a99fe3d45dcf624788c1724d9989f3382dc
--
2.34.1
view thread (26+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Extension security improvement: Add support for extensions with an owned schema
In-Reply-To: <CAGECzQS02M6YPDXemo36tShO-ZYObjqnyTJyVttua1PGyN4xRw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox