Re: Marking some contrib modules as trusted extensions
On Wed, Feb 26, 2020 at 12:17:37AM -0800, Andres Freund wrote: > Hi, > > On 2020-02-26 09:11:21 +0100, Sandro Santilli wrote: > > PostGIS uses unpackaged-to-XXX pretty heavily, and has it under > > automated testing (which broke since "FROM unpackaged" support was > > removed, see 14514.1581638...@sss.pgh.pa.us) > > > > We'd be ok with requiring SUPERUSER for doing that, since that's > > what is currently required so nothing would change for us. > > > > Instead, dropping UPGRADE..FROM completely puts us in trouble of > > having to find another way to "package" postgis objects. > > Coul you explain what postgis is trying to achieve with FROM unpackaged? We're turning a non-extension based install into an extension-based one. Common need for those who came to PostGIS way before EXTENSION was even invented and for those who remained there for the bigger flexibility (for example to avoid the raster component, which was unavoidable with EXTENSION mechanism until PostGIS 3.0). For the upgrades to 3.0.0 when coming from a previous version we're using that `FROM unpackaged` SYNTAX for re-packaging the raster component for those who still want it (raster objects are unpackaged from 'postgis' extension on EXTENSION UPDATE because there was no other way to move them from an extension to another). I guess it would be ok for us to do the packaging directly from the scripts that would run on `CREATE EXTENSION postgis`, but would that mean we'd take the security risk you're trying to avoid by dropping the `FROM unpackaged` syntax ? --strk;
Re: Marking some contrib modules as trusted extensions
Hi, On 2020-02-26 09:11:21 +0100, Sandro Santilli wrote: > PostGIS uses unpackaged-to-XXX pretty heavily, and has it under > automated testing (which broke since "FROM unpackaged" support was > removed, see 14514.1581638...@sss.pgh.pa.us) > > We'd be ok with requiring SUPERUSER for doing that, since that's > what is currently required so nothing would change for us. > > Instead, dropping UPGRADE..FROM completely puts us in trouble of > having to find another way to "package" postgis objects. Coul you explain what postgis is trying to achieve with FROM unpackaged? Greetings, Andres Freund
Re: Marking some contrib modules as trusted extensions
On Thu, Feb 13, 2020 at 07:09:18PM -0500, Tom Lane wrote: > I wrote: > > Andres Freund writes: > >> It seems to me that FROM UNPACKAGED shouldn't support trusted. > > > Hmm, seems like a reasonable idea, but I'm not quite sure how to mechanize > > it given that "unpackaged" isn't magic in any way so far as extension.c > > is concerned. Maybe we could decide that the time for supporting easy > > updates from pre-9.1 is past, and just remove all the unpackaged-to-XXX > > scripts? Maybe even remove the "FROM version" option altogether. > > [ thinks some more... ] A less invasive idea would be to insist that > you be superuser to use the FROM option. But I'm thinking that the > unpackaged-to-XXX scripts are pretty much dead letters anymore. Has > anyone even tested them in years? How much longer do we want to be > on the hook to fix them? PostGIS uses unpackaged-to-XXX pretty heavily, and has it under automated testing (which broke since "FROM unpackaged" support was removed, see 14514.1581638...@sss.pgh.pa.us) We'd be ok with requiring SUPERUSER for doing that, since that's what is currently required so nothing would change for us. Instead, dropping UPGRADE..FROM completely puts us in trouble of having to find another way to "package" postgis objects. --strk;
Re: Marking some contrib modules as trusted extensions
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Andres Freund writes: > > On 2020-01-29 14:41:16 -0500, Tom Lane wrote: > >> pgcrypto > > > FWIW, given the code quality, I'm doubtful about putting itq into the > > trusted > > section. > > I don't particularly have an opinion about that --- is it really that > awful? If there is anything broken in it, wouldn't we consider that > a security problem anyhow? I would certainly hope so- and I would expect that to go for any of the other extensions which are included in core. If we aren't going to maintain them and deal with security issues in them, then we should drop them. Which goes back to my earlier complaint that having extensions in core which aren't or can't be marked as trusted is not a position we should put our users in. Either they're maintained and have been vetted through our commit process, or they aren't and should be removed. > > Especially with FROM UNPACKAGED it seems like it'd be fairly easy to get > > an extension script to do dangerous things (as superuser). One could > > just create pre-existing objects that have *not* been created by a > > previous version, and some upgrade scripts would do pretty weird > > stuff. There's several that do things like updating catalogs directly > > etc. It seems to me that FROM UNPACKAGED shouldn't support trusted. > > Hmm, seems like a reasonable idea, but I'm not quite sure how to mechanize > it given that "unpackaged" isn't magic in any way so far as extension.c > is concerned. Maybe we could decide that the time for supporting easy > updates from pre-9.1 is past, and just remove all the unpackaged-to-XXX > scripts? Maybe even remove the "FROM version" option altogether. I agree in general with dropping the unpackaged-to-XXX bits. Thanks, Stephen signature.asc Description: PGP signature
Re: Marking some contrib modules as trusted extensions
Andres Freund writes:
> On 2020-02-13 18:57:10 -0500, Tom Lane wrote:
>> Maybe we could decide that the time for supporting easy updates from
>> pre-9.1 is past, and just remove all the unpackaged-to-XXX scripts?
>> Maybe even remove the "FROM version" option altogether.
> Yea, that strikes me as a reasonable thing to do. These days that just
> seems to be dangerous, without much advantage.
Here's a patch to remove the core-code support and documentation for
that. I have not included the actual deletion of the contrib modules'
'unpackaged' scripts, as that seems both long and boring.
regards, tom lane
diff --git a/doc/src/sgml/contrib.sgml b/doc/src/sgml/contrib.sgml
index 08bb110..261a559 100644
--- a/doc/src/sgml/contrib.sgml
+++ b/doc/src/sgml/contrib.sgml
@@ -88,22 +88,6 @@ CREATE EXTENSION module_name;
- If your database was brought forward by dump and reload from a pre-9.1
- version of PostgreSQL, and you had been using the pre-9.1
- version of the module in it, you should instead do
-
-
-CREATE EXTENSION module_name FROM unpackaged;
-
-
- This will update the pre-9.1 objects of the module into a proper
- extension object. Future updates to the module will be
- managed by .
- For more information about extension updates, see
- .
-
-
-
Note, however, that some of these modules are not extensions
in this sense, but are loaded into the server in some other way, for instance
by way of
diff --git a/doc/src/sgml/extend.sgml b/doc/src/sgml/extend.sgml
index ffe068b..9ec1af7 100644
--- a/doc/src/sgml/extend.sgml
+++ b/doc/src/sgml/extend.sgml
@@ -917,33 +917,6 @@ SELECT pg_catalog.pg_extension_config_dump('my_config', 'WHERE NOT standard_entr
- The update mechanism can be used to solve an important special case:
- converting a loose collection of objects into an extension.
- Before the extension mechanism was added to
- PostgreSQL (in 9.1), many people wrote
- extension modules that simply created assorted unpackaged objects.
- Given an existing database containing such objects, how can we convert
- the objects into a properly packaged extension? Dropping them and then
- doing a plain CREATE EXTENSION is one way, but it's not
- desirable if the objects have dependencies (for example, if there are
- table columns of a data type created by the extension). The way to fix
- this situation is to create an empty extension, then use ALTER
- EXTENSION ADD to attach each pre-existing object to the extension,
- then finally create any new objects that are in the current extension
- version but were not in the unpackaged release. CREATE
- EXTENSION supports this case with its FROM old_version option, which causes it to not run the
- normal installation script for the target version, but instead the update
- script named
- extension--old_version--target_version.sql.
- The choice of the dummy version name to use as old_version is up to the extension author, though
- unpackaged is a common convention. If you have multiple
- prior versions you need to be able to update into extension style, use
- multiple dummy version names to identify them.
-
-
-
ALTER EXTENSION is able to execute sequences of update
script files to achieve a requested update. For example, if only
foo--1.0--1.1.sql and foo--1.1--2.0.sql are
diff --git a/doc/src/sgml/ref/create_extension.sgml b/doc/src/sgml/ref/create_extension.sgml
index d76ac3e..6a21bff 100644
--- a/doc/src/sgml/ref/create_extension.sgml
+++ b/doc/src/sgml/ref/create_extension.sgml
@@ -24,7 +24,6 @@ PostgreSQL documentation
CREATE EXTENSION [ IF NOT EXISTS ] extension_name
[ WITH ] [ SCHEMA schema_name ]
[ VERSION version ]
- [ FROM old_version ]
[ CASCADE ]
@@ -48,8 +47,9 @@ CREATE EXTENSION [ IF NOT EXISTS ] extension_name
The user who runs CREATE EXTENSION becomes the
- owner of the extension for purposes of later privilege checks, as well
- as the owner of any objects created by the extension's script.
+ owner of the extension for purposes of later privilege checks, and
+ normally also becomes the owner of any objects created by the
+ extension's script.
@@ -142,33 +142,6 @@ CREATE EXTENSION [ IF NOT EXISTS ] extension_name
- old_version
-
-
-FROM old_version
-must be specified when, and only when, you are attempting to install
-an extension that replaces an old style module that is just
-a collection of objects not packaged into an extension. This option
-causes CREATE EXTENSION to run an alternative installation
-script that absorbs the existing objects into the extension, instead
-of creating new objects. Be careful that SCHEMA specifies
-the schema containing these pre-existing objects.
-
-
-
-
Re: Marking some contrib modules as trusted extensions
Hi, On 2020-02-13 18:57:10 -0500, Tom Lane wrote: > Maybe we could decide that the time for supporting easy updates from > pre-9.1 is past, and just remove all the unpackaged-to-XXX scripts? > Maybe even remove the "FROM version" option altogether. Yea, that strikes me as a reasonable thing to do. These days that just seems to be dangerous, without much advantage. Greetings, Andres Freund
Re: Marking some contrib modules as trusted extensions
I wrote: > Andres Freund writes: >> It seems to me that FROM UNPACKAGED shouldn't support trusted. > Hmm, seems like a reasonable idea, but I'm not quite sure how to mechanize > it given that "unpackaged" isn't magic in any way so far as extension.c > is concerned. Maybe we could decide that the time for supporting easy > updates from pre-9.1 is past, and just remove all the unpackaged-to-XXX > scripts? Maybe even remove the "FROM version" option altogether. [ thinks some more... ] A less invasive idea would be to insist that you be superuser to use the FROM option. But I'm thinking that the unpackaged-to-XXX scripts are pretty much dead letters anymore. Has anyone even tested them in years? How much longer do we want to be on the hook to fix them? regards, tom lane
Re: Marking some contrib modules as trusted extensions
Andres Freund writes: > On 2020-01-29 14:41:16 -0500, Tom Lane wrote: >> pgcrypto > FWIW, given the code quality, I'm doubtful about putting itq into the trusted > section. I don't particularly have an opinion about that --- is it really that awful? If there is anything broken in it, wouldn't we consider that a security problem anyhow? > Especially with FROM UNPACKAGED it seems like it'd be fairly easy to get > an extension script to do dangerous things (as superuser). One could > just create pre-existing objects that have *not* been created by a > previous version, and some upgrade scripts would do pretty weird > stuff. There's several that do things like updating catalogs directly > etc. It seems to me that FROM UNPACKAGED shouldn't support trusted. Hmm, seems like a reasonable idea, but I'm not quite sure how to mechanize it given that "unpackaged" isn't magic in any way so far as extension.c is concerned. Maybe we could decide that the time for supporting easy updates from pre-9.1 is past, and just remove all the unpackaged-to-XXX scripts? Maybe even remove the "FROM version" option altogether. regards, tom lane
Re: Marking some contrib modules as trusted extensions
Hi, On 2020-01-29 14:41:16 -0500, Tom Lane wrote: > pgcrypto FWIW, given the code quality, I'm doubtful about putting itq into the trusted section. Have you audited how safe the create/upgrade scripts are against being used to elevate privileges? Especially with FROM UNPACKAGED it seems like it'd be fairly easy to get an extension script to do dangerous things (as superuser). One could just create pre-existing objects that have *not* been created by a previous version, and some upgrade scripts would do pretty weird stuff. There's several that do things like updating catalogs directly etc. It seems to me that FROM UNPACKAGED shouldn't support trusted. Regards, Andres Freund
Re: Marking some contrib modules as trusted extensions
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Our previous >> discussions about what privilege level is needed to look at >> pg_stat_statements info were all made against a background assumption >> that you needed some extra privilege to set up the view in the first >> place. I think that would need another look or two before being >> comfortable that we're not shifting the goal posts too far. > While you could maybe argue that's true for pg_stat_statements, it's > certainly not true for pg_stat_activity, so I don't buy it for either. The analogy of pg_stat_activity certainly suggests that there shouldn't be a reason *in principle* why pg_stat_statements couldn't be made trusted. There's a difference between that statement and saying that *in practice* pg_stat_statements is ready to be trusted right now with no further changes. I haven't done the analysis needed to conclude that, and don't care to do so as part of this patch proposal. > The same goes for just about everything else (I sure hope, at least) in > our extensions set- none of the core extensions should be allowing > access to things which break our security model, even if they're > installed, unless some additional privileges are granted out. Maybe not, but the principle of defense-in-depth still says that admins could reasonably want to not let dangerous tools get installed in the first place. > As such, I really don't agree with this entire line of thinking when it > comes to our core extensions. I view the 'trusted extension' model as > really for things where the extension author doesn't care about, and > doesn't wish to care about, dealing with our security model and making > sure that it's followed. We do care, and we do maintain, the security > model that we have throughout the core extensions. I am confused as to what "entire line of thinking" you are objecting to. Are you now saying that we should forget the trusted-extension model? Or maybe that we can just mark *everything* we ship as trusted? I'm not going to agree with either. >> The bigger picture here is that I don't want to get push-back that >> we've broken somebody's security posture by marking too many extensions >> trusted. So for anything where there's any question about security >> implications, we should err in the conservative direction of leaving >> it untrusted. > This is just going to a) cause our users to complain about not being > able to install extensions that they've routinely installed in the past, That's utter nonsense. Nothing here is taking away privileges that existed before; if you could install $whatever as superuser before, you still can. OTOH, we *would* have a problem of that sort if we marked $whatever as trusted and then later had to undo it. So I think there's plenty of reason to be conservative about the first wave of what-to-mark-as-trusted. Once we've got more experience with this mechanism under our belts, we might decide we can be more liberal about it. > and b) make our users wonder what it is about these extensions that > we've decided can't be trusted to even just be installed and if they're > at risk today because they've installed them. Yep, you're right, this patch does make value judgements of that sort, and I'll stand behind them. Giving people the impression that, say, postgres_fdw isn't any more dangerous than cube isn't helpful. > While it might not seem obvious, the discussion over on the thread about > DEFAULT PRIVILEGES and pg_init_privs is actually a lot more relevant > here- there's extensions we have that expect certain functions, once > installed, to be owned by a superuser (which will still be the case > here, thanks to how you've addressed that), but then to not have EXECUTE > rights GRANT'd to anyone (thanks to the REVERT FROM PUBLIC in the > installation), but that falls apart when someone's decided to set > up DEFAULT PRIVILEGES for the superuser. While no one seems to want to > discuss that with me, unfortunately, it's becoming more and more clear > that we need to skip DEFAULT PRIVILEGES from being applied during > extension creation. Or that we can't let people apply default privileges to superuser-created objects at all. But I agree that that's a different discussion. regards, tom lane
Re: Marking some contrib modules as trusted extensions
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Julien Rouhaud writes: > >>> Probably NO, if only because you'd need additional privileges > >>> to use these anyway: > >>> pg_stat_statements > > > But the additional privileges are global, so assuming the extension > > has been properly setup, wouldn't it be sensible to ease the > > per-database installation? If not properly setup, there's no harm in > > creating the extension anyway. > > Mmm, I'm not convinced --- the ability to see what statements are being > executed in other sessions (even other databases) is something that > paranoid installations might not be so happy about. Of course, but that's why we have a default role which allows installations to control access to that kind of information- and it's already being checked in the pg_stat_statements case and in the pg_stat_activity case: /* Superusers or members of pg_read_all_stats members are allowed */ is_allowed_role = is_member_of_role(GetUserId(), DEFAULT_ROLE_READ_ALL_STATS); > Our previous > discussions about what privilege level is needed to look at > pg_stat_statements info were all made against a background assumption > that you needed some extra privilege to set up the view in the first > place. I think that would need another look or two before being > comfortable that we're not shifting the goal posts too far. While you could maybe argue that's true for pg_stat_statements, it's certainly not true for pg_stat_activity, so I don't buy it for either. This looks like revisionist history to justify paranoia. I understand the general concern, but if we were really depending on the mere installation of the extension to provide security then we wouldn't have bothered putting in checks like the one above, and, worse, I think our users would be routinely complaining that our extensions don't follow our security model and how they can't install them. Lots of people want to use pg_stat_statements, even in environments where not everyone on the database server, or even in the database you want pg_stat_statements in, is trusted, and therefore we have to have these additional checks inside the extension itself. The same goes for just about everything else (I sure hope, at least) in our extensions set- none of the core extensions should be allowing access to things which break our security model, even if they're installed, unless some additional privileges are granted out. The act of installing a core extension should not create a security risk for our users- if it did, it'd be a security issue and CVE-worthy. As such, I really don't agree with this entire line of thinking when it comes to our core extensions. I view the 'trusted extension' model as really for things where the extension author doesn't care about, and doesn't wish to care about, dealing with our security model and making sure that it's followed. We do care, and we do maintain, the security model that we have throughout the core extensions. What I expect and hope will happen is that people will realize that, now that they can have non-superusers installing these extensions and therefore they don't have to give out superuser-level rights as much, there will be asks for more default roles to allow granting out of access to formerly superuser-only capabilities. There's a bit of a complication there since there might be privileges that only make sense for a specific extension, but an extension can't really install a new default role (and, even if it did, the role would have to be only available to the superuser initially anyway), so we might have to try and come up with some more generic and reusable default role for that case. Still, we can try to deal with that when it happens. Consider that you may wish to have a system that, once installed, a superuser will virtually never access again, but one where you want users to be able to install and use extensions like postgis and pg_stat_statements. That can be done with these changes, and that's fantastic progress- you just install PG, create a non-superuser role, make them the DB owner, and then GRANT things like pg_read_all_stats to their role with admin rights, and boom, they're good to go and you didn't have to hack up the PG source code at all. > The bigger picture here is that I don't want to get push-back that > we've broken somebody's security posture by marking too many extensions > trusted. So for anything where there's any question about security > implications, we should err in the conservative direction of leaving > it untrusted. This is just going to a) cause our users to complain about not being able to install extensions that they've routinely installed in the past, and b) make our users wonder what it is about these extensions that we've decided can't be trusted to even just be installed and if they're at risk today because they've installed them. While it might not seem obvious, the discussion over on the thread about DEFAULT PRIVILEGES and pg_
Re: Marking some contrib modules as trusted extensions
After looking more closely at these modules, I'm kind of inclined *not* to put the trusted marker on intagg. That module is just a backwards-compatibility wrapper around functionality that exists in the core code nowadays. So I think what we ought to be doing with it is deprecating and eventually removing it, not encouraging people to keep using it. Given that and the other discussion in this thread, I think the initial list of modules to trust is: btree_gin btree_gist citext cube dict_int earthdistance fuzzystrmatch hstore hstore_plperl intarray isn jsonb_plperl lo ltree pg_trgm pgcrypto seg tablefunc tcn tsm_system_rows tsm_system_time unaccent uuid-ossp So attached is a patch to do that. The code changes are trivial; just add "trusted = true" to each control file. We don't need to bump the module version numbers, since this doesn't change the contents of any extension, just who can install it. I do not think any regression test changes are needed either. (Note that commit 50fc694e4 already added a test that trusted extensions behave as expected, see src/pl/plperl/sql/plperl_setup.sql.) So it seems like the only thing that needs much discussion is the documentation changes. I adjusted contrib.sgml's discussion of how to install these modules in general, and then labeled the individual modules if they are trusted. regards, tom lane diff --git a/contrib/btree_gin/btree_gin.control b/contrib/btree_gin/btree_gin.control index d576da7..67d0c99 100644 --- a/contrib/btree_gin/btree_gin.control +++ b/contrib/btree_gin/btree_gin.control @@ -3,3 +3,4 @@ comment = 'support for indexing common datatypes in GIN' default_version = '1.3' module_pathname = '$libdir/btree_gin' relocatable = true +trusted = true diff --git a/contrib/btree_gist/btree_gist.control b/contrib/btree_gist/btree_gist.control index 81c8509..cd2d7eb 100644 --- a/contrib/btree_gist/btree_gist.control +++ b/contrib/btree_gist/btree_gist.control @@ -3,3 +3,4 @@ comment = 'support for indexing common datatypes in GiST' default_version = '1.5' module_pathname = '$libdir/btree_gist' relocatable = true +trusted = true diff --git a/contrib/citext/citext.control b/contrib/citext/citext.control index a872a3f..ccf4454 100644 --- a/contrib/citext/citext.control +++ b/contrib/citext/citext.control @@ -3,3 +3,4 @@ comment = 'data type for case-insensitive character strings' default_version = '1.6' module_pathname = '$libdir/citext' relocatable = true +trusted = true diff --git a/contrib/cube/cube.control b/contrib/cube/cube.control index f39a838..3e238fc 100644 --- a/contrib/cube/cube.control +++ b/contrib/cube/cube.control @@ -3,3 +3,4 @@ comment = 'data type for multidimensional cubes' default_version = '1.4' module_pathname = '$libdir/cube' relocatable = true +trusted = true diff --git a/contrib/dict_int/dict_int.control b/contrib/dict_int/dict_int.control index 6e2d2b3..ec04cce 100644 --- a/contrib/dict_int/dict_int.control +++ b/contrib/dict_int/dict_int.control @@ -3,3 +3,4 @@ comment = 'text search dictionary template for integers' default_version = '1.0' module_pathname = '$libdir/dict_int' relocatable = true +trusted = true diff --git a/contrib/earthdistance/earthdistance.control b/contrib/earthdistance/earthdistance.control index 5816d22..3df666d 100644 --- a/contrib/earthdistance/earthdistance.control +++ b/contrib/earthdistance/earthdistance.control @@ -3,4 +3,5 @@ comment = 'calculate great-circle distances on the surface of the Earth' default_version = '1.1' module_pathname = '$libdir/earthdistance' relocatable = true +trusted = true requires = 'cube' diff --git a/contrib/fuzzystrmatch/fuzzystrmatch.control b/contrib/fuzzystrmatch/fuzzystrmatch.control index 6b2832a..3cd6660 100644 --- a/contrib/fuzzystrmatch/fuzzystrmatch.control +++ b/contrib/fuzzystrmatch/fuzzystrmatch.control @@ -3,3 +3,4 @@ comment = 'determine similarities and distance between strings' default_version = '1.1' module_pathname = '$libdir/fuzzystrmatch' relocatable = true +trusted = true diff --git a/contrib/hstore/hstore.control b/contrib/hstore/hstore.control index 93688cd..e0fbb8b 100644 --- a/contrib/hstore/hstore.control +++ b/contrib/hstore/hstore.control @@ -3,3 +3,4 @@ comment = 'data type for storing sets of (key, value) pairs' default_version = '1.6' module_pathname = '$libdir/hstore' relocatable = true +trusted = true diff --git a/contrib/hstore_plperl/hstore_plperl.control b/contrib/hstore_plperl/hstore_plperl.control index 16277f6..4b9fd13 100644 --- a/contrib/hstore_plperl/hstore_plperl.control +++ b/contrib/hstore_plperl/hstore_plperl.control @@ -3,4 +3,5 @@ comment = 'transform between hstore and plperl' default_version = '1.0' module_pathname = '$libdir/hstore_plperl' relocatable = true +trusted = true requires = 'hstore,plperl' diff --git a/contrib/intarray/intarray.control b/contrib/intarray/intarray.control index 7e50cc3..bf28804 100644 --- a/contrib/intarray/intarray.control +++ b/contrib/int
Re: Marking some contrib modules as trusted extensions
Dean Rasheed writes: > On Wed, 29 Jan 2020 at 21:39, Tom Lane wrote: >> The bigger picture here is that I don't want to get push-back that >> we've broken somebody's security posture by marking too many extensions >> trusted. So for anything where there's any question about security >> implications, we should err in the conservative direction of leaving >> it untrusted. > I wonder if the same could be said about pgrowlocks. Good point. I had figured it was probably OK given that it's analogous to the pg_locks view (which is unrestricted AFAIR), and that it already has some restrictions on what you can see. I'd have no hesitation about dropping it off this list though, since it's probably not used that much and it could also be seen as exposing internals. regards, tom lane
Re: Marking some contrib modules as trusted extensions
On Wed, 29 Jan 2020 at 21:39, Tom Lane wrote: > > >>> pg_stat_statements > > Mmm, I'm not convinced --- the ability to see what statements are being > executed in other sessions (even other databases) is something that > paranoid installations might not be so happy about. Our previous > discussions about what privilege level is needed to look at > pg_stat_statements info were all made against a background assumption > that you needed some extra privilege to set up the view in the first > place. I think that would need another look or two before being > comfortable that we're not shifting the goal posts too far. > > The bigger picture here is that I don't want to get push-back that > we've broken somebody's security posture by marking too many extensions > trusted. So for anything where there's any question about security > implications, we should err in the conservative direction of leaving > it untrusted. > +1 I wonder if the same could be said about pgrowlocks. Regards, Dean
Re: Marking some contrib modules as trusted extensions
Julien Rouhaud writes: >>> Probably NO, if only because you'd need additional privileges >>> to use these anyway: >>> pg_stat_statements > But the additional privileges are global, so assuming the extension > has been properly setup, wouldn't it be sensible to ease the > per-database installation? If not properly setup, there's no harm in > creating the extension anyway. Mmm, I'm not convinced --- the ability to see what statements are being executed in other sessions (even other databases) is something that paranoid installations might not be so happy about. Our previous discussions about what privilege level is needed to look at pg_stat_statements info were all made against a background assumption that you needed some extra privilege to set up the view in the first place. I think that would need another look or two before being comfortable that we're not shifting the goal posts too far. The bigger picture here is that I don't want to get push-back that we've broken somebody's security posture by marking too many extensions trusted. So for anything where there's any question about security implications, we should err in the conservative direction of leaving it untrusted. regards, tom lane
Re: Marking some contrib modules as trusted extensions
On Wed, Jan 29, 2020 at 9:46 PM Darafei "Komяpa" Praliaskouski wrote: > > Hello, > >> >> btree_gin >> btree_gist > > > I would even ask btree_gin and btree_gist to be moved to core. Without going that far, I also agree that I relied on those extension quite often, so +1 for marking them as trusted. >> Probably NO, if only because you'd need additional privileges >> to use these anyway: >> pg_stat_statements But the additional privileges are global, so assuming the extension has been properly setup, wouldn't it be sensible to ease the per-database installation? If not properly setup, there's no harm in creating the extension anyway.
Re: Marking some contrib modules as trusted extensions
=?UTF-8?Q?Darafei_=22Kom=D1=8Fpa=22_Praliaskouski?= writes: >> btree_gin >> btree_gist > I would even ask btree_gin and btree_gist to be moved to core. That's not in scope here. Our past experience with trying to move extensions into core is that it creates a pretty painful upgrade experience for users, so that's not something I'm interested in doing ... especially for relatively marginal cases like these. There's also a more generic question of why we should want to move anything to core anymore. The trusted-extension mechanism removes one of the biggest remaining gripes about extensions, namely the pain level for installing them. (But please, let's not have that debate on this thread.) regards, tom lane
Re: Marking some contrib modules as trusted extensions
Hello, > btree_gin > btree_gist I would even ask btree_gin and btree_gist to be moved to core. btree_gist is shipping opclasses for built in types to be used in gist indexes. btree_* is confusing part in the name pretending there's some magic happening linking btree and gist. gist is the most popular way to get geometric indexes, and these often need to be combined with some class identifier that's used in lookups together. CREATE INDEX on geom_table using gist (zooom_level, geom); fails for no reason without btree_gist - types are shipped in core, gist itself is not an extension, but letting to use one core mechanism with another in an obvious way is for some reason split out. -- Darafei Praliaskouski Support me: http://patreon.com/komzpa
Re: Marking some contrib modules as trusted extensions
On 2020-Jan-29, Tom Lane wrote: > Not sure what I think about these: > > bloom (are these useful in production?) > btree_gin > btree_gist > pgrowlocks(seems safe, but are there security issues?) > spi/autoinc (I doubt that these four are production grade) > spi/insert_username > spi/moddatetime > spi/refint > sslinfo (seems safe, but are there security issues?) > xml2 (nominally safe, but deprecated, and libxml2 >has been a fertile source of security issues) Of these, btree_gist is definitely useful from a user perspective, because it enables creation of certain exclusion constraints. I've never heard of anyone using bloom indexes in production. I'd argue that if the feature is useful, then we should turn it into a core-included index AM with regular WAL logging for improved performance, and add a stripped-down version to src/test/modules to cover the WAL-log testing needs. Maybe exposing it more, as promoting it as a trusted extension would do, would help find more use cases for it. > Also, how should we document this, if we do it? Add a boilerplate > sentence to each module's description about whether it is trusted > or not? Put a table up at the front of Appendxix F? Both? If it were possible to do both from a single source of truth, that would be great. Failing that, I'd just list it in each module's section. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
