Standards compliance of SET ROLE / SET SESSION AUTHORIZATION

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Standards compliance of SET ROLE / SET SESSION AUTHORIZATION
Date: 2020-02-14 21:01:01
Message-ID: 6076.1581714061@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

[ Starting a new thread about this, since the old one about GUC reporting
is only marginally related to this point ... if it were more so, maybe I'd
have found it when I went looking for it yesterday ]

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Nov 5, 2019 at 10:02 AM Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>> There's a reason the SQL standard defines SET SESSION AUTHORIZATION but
>> no RESET SESSION AUTHORIZATION: once you enter a security context, you
>> cannot escape it. ISTM that essentially we broke feature F321 "User
>> authorization" by adding RESET into the mix. (I think RESET ROLE breaks
>> the spirit of feature T331 too.) The SQL:2016 standard describes how
>> this is supposed to work in Foundation "4.40.1.1 SQL-session
>> authorization identifiers" (same section is numbered 4.35.1.1 in
>> SQL:2011), and ISTM we made a huge mess of it.
>>
>> I don't see how to fix it, though. If we were to adopt the standard's
>> mechanism, we'd probably break tons of existing code.

> It wouldn't be difficult to introduce a new protocol-level option that
> prohibits RESET SESSION AUTHORIZATION; and it would also be possible
> to introduce a new protocol message that has the same effect as RESET
> SESSION AUTHORIZATION. If you do those two things, then it's possible
> to create a sandbox which the end client cannot escape but which the
> pooler can escape easily.

I went looking into the SQL standard to see just what it says about this,
and I'm darned if I see anything supporting Alvaro's argument. I do not
have SQL:2016 at hand, but in SQL:2011 what I see is that section 4.35.1.1
describes a stack of authorization identifiers and/or roles that controls
the currently-applicable privileges. It says

Let E be an externally-invoked procedure, SQL-invoked routine,
triggered action, prepared statement, or directly executed
statement. When E is invoked, a copy of the top cell is pushed onto
the authorization stack. If the invocation of E is to be under
definer's rights, then the contents of the top cell are replaced with
the authorization identifier of the owner of E. On completion of the
execution of E, the top cell is removed.
...
The <set session user identifier statement> changes the value of the
current user identifier and of the SQL- session user identifier. The
<set role statement> changes the value of the current role name.
...
The term current authorization identifier denotes an authorization
identifier in the top cell of the authorization stack.

There is nothing anywhere in 4.35 that constrains the allowable
transitions of authorization identifiers. The only thing I can find on
that point is in the General Rules of 19.2 <set session user identifier
statement> (a/k/a SET SESSION AUTHORIZATION), which says:

4) If V is not equal to the current value of the SQL-session user
identifier of the current SQL-session context, then the restrictions
on the permissible values for V are implementation-defined.

5) If the current user identifier and the current role name are
restricted from setting the user identifier to V, then an exception
condition is raised: invalid authorization specification.

So as far as I can see, restrictions on what SET SESSION AUTHORIZATION
can set the authorization ID to are implementation-defined, full stop.
There might be considerable value in the semantics Alvaro suggests,
but I think arguing that the spec requires 'em is just wrong.

On the other hand, the restrictions on SET ROLE in 19.3 are much less
squishy:

3) If <role specification> contains a <value specification>, then:

c) If no role authorization descriptor exists that indicates that
the role identified by V has been granted to either the current
user identifier or to PUBLIC, then an exception condition is
raised: invalid role specification.

d) The SQL-session role name and the current role name are set to
V.

4) If NONE is specified, then the current role name is removed.

As best I can tell, we actually are entirely compliant with that, modulo
the fact that we don't think of the current state as an <auth ID, role>
pair. What you can SET ROLE to is determined by your authorization
identifier, not your current role, and so doing a SET ROLE doesn't change
what you can SET ROLE to later. The argument that "RESET ROLE" is somehow
invalid seems a little silly when "SET ROLE NONE" does the same thing.

What I'm now thinking is that we shouldn't mess with the behavior of
SET ROLE, as I mused about doing yesterday in [1]. It's spec-compliant,
or close enough, so let's leave it be. On the other hand, changing the
behavior of SET SESSION AUTHORIZATION is not constrained by spec
compliance concerns, only backwards compatibility. We could address the
pg_dump concerns I had in [1] by tweaking what SET SESSION AUTHORIZATION
can do and then adjusting pg_dump to swap its usage of SET SESSION
AUTHORIZATION (do that just once, in response to --role) and SET ROLE
(do that per-object, to establish ownership).

The only thing stopping us from addressing Alvaro's concern is backwards
compatibility. Perhaps a reasonable solution that preserves that is
to add an option to the command, say

SET SESSION AUTHORIZATION foo PERMANENT;

which would check that you're allowed to become foo and then establish
that as the logged-in userid, with no going back being possible (unless
of course foo has privilege enough to do so). A protocol-level message
to set session auth could also be possible, of course.

regards, tom lane

[1] https://www.postgresql.org/message-id/11496.1581634533%40sss.pgh.pa.us

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message James Coleman 2020-02-14 21:09:30 Re: [DOC] Document concurrent index builds waiting on each other
Previous Message Dave Cramer 2020-02-14 20:43:24 Re: Error on failed COMMIT