Thread: Bug #6593, extensions, and proposed new patch policy
Hackers, Per bug #6593, REASSIGN OWNED fails when the affected role owns an extension. This would be trivial to fix if extensions had support code for ALTER EXTENSION / OWNER, but they don't. So the only back-patchable fix right now seems to be to throw an error on REASSIGN OWNED when the user owns an extension. (If anyone wants to claim that we ought to work on a real fix that allows changing the owner internally from REASSIGN OWNED, without introducing ALTER EXTENSION support for doing so, let me know and I'll see about it.) In HEAD we can do the more invasive fix of actually adding support code for changing an extension's owner. And it seems to me that, going further, we should have a policy that any ownable object type we add must come with appropriate support for changing owner. Thoughts? -- Álvaro Herrera <alvherre@alvh.no-ip.org>
On Wed, Apr 18, 2012 at 11:41 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote: > Per bug #6593, REASSIGN OWNED fails when the affected role owns an > extension. This would be trivial to fix if extensions had support code > for ALTER EXTENSION / OWNER, but they don't. So the only back-patchable > fix right now seems to be to throw an error on REASSIGN OWNED when the > user owns an extension. (If anyone wants to claim that we ought to work > on a real fix that allows changing the owner internally from REASSIGN > OWNED, without introducing ALTER EXTENSION support for doing so, let me > know and I'll see about it.) I would be OK with the latter. > In HEAD we can do the more invasive fix of actually adding support code > for changing an extension's owner. And it seems to me that, going > further, we should have a policy that any ownable object type we add > must come with appropriate support for changing owner. +1. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Excerpts from Robert Haas's message of mié abr 18 13:05:03 -0300 2012: > On Wed, Apr 18, 2012 at 11:41 AM, Alvaro Herrera > <alvherre@alvh.no-ip.org> wrote: > > Per bug #6593, REASSIGN OWNED fails when the affected role owns an > > extension. This would be trivial to fix if extensions had support code > > for ALTER EXTENSION / OWNER, but they don't. So the only back-patchable > > fix right now seems to be to throw an error on REASSIGN OWNED when the > > user owns an extension. (If anyone wants to claim that we ought to work > > on a real fix that allows changing the owner internally from REASSIGN > > OWNED, without introducing ALTER EXTENSION support for doing so, let me > > know and I'll see about it.) > > I would be OK with the latter. Here's a patch for that. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachment
On Wed, Apr 18, 2012 at 5:27 PM, Alvaro Herrera <alvherre@commandprompt.com> wrote: > Excerpts from Robert Haas's message of mié abr 18 13:05:03 -0300 2012: >> On Wed, Apr 18, 2012 at 11:41 AM, Alvaro Herrera >> <alvherre@alvh.no-ip.org> wrote: >> > Per bug #6593, REASSIGN OWNED fails when the affected role owns an >> > extension. This would be trivial to fix if extensions had support code >> > for ALTER EXTENSION / OWNER, but they don't. So the only back-patchable >> > fix right now seems to be to throw an error on REASSIGN OWNED when the >> > user owns an extension. (If anyone wants to claim that we ought to work >> > on a real fix that allows changing the owner internally from REASSIGN >> > OWNED, without introducing ALTER EXTENSION support for doing so, let me >> > know and I'll see about it.) >> >> I would be OK with the latter. > > Here's a patch for that. Looks sane on a quick once-over. I do wonder about the comment, though. If we add ALTER EXTENSION .. OWNER, should that try to change the ownership of the objects contained inside the extension? Your comment implies that the answer should be yes, but I'm not totally convinced... what if the user has altered the ownership of the objects manually, for example? I guess that's a question for another day, just wondering out loud. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Apr 18, 2012 at 5:27 PM, Alvaro Herrera
> <alvherre@commandprompt.com> wrote:
>> Here's a patch for that.
> Looks sane on a quick once-over. I do wonder about the comment,
> though. If we add ALTER EXTENSION .. OWNER, should that try to change
> the ownership of the objects contained inside the extension?
I would certainly think that not doing so would violate the principle
of least astonishment.
> Your
> comment implies that the answer should be yes, but I'm not totally
> convinced... what if the user has altered the ownership of the
> objects manually, for example?
So? ALTER OWNER doesn't care about the previous ownership of objects,
it just reassigns them as told. So even if that had been done, I'd
expect the post-ALTER state to be that everything has the new owner.
However, ignoring that issue for the moment, this patch is making me
uncomfortable. I have a vague recollection that we deliberately omitted
ALTER EXTENSION OWNER because of security or definitional worries.
(Dimitri, does that ring any bells?) I wonder whether we should insist
that the new owner be a superuser, as the original owner must have been.
regards, tom lane
On Wed, Apr 18, 2012 at 7:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > However, ignoring that issue for the moment, this patch is making me > uncomfortable. I have a vague recollection that we deliberately omitted > ALTER EXTENSION OWNER because of security or definitional worries. > (Dimitri, does that ring any bells?) I wonder whether we should insist > that the new owner be a superuser, as the original owner must have been. Don't we have non-superuser extensions, that can be installed with just DBA privileges? Anyhow, it seems a bit nannyish, unless I'm missing something. If the current owner is a superuser and s/he wants to give the object to a non-superuser, you can't really stop them. They can just make the target user a superuser, give 'em the object, and make them not a superuser, all in one transaction no less. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi, Robert Haas <robertmhaas@gmail.com> writes: > On Wed, Apr 18, 2012 at 7:54 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> However, ignoring that issue for the moment, this patch is making me >> uncomfortable. I have a vague recollection that we deliberately omitted >> ALTER EXTENSION OWNER because of security or definitional worries. >> (Dimitri, does that ring any bells?) I wonder whether we should insist >> that the new owner be a superuser, as the original owner must have been. I remember not having included any OWNER in my submitted patch, because I though we didn't really need one as far as the pg_extension entry is concerned. You added one to better integrate with our standard practice and for pg_dump purposes IIRC, but that's about it. I don't remember that we talked about specific extension's objects owner: they get owned by the user installing them. > Don't we have non-superuser extensions, that can be installed with > just DBA privileges? We do, so the extension owner could well be the database owner and not a superuser. That can only happens if the extension contains no C coded parts, obviously, and that's why it's so easy to forget about --- plans have been discussed to work on per-database module installation, not yet about non-superuser installing a C module. I think the open possibility of crashing the whole cluster is driving us not to relax the trusted bit for the C language ever. > Anyhow, it seems a bit nannyish, unless I'm missing something. If the > current owner is a superuser and s/he wants to give the object to a > non-superuser, you can't really stop them. They can just make the > target user a superuser, give 'em the object, and make them not a > superuser, all in one transaction no less. The interesting use case would be reassigning the extension from a database owner to a superuser so that the database owner can't remove it nor upgrade it no more. I think we want to allow that. The reassign from a superuser to a non-superuser can be argued both ways, unless the extension is marked as superuser = true in the control file. Which means we should better register that into the catalogs because the control file can be updated on the file system and that's out of reach for the cluster. What about only issuing a WARNING that the extensions are not supported by reassign owned in 9.1 (and probably 9.2)? In next versions, we will be able to register the control file superuser parameter and allow reassigning in a more controlled fashion, both the catalog entry in pg_extension and the extension's objects. Maybe rather than adding the control file into the catalogs piece after piece we should install a copy of it at create/alter time in a database specific location from the per major version shared location in the OS. That way the user could be updating the OS shipped file then decide which cluster and which database to upgrade with that, and you could have PostGIS 1.5 in database foo and 2.0 in database bar. The OS would still need to provide for a way to upgrade them separately, but that would be much better than what we have now. When doing things this way, we could trust the extension's control file in cases such as ALTER EXTENSION OWNER, because it's now a private copy of it. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Excerpts from Dimitri Fontaine's message of jue abr 19 12:42:00 -0300 2012: > What about only issuing a WARNING that the extensions are not supported > by reassign owned in 9.1 (and probably 9.2)? Raise a warning and then do what? While you can continue reassigning the rest of the objects to someone else, this doesn't help the poor fella who's just trying to drop the owner of the extension -- it still can't be dropped. Moreover, since there's no ALTER OWNER command for extensions, the user can't just change it to someone else manually. The only option is to do DROP OWNED, which will drop the extension along with all the objects that belong to it. In fact, the documentation states that the way to drop a user that owns objects is to run REASSIGN OWNED, then DROP OWNED, (repeat for all databases), then DROP ROLE. So if the DBA does that, he might end up dropping the extension by accident. Maybe we could just add a protection that the user to which the extension is reassigned must be a superuser or the database owner. Remember that we're talking about REASSIGN OWNED here, which will automatically reassign not only the extension itself, but also the contained objects. There is no danger that we will end up with an inconsistent installation. Also, if the objects in the extension have been manually given to someone else, they will stay owned by that other user, precisely because the code as written does not recurse. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes: > Remember that we're talking about REASSIGN OWNED here, which will > automatically reassign not only the extension itself, but also the > contained objects. There is no danger that we will end up with an > inconsistent installation. Also, if the objects in the extension have > been manually given to someone else, they will stay owned by that other > user, precisely because the code as written does not recurse. Oh, right, I forgot the scope of the command. Given those bits of missed context, +1 from me here. Sorry about missing that in my previous email. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Excerpts from Alvaro Herrera's message of mié abr 18 18:27:27 -0300 2012: > Excerpts from Robert Haas's message of mié abr 18 13:05:03 -0300 2012: > > On Wed, Apr 18, 2012 at 11:41 AM, Alvaro Herrera > > <alvherre@alvh.no-ip.org> wrote: > > > Per bug #6593, REASSIGN OWNED fails when the affected role owns an > > > extension. This would be trivial to fix if extensions had support code > > > for ALTER EXTENSION / OWNER, but they don't. So the only back-patchable > > > fix right now seems to be to throw an error on REASSIGN OWNED when the > > > user owns an extension. (If anyone wants to claim that we ought to work > > > on a real fix that allows changing the owner internally from REASSIGN > > > OWNED, without introducing ALTER EXTENSION support for doing so, let me > > > know and I'll see about it.) > > > > I would be OK with the latter. > > Here's a patch for that. Applied to master, 9.2 and 9.1. Sorry for dropping the ball on it. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes: > Applied to master, 9.2 and 9.1. Thank you! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support