Subject: Re: v16 roles, SET FALSE, INHERIT FALSE, ADMIN FALSE

From: Tom Lane <[email-masked]>
Date: Mon, 08 Jul 2024 18:08:50 -0400

"David G. Johnston" <[email-masked]> writes:
> On Mon, Jul 8, 2024 at 2:16 PM Tom Lane <[email-masked]> wrote:
>> Pavel Luzanov <[email-masked]> writes:
> On 08.07.2024 22:22, Christophe Pettus wrote:
>>>> This is more curiosity than anything else.  In the v16 role system, is
>>>> there actually any reason to grant membership in a role to a different
>>>> role, but with SET FALSE, INHERIT FALSE, and ADMIN FALSE?  Does the role
>>>> granted membership gain any ability it didn't have before in that case?

>>> Looks like there is one ability.
>>> Authentication in pg_hba.conf "USER" field via +role syntax.

>> Hmm, if that check doesn't require INHERIT TRUE I'd say it's
>> a bug.

> The code doesn't support that claim.

That doesn't make it not a bug.  Robert, what do you think?  If this
is correct behavior, why is it correct?

			regards, tom lane

---

From: "David G. Johnston" <[email-masked]>
Date: Mon, 8 Jul 2024 15:39:11 -0700

On Mon, Jul 8, 2024 at 3:08 PM Tom Lane <[email-masked]> wrote:

> That doesn't make it not a bug.
>

Fair, the code was from a time when membership implied SET permission which
apparently was, IMO, still is, a sufficient reason to allow a member of
that group to login.

By making SET optional we removed this presumption and broke this code and
now need to decide what is a valid setup to allow logging in.  So, yes, a
bug.

So long as we document that being a member of a group grants you the right
to login if that group is named in pg_hba.conf I don't see how that is an
invalid specification.  It also isn't self-evidently correct.  If we do
agree that the status quo is undesirable the change I'd expect is to
require at least one of the membership options to be true in order to be
allowed to login.

But as we documented that membership is the only determinant here, by
design or happenstance, we are behaving exactly as our documentation says.

As we are talking about logging in I'd be fine with potential breakage and
implementing the "At least one" requirement.  I wish we could break even
allowing all-false as an active state but that seems too late to try and
do.  Better just to truly make it pointless and thus let there be no
surprises.

David J.

---

From: Tom Lane <[email-masked]>
Date: Mon, 08 Jul 2024 18:58:00 -0400

> now need to decide what is a valid setup to allow logging in.  So, yes, a

Yeah, that's what I'm thinking --- I also think the current behavior
was an oversight rather than an intentional decision.  Possibly Robert
remembers differently though.

> require at least one of the membership options to be true in order to be

I'd argue that INHERIT TRUE should be required.  The point of SET TRUE
with INHERIT FALSE is that you must *explicitly* do SET ROLE or
equivalent in order to have access to the privileges of the referenced
role.  There is no opportunity to issue SET ROLE before logging in,
but ISTM that means you don't get the privilege, not that you
magically get it without asking.  Otherwise, why did we build this
additional level of grant specificity at all?

I follow the use-case of wanting to be able to log into a database
where your only useful privileges are locked behind SET requirements.
But I'd still argue that you should need to be able to log into that
database using your own privileges.  An analogous case is that having
some privileges on table(s) in some schema does not give you the right
to bypass needing USAGE on the schema in order to access those tables.

> I wish we could break even
> do.

I suppose that was intentional, but I too wonder about the use-case.
I guess it's analogous to, for example, zero-column tables.  That is,
it's not terribly useful as a static state but it might be a state
you pass through in a series of manipulations.  Forbidding it would
just create a roadblock that'd have to be worked around, for example
by being very careful about the ordering of GRANT/REVOKE commands.

			regards, tom lane

---

From: "David G. Johnston" <[email-masked]>
Date: Mon, 8 Jul 2024 16:24:32 -0700

On Mon, Jul 8, 2024 at 3:58 PM Tom Lane <[email-masked]> wrote:

> role.

I think that blast radius is too large for v16.  Arguable for v17.  All
pre-v-16 roles encountered during upgrade will have SET true and those
roles are able to login today but a non-trivial number I would expect to be
unable to if we require INHERIT.

It would be nicer to do away with the default connect grant to public,
expect the group role to have the connect privilege on the desired
database, and let the user role inherit that in lieu of granting it out
explicitly, if we are into v18 feature requests.  By the time the newbie
stops using postgres superuser for everything they can add a connect grant
here or there.

There is no opportunity to issue SET ROLE before logging in,
> additional level of grant specificity at all?

Mostly for the pre-defined roles, and just general explicitness.  So
inherit could be per-role-membership instead per-role.  HBA group
specification rules wasn't a motivating drive if it was considered at all.
Unbreaking CREATEROLE.

An analogous case is that having
> to bypass needing USAGE on the schema in order to access those tables.

That sounds like the Connect privilege noted above.  While +role could be
seen as a sImple text-expander for pg_hba.conf, not a privilege check in
its own right.  We don't explicitly exclude users lacking the login
attribute from the 'all' specification.  They are allowed to match and then
their login attribute and database-specific connect privilege is checked.

David J.

---

From: Robert Haas <[email-masked]>
Date: Mon, 8 Jul 2024 19:37:44 -0400

> is correct behavior, why is it correct?

Correct is debatable, but it's definitely intentional. I didn't think
that referencing a group in pg_hba.conf constituted either (a) the
group inheriting the privileges of the role -- which would make it
governed by INHERIT -- or (b) the group being able to SET ROLE to the
role -- which would make it controlled by SET. I guess you're arguing
for INHERIT which is probably the more logical of the two, but I'm not
really sold on it. I think the pg_hba.conf matching is just asking
whether X is in set S, not whether S has the privileges of X.

For contemporaneous evidence of my thinking on this subject see
https://www.postgresql.org/message-id/[email-masked]
particularly the paragraph that starts with "That's it".

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