Re: SET ROLE and reserved roles - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: SET ROLE and reserved roles
Date
Msg-id 20160413225337.GS10850@tamriel.snowman.net
Whole thread Raw
In response to Re: SET ROLE and reserved roles  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: SET ROLE and reserved roles  ("David G. Johnston" <david.g.johnston@gmail.com>)
Re: SET ROLE and reserved roles  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Stephen Frost <sfrost@snowman.net> writes:
> > On Wednesday, April 13, 2016, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> If you want to prevent that, I think it needs to be done somewhere else
> >> than here.  What about "ALTER OWNER TO pg_signal_backend", for instance?
>
> > Checks are included in that code path to disallow it.
>
> If there are such low-level checks, why do we need to disallow SET ROLE?
> It seems like the wrong place to be inserting such defenses.

The low-level checks were placed all along the paths which used
get_rolespace_oid/tuple.  SET ROLE is the only place where a user could
change to another role and then do things which result in objects being
owned by that role without going through one of those paths.

Requiring that SET ROLE be allowed will mean that many more paths must
be checked and adjusted, such as in all of the CreateObject statements
and potentionally many other paths that I'm not thinking of here, not
the least of which are all of the *existing* checks near
get_rolespec_oid/tuple which will require additional checks for the
CURRENT_USER case to see if the current user is a default role.

> >> But perhaps more to the point, why is it a strange corner case for one
> >> of these roles to own objects?
>
> > If the default roles can own objects and otherwise be used for other
> > purposes then we can never, ever, be rid of any which we create.
>
> This argument seems quite bogus to me, because granted privileges are
> pretty darn close to being objects.  Certainly, if you have some
> "GRANT pg_signal_backend TO joe_user" commands in a pg_dump script,
> those will fail for lack of the role just as surely as ALTER OWNER
> commands would.  So we would need some kind of special hack in pg_dump
> either way, unless we make it incumbent on users to clean up before
> upgrading (which doesn't seem out of the question to me ...)

I don't think we would have any concerns with a hack in pg_dump to
remove those GRANTs if that default role goes away.  There's certainly
no way we're going to do that for tables which have data in them,
however.  Therefore, the user would have to address any such cases
before being able to an upgrade, and I don't see why we want to allow
such a case.

As for present-vs-future cruft, we can't put this genie back in the
bottle once we allow a default role to own objects, which is why I felt
it was prudent to address that concern from the get-go.

> I think you're replacing hypothetical future cruft with actual present
> cruft, and it doesn't seem like a very good tradeoff.

If we don't care about default roles being able to own objects, then we
can remove the check in SET ROLE and simply accept that we can never
remove those roles without user intervention.  There's a number of other
checks in the tree which we can debate (should a default role be allowed
to have a USER MAPPING?  What about additional privileges beyond those
we define for it?  Perhaps DEFAULT privileges?).  If we're going to
allow default roles to own objects, it seems like we could just about
remove all those other checks also.

If we don't want default roles to be able to own objects, but we do want
users to be able to SET ROLE to them, then there's a whole slew of
*additional* checks which have to be added, which is surely a
net-increase in code.

I'm happy to do my best to implement the concensus opinion, just wanted
to be clear on what the options are.  If I could get responses regarding
everyone's preferences on the above, then we can get to what the
desired behavior should be and I'll implement it.

Thanks!

Stephen

pgsql-hackers by date:

Previous
From: Daniel Lenski
Date:
Subject: Re: sign function with INTERVAL?
Next
From: Michael Paquier
Date:
Subject: Re: Why doesn't src/backend/port/win32/socket.c implement bind()?