Thread: Problem with pg_upgrade (8.4 -> 9.0) due to ALTER DATABASE SET ROLE
Hi I've just ran into a problem while upgrading from 8.4 to 9.0. pg_upgrade aborted during the step "Adding support functions to new cluster" with "ERROR: permission denied for languagec" error. Unfortunately, the log didn't include the name of the database where the error occurred, so it took mea while to figure out that the culprit was a "ALTER DATABASE SET ROLE = <non-superuser>" I had done on one of my databases,which effectively prevented pg_upgrade from connection with superuser privileges. While one could argue that this behaviour is perfectly consistent, I believe most users will expect pg_upgrade (and to alesser extent also pg_dump and pg_restore) to be unaffected by such settings. Should we provide a way (for super-users only, of course) to skip per-database/per-role settings when connecting? best regards Florian Pflug
Florian Pflug <fgp@phlo.org> writes:
> pg_upgrade aborted during the step "Adding support functions to new cluster" with "ERROR: permission denied for
languagec" error. Unfortunately, the log didn't include the name of the database where the error occurred, so it took
mea while to figure out that the culprit was a "ALTER DATABASE SET ROLE = <non-superuser>" I had done on one of my
databases,which effectively prevented pg_upgrade from connection with superuser privileges.
That seems like a pretty stupid thing to have done; it would prevent
*any* connection to that database with superuser privileges, no?
> While one could argue that this behaviour is perfectly consistent, I believe most users will expect pg_upgrade (and
toa lesser extent also pg_dump and pg_restore) to be unaffected by such settings.
This is about like arguing that pg_dump and pg_upgrade should still work
after you've done "delete from pg_proc;". Superusers are assumed to
know what they're doing and not break fundamental operations.
I'm thinking that if there's anything we should forbid here, it's the
ALTER ... SET itself. In particular, some experimentation suggests that
a non-superuser database owner can do it:
regression=# create user joe;
CREATE ROLE
regression=# create database joe with owner joe;
CREATE DATABASE
regression=# \c joe joe
You are now connected to database "joe" as user "joe".
joe=> alter database joe set role joe;
ALTER DATABASE
which seems to me at least a bad idea and arguably a security hazard.
regards, tom lane
On Dec12, 2010, at 17:01 , Tom Lane wrote: > Florian Pflug <fgp@phlo.org> writes: >> pg_upgrade aborted during the step "Adding support functions to new cluster" with "ERROR: permission denied for languagec" error. Unfortunately, the log didn't include the name of the database where the error occurred, so it took mea while to figure out that the culprit was a "ALTER DATABASE SET ROLE = <non-superuser>" I had done on one of my databases,which effectively prevented pg_upgrade from connection with superuser privileges. > > That seems like a pretty stupid thing to have done; it would prevent > *any* connection to that database with superuser privileges, no? I had two developers working with that database who regularly modify the schema, often creating new objects (it's a developmentmachine). They both were annoyed that if one of them created a table, he'd be the owner and some operations onthat table would be restricted to him and superusers. The "ALTER DATABASE SET ROLE" fixes that nicely for me. When I neededto work as a superuser with that database, I simply did "SET ROLE <superuser role>" to restore my superuser powers. Nowadays, I could probably do the "SET ROLE" just for some specific combination of user and database. That option, however,wasn't there at the time I did the "ALTER DATABASE SET ROLE". >> While one could argue that this behaviour is perfectly consistent, I believe most users will expect pg_upgrade (and toa lesser extent also pg_dump and pg_restore) to be unaffected by such settings. > > This is about like arguing that pg_dump and pg_upgrade should still work > after you've done "delete from pg_proc;". Superusers are assumed to > know what they're doing and not break fundamental operations. Sure. If you believe in proof by exaggeration, which I don't. The way I see it, how is a DBA supposed to know that setting a per-database ROLE is a bad idea, but per-database settingsfor other GUCs are fine. For example, what about synchronous_commit=off vacuum_freeze_min_age datestyle sql_inheritance standard_conforming_stringsarray_nulls default_with_oids ... Without checking the code, all of these have about the same chance of breaking pg_upgrade. But then, by your line of reasoning,"ALTER DATABASE SET ROLE" shouldn't haven been invented in the first place. Which maybe even true, but it's toolate for that. So the next best thing, IMHO, is to give superusers a way to avoid the hazard it poses. > I'm thinking that if there's anything we should forbid here, it's the > ALTER ... SET itself. In particular, some experimentation suggests that > a non-superuser database owner can do it: > > regression=# create user joe; > CREATE ROLE > regression=# create database joe with owner joe; > CREATE DATABASE > regression=# \c joe joe > You are now connected to database "joe" as user "joe". > joe=> alter database joe set role joe; > ALTER DATABASE > > which seems to me at least a bad idea and arguably a security hazard. I'm sorry, I don't see that security hazard there. Care to explain? best regards, Florian Pflug
On Sun, Dec 12, 2010 at 11:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > This is about like arguing that pg_dump and pg_upgrade should still work > after you've done "delete from pg_proc;". Superusers are assumed to > know what they're doing and not break fundamental operations. No, it isn't like that at all. You've made that argument in the past, and it carries no water with me at all. There's no help for the fact that direct modification of the system catalog contents can fundamentally break things, but DDL commands should not. I'm willing to reserve judgment on whether ALTER DATABASE .. SET ROLE should be disallowed, or whether it should be made to not break things, but blaming the DBA for shooting himself with the loaded foot-gun we thoughtfully provided is unreasonable. And in fact it strikes me that we might not have much choice about how to fix this. I think we are not going to retroactively change the behavior of ALTER DATABASE .. SET ROLE in a released version, but yet we do, I think, want to make pg_upgrade work. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Dec13, 2010, at 00:16 , Robert Haas wrote: > And in fact it strikes me that we might not have much choice about how > to fix this. I think we are not going to retroactively change the > behavior of ALTER DATABASE .. SET ROLE in a released version, but yet > we do, I think, want to make pg_upgrade work. A simple fix is to teach pg_upgrade to issue "RESET SESSION AUTHORIZATION" immediately after connecting to a database. Idon't see any downside of this currently - it seems that the only case where this wouldn't be a NO-OP is if someone setROLE to to something else either per-database, per-user or both. Actually, I'd like to provide an option for pg_dump and pg_restore to do that too (not by default, though). If people thinkthis is a good idea, I could come up with a patch. best regards, Florian Pflug
Florian Pflug <fgp@phlo.org> writes:
> On Dec13, 2010, at 00:16 , Robert Haas wrote:
>> And in fact it strikes me that we might not have much choice about how
>> to fix this. I think we are not going to retroactively change the
>> behavior of ALTER DATABASE .. SET ROLE in a released version, but yet
>> we do, I think, want to make pg_upgrade work.
> A simple fix is to teach pg_upgrade to issue "RESET SESSION
AUTHORIZATION" immediately after connecting to a database.
How is that a fix? RESET is defined to reset to the state you get
immediately after connection. Including anything established by those
ALTER commands.
regards, tom lane
On Dec13, 2010, at 16:40 , Tom Lane wrote: > Florian Pflug <fgp@phlo.org> writes: >> On Dec13, 2010, at 00:16 , Robert Haas wrote: >>> And in fact it strikes me that we might not have much choice about how >>> to fix this. I think we are not going to retroactively change the >>> behavior of ALTER DATABASE .. SET ROLE in a released version, but yet >>> we do, I think, want to make pg_upgrade work. > >> A simple fix is to teach pg_upgrade to issue "RESET SESSION > AUTHORIZATION" immediately after connecting to a database. > > How is that a fix? RESET is defined to reset to the state you get > immediately after connection. Including anything established by those > ALTER commands. I thought so too until yesterday when I tried it (the database "db" has ROLE set to "db"). fgp@master:~$ psql db psql (9.0.1) db=> select session_user, current_user;session_user | current_user --------------+----------------fgp | db (1 row) db=> reset session authorization; RESET db=> select session_user, current_user;session_user | current_user --------------+--------------fgp | fgp (1 row) The manual agrees with this behaviour, it states "The session user identifier is initially set to be the (possibly authenticated) user name provided by the client." and then goes on to explain "The current user identifier is normally equal to the session user identifier, but might change temporarily in the contextof SECURITY DEFINER functions and similar mechanisms; it can also be changed by SET ROLE." best regards, Florian Pflug
Florian Pflug wrote: > Hi > > I've just ran into a problem while upgrading from 8.4 to 9.0. > > pg_upgrade aborted during the step "Adding support functions to new > cluster" with "ERROR: permission denied for language c" error. > Unfortunately, the log didn't include the name of the database where > the error occurred, so it took me a while to figure out that the culprit > was a "ALTER DATABASE SET ROLE = <non-superuser>" I had done on one of > my databases, which effectively prevented pg_upgrade from connection > with superuser privileges. > > While one could argue that this behaviour is perfectly consistent, I > believe most users will expect pg_upgrade (and to a lesser extent also > pg_dump and pg_restore) to be unaffected by such settings. > > Should we provide a way (for super-users only, of course) to skip > per-database/per-role settings when connecting? I think pg_dumpall would have failed with this setup too, so I don't see this as a pg_upgrade bug, nor something that I am willing to risk adding to pg_upgrade. Perhaps the error report can be improved. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Wed, Jan 5, 2011 at 9:44 PM, Bruce Momjian <bruce@momjian.us> wrote: > I think pg_dumpall would have failed with this setup too, so I don't see > this as a pg_upgrade bug, nor something that I am willing to risk adding > to pg_upgrade. If adding RESET SESSION AUTHORIZATION fixes the bug, I think we should consider doing that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Wed, Jan 5, 2011 at 9:44 PM, Bruce Momjian <bruce@momjian.us> wrote: > > I think pg_dumpall would have failed with this setup too, so I don't see > > this as a pg_upgrade bug, nor something that I am willing to risk adding > > to pg_upgrade. > > If adding RESET SESSION AUTHORIZATION fixes the bug, I think we should > consider doing that. If we add every fix that could conceivably break a pg_dumpall restore, pg_upgrade will be less stable than it is now. I don't see why adding this should be any different. If you want to argue that pg_dumpall should be doing it, that is a separate issue and not related to pg_upgrade. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Jan 5, 2011 at 9:44 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> I think pg_dumpall would have failed with this setup too, so I don't see
>> this as a pg_upgrade bug, nor something that I am willing to risk adding
>> to pg_upgrade.
> If adding RESET SESSION AUTHORIZATION fixes the bug, I think we should
> consider doing that.
I think an appropriate response would be to prevent ALTER DATABASE SET
ROLE. I really cannot believe that there are any situations where
that's a good idea.
Or we could take the approach somebody was just espousing about
> Our job is to prevent the user from *accidentally*
> shooting themselves in the foot.
If they want to deliberately shoot themselves in the foot by hosing the
login system like that, it's not our job to prevent it. But it's not
our job to try to work around it, either.
regards, tom lane
Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: > > On Wed, Jan 5, 2011 at 9:44 PM, Bruce Momjian <bruce@momjian.us> wrote: > >> I think pg_dumpall would have failed with this setup too, so I don't see > >> this as a pg_upgrade bug, nor something that I am willing to risk adding > >> to pg_upgrade. > > > If adding RESET SESSION AUTHORIZATION fixes the bug, I think we should > > consider doing that. > > I think an appropriate response would be to prevent ALTER DATABASE SET > ROLE. I really cannot believe that there are any situations where > that's a good idea. > > Or we could take the approach somebody was just espousing about > > > Our job is to prevent the user from *accidentally* > > shooting themselves in the foot. > > If they want to deliberately shoot themselves in the foot by hosing the > login system like that, it's not our job to prevent it. But it's not > our job to try to work around it, either. Yep. We should probably make a decision on foot-guns and be consistent, at least. Doing it half-way isn't helping anyone. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On 01/05/2011 11:08 PM, Tom Lane wrote: > If they want to deliberately shoot themselves in the foot by hosing the > login system like that, it's not our job to prevent it. But it's not > our job to try to work around it, either. > > I think this is especially true in this case, when the problem is easily and quickly worked around by the end user. cheers andrew
On Wed, Jan 5, 2011 at 11:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Or we could take the approach somebody was just espousing about > >> Our job is to prevent the user from *accidentally* >> shooting themselves in the foot. I don't see how you can compare those two cases with a straight face. In the FOREIGN KEY NOT ENFORCED case, there is an explicit piece of syntax by means of which the user is asking for the dangerous behavior. In this case, the user made a settings change which was allowed by the system and solved his problem, and then pg_upgrade broke. If he had typed ALTER DATABASE .. SET ROLE .. BREAK PG_UPGRADE, the two cases would be comparable. Or if we failed to enforce foreign keys by default, that'd be comparable, too. How exactly is the user supposed to know that ALTER DATABASE .. SET ROLE is a bad idea? You've repeatedly made remarks about "deliberately hosing the login system", but you've offered no evidence that the user "deliberately hosed" anything. Changed the behavior? Well, yeah. And fixed his problem, too! I even sympathize with his use case. Hosed? Well, maybe. It worked for him, until he tried to run pg_upgrade. Deliberately hosed, like he did it just to break things? Doesn't seem that way. Your argument rests on the presumption that the user should have known better than to execute a command which didn't produce an error and did solve his problem. Perhaps that's a reasonable argument in some cases - a user might be reasonably expected to foresee that setting work_mem to 100GB could cause problems even if it happens to fix the immediate issue, based on the description of the parameter - but neither you nor anyone else on this thread have offered more than hand-waving to explain how the user was supposed to know that it was unwise, or even to substantiate your position that it WAS unwise. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Jan6, 2011, at 04:13 , Bruce Momjian wrote: > Robert Haas wrote: >> On Wed, Jan 5, 2011 at 9:44 PM, Bruce Momjian <bruce@momjian.us> wrote: >>> I think pg_dumpall would have failed with this setup too, so I don't see >>> this as a pg_upgrade bug, nor something that I am willing to risk adding >>> to pg_upgrade. >> >> If adding RESET SESSION AUTHORIZATION fixes the bug, I think we should >> consider doing that. > > If we add every fix that could conceivably break a pg_dumpall restore, > pg_upgrade will be less stable than it is now. I don't see why adding > this should be any different. The issue is more complicted. In my situation, it's not the pg_dumpall restore that's failing, but rather pg_upgrade's attempt to install the support functions necessary for the upgrade. But in principle, you're right I think. pg_dumpall *would* fail if my database contained any objects that required superuser privileges to create, like C-language functions. > If you want to argue that pg_dumpall should be doing it, that is a > separate issue and not related to pg_upgrade. I think both need the fix. best regards, Florian Pflug
On Jan6, 2011, at 05:08 , Tom Lane wrote: > I think an appropriate response would be to prevent ALTER DATABASE SET > ROLE. I really cannot believe that there are any situations where > that's a good idea. I explained up-thread why, in my situation, doing this *is* a perfectly good idea. You have neither offered an alternative solution nor argued why *exactly* this is supposed to be such a bad idea, other than the obvious "it breaks pg_upgrade". Which isn't a very convincing argument that this isn't simply a pg_upgrade bug... To reiterate: I did ALTER DATABASE SET ROLE to allow different developers to work on the same database without the permission system getting into their way. Without that, objects created by one developer couldn't be modified by another, which obviously didn't work very well... > Or we could take the approach somebody was just espousing about > >> Our job is to prevent the user from *accidentally* >> shooting themselves in the foot. > > If they want to deliberately shoot themselves in the foot by hosing the > login system like that, it's not our job to prevent it. But it's not > our job to try to work around it, either. Nothing was hosed here. I simply solved a very real problem with the tools made available by postgres. Telling me after *years* of this solution working perfectly, and after I discovered that a *new* tool doesn't handle the situation well, that I deliberately hosed things is downright unfriendly from where I stand. best regards, Florian Pflug
Florian Pflug wrote: > On Jan6, 2011, at 04:13 , Bruce Momjian wrote: > > Robert Haas wrote: > >> On Wed, Jan 5, 2011 at 9:44 PM, Bruce Momjian <bruce@momjian.us> wrote: > >>> I think pg_dumpall would have failed with this setup too, so I don't see > >>> this as a pg_upgrade bug, nor something that I am willing to risk adding > >>> to pg_upgrade. > >> > >> If adding RESET SESSION AUTHORIZATION fixes the bug, I think we should > >> consider doing that. > > > > If we add every fix that could conceivably break a pg_dumpall restore, > > pg_upgrade will be less stable than it is now. I don't see why adding > > this should be any different. > > The issue is more complicted. In my situation, it's not the pg_dumpall > restore that's failing, but rather pg_upgrade's attempt to install > the support functions necessary for the upgrade. > > But in principle, you're right I think. pg_dumpall *would* fail if my > database contained any objects that required superuser privileges to > create, like C-language functions. Right, it was only the pg_upgrade support functions that failed first. > > If you want to argue that pg_dumpall should be doing it, that is a > > separate issue and not related to pg_upgrade. > > I think both need the fix. Actually, pg_dump would need to be doing it, so would need a line in every pg_dump file with a RESET SESSION AUTHORIZATION, but because the fact that the command actually reset the username suprised many of us, you would also need an SQL command stating why it is there. And at that point, it seems like complete overkill. Also, remember, pg_upgrade does as liittle as possible (like me :-) ) and relies as much as possible on the existing Postgres facilities to improve its reliability and reduce the churn needed for each new major release. As far as telling you what database you failed in, pg_upgrade can't because it blindly runs the pg_dumpall file through psql and just exits on _any_ error, hence the failure you saw, but we don't know what database you were in when the failure happened. We would need to modify psql to report the database in the error message. Looking at your use case of ALTER DATABASE SET, shouldn't you be using the new default schema object permission feature? -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
Robert Haas wrote: > On Wed, Jan 5, 2011 at 11:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Or we could take the approach somebody was just espousing about > > > >> Our job is to prevent the user from *accidentally* > >> shooting themselves in the foot. > > I don't see how you can compare those two cases with a straight face. > In the FOREIGN KEY NOT ENFORCED case, there is an explicit piece of > syntax by means of which the user is asking for the dangerous > behavior. In this case, the user made a settings change which was > allowed by the system and solved his problem, and then pg_upgrade > broke. If he had typed ALTER DATABASE .. SET ROLE .. BREAK > PG_UPGRADE, the two cases would be comparable. Or if we failed to > enforce foreign keys by default, that'd be comparable, too. > > How exactly is the user supposed to know that ALTER DATABASE .. SET > ROLE is a bad idea? You've repeatedly made remarks about > "deliberately hosing the login system", but you've offered no evidence > that the user "deliberately hosed" anything. Changed the behavior? > Well, yeah. And fixed his problem, too! I even sympathize with his > use case. Hosed? Well, maybe. It worked for him, until he tried to > run pg_upgrade. Deliberately hosed, like he did it just to break > things? Doesn't seem that way. Your argument rests on the > presumption that the user should have known better than to execute a > command which didn't produce an error and did solve his problem. > Perhaps that's a reasonable argument in some cases - a user might be > reasonably expected to foresee that setting work_mem to 100GB could > cause problems even if it happens to fix the immediate issue, based on > the description of the parameter - but neither you nor anyone else on > this thread have offered more than hand-waving to explain how the user > was supposed to know that it was unwise, or even to substantiate your > position that it WAS unwise. Well, if everyone who logs in gets the same username, you can easily conclude that trying to dump/restore the database will cause problems if you have objects in there that are not owned by that user. I now realize the pg_upgrade problem was that it requires super-user objects. You could argue that requiring the ability for a super-user to do things in every database is either reasonable or overly-restrictive. I am not sure which it is, but I do know pg_upgrade requires it. Does anything else require super-user in every database. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Thu, Jan 6, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: > Well, if everyone who logs in gets the same username, you can easily > conclude that trying to dump/restore the database will cause problems if > you have objects in there that are not owned by that user. I can't, and neither could Florian. I'm not sure why this is so obvious to you and Tom. Unless I've made some catastrophic *manual* change to the system catalogs, like nuking pg_proc, I expect dump and restore to just work. pg_dump's job is to emit a series of commands that will work. Every time I run across a case where it doesn't, I'm violently annoyed, because it's happened to me as a user and it feels like a bug every time. Florian is probably made of a bit sterner stuff than the typical user, but a typical user doesn't go "Oh, gee, dump and restore didn't work, I guess that setting I installed in there six years ago must actually be something that the developers never intended for me to do." First they cuss, and then they blame us for not being able to dump the database that we let them create, and then if they're really ticked they go use some other product. When someone actually takes the time to troubleshoot what broke and let us know, the only correct response from our end is to say "thanks, we'll work on making that less confusing", not "well that was a stupid thing to do". > I now realize the pg_upgrade problem was that it requires super-user > objects. You could argue that requiring the ability for a super-user to > do things in every database is either reasonable or overly-restrictive. > I am not sure which it is, but I do know pg_upgrade requires it. Does > anything else require super-user in every database. Monitoring and/or management applications of any sort, I would assume. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas wrote: > On Thu, Jan 6, 2011 at 1:59 PM, Bruce Momjian <bruce@momjian.us> wrote: > > Well, if everyone who logs in gets the same username, you can easily > > conclude that trying to dump/restore the database will cause problems if > > you have objects in there that are not owned by that user. > > I can't, and neither could Florian. I'm not sure why this is so > obvious to you and Tom. Unless I've made some catastrophic *manual* > change to the system catalogs, like nuking pg_proc, I expect dump and > restore to just work. pg_dump's job is to emit a series of commands > that will work. Every time I run across a case where it doesn't, I'm > violently annoyed, because it's happened to me as a user and it feels > like a bug every time. Florian is probably made of a bit sterner > stuff than the typical user, but a typical user doesn't go "Oh, gee, > dump and restore didn't work, I guess that setting I installed in > there six years ago must actually be something that the developers > never intended for me to do." First they cuss, and then they blame us > for not being able to dump the database that we let them create, and > then if they're really ticked they go use some other product. When > someone actually takes the time to troubleshoot what broke and let us > know, the only correct response from our end is to say "thanks, we'll > work on making that less confusing", not "well that was a stupid thing > to do". Well, we usually tell people to restore as super-user, particularly pg_dumpall, but in this case, it is impossible. Certainly pg_upgrade requires it, which is the root of the problem. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. +
On Thu, Jan 6, 2011 at 3:54 PM, Bruce Momjian <bruce@momjian.us> wrote: > Well, we usually tell people to restore as super-user, particularly > pg_dumpall, but in this case, it is impossible. Certainly pg_upgrade > requires it, which is the root of the problem. True. Although it's not really impossible, it just requires one additional step that we don't currently perform. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company