On Fri, Jan 13, 2023 at 8:29 PM David G. Johnston <david.g.johns...@gmail.com> wrote: >> The point of the security definer section is to explain how to safely write >> security definer functions that you grant to less privileged users > > Yeah, we are really good at "how". > > + If the security definer function intends to create roles, and if it > + is running as a non-superuser, <varname>createrole_self_grant</varname> > + should also be set to a known value using the <literal>SET</literal> > + clause. > > I'd like to know "why". Without knowing why we are adding this I can't give > it a +1. I want the patch to include the why.
What don't you understand about the "why"? If your security-definer function relies on some GUC having some particular value for some security-critical purpose, and the caller can substitute some other value, they might be able to create a security compromise. Since this GUC has some connection to security, there is at least some distant possibility of that happening. Reasonable people can, perhaps, differ about how likely that is, but I don't really see what's confusing about the theory. As a general statement about the human condition, if you know that someone may be untrustworthy, you should be careful about letting them influence your decisions. If you aren't, something bad may happen to you. The whole thing about SECURITY INVOKER functions is really a separate issue. You can tell people how to write SECURITY DEFINER functions more safely, and we do. You cannot tell them how to write SECURITY INVOKER functions more safely, because the direction of the attack is reversed. In the case of SECURITY DEFINER, the caller of the function can attack the owner of the function. In the case of a SECURITY INVOKER function, the owner of the function can attack the caller of the function. We can't document how to write security invoker functions safely because the author of the function is the one potentially making an attack, and therefore would do the opposite of whatever advice we gave. We *could* add whole new sections to the documentation telling people to be careful about calling security invoker functions, and that's a fine thing to discuss, but what I'm doing here is following up an already-committed patch by adjusting parts of the existing documentation to account for the changes. Inventing whole new sections of the documentation would be a job for a new patch on a new thread, not a follow-up patch on an existing thread. -- Robert Haas EDB: http://www.enterprisedb.com
