Thread: BUG #1145: silent REVOKE failures
The following bug has been logged online:
Bug reference: 1145
Logged by: Fabien Coelho
Email address: coelho@cri.ensmp.fr
PostgreSQL version: 7.5 Dev
Operating system: linux debian
Description: silent REVOKE failures
Details:
REVOKE fails silently.
It may be a feature, but if so it is not a good one.
The documentation does not say this is a feature.
-- calvin has no "grant" rights on database comics nor -- on schema public.
calvin@[local]:5432
comics=> REVOKE ALL ON SCHEMA public FROM calvin;
REVOKE
calvin@[local]:5432
comics=> SELECT * FROM pg_namespace WHERE nspname='public';
nspname | nspowner | nspacl
---------+----------+----------------------------------------------------
public | 1 | {postgres=U*C*/postgres,"groupeleves=U/postgres"}
(1 row)
The REVOKE failure should be reported.
"PostgreSQL Bugs List" <pgsql-bugs@postgresql.org> writes:
> The REVOKE failure should be reported.
What failure? This looks perfectly fine to me.
regards, tom lane
Fabien COELHO <coelho@cri.ensmp.fr> writes:
> Well, if I issue a "REVOKE" and the rights are not revoked and could never
> have been because I have no right to issue such statement on the object, I
> tend to call this deep absence of success a "failure".
> If I do the very opposite GRANT, I have a clear "permission denied".
Oh, I thought you were complaining that revoking rights not previously
granted should be an error. I agree with the above; in fact it's a
duplicate of a previous complaint.
regards, tom lane
Dear Tom, > "PostgreSQL Bugs List" <pgsql-bugs@postgresql.org> writes: > > The REVOKE failure should be reported. > > What failure? This looks perfectly fine to me. "Ex nihilo dixit quod libet", as we used to say in latin and in maths. Sorry if say something stupid, but I cannot see why it is fine. Well, if I issue a "REVOKE" and the rights are not revoked and could never have been because I have no right to issue such statement on the object, I tend to call this deep absence of success a "failure". If I do the very opposite GRANT, I have a clear "permission denied". I wish I had the very same error on REVOKE, because for both operations you should need to be either a super user, the owner or to have a relevant grant options? Look at the very same with unix: sh> chmod o-r /tmp/ chmod: changing permissions of `/tmp/': Operation not permitted If you want to call that a "feature", I disagree without further strong argument, and anyway the documentation should be clear about that. Have a nice day, -- Fabien Coelho - coelho@cri.ensmp.fr
Tom Lane wrote: > Fabien COELHO <coelho@cri.ensmp.fr> writes: > > Well, if I issue a "REVOKE" and the rights are not revoked and could never > > have been because I have no right to issue such statement on the object, I > > tend to call this deep absence of success a "failure". > > > If I do the very opposite GRANT, I have a clear "permission denied". > > Oh, I thought you were complaining that revoking rights not previously > granted should be an error. I agree with the above; in fact it's a > duplicate of a previous complaint. Did we resolve this? Is it a TODO? -- 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
Fabien COELHO <coelho@cri.ensmp.fr> writes:
> For the TODO, I would suggest something general:
> - fix grant/revoke wrt SQL standard, validate errors, warnings and successes.
I have a patch-in-progress that addresses these issues. I didn't get it
quite finished before leaving for vacation, but hope to clean it up and
commit soon. There was an open issue, which was whether we want the
owner's grant options to be shown in the default ACL value or not.
(This would be only cosmetic and not functional, because the code will
treat the owner as having grant options in any case.) A related
question is whether the information_schema display of grant options
should track what the ACL says or tell the truth, namely that the owner
has grant options regardless. (Doing the latter would force changing
the API of aclcontains(), hence initdb.)
I asked for opinions on this but got little feedback.
regards, tom lane
Dear Bruce, > > > Well, if I issue a "REVOKE" and the rights are not revoked and could never > > > have been because I have no right to issue such statement on the object, I > > > tend to call this deep absence of success a "failure". > > > > > If I do the very opposite GRANT, I have a clear "permission denied". > > > > Oh, I thought you were complaining that revoking rights not previously > > granted should be an error. I agree with the above; in fact it's a > > duplicate of a previous complaint. > > Did we resolve this? Is it a TODO? No? No? There has been a lot of off-line discussion about how to interpret the standard on this point. I'm not even sure we perfectly agreed in the end, although our understanding of the issues improved a lot through the discussion. As a summary, it is pretty subtle, especially as the standard wording is contrived, and postgres does not do what should be done in a lot of cases. There are also actual "security" bugs. For the TODO, I would suggest something general: - fix grant/revoke wrt SQL standard, validate errors, warnings and successes. -- Fabien Coelho - coelho@cri.ensmp.fr
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> For the TODO, I would suggest something general:
>> - fix grant/revoke wrt SQL standard, validate errors, warnings and successes.
> Tom, is this done?
I think so. At least, we're a lot closer to spec than we were.
regards, tom lane
Fabien COELHO wrote: > > Dear Bruce, > > > > > Well, if I issue a "REVOKE" and the rights are not revoked and could never > > > > have been because I have no right to issue such statement on the object, I > > > > tend to call this deep absence of success a "failure". > > > > > > > If I do the very opposite GRANT, I have a clear "permission denied". > > > > > > Oh, I thought you were complaining that revoking rights not previously > > > granted should be an error. I agree with the above; in fact it's a > > > duplicate of a previous complaint. > > > > Did we resolve this? Is it a TODO? > > No? No? > > There has been a lot of off-line discussion about how to interpret the > standard on this point. I'm not even sure we perfectly agreed in the end, > although our understanding of the issues improved a lot through the > discussion. As a summary, it is pretty subtle, especially as the standard > wording is contrived, and postgres does not do what should be done in a > lot of cases. There are also actual "security" bugs. > > For the TODO, I would suggest something general: > > - fix grant/revoke wrt SQL standard, validate errors, warnings and successes. Tom, is this done? -- 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
> >> For the TODO, I would suggest something general: > >> - fix grant/revoke wrt SQL standard, validate errors, warnings and successes. > > > Tom, is this done? > > I think so. At least, we're a lot closer to spec than we were. Indeed. Maybe the validation part could be improved somehow, with test cases build from the sql specification and the expected behavior checked against the actual behavior. What I derived from the many discussion and time I spent on the standard is that there is a lot of subtlety involved. So maybe the following TODO could be kept: - validate grant/revoke (error, warning, success0 wrt sql standard I may be interested in implementing ROLEs someday, and such tests would be welcome just to check that nothing is broken. -- Fabien Coelho - coelho@cri.ensmp.fr
Fabien COELHO wrote: > > > >> For the TODO, I would suggest something general: > > >> - fix grant/revoke wrt SQL standard, validate errors, warnings and successes. > > > > > Tom, is this done? > > > > I think so. At least, we're a lot closer to spec than we were. > > Indeed. > > Maybe the validation part could be improved somehow, with test cases > build from the sql specification and the expected behavior checked > against the actual behavior. > > What I derived from the many discussion and time I spent on the standard > is that there is a lot of subtlety involved. > > So maybe the following TODO could be kept: > > - validate grant/revoke (error, warning, success0 wrt sql standard > > I may be interested in implementing ROLEs someday, and such tests would be > welcome just to check that nothing is broken. Unless someone can say it is wrong I am not inclinded to add a TODO item that is only possible. -- 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
Dear Bruce, > > So maybe the following TODO could be kept: > > - validate grant/revoke (error, warning, success0 wrt sql standard > > > > I may be interested in implementing ROLEs someday, and such tests would be > > welcome just to check that nothing is broken. > > Unless someone can say it is wrong I am not inclinded to add a TODO item > that is only possible. Sorry, I do not understand your argument, what you mean by "only possible". Or are you talking about roles?? I see TODO items as wishes, and I'm not sure I can see what is wrong with wishing better/full testing of postgresql data access controls and compare the results with what is defined by the norm? -- Fabien Coelho - coelho@cri.ensmp.fr
Fabien COELHO wrote: > > Dear Bruce, > > > > So maybe the following TODO could be kept: > > > - validate grant/revoke (error, warning, success0 wrt sql standard > > > > > > I may be interested in implementing ROLEs someday, and such tests would be > > > welcome just to check that nothing is broken. > > > > Unless someone can say it is wrong I am not inclinded to add a TODO item > > that is only possible. > > Sorry, I do not understand your argument, what you mean by "only > possible". Or are you talking about roles?? > > I see TODO items as wishes, and I'm not sure I can see what is wrong with > wishing better/full testing of postgresql data access controls and compare > the results with what is defined by the norm? I guess I am looking for a more detailed analysis that there is a problem to be fixed. Yes, I would like more testing too, but we need more testing in lots of areas, but it doesn't make it a TODO item. I guess I am asking why this area needs more testing for spec compliance. -- 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
Dear Bruce, > > I see TODO items as wishes, and I'm not sure I can see what is wrong with > > wishing better/full testing of postgresql data access controls and compare > > the results with what is defined by the norm? > > I guess I am looking for a more detailed analysis that there is a > problem to be fixed. Yes, I would like more testing too, but we need > more testing in lots of areas, but it doesn't make it a TODO item. > > I guess I am asking why this area needs more testing for spec > compliance. Ok. I can state new arguments and repeat old ones. Because security is not really tested by users. If there is a problem in SELECT, you would hear quite quickly about it. Security looks like an important issue, but people/admin just assume that it works properly. Probing the walls is not what the average user or admin is expected to do with the DB anyway. Moreover, the sql specs is quite contrived in the area, although I haven't looked at others areas;-) There were bugs in the past that where solved, there may be others yet to be find, but there is no real validation, so a "make check" would not notice if some old bugs is brought back, which goes with the next point: If roles are to be implemented, is will touch this "sensitive" area, and anyone should feel safer to accept such changes if deep exhaustive tests are actually performed. You juge what these arguments are worth wrt to justify a grand "TODO" item;-) IMHO, this should be a prerequisite to adding "roles". Hace a nice day, -- Fabien Coelho - coelho@cri.ensmp.fr