Re: [PATCHES] Proposed patch to remove USERLIMIT
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: I would be glad to see USERLIMIT gone, even though I wrote it. However, I feel we are removing user's ability of non-super users to turn logging on and off easily without really having thought through a mechanism to give them that. I think that is a very weak argument. Remember that the discussion Well, I disagree or I wouldn't have made the argument, would I? started because setting these variables via ALTER USER fails to work. It fails to work if logging was already on and someone wants to turn it off via ALTER USER, and that matches the expected behavior. Why is that of less concern than the fact that unprivileged users won't be able to increase the log level by themselves? I think it's pretty Because it is working as planned, meaning normal users can't turn off logging. debatable whether the current behavior is a feature at all, rather than a security hole. I just don't see people creating secuirty definer functions easily to do this create or replace function enable_logging(bool) returns void as $$ begin if $1 then set log_statement = 'all'; else set log_statement = 'ddl'; end if; return; end$$ language plpgsql security definer; revoke execute on function enable_logging(bool) from public; grant execute on function enable_logging(bool) to joe_user; (My, that was hard ...) Well, you have just made the system less secure by creating that function. Why didn't you create a function that has the existing behavior of only allowing users to increase the log level? Because it is harder to do, and certainly not something trivial you would write during a debugging session. You seem to be supposing that anyone who needs this will be needing a bug-for-bug equivalent to the existing USERLIMIT behavior. I don't What bug? think so. I think that in the field, people are going to have at most a couple of logging configurations they want to allow users to select, and they will use code not significantly more complex than the above. Can you show me the function? Arguably, this approach is better than directly calling the SET commands anyway, because it insulates the application code from the changing details of the specific GUC variables. Need I remind you that any app that directly issues SET log_statement is going to be broken by 8.0? In 7.4 this variable took on and off, now it does not. The argument of preserving backwards-compatible behavior looks a bit funny beside that reality. I realize that but I think we need to give people the existing functionality out of the box, or at least not make such a change so close to final without time to discuss. You should ask on general where we usually do such polls of feature changes? People are already complaining we don't make logging easy enough (pg_stat_activity) and now we are going to make it harder still? Doesn't make sense to me, and I don't see your change as a bug fix. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Proposed patch to remove USERLIMIT
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: started because setting these variables via ALTER USER fails to work. It fails to work if logging was already on and someone wants to turn it off via ALTER USER, and that matches the expected behavior. Not if a superuser does it; superusers should be allowed to set it however they want. In current code a superuser can't even set it that way for himself, let alone for other nonprivileged users. You seem to define expected behavior as whatever the code does now, but in point of fact it's got lots of deficiencies. I just turned logging on in my postgresql.conf file, did as super-user: test= ALTER USER postgres SET log_statement = 'none'; and it seemed to work fine. Are you saying the problem is that the super-user can't set log_statement for non-super users via ALTER USER? Yes, that doesn't work, but I have seen only one user request to do that, while I have seen a lot of requests of people wanting to turn log_statement on and off as non-super users in their applications. Well, you have just made the system less secure by creating that function. Why didn't you create a function that has the existing behavior of only allowing users to increase the log level? I repeat my point: I don't think DBAs actually need that. In the function as illustrated, I gave joe_user (and nobody else) the choice between 'all' and 'ddl' (and nothing else), where I suppose 'ddl' is the global default. This is in fact a whole lot *more* secure than the existing behavior, in the sense that I can constrain what's allowed a lot more. If I later decide I want 'mod' as the global default, I can alter the function to map it that way, and joe_user's application is entirely unaffected. This is *not* true if I expect joe_user's app to set the variable directly. He has to go change his app to conform to the new global default. OK, so you are saying your function has to be aligned with the global default. think so. I think that in the field, people are going to have at most a couple of logging configurations they want to allow users to select, and they will use code not significantly more complex than the above. Can you show me the function? I did. I want to see one that allows a non-super user to only to increase the log level without predefining the lowest log level allowed. You should ask on general where we usually do such polls of feature changes? Where was the poll in which we decided that the logging variables needed to be secured at all? In 7.3 these variables were all plain USERSET. We didn't need a pool because we had repeated user complaints stating our current setup was insecure, and we weren't in feature freeze or near final release. This whole discussion is taking time away from getting us to final. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Proposed patch to remove USERLIMIT
Bruce Momjian [EMAIL PROTECTED] writes: I would be glad to see USERLIMIT gone, even though I wrote it. However, I feel we are removing user's ability of non-super users to turn logging on and off easily without really having thought through a mechanism to give them that. I think that is a very weak argument. Remember that the discussion started because setting these variables via ALTER USER fails to work. Why is that of less concern than the fact that unprivileged users won't be able to increase the log level by themselves? I think it's pretty debatable whether the current behavior is a feature at all, rather than a security hole. I just don't see people creating secuirty definer functions easily to do this create or replace function enable_logging(bool) returns void as $$ begin if $1 then set log_statement = 'all'; else set log_statement = 'ddl'; end if; return; end$$ language plpgsql security definer; revoke execute on function enable_logging(bool) from public; grant execute on function enable_logging(bool) to joe_user; (My, that was hard ...) You seem to be supposing that anyone who needs this will be needing a bug-for-bug equivalent to the existing USERLIMIT behavior. I don't think so. I think that in the field, people are going to have at most a couple of logging configurations they want to allow users to select, and they will use code not significantly more complex than the above. Arguably, this approach is better than directly calling the SET commands anyway, because it insulates the application code from the changing details of the specific GUC variables. Need I remind you that any app that directly issues SET log_statement is going to be broken by 8.0? In 7.4 this variable took on and off, now it does not. The argument of preserving backwards-compatible behavior looks a bit funny beside that reality. regards, tom lane ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
[PATCHES] Proposed patch to remove USERLIMIT
The attached patch removes GUC's USERLIMIT variable category, changing all the affected variables to be plain SUSET, as per recent discussion. I also modified postgres.c so that variable settings coming from the client connection request packet (eg, from PGOPTIONS on the client side) are processed only after we have determined whether the user is a superuser. This allows a superuser to set SUSET options from PGOPTIONS, which has never worked before. I consider this to be a lot cleaner and more flexible than what we have now. Comments? regards, tom lane bin8rEOQ7lV8m.bin Description: no-userlimit.patch.gz ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Proposed patch to remove USERLIMIT
Tom Lane wrote: The attached patch removes GUC's USERLIMIT variable category, changing all the affected variables to be plain SUSET, as per recent discussion. I also modified postgres.c so that variable settings coming from the client connection request packet (eg, from PGOPTIONS on the client side) are processed only after we have determined whether the user is a superuser. This allows a superuser to set SUSET options from PGOPTIONS, which has never worked before. I consider this to be a lot cleaner and more flexible than what we have now. Comments? I would be glad to see USERLIMIT gone, even though I wrote it. However, I feel we are removing user's ability of non-super users to turn logging on and off easily without really having thought through a mechanism to give them that. I just don't see people creating secuirty definer functions easily to do this and perhaps we should ship some capability with our source. I am thinking we should hold until 8.1. I do like the idea of moving things to ALTER USER but I think there is more work for us to do. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
