Thread: ALTER USER SET log_* not allowed...
I've got a particular database account that connects/disconnects hundreds of times a second, literally (email delivery agent). Being the ever ready logger that I am, I have a number of logging bits turned on to help me identify problems when they come up. In this case, I'm not worried about a problem and want to turn off most logging for this particular user. I can issue an ALTER USER [usr] SET log_* = false/none in all cases, except for log_duration. db=# ALTER USER dbmail SET log_disconnections = false; ERROR: parameter "log_disconnections" cannot be set after connection start :-/ I use disconnections as the way of noting the number of connections. Regardless, I think I'm off to the races... BUT! not quite: INFO: permission denied to set parameter "log_statement" HINT: Must be superuser to increase this value. INFO: permission denied to set parameter "log_duration" HINT: Must be superuser to change this value to false. doh! So much for that idea. There's no real way to have some useconfig variables run by the super user and some run by the session user. My workaround is to have the program call a SECURITY DEFINER function, but I'd be nice to be able to remove that hack. Anyway, something to consider next time the system catalogs are being tweaked. -sc -- Sean Chittenden
Sean Chittenden <sean@chittenden.org> writes: > doh! So much for that idea. There's no real way to have some > useconfig variables run by the super user and some run by the session > user. My workaround is to have the program call a SECURITY DEFINER > function, but I'd be nice to be able to remove that hack. Yeah, the whole concept of USERLIMIT GUC variables is fairly dubious, because of the fact that we have to be able to set these values at times when we don't necessarily know whether the user is a superuser or not. It occurs to me though that we might be able to handle this one case. Variable values coming from ALTER USER/DATABASE SET ... are currently treated as untrusted (because of the ordering of enum GucSource). However, we *also* test whether the user executing the ALTER has permissions: regression=> ALTER USER tgl SET log_duration = false; ERROR: permission denied to set parameter "log_duration" HINT: Must be superuser to change this value to false. At first glance it seems like the ALTER-time test is a sufficient security check and therefore PGC_S_DATABASE and PGC_S_USER sources could be treated as privileged. This doesn't entirely work, because the USERLIMIT checks are context sensitive: if the current config-file setting of log_duration is FALSE then I will be allowed to install the above per-user setting, and without the time-of-use check it would override any change in the global setting that the DBA tried to make. It strikes me that a more useful definition would be to say that you must be superuser, period, to install an ALTER USER/DATABASE value for any USERLIMIT variable; and then we could treat these values as privileged for USERLIMIT purposes. This would lose the ability for non-superusers to set allowable values for themselves this way, but I think the case we'd gain is more useful. Comments? regards, tom lane
>> doh! So much for that idea. There's no real way to have some >> useconfig variables run by the super user and some run by the session >> user. My workaround is to have the program call a SECURITY DEFINER >> function, but I'd be nice to be able to remove that hack. > > Yeah, the whole concept of USERLIMIT GUC variables is fairly dubious, > because of the fact that we have to be able to set these values at > times > when we don't necessarily know whether the user is a superuser or not. [snip] > It strikes me that a more useful definition would be to say that you > must be superuser, period, to install an ALTER USER/DATABASE value for > any USERLIMIT variable; and then we could treat these values as > privileged for USERLIMIT purposes. This would lose the ability for > non-superusers to set allowable values for themselves this way, but > I think the case we'd gain is more useful. > > Comments? Oh! Please! I thought about suggesting that but didn't think it'd pass your litmus test and figured a pg_shadow.useconfig and an pg_shadow.admconfig would be received better, but, absolutely! The reason I hesitated to suggest such a change was SET search_path = foo;, which a user should be able to set on their own. Sure it'd be easy to have it a one-off exception, but then it'd be a one-off exception and having an 'ALTER USER usr ADMIN SET guc = val' would skirt that issue completely. That's the only concern that I have. How about this, leave the existing system in place, but since useconfig is just a TEXT[], if an admin sets the value (ALTER USER usr ADMIN SET), prefix the guc name with an 'A:'. As things currently stand, useconfig looks like, '{search_path=dbmail,log_statements=none}', but after, it'd look like: '{search_path=dbmail,A:log_statements=none}'. Then log_statements gets set with admin privs, where as search_path is set with user privs. Pros: *) Preserves backwards compatibility with existing databases and the GUC security infrastructure (no need to bump catalog version) *) Allows GUC variables to be set with differing permission levels and still be set by the user *) At ALTER USER time, a permission message can be returned that tells the user a GUC has already been set by the admin, go bug them to change this value Cons: *) Places a special value on the prefix of GUC variable names. *) Requires adding a new keyword in the ALTER USER syntax. *) Feels a tad like a "miscellaneous" column that is on the verge of being abused. Then again, isn't it on the horizon to have GUC reworked? Maybe this would be an acceptable addition. *shrug* Just an idea. -sc -- Sean Chittenden
Sean Chittenden <sean@chittenden.org> writes: >> It strikes me that a more useful definition would be to say that you >> must be superuser, period, to install an ALTER USER/DATABASE value for >> any USERLIMIT variable; and then we could treat these values as >> privileged for USERLIMIT purposes. This would lose the ability for >> non-superusers to set allowable values for themselves this way, but >> I think the case we'd gain is more useful. > Oh! Please! I thought about suggesting that but didn't think it'd > pass your litmus test and figured a pg_shadow.useconfig and an > pg_shadow.admconfig would be received better, but, absolutely! The > reason I hesitated to suggest such a change was SET search_path = foo;, > which a user should be able to set on their own. But search_path is not USERLIMIT. The only variables that are in that category are the ones that control logging. Bruce and I were chatting about this on the phone today, and we were seriously considering a more radical proposal: get rid of the whole concept of USERLIMIT variables, and make the logging variables be plain SUSET (ie, only superusers can change 'em). This would eliminate the current ability of a non-superuser to increase the logging verbosity of his session, but it's not real clear that that's such a good idea anyway. (Cranking the log verbosity up far past what the DBA wants could be seen as a primitive form of DOS attack; and anyway, if you are not a superuser then you can't see what's in the log, so why should you care what the verbosity is, much less be able to affect it?) Given the code complexity of the USERLIMIT stuff and the number of bugs already found in it, getting rid of it seems awfully attractive. Without USERLIMIT, we could easily change the rules to make ALTER USER work the way you want: we'd say you have to be superuser to issue ALTER USER or ALTER DATABASE with a SUSET variable (already true), and then the value can be adopted at session start in all cases since we can then treat pg_shadow and pg_database as secure sources. regards, tom lane
> Without USERLIMIT, we could easily change the rules to make ALTER USER > work the way you want: we'd say you have to be superuser to issue ALTER > USER or ALTER DATABASE with a SUSET variable (already true), and then > the value can be adopted at session start in all cases since we can > then > treat pg_shadow and pg_database as secure sources. Nevermind, you win. That's far more elegant/easier... is 8.0 looking like a good time to make this break from tradition? -sc -- Sean Chittenden
On Tue, 2004-11-09 at 13:58 -0500, Tom Lane wrote: >=20 > Bruce and I were chatting about this on the phone today, and we were > seriously considering a more radical proposal: get rid of the whole > concept of USERLIMIT variables, and make the logging variables be plain > SUSET (ie, only superusers can change 'em). This would eliminate the > current ability of a non-superuser to increase the logging verbosity > of his session, but it's not real clear that that's such a good idea > anyway. (Cranking the log verbosity up far past what the DBA wants > could be seen as a primitive form of DOS attack; and anyway, if you are > not a superuser then you can't see what's in the log, so why should > you care what the verbosity is, much less be able to affect it?) Given > the code complexity of the USERLIMIT stuff and the number of bugs > already found in it, getting rid of it seems awfully attractive. The current functionality could be useful inside particular code paths of an application, where you want to increase the log verbosity in a particular part of the code, when it (unpredictably) happens, without nuking the logs entirely. Of course you are superuser when you review such logs, but I wouldn't usually want the db connection from the application to have to run as superuser if I could help it... especially not a web application. Regards, Andrew McMillan. ------------------------------------------------------------------------- Andrew @ Catalyst .Net .NZ Ltd, PO Box 11-053, Manners St, Wellington WEB: http://catalyst.net.nz/ PHYS: Level 2, 150-154 Willis St DDI: +64(4)803-2201 MOB: +64(272)DEBIAN OFFICE: +64(4)499-2267 Survey for nothing with http://survey.net.nz/ -------------------------------------------------------------------------
Andrew McMillan <andrew@catalyst.net.nz> writes: > The current functionality could be useful inside particular code paths > of an application, where you want to increase the log verbosity in a > particular part of the code, when it (unpredictably) happens, without > nuking the logs entirely. > Of course you are superuser when you review such logs, but I wouldn't > usually want the db connection from the application to have to run as > superuser if I could help it... especially not a web application. Sure. There is a workaround for that though, which is to provide a SECURITY DEFINER function for the app to call that will adjust the logging level for it, rather than trying to do the SET directly in unprivileged code. regards, tom lane
Tom Lane wrote: > Andrew McMillan <andrew@catalyst.net.nz> writes: > > The current functionality could be useful inside particular code paths > > of an application, where you want to increase the log verbosity in a > > particular part of the code, when it (unpredictably) happens, without > > nuking the logs entirely. > > Of course you are superuser when you review such logs, but I wouldn't > > usually want the db connection from the application to have to run as > > superuser if I could help it... especially not a web application. > > Sure. There is a workaround for that though, which is to provide a > SECURITY DEFINER function for the app to call that will adjust the > logging level for it, rather than trying to do the SET directly in > unprivileged code. But if they go that way can it done securely, turned on and off? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes: > Tom Lane wrote: >> Sure. There is a workaround for that though, which is to provide a >> SECURITY DEFINER function for the app to call that will adjust the >> logging level for it, rather than trying to do the SET directly in >> unprivileged code. > But if they go that way can it done securely, turned on and off? Why not? You can put whatever restrictions you like in such a function. It'd certainly be more "secure" than the existing USERLIMIT behavior, because the DBA can decide exactly what policy he wants and code it into the function he gives his users (maybe even multiple functions for different users). USERLIMIT effectively dictates to the DBA what will be allowed. regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> Sure. There is a workaround for that though, which is to provide a > >> SECURITY DEFINER function for the app to call that will adjust the > >> logging level for it, rather than trying to do the SET directly in > >> unprivileged code. > > > But if they go that way can it done securely, turned on and off? > > Why not? You can put whatever restrictions you like in such a function. > > It'd certainly be more "secure" than the existing USERLIMIT behavior, > because the DBA can decide exactly what policy he wants and code it > into the function he gives his users (maybe even multiple functions for > different users). USERLIMIT effectively dictates to the DBA what will > be allowed. But right now the userlimit codes allows a non-super user to turn logging on only if it was off, and he can then turn it off again. I assume to do this in a function you would have to create a temp table to store the original setting? Doesn't sound trivial, nor does it sound like something someone is going to write just for a single debug session. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes: > Tom Lane wrote: >> Why not? You can put whatever restrictions you like in such a function. > I assume to do this in a function you would have to create a temp table > to store the original setting? Not at all. You could use "RESET foo" to see what the "original" value of foo was. regards, tom lane
Andrew McMillan wrote: -- Start of PGP signed section. > On Tue, 2004-11-09 at 13:58 -0500, Tom Lane wrote: > > > > Bruce and I were chatting about this on the phone today, and we were > > seriously considering a more radical proposal: get rid of the whole > > concept of USERLIMIT variables, and make the logging variables be plain > > SUSET (ie, only superusers can change 'em). This would eliminate the > > current ability of a non-superuser to increase the logging verbosity > > of his session, but it's not real clear that that's such a good idea > > anyway. (Cranking the log verbosity up far past what the DBA wants > > could be seen as a primitive form of DOS attack; and anyway, if you are > > not a superuser then you can't see what's in the log, so why should > > you care what the verbosity is, much less be able to affect it?) Given > > the code complexity of the USERLIMIT stuff and the number of bugs > > already found in it, getting rid of it seems awfully attractive. > > The current functionality could be useful inside particular code paths > of an application, where you want to increase the log verbosity in a > particular part of the code, when it (unpredictably) happens, without > nuking the logs entirely. > > Of course you are superuser when you review such logs, but I wouldn't > usually want the db connection from the application to have to run as > superuser if I could help it... especially not a web application. As much as I would like the URERLIMIT hacks removed for 8.0 I am thinking we are too far along in release and don't have enough time to figure out how to do the security definer function cleanly. I am thinking we should wait for 8.1 and maybe have the USERLIMIT capability integrated intot a security definer capability function we ship with our code. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes: > As much as I would like the URERLIMIT hacks removed for 8.0 I am thinking > we are too far along in release and don't have enough time to figure out > how to do the security definer function cleanly. What does that have to do with it? We aren't the ones who would be writing such a function. regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > As much as I would like the URERLIMIT hacks removed for 8.0 I am thinking > > we are too far along in release and don't have enough time to figure out > > how to do the security definer function cleanly. > > What does that have to do with it? We aren't the ones who would be > writing such a function. I am thinking it is too much to remove such functionality and require others to write it. They will not write it until they are debugging something, and then they will not have time to do it. I think we need to provide such functionality in a cleaner way than just saying "write a function". I think it needs more thought and we don't have the time. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073