Re: has_privs_of_role vs. is_member_of_role, redux

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Wolfgang Walther <walther(at)technowledgy(dot)de>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: has_privs_of_role vs. is_member_of_role, redux
Date: 2022-09-26 20:24:06
Message-ID: CA+TgmoZVEPsxzEZ3e2VWLb_Z6M0n252Kp5N2cho3cxWvo94ALQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 26, 2022 at 3:16 PM Wolfgang Walther
<walther(at)technowledgy(dot)de> wrote:
> Robert Haas:
> > I don't think we're going to be very happy if we redefine inheriting
> > the privileges of another role to mean inheriting only some of them.
> > That seems pretty counterintuitive to me. I also think that this
> > particular definition is pretty fuzzy.
>
> Scratch my previous suggestion. A new, less fuzyy definition would be:
> Ownership is not a privilege itself and as such not inheritable.
>
> When role A is granted to role B, two things happen:
> 1. Role B now has the right to use the GRANTed privileges of role A.
> 2. Role B now has the right to become role A via SET ROLE.
>
> WITH SET controls whether point 2 is the case or not.
>
> WITH INHERIT controls whether role B actually executes their right to
> use those privileges ("inheritance") **and** whether the set role is
> done implicitly for anything that requires ownership, but of course only
> WITH SET TRUE.

If I'm understanding correctly, this would amount to a major
redefinition of what it means to inherit privileges, and I think the
chances of such a change being accepted are approximately zero.
Inheriting privileges needs to keep meaning what it means now, namely,
you inherit all the rights of the granted role.

> > Here, though, it doesn't really seem simple enough to explain in one
> > sentence, nor does it seem easy to reason about.
>
> I think the "ownership is not inheritable" idea is easy to explain.

I don't. And even if I did think it were easy to explain, I don't
think it would be a good idea. One of my first patches to PostgreSQL
added a grantable TRUNCATE privilege to tables. I think that, under
your proposed definitions, the addition of this privilege would have
had the result that a role grant would cease to allow the recipient to
truncate tables owned by the granted role. There is currently a
proposal on the table to make VACUUM and ANALYZE grantable permissions
on tables, which would have the same issue. I think that if I made it
so that adding such privileges resulted in role inheritance not
working for those operations any more, people would come after me with
pitchforks. And I wouldn't blame them: that sounds terrible.

I think the only thing we should be discussing here is how to tighten
up the tests for operations in categories (1) and (2) in my original
email. The options so far proposed are: (a) do nothing, which makes
the proposed SET option on grants a lot less useful; (b) restrict
those operations by has_privs_of_role(), basically making them
dependent on the INHERIT option, (c) restrict them by
has_privs_of_role() || member_can_set_role(), requiring either the
INHERIT option or the SET option, or (d) restrict them by
member_can_set_role() only, i.e. making them depend on the SET option
alone. A broader reworking of what the INHERIT option means is not on
the table: I don't want to write a patch for it, I don't think it's a
good idea, and I don't think the community would accept it even if I
did want to write a patch for it and even if I did think it was a good
idea.

I would like to hear more opinions on that topic. I understand your
vote from among those four options to be (d). I do not know what
anyone else thinks.

Thanks,

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-09-26 20:27:06 Re: [RFC] building postgres with meson - v13
Previous Message Tom Lane 2022-09-26 20:16:25 Re: [PATCH] Introduce array_shuffle() and array_sample()