On 2/7/22 10:35, Robert Haas wrote:
On Sun, Feb 6, 2022 at 12:24 PM Tom Lane <t...@sss.pgh.pa.us> wrote:
Joe Conway <m...@joeconway.com> writes:
> I'd like to pick this patch up and see it through to commit/push.
> Presumably that will include back-patching to all supported pg versions.
> Before I go through the effort to back-patch, does anyone want to argue
> that this should *not* be back-patched?
Hm, I'm -0.5 or so. I think changing security-related behaviors
in a stable branch is a hard sell unless you are closing a security
hole. This is a fine improvement for HEAD but I'm inclined to
leave the back branches alone.
I think the threshold to back-patch a clear behavior change is pretty
high, so I do not think it should be back-patched.
ok
I am also not convinced that a sufficient argument has been made for
changing this in master. This isn't the only thread where NOINHERIT
has come up lately, and I have to admit that I'm not a fan. Let's
suppose that I have two users, let's say sunita and sri. In the real
world, Sunita is Sri's manager and needs to be able to perform actions
as Sri when Sri is out of the office, but it isn't desirable for
Sunita to have Sri's privileges in all situations all the time. So we
mark role sunita as NOINHERIT and grant sri to sunita. Then it turns
out that Sunita also needs to be COPY TO/FROM PROGRAM, so we give her
pg_execute_server_program. Now, if she can't exercise this privilege
without setting role to the prefined role, that's bad, isn't it? I
mean, we want her to be able to copy between *her* tables and various
shell commands, not the tables owned by pg_execute_server_program, of
which there are presumably none.
Easily worked around with one additional level of role:
8<---------------------------------------
nmx=# create user sunita;
CREATE ROLE
nmx=# create user sri superuser;
CREATE ROLE
nmx=# create user sri_alt noinherit;
CREATE ROLE
nmx=# grant sri to sri_alt;
GRANT ROLE
nmx=# grant sri_alt to sunita;
GRANT ROLE
nmx=# grant pg_execute_server_program to sunita;
GRANT ROLE
nmx=# set session authorization sri;
SET
nmx=# create table foo(id int);
CREATE TABLE
nmx=# insert into foo values(42);
INSERT 0 1
nmx=# set session authorization sunita;
SET
nmx=> select * from foo;
ERROR: permission denied for table foo
nmx=> set role sri;
SET
nmx=# select * from foo;
id
----
42
(1 row)
nmx=# reset role;
RESET
nmx=> select current_user;
current_user
--------------
sunita
(1 row)
nmx=> create temp table sfoo(f1 text);
CREATE TABLE
nmx=> copy sfoo(f1) from program 'id';
COPY 1
nmx=> select f1 from sfoo;
f1
----------------------------------------------------------------------------------------
uid=1001(postgres) gid=1001(postgres)
groups=1001(postgres),108(ssl-cert),1002(pgconf)
(1 row)
8<---------------------------------------
It seems to me that the INHERIT role flag isn't very well-considered.
Inheritance, or the lack of it, ought to be decided separately for
each inherited role. However, that would be a major architectural
change.
Agreed -- that would be useful.
But in the absence of that, it seems clearly better for predefined
roles to disregard INHERIT and just always grant the rights they are
intended to give. Because if we don't do that, then we end up with
people having to SET ROLE to the predefined role and perform actions
directly as that role, which seems like it can't be what we want. I
almost feel like we ought to be looking for ways of preventing people
from doing SET ROLE to a predefined role altogether, not encouraging
them to do it.
I disagree with this though.
It is confusing and IMHO dangerous that the predefined roles currently
work differently than regular roles eith respect to privilege inheritance.
In fact, I would extend that argument to the pseudo-role PUBLIC.
Joe
--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development
