Re: [PATCH] Simplify permission checking logic in user.c

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Paul Martinez <paulmtz(at)google(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Simplify permission checking logic in user.c
Date: 2020-12-30 15:39:22
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


* Michael Paquier (michael(at)paquier(dot)xyz) wrote:
> On Tue, Dec 29, 2020 at 02:26:19PM -0600, Paul Martinez wrote:
> > The checks for whether the current user can create a user with the SUPERUSER,
> > REPLICATION, or BYPASSRLS attributes are chained together using if/else-if,
> > before finally checking whether the user has CREATEROLE privileges in a
> > final else. This construction is error-prone, since once one branch is visited,
> > later ones are skipped, and it implicitly assumes that the permissions needed
> > for each subsequent action are subsets of the permissions needed for the
> > previous action. Since each branch requires SUPERUSER this is fine, but
> > changing one of the checks could inadvertently allow users without the
> > CREATEROLE permission to create users.
> Hmm. I agree with your position here. A careless change could easily
> miss that all those if branches have to use a superuser check.

That really strikes me as pretty darn unlikely to happen, it's not like
this code is really spread out across very many lines or that it's hard
to see the pretty clear pattern.

In thinking about these role attributes and the restrictions we have
about what attributes don't require superuser to set and what do, I do
feel like we're probably missing a bet regarding replication and
bypassrls.. In other words, I suspect people would be happier if we
provided a way for non-superusers a way to create replication roles and
bypassrls roles. Exactly how we do that is a bit tricky to figure out
but that's certainly a much more productive direction to be going in.



In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Verite 2020-12-30 16:27:04 Re: popcount
Previous Message Stephen Frost 2020-12-30 15:26:28 Re: [PATCH] Simplify permission checking logic in user.c