Re: Reworks for Access Control facilities (r2363)
От | Tom Lane |
---|---|
Тема | Re: Reworks for Access Control facilities (r2363) |
Дата | |
Msg-id | 10495.1255627322@sss.pgh.pa.us обсуждение исходный текст |
Ответ на | Reworks for Access Control facilities (r2363) (KaiGai Kohei <kaigai@ak.jp.nec.com>) |
Ответы |
Re: Reworks for Access Control facilities (r2363)
(KaiGai Kohei <kaigai@ak.jp.nec.com>)
Re: Reworks for Access Control facilities (r2363) (Robert Haas <robertmhaas@gmail.com>) |
Список | pgsql-hackers |
KaiGai Kohei <kaigai@ak.jp.nec.com> writes: > [ patch r2363 ] I promised I would review this today, but I just can't make myself do it in any detail. This is too large, too ugly, and it is going in a direction that I do not like or want to spend any of my time on. The overwhelming impression after a brief read-through is that the code has been hacked apart with a chainsaw and reassembled into a Frankenstein's monster --- it's alive, but man is it ugly. Code comments that refer to something "above" or "below" are still there, but the referent is no longer close enough for that to be a reasonable way of referring to it. It's impossible to follow what's going on or why, either in the shim functions or in the callers --- in the original coding there was context for the aclcheck calls, now there isn't. I don't have any confidence that this is a sane way to proceed forward. The shim layer knows everything about everything --- there may still be a few backend .h files it doesn't include, but that's not for lack of trying. The direction this is heading in is an unmaintainable giant-bowl-of-spaghetti security module, rather than something that can be divided into understandable parts. And I don't think it's really removed any complexity from the callers, nor do I believe that it's going to be a useful basis for imposing a different security policy than the one we have now. Two specific examples of why not: * The "skip permissions checks" arguments that have been added to various random functions suggest strongly that the factoring still isn't right --- I especially don't believe in that in the context of performDeletion and friends. * There are two special-purpose shims, ac_database_calculate_size and ac_tablespace_calculate_size, that got added for the benefit of utils/adt/dbsize.c. What if that code were still in contrib? How is it different from a lot of the code that is in contrib now, eg dblink or pgrowlocks, to say nothing of third-party modules? Presuming that the shim layer can know explicitly about each individual permission-checking requirement is a dead-end design. Maybe if I weren't burned out after a month of CommitFesting, I could muster a more positive reaction, but right now I just can't summon any enthusiasm for this. regards, tom lane
В списке pgsql-hackers по дате отправления: