Re: RFC: Non-user-resettable SET SESSION AUTHORISATION

Craig,

* Craig Ringer (craig@2ndquadrant.com) wrote:
> On 12 May 2015 at 21:10, Stephen Frost <sfrost@snowman.net> wrote:
> > > This can be done without protocol-level changes and with no backward
> > > compatibility impact to existing applications. Any objections?
> >
> > I don't particularly object but I'm not entirely sure that it's that
> > simple to clear out the cookie value- look at the previous discussion
> > about ALTER ROLE / password resets and trying to keep them from ending
> > up in the logs.
>
> In this case we don't have to care about the cookie values ending up in the
> logs. They're just single use tokens. If an app can read the PostgreSQL
> logs or access privileged pg_stat_activity then it's already privileged
> enough that it's outside the scope of something like this.

Perhaps that would work, but the issue still exists about the connection
on which it's set potentially being able to see the value if we don't
scrub it, no?

> > Perhaps people will feel differently about this since
> > it's expected to be programatically used, but, personally, I'd favor
> > trying to do something at the protocol level instead.
>
> I would also prefer a protocol level solution, but prior discussion has
> shown that Tom in particular sees it as unsafe to add new protocol messages
> in the v3 protocol. So I didn't think it was productive to start with a
> protocol-level approach when it can be done without protocol changes.

I hope it wasn't quite so cut-and-dry as "can't change anything"..
Especially if it's a client initiated request which clearly indicates
that the client supports the response then, hopefully, we'd be able to
make a change.  More specifics or a link to the prior discussion would
help here.

> I'm very happy to do this at the protocol level, I just don't want to
> implement that and then have to do the SQL-level approach or implement a
> whole v4 protocol ;-)

I certainly agree that we should discuss it first.

> > > As is currently the case, poolers will still have to use a superuser
> > > connection if they want to pool across users.
> >
> > That just needs to be fixed. :(  Having poolers connect as superuser is
> > *not* ok..
>
> While I agree, that's existing common (and horrible) practice, and a
> separate problem.
>
> We need a way to say "this pooler connection can become <this set of
> users>". Right now SET SESSION AUTHORIZATION is all or nothing.

That's exactly what SET ROLE provides ...

> Is there a reason why we would need to support both?  Clearly this is
> > all post-9.5 work and seems like it might be reasonable to have
> > completed for 9.6.  Have you thought much about how Heikki's work on
> > SCRAM might play into this?
>
> I haven't looked at the SCRAM work, and will take a look.

Ok.

> If it can offer separation of authentication and authorization then it
> would be in a good position to handle letting SET SESSION AUTHORIZATION run
> as non-superuser, which would be great. I don't see how it could help with
> making it non-resettable though.

It's not going to provide that right off the bat, but it is defining a
new authentication mechanism with changes to the protocol..  Perhaps
there's a way to work in the other things you're looking for as part of
that change?  That is to say, if the client says "I support SCRAM" and
we authenticate that way then perhaps that can also mean "I can do SCRAM
to re-authenticate later too" and we can feel comfortable about the
protocol level changes to make what we're talking about here work, at
least when SCRAM is involved.

If that works, we could probably figure out a way to make the other auth
methods able to work with this too.

> > I guess I'm a bit confused at why SET ROLE being visible to apps is a
> > problem..?
>
> If we used SET ROLE rather than SET SESSION AUTHORIZATION for this, then
> apps that use SET ROLE and RESET ROLE couldn't do so anymore, so a pooled
> connection wouldn't be quite the same as an unpooled connection.

There's two pieces here- is it really a sensible use-case for the
connection pooler to need to do SET ROLE and the app need to do it?  I'm
not convinced that actually makes sense or is a use-case we need to
support.

> session_user would also report the pooler's user identity.

So?  I'm not sure that's an issue, though if it is, we could probably
deal with it in some way.

> postgres=> SET ROLE craig;
> SET
> postgres=> SELECT session_user, current_user;
>  session_user | current_user
> --------------+--------------
>  postgres     | craig
> (1 row)
>
> ...  and IIRC it affects some of the privilege and role membership tests,
> which distinguish between currently active role and role membership.

Uhh, setting role had better change the privilege tests to operate
against the role that you changed to.  If it doesn't somewhere, then
that's a bug we need to fix.

> I think if we want to SET the session authorization, SET SESSION
> AUTHORIZATION or a protocol level equivalent is more appropriate. It has
> the desired behaviour, barring a limit on being reset.

What you're asking for above wrt saying a given user can become a set of
users is what we've already *got* with roles.  The only thing missing
here is that the session can detect that you used set role, the reset
issue that exists for both, and the how-to-auth issue which also exists
for both.  If it's really an issue that the session could detect that a
set role was used then perhaps we work out a way to deal with that.
Seems like it'd be easier to do than reinventing pg_authid independently
for set-session-authorization.
Thanks!
    Stephen