Re: fixing CREATEROLE

On Wed, Nov 23, 2022 at 3:33 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Because they are role-level properties, they can be set by whoever has
> > ADMIN OPTION on the role. That always includes every superuser, and it
> > never includes Alice herself (except if she's a superuser).
>
> That is just bizarre. Alice can do X, and she can do Y, but she
> can't control a flag that says which of those happens by default?
> How is that sane (disregarding the question of whether the existence
> of the flag is a good idea, which I'm now even less sold on)?

Look, I admitted later in that same email that I don't really know
what the rules for setting role-level properties ought to be. If you
have an idea, I'd love to hear it, but I'd rather if you didn't just
label things into which I have put quite a bit of work as insane
without giving any constructive feedback, especially if you haven't
yet fully understood the proposal.

Your description of the behavior here is not quite accurate.
Regardless of how the flags are set, alice, as a CREATEROLE user, can
gain access to all the privileges of the target role, and she can
arrange to have a grant of permissions on that role with INHERIT TRUE
and SET TRUE. However, there's a difference between the case where (a)
INHERITCREATEDROLE and SETCREATEDROLE are set, and alice gets the
permissions of the role by default and the one where (b)
NOINHERITCREATEDROLE and NOSETCREATEDROLE are set, and therefore alice
gets the permissions only if she does GRANT created_role TO ALICE WITH
INHERIT TRUE, SET TRUE. In the former case, there is only one grant,
and it has grantor=bootstrap_superuser/admin_option=true/inherit_option=true/set_option=true.
In the latter case there are two, one with
grantor=bootstrap_supeuser/admin_option=true/set_option=false/inherit_option=false
and a second with
grantor=alice/admin_option=false/set_option=true/inherit_option=true.
That is pretty nearly equivalent, but it is not the same, and it will
not, for example, be dumped in the same way. Furthermore, it's not
equivalent in the other direction at all. If the superuser gives alice
INHERITCREATEDROLES and SETCREATEDROLES, she can't renounce those
permissions in the patch as written. All of which is to say that I
don't think your characterization of this as "Alice can do X, and she
can do Y, but she can't control a flag that says which of those
happens by default?" is really correct. It's subtler than that.

But having said that, I could certainly change the patches so that any
user, or maybe just a createrole user since it's otherwise irrelevant,
can flip the INHERITCREATEDROLE and SETCREATEDROLE bits on their own
account. There would be no harm in that from a security or auditing
perspective AFAICS. It would, however, make the handling of those
flags different from the handling of most other role-level properties.
That is in fact why things ended up the way that they did: I just made
the new role-level properties which I added work like most of the
existing ones. I don't think that's insane at all. I even think it
might be the right decision. But it's certainly arguable. If you think
it should work differently, make an argument for that. What I would
particularly like to hear in such an argument, though, is a theory
that goes beyond those two particular properties and addresses what
ought to be done with all the other ones, especially CREATEDB and
CREATEROLE. If we can't come up with such a grand unifying theory but
are confident we know what to do about this case, so be it, but we
shouldn't make an idiosyncratic rule for this case without at least
thinking about the overall picture.

--
Robert Haas
EDB: http://www.enterprisedb.com