[PATCH] New predefined role pg_manage_extensions

Started by Michael Banckover 2 years ago27 messageshackers
Jump to latest
#1Michael Banck
michael.banck@credativ.de

Hi,

I propose to add a new predefined role to Postgres,
pg_manage_extensions. The idea is that it allows Superusers to delegate
the rights to create, update or delete extensions to other roles, even
if those extensions are not trusted or those users are not the database
owner.

I have attached a WIP patch for this.

Thoughts?

Michael

Attachments:

0001-Add-new-pg_manage_extensions-predefined-role.patchtext/x-diff; charset=us-asciiDownload+16-6
#2Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Michael Banck (#1)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, 12 Jan 2024 at 15:53, Michael Banck <mbanck@gmx.net> wrote:

I propose to add a new predefined role to Postgres,
pg_manage_extensions. The idea is that it allows Superusers to delegate
the rights to create, update or delete extensions to other roles, even
if those extensions are not trusted or those users are not the database
owner.

I agree that extension creation is one of the main reasons people
require superuser access, and I think it would be beneficial to try to
reduce that. But I'm not sure that such a pg_manage_extensions role
would have any fewer permissions than superuser in practice. Afaik
many extensions that are not marked as trusted, are not trusted
because they would allow fairly trivial privilege escalation to
superuser if they were.

#3Michael Banck
michael.banck@credativ.de
In reply to: Jelte Fennema-Nio (#2)
Re: [PATCH] New predefined role pg_manage_extensions

Hi,

On Fri, Jan 12, 2024 at 04:13:27PM +0100, Jelte Fennema-Nio wrote:

But I'm not sure that such a pg_manage_extensions role would have any
fewer permissions than superuser in practice.

Note that just being able to create an extension does not give blanket
permission to use it. I did a few checks with things I thought might be
problematic like adminpack or plpython3u, and a pg_manage_extensions
user is not allowed to call those functions or use the untrusted
language.

Afaik many extensions that are not marked as trusted, are not trusted
because they would allow fairly trivial privilege escalation to
superuser if they were.

While that might be true (or we err on the side of caution), I thought
the rationale was more that they either disclose more information about
the database server than we want to disclose to ordinary users, or that
they allow access to the file system etc.

I think if we have extensions in contrib that trivially allow
non-superusers to become superusers just by being installed, that should
be a bug and be fixed by making it impossible for ordinary users to
use those extensions without being granted some access to them in
addition.

After all, socially engineering a DBA into installing an extension due
to user demand would be a thing anyway (even if most DBAs might reject
it) and at least DBAs should be aware of the specific risks of a
particular extension probably?

Michael

#4Michael Banck
michael.banck@credativ.de
In reply to: Michael Banck (#3)
Re: [PATCH] New predefined role pg_manage_extensions

Hi,

Even though there has not been a lot of discussion on this, here is a
rebased patch. I have also added it to the upcoming commitfest.

On Sat, Jan 13, 2024 at 09:20:40AM +0100, Michael Banck wrote:

On Fri, Jan 12, 2024 at 04:13:27PM +0100, Jelte Fennema-Nio wrote:

But I'm not sure that such a pg_manage_extensions role would have any
fewer permissions than superuser in practice.

Note that just being able to create an extension does not give blanket
permission to use it. I did a few checks with things I thought might be
problematic like adminpack or plpython3u, and a pg_manage_extensions
user is not allowed to call those functions or use the untrusted
language.

Afaik many extensions that are not marked as trusted, are not trusted
because they would allow fairly trivial privilege escalation to
superuser if they were.

While that might be true (or we err on the side of caution), I thought
the rationale was more that they either disclose more information about
the database server than we want to disclose to ordinary users, or that
they allow access to the file system etc.

I think if we have extensions in contrib that trivially allow
non-superusers to become superusers just by being installed, that should
be a bug and be fixed by making it impossible for ordinary users to
use those extensions without being granted some access to them in
addition.

After all, socially engineering a DBA into installing an extension due
to user demand would be a thing anyway (even if most DBAs might reject
it) and at least DBAs should be aware of the specific risks of a
particular extension probably?

Michael

Attachments:

v2-0001-Add-new-pg_manage_extensions-predefined-role.patchtext/x-diff; charset=us-asciiDownload+22-6
#5Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Michael Banck (#4)
Re: [PATCH] New predefined role pg_manage_extensions

On Thu, 31 Oct 2024 at 22:47, Michael Banck <mbanck@gmx.net> wrote:

Even though there has not been a lot of discussion on this, here is a
rebased patch. I have also added it to the upcoming commitfest.

After considering this again, I actually think this is a good change.
Basically all cloud providers already provide something similar. It
would be great if we could have a standardized way of doing this.

Regarding the actual patch: I think it looks good, but it needs some tests.

#6Kirill Reshke
reshkekirill@gmail.com
In reply to: Michael Banck (#4)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, 1 Nov 2024 at 02:47, Michael Banck <mbanck@gmx.net> wrote:

Hi,

Even though there has not been a lot of discussion on this, here is a
rebased patch. I have also added it to the upcoming commitfest.

Hi!

+       <literal>pg_manage_extensions</literal> allows creating, removing or
+       updating extensions, even if the extensions are untrusted or the user is
+       not the database owner.

Users are not required to be a database owner to create extensions.
They are required to have CREATE priv on database.

On Sat, Jan 13, 2024 at 09:20:40AM +0100, Michael Banck wrote:

On Fri, Jan 12, 2024 at 04:13:27PM +0100, Jelte Fennema-Nio wrote:

But I'm not sure that such a pg_manage_extensions role would have any
fewer permissions than superuser in practice.

Note that just being able to create an extension does not give blanket
permission to use it. I did a few checks with things I thought might be
problematic like adminpack or plpython3u, and a pg_manage_extensions
user is not allowed to call those functions or use the untrusted
language.

Afaik many extensions that are not marked as trusted, are not trusted
because they would allow fairly trivial privilege escalation to
superuser if they were.

While that might be true (or we err on the side of caution), I thought
the rationale was more that they either disclose more information about
the database server than we want to disclose to ordinary users, or that
they allow access to the file system etc.

Extension installation script can execute arbitrary code with
superuser privilege if markled trusted.

Take this example

```

reshke@yezzey-cbdb:~/postgres/contrib/fooe$ cat fooe.control
# fooe extnesion
comment = 'foo bar baz'
default_version = '1.0'
module_pathname = '$libdir/fooe'
relocatable = true
trusted = true
reshke@yezzey-cbdb:~/postgres/contrib/fooe$ cat fooe--1.0.sql
/* contrib/fooe/fooe--1.0.sql */

-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "CREATE EXTENSION fooe" to load this file. \quit

CREATE ROLE pwned WITH LOGIN SUPERUSER;

reshke@yezzey-cbdb:~/postgres/contrib/fooe$ ../../pgbin/bin/psql -d db2
db2=# create role user_no_sup with login;
CREATE ROLE
db2=# ^C
\q

reshke@yezzey-cbdb:~/postgres/contrib/fooe$ ../../pgbin/bin/psql -U
user_no_sup -d db2
psql (18devel)
Type "help" for help.

db2=> create extension fooe ;
CREATE EXTENSION
db2=> \du+
List of roles
Role name | Attributes
| Description
-------------+------------------------------------------------------------+-------------
pwned | Superuser |
reshke | Superuser, Create role, Create DB, Replication, Bypass RLS |
user1 | |
user2 | |
user_no_sup | |

db2=> ^C

```

I think if we have extensions in contrib that trivially allow
non-superusers to become superusers just by being installed, that should
be a bug and be fixed by making it impossible for ordinary users to
use those extensions without being granted some access to them in
addition.

After all, socially engineering a DBA into installing an extension due
to user demand would be a thing anyway (even if most DBAs might reject
it) and at least DBAs should be aware of the specific risks of a
particular extension probably?

Michael

In general, this concept is rather dubious. Why should we have such a
dangerous pre-defined role? I would prefer to have complete control
over what gets installed in the database if I were a superuser.
Additionally, if a dangerous extension is inadvertently or otherwise
loaded on the host, I never want a normal user to run code with
superuser privileges.

For a thorough understanding of the current situation and the
rationale behind the design, you can read this[1]/messages/by-id/5889.1566415762@sss.pgh.pa.us discussion.

[1]: /messages/by-id/5889.1566415762@sss.pgh.pa.us

--
Best regards,
Kirill Reshke

#7Kirill Reshke
reshkekirill@gmail.com
In reply to: Kirill Reshke (#6)
Re: [PATCH] New predefined role pg_manage_extensions

On Mon, 18 Nov 2024 at 11:26, Kirill Reshke <reshkekirill@gmail.com> wrote:

On Fri, 1 Nov 2024 at 02:47, Michael Banck <mbanck@gmx.net> wrote:

Hi,

Even though there has not been a lot of discussion on this, here is a
rebased patch. I have also added it to the upcoming commitfest.

Hi!

+       <literal>pg_manage_extensions</literal> allows creating, removing or
+       updating extensions, even if the extensions are untrusted or the user is
+       not the database owner.

Users are not required to be a database owner to create extensions.
They are required to have CREATE priv on database.

On Sat, Jan 13, 2024 at 09:20:40AM +0100, Michael Banck wrote:

On Fri, Jan 12, 2024 at 04:13:27PM +0100, Jelte Fennema-Nio wrote:

But I'm not sure that such a pg_manage_extensions role would have any
fewer permissions than superuser in practice.

Note that just being able to create an extension does not give blanket
permission to use it. I did a few checks with things I thought might be
problematic like adminpack or plpython3u, and a pg_manage_extensions
user is not allowed to call those functions or use the untrusted
language.

Afaik many extensions that are not marked as trusted, are not trusted
because they would allow fairly trivial privilege escalation to
superuser if they were.

While that might be true (or we err on the side of caution), I thought
the rationale was more that they either disclose more information about
the database server than we want to disclose to ordinary users, or that
they allow access to the file system etc.

Extension installation script can execute arbitrary code with
superuser privilege if markled trusted.

Take this example

```

reshke@yezzey-cbdb:~/postgres/contrib/fooe$ cat fooe.control
# fooe extnesion
comment = 'foo bar baz'
default_version = '1.0'
module_pathname = '$libdir/fooe'
relocatable = true
trusted = true
reshke@yezzey-cbdb:~/postgres/contrib/fooe$ cat fooe--1.0.sql
/* contrib/fooe/fooe--1.0.sql */

-- complain if script is sourced in psql, rather than via CREATE EXTENSION
\echo Use "CREATE EXTENSION fooe" to load this file. \quit

CREATE ROLE pwned WITH LOGIN SUPERUSER;

reshke@yezzey-cbdb:~/postgres/contrib/fooe$ ../../pgbin/bin/psql -d db2
db2=# create role user_no_sup with login;
CREATE ROLE
db2=# ^C
\q

I'm sorry, the example is incomplete.

Here is missing command:

db2=# grant create on database db2 to user_no_sup ;
GRANT

reshke@yezzey-cbdb:~/postgres/contrib/fooe$ ../../pgbin/bin/psql -U
user_no_sup -d db2
psql (18devel)
Type "help" for help.

db2=> create extension fooe ;
CREATE EXTENSION
db2=> \du+
List of roles
Role name | Attributes
| Description
-------------+------------------------------------------------------------+-------------
pwned | Superuser |
reshke | Superuser, Create role, Create DB, Replication, Bypass RLS |
user1 | |
user2 | |
user_no_sup | |

db2=> ^C

```

I think if we have extensions in contrib that trivially allow
non-superusers to become superusers just by being installed, that should
be a bug and be fixed by making it impossible for ordinary users to
use those extensions without being granted some access to them in
addition.

After all, socially engineering a DBA into installing an extension due
to user demand would be a thing anyway (even if most DBAs might reject
it) and at least DBAs should be aware of the specific risks of a
particular extension probably?

Michael

In general, this concept is rather dubious. Why should we have such a
dangerous pre-defined role? I would prefer to have complete control
over what gets installed in the database if I were a superuser.
Additionally, if a dangerous extension is inadvertently or otherwise
loaded on the host, I never want a normal user to run code with
superuser privileges.

For a thorough understanding of the current situation and the
rationale behind the design, you can read this[1] discussion.

[1] /messages/by-id/5889.1566415762@sss.pgh.pa.us

--
Best regards,
Kirill Reshke

--
Best regards,
Kirill Reshke

#8Michael Banck
michael.banck@credativ.de
In reply to: Kirill Reshke (#6)
Re: [PATCH] New predefined role pg_manage_extensions

Hi,

first, sorry for the late reply :-/

On Mon, Nov 18, 2024 at 11:26:40AM +0500, Kirill Reshke wrote:

On Fri, 1 Nov 2024 at 02:47, Michael Banck <mbanck@gmx.net> wrote:

Even though there has not been a lot of discussion on this, here is a
rebased patch. I have also added it to the upcoming commitfest.

+       <literal>pg_manage_extensions</literal> allows creating, removing or
+       updating extensions, even if the extensions are untrusted or the user is
+       not the database owner.

Users are not required to be a database owner to create extensions.
They are required to have CREATE priv on database.

Ah right, thanks, I have changed that.

On Sat, Jan 13, 2024 at 09:20:40AM +0100, Michael Banck wrote:

On Fri, Jan 12, 2024 at 04:13:27PM +0100, Jelte Fennema-Nio wrote:

But I'm not sure that such a pg_manage_extensions role would have any
fewer permissions than superuser in practice.

Note that just being able to create an extension does not give blanket
permission to use it. I did a few checks with things I thought might be
problematic like adminpack or plpython3u, and a pg_manage_extensions
user is not allowed to call those functions or use the untrusted
language.

Afaik many extensions that are not marked as trusted, are not trusted
because they would allow fairly trivial privilege escalation to
superuser if they were.

While that might be true (or we err on the side of caution), I thought
the rationale was more that they either disclose more information about
the database server than we want to disclose to ordinary users, or that
they allow access to the file system etc.

Extension installation script can execute arbitrary code with
superuser privilege if markled trusted.

Take this example

[...]

Right, but this implies that a superuser installed that
rogue/sketchy/unsafe-but-declared-safe extension in the first place.
I would assume that the person having pg_manage_extensions privs not
have the ability to install extensions at the system level. Maybe this
should be cautioned some more in the documentation part of this patch,
though.

Unless one of the current untrusted contrib extensions allows to attain
superuser rights by being created?

My opinion was that superusers (or system administrators) would need to
explicitly install this (presumably external) extension somehow, either
via package management or by compile-installing it. So, what is the
threat vector here? I think the system administrator has (in most/all(?)
cases) superuser access to Postgres in practise anyway, via sudo/root
access to the postgres user.

Maybe this should be revisited in light of the extension_destdir GUC
patch, but I think in that case the system admins/postgres superuser
should make sure that this auxiliary directory is not writable to
others.

In general, this concept is rather dubious. Why should we have such a
dangerous pre-defined role?

Well, I would say pg_execute_server_program could be regarded as a
precedent.

I would prefer to have complete control over what gets installed in
the database if I were a superuser.

A superuser will have to grant this attribute to somebody, so there is
certainly some gate-keeping here.

As a side note, maybe what we are missing is a way for site admins to
disable some of the predefined roles (i.e. something better than just
DELETE FROM pg_authid).

Additionally, if a dangerous extension is inadvertently or otherwise
loaded on the host, I never want a normal user to run code with
superuser privileges.

Well again, normal users with pg_execute_server_program rights can
basically already do this. I would consider a user with a
pg_manage_extensions right not a normal user, but an application DBA
or a pseudo-superuser in the managed Postgres cloud parlance.

For a thorough understanding of the current situation and the
rationale behind the design, you can read this[1] discussion.

I have re-read this since, thanks.

I do think having a whitelist of allowed-to-be-installed extensions
(similar/like https://github.com/dimitri/pgextwlist) makes sense
additionally in today's container/cloud word where the local Postgres
admin might not have control over which packages get installed but wants
to have control over which extension the application admins (or whoever)
may create, but that is another topic I think.

Michael

#9Shinya Kato
shinya11.kato@gmail.com
In reply to: Michael Banck (#8)
Re: [PATCH] New predefined role pg_manage_extensions

On Thu, Jan 16, 2025 at 3:31 PM Michael Banck <mbanck@gmx.net> wrote:

I agree with this idea.
I think it is natural to delegate a part of superuser privileges to
another role because superuser privilege is too strong.

In general, this concept is rather dubious. Why should we have such a
dangerous pre-defined role?

Well, I would say pg_execute_server_program could be regarded as a
precedent.

Exactly. pg_execute_server_program can escalate to superuser
privileges, so pg_manage_extensions is not the only dangerous
pre-defined role.

I do think having a whitelist of allowed-to-be-installed extensions
(similar/like https://github.com/dimitri/pgextwlist) makes sense
additionally in today's container/cloud word where the local Postgres
admin might not have control over which packages get installed but wants
to have control over which extension the application admins (or whoever)
may create, but that is another topic I think.

To use a certain extension, you may need to install the
postgresql-contrib package. In that case, is there a way to restrict
extensions other than the required one? Or is it unnecessary to impose
such restrictions?

Regards,
Shinya Kato

#10Michael Banck
michael.banck@credativ.de
In reply to: Shinya Kato (#9)
Re: [PATCH] New predefined role pg_manage_extensions

Hi,

On Thu, Jan 16, 2025 at 04:09:44PM +0900, Shinya Kato wrote:

On Thu, Jan 16, 2025 at 3:31 PM Michael Banck <mbanck@gmx.net> wrote:

I do think having a whitelist of allowed-to-be-installed extensions
(similar/like https://github.com/dimitri/pgextwlist) makes sense
additionally in today's container/cloud word where the local Postgres
admin might not have control over which packages get installed but wants
to have control over which extension the application admins (or whoever)
may create, but that is another topic I think.

To use a certain extension, you may need to install the
postgresql-contrib package. In that case, is there a way to restrict
extensions other than the required one? Or is it unnecessary to impose
such restrictions?

I was thinking about the following (increasinly common, I think)
use-case: we have a largish organisation where the platform/whatever
team wants to deploy Postgres in a uniform way and install the common
set of all contrib and external extensions that might be needed for each
instance. But then you have instance-specific admins that might want to
restrict the set of extensions their instance (or their developers/app
admins/whatever) is allowed to use. However, this is not the purpose of
the patch in discussion, just a side-remark that this functionality
would be good to have in addition.

Michael

#11Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Michael Banck (#4)
Re: [PATCH] New predefined role pg_manage_extensions

On Thu, 2024-10-31 at 22:47 +0100, Michael Banck wrote:

Even though there has not been a lot of discussion on this, here is a
rebased patch.  I have also added it to the upcoming commitfest.

I had a look at the patch.

--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -669,6 +669,17 @@ GRANT pg_signal_backend TO admin_user;
</listitem>
</varlistentry>
+    <varlistentry id="predefined-role-pg-manage-extensions" xreflabel="pg_manage_extensions">
+     <term><varname>pg_manage_extensions</varname></term>
+     <listitem>
+      <para>
+       <literal>pg_manage_extensions</literal> allows creating, removing or
+       updating extensions, even if the extensions are untrusted or the user is
+       not the database owner.
+      </para>
+     </listitem>
+    </varlistentry>
+

The inaccuracy of the database owner has already been mentioned.

Should we say "creating, altering or dropping extensions" to make the connection to
the respective commands obvious?

--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -994,13 +994,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
ListCell   *lc2;
/*
-    * Enforce superuser-ness if appropriate.  We postpone these checks until
-    * here so that the control flags are correctly associated with the right
+    * Enforce superuser-ness/membership of the pg_manage_extensions
+    * predefined role if appropriate.  We postpone these checks until here
+    * so that the control flags are correctly associated with the right
* script(s) if they happen to be set in secondary control files.
*/

This is just an attempt to improve the English:

If appropriate, enforce superuser-ness or membership of the predefined role
pg_manage_extensions.

-                    : errhint("Must be superuser to create this extension.")));
+                    : errhint("Only roles with privileges of the \"%s\" role are allowed to create this extension.", "pg_manage_extensions")));

I don't see the point of breaking out the role name from the message.
We don't do that in other places.
Besides, I think that the mention of the superuser should be retained.
Moreover, I think that commit 8d9978a717 pretty much established that we
should not quote names if they contain underscores.
Perhaps:

errhint("Must be superuser or member of pg_manage_extensions to create this extension.")));

Yours,
Laurenz Albe

#12Michael Banck
michael.banck@credativ.de
In reply to: Laurenz Albe (#11)
Re: [PATCH] New predefined role pg_manage_extensions

Hi,

On Fri, Jan 17, 2025 at 10:03:17AM +0100, Laurenz Albe wrote:

On Thu, 2024-10-31 at 22:47 +0100, Michael Banck wrote:

Even though there has not been a lot of discussion on this, here is a
rebased patch.� I have also added it to the upcoming commitfest.

I had a look at the patch.

Thanks! And sorry for the long delay...

--- a/doc/src/sgml/user-manag.sgml
+++ b/doc/src/sgml/user-manag.sgml
@@ -669,6 +669,17 @@ GRANT pg_signal_backend TO admin_user;
</listitem>
</varlistentry>
+    <varlistentry id="predefined-role-pg-manage-extensions" xreflabel="pg_manage_extensions">
+     <term><varname>pg_manage_extensions</varname></term>
+     <listitem>
+      <para>
+       <literal>pg_manage_extensions</literal> allows creating, removing or
+       updating extensions, even if the extensions are untrusted or the user is
+       not the database owner.
+      </para>
+     </listitem>
+    </varlistentry>
+

The inaccuracy of the database owner has already been mentioned.

Right, I had already changed that in my tree but never sent out an
updated version with this.

Should we say "creating, altering or dropping extensions" to make the connection to
the respective commands obvious?

Alright, did so.

--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -994,13 +994,14 @@ execute_extension_script(Oid extensionOid, ExtensionControlFile *control,
ListCell   *lc2;
/*
-    * Enforce superuser-ness if appropriate.  We postpone these checks until
-    * here so that the control flags are correctly associated with the right
+    * Enforce superuser-ness/membership of the pg_manage_extensions
+    * predefined role if appropriate.  We postpone these checks until here
+    * so that the control flags are correctly associated with the right
* script(s) if they happen to be set in secondary control files.
*/

This is just an attempt to improve the English:

If appropriate, enforce superuser-ness or membership of the predefined role
pg_manage_extensions.

Done.

-                    : errhint("Must be superuser to create this extension.")));
+                    : errhint("Only roles with privileges of the \"%s\" role are allowed to create this extension.", "pg_manage_extensions")));

I don't see the point of breaking out the role name from the message.
We don't do that in other places.

We actually do, I think I modelled it after other predefined roles,
e.g.:

|src/backend/commands/subscriptioncmds.c: errdetail("Only roles with privileges of the \"%s\" role may create subscriptions.",
|src/backend/commands/subscriptioncmds.c- "pg_create_subscription")));
|--
|src/backend/commands/copy.c: errdetail("Only roles with privileges of the \"%s\" role may COPY to or from an external program.",
|src/backend/commands/copy.c- "pg_execute_server_program"),
|--
|src/backend/commands/copy.c: errdetail("Only roles with privileges of the \"%s\" role may COPY from a file.",
|src/backend/commands/copy.c- "pg_read_server_files"),
|--
|src/backend/commands/copy.c: errdetail("Only roles with privileges of the \"%s\" role may COPY to a file.",
|src/backend/commands/copy.c- "pg_write_server_files"),

However, those are all errdetail, while the previous "Must be superuser
to [...]" is errhint, and that error message has different hints based
on whether the extension is trusted or not...

Besides, I think that the mention of the superuser should be retained.
Moreover, I think that commit 8d9978a717 pretty much established that we
should not quote names if they contain underscores.
Perhaps:

errhint("Must be superuser or member of pg_manage_extensions to create this extension.")));

Alright, I think it makes more sense to have it like that than the
above, so changed it to that.

New version 3 attached.

Michael

Attachments:

v3-0001-Add-new-pg_manage_extensions-predefined-role.patchtext/x-diff; charset=us-asciiDownload+22-6
#13Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Michael Banck (#12)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, 2025-02-28 at 19:16 +0100, Michael Banck wrote:

New version 3 attached.

I am fine with v3, and I'll mark it "ready for committer".

I have been wondering about regression tests.
We cannot have them in the core tests, because we cannot rely on any
extension being available.
We could shove a test into a regression test for an existing contrib,
but that would be somewhat off-topic there. Not sure what would be best.

Yours,
Laurenz Albe

In reply to: Laurenz Albe (#13)
Re: [PATCH] New predefined role pg_manage_extensions

Laurenz Albe <laurenz.albe@cybertec.at> writes:

On Fri, 2025-02-28 at 19:16 +0100, Michael Banck wrote:

New version 3 attached.

I am fine with v3, and I'll mark it "ready for committer".

I have been wondering about regression tests.
We cannot have them in the core tests, because we cannot rely on any
extension being available.

That's what the extensions in src/test/modules/ are for. The
test_extensions subdirectory seems suitable, it has tests for all sorts
of behaviour around extensions.

- ilmari

#15Robert Haas
robertmhaas@gmail.com
In reply to: Jelte Fennema-Nio (#2)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, Jan 12, 2024 at 10:13 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:

On Fri, 12 Jan 2024 at 15:53, Michael Banck <mbanck@gmx.net> wrote:

I propose to add a new predefined role to Postgres,
pg_manage_extensions. The idea is that it allows Superusers to delegate
the rights to create, update or delete extensions to other roles, even
if those extensions are not trusted or those users are not the database
owner.

I agree that extension creation is one of the main reasons people
require superuser access, and I think it would be beneficial to try to
reduce that. But I'm not sure that such a pg_manage_extensions role
would have any fewer permissions than superuser in practice. Afaik
many extensions that are not marked as trusted, are not trusted
because they would allow fairly trivial privilege escalation to
superuser if they were.

I see that Jelte walked this comment back, but I think this issue
needs more discussion. I'm not intrinsically against having a role
like pg_execute_server_programs that allows escalation to superuser,
but I don't see how it would help a cloud provider whose goal is to
NOT allow administrators to escalate to superuser.

What am I missing?

--
Robert Haas
EDB: http://www.enterprisedb.com

#16Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Robert Haas (#15)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, 7 Mar 2025 at 14:58, Robert Haas <robertmhaas@gmail.com> wrote:

I see that Jelte walked this comment back, but I think this issue
needs more discussion. I'm not intrinsically against having a role
like pg_execute_server_programs that allows escalation to superuser,
but I don't see how it would help a cloud provider whose goal is to
NOT allow administrators to escalate to superuser.

What am I missing?

The reason why I walked back my comment was that cloud providers can
simply choose which extensions they actually add to the image. If an
extension is marked as not trusted by the author, then with this role
they can still choose to add it without having to make changes to the
control file if they think it's "secure enough".

#17Robert Haas
robertmhaas@gmail.com
In reply to: Jelte Fennema-Nio (#16)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, Mar 7, 2025 at 9:02 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:

The reason why I walked back my comment was that cloud providers can
simply choose which extensions they actually add to the image. If an
extension is marked as not trusted by the author, then with this role
they can still choose to add it without having to make changes to the
control file if they think it's "secure enough".

Hmm. It would be easy to do dumb things here, but I agree there are
probably a bunch of debatable cases. Maybe it would be smart if we
labelled our untrusted extensions somehow with why they're untrusted,
or documented that.

Why wouldn't the cloud provider just change add 'trusted = true' to
the relevant control files instead of doing this?

--
Robert Haas
EDB: http://www.enterprisedb.com

#18Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Robert Haas (#17)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, 2025-03-07 at 09:17 -0500, Robert Haas wrote:

On Fri, Mar 7, 2025 at 9:02 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:

The reason why I walked back my comment was that cloud providers can
simply choose which extensions they actually add to the image. If an
extension is marked as not trusted by the author, then with this role
they can still choose to add it without having to make changes to the
control file if they think it's "secure enough".

Hmm. It would be easy to do dumb things here, but I agree there are
probably a bunch of debatable cases. Maybe it would be smart if we
labelled our untrusted extensions somehow with why they're untrusted,
or documented that.

Why wouldn't the cloud provider just change add 'trusted = true' to
the relevant control files instead of doing this?

That's quite true. Perhaps the patch should be rejected after all.

Yours,
Laurenz Albe

#19Michael Banck
michael.banck@credativ.de
In reply to: Robert Haas (#17)
Re: [PATCH] New predefined role pg_manage_extensions

Hi,

On Fri, Mar 07, 2025 at 09:17:46AM -0500, Robert Haas wrote:

Why wouldn't the cloud provider just change add 'trusted = true' to
the relevant control files instead of doing this?

That would be possible, but maybe the cloud provider is using
distribution packages and does not want to muck around in the file
system (as is usually frowned upon), or, maybe more likely, is using
container images based on (what I've seen most of them are) the Debian
packages and cannot (or does not want to anyway) muck around in the file
system easily.

Also, I think there is case to be made that a cloud provider (or site
admin) would like to delegate the decision whether users with CREATE
rights on a particular database are allowed to install some extensions
or not. Or rather, assign somebody they believe would make the right
call to do that, by granting pg_manage_extensions.

On the other hand, maybe trusted should be part of the catalog and not
(just) the extension control file, so that somebody with appropriate
permissions (like the cloud provider during instance bootstrap) could do
"ALTER EXTENSION foo (SET trusted|TRUSTED);" or whatever. ISTR that I
reviewed the discussion around trusted back then and did not see that
possiblity discussed at all, but I might be misremembering, it's been a
while.

Michael

#20Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Michael Banck (#19)
Re: [PATCH] New predefined role pg_manage_extensions

On Fri, 7 Mar 2025 at 15:37, Michael Banck <mbanck@gmx.net> wrote:

On Fri, Mar 07, 2025 at 09:17:46AM -0500, Robert Haas wrote:

Why wouldn't the cloud provider just change add 'trusted = true' to
the relevant control files instead of doing this?

That would be possible, but maybe the cloud provider is using
distribution packages and does not want to muck around in the file
system (as is usually frowned upon), or, maybe more likely, is using
container images based on (what I've seen most of them are) the Debian
packages and cannot (or does not want to anyway) muck around in the file
system easily.

Yeah exactly, having to do this for every extension that you onboard
is quite a hassle to maintain. It seems much nicer to allow people to
assign a single role and be done with it.

Also many cloud providers have some slightly forked/extended postgres
to allow this already.

Also, I think there is case to be made that a cloud provider (or site
admin) would like to delegate the decision whether users with CREATE
rights on a particular database are allowed to install some extensions
or not. Or rather, assign somebody they believe would make the right
call to do that, by granting pg_manage_extensions.

I think this is a really good point. Adding trusted=true gives any
database owner the ability to install these more dangerous extensions.
While by using pg_manage_extensions you can limit this ability to the
cluster administrator.

On the other hand, maybe trusted should be part of the catalog and not
(just) the extension control file, so that somebody with appropriate
permissions (like the cloud provider during instance bootstrap) could do
"ALTER EXTENSION foo (SET trusted|TRUSTED);" or whatever. ISTR that I
reviewed the discussion around trusted back then and did not see that
possiblity discussed at all, but I might be misremembering, it's been a
while.

I think that would be hard because there's no record in the
pg_extension for extensions that are not installed. So there's also no
way to mark such an extension as trusted. To be able to do this we'd
probably need a system-wide catalog. If we'd go this route then I
think what we'd really want is a way to do:

GRANT INSTALL ON EXTENSION TO user;

And that seems orthogonal to having this pg_manage_extensions role,
because then pg_manage_extensions could grant that permission to
people.

#21Robert Haas
robertmhaas@gmail.com
In reply to: Michael Banck (#19)
#22Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#21)
#23Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jelte Fennema-Nio (#16)
#24Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#22)
#25Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#24)
#26Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Tom Lane (#25)
#27John H
johnhyvr@gmail.com
In reply to: Laurenz Albe (#26)