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

From: Craig Ringer <craig(at)2ndquadrant(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: RFC: Non-user-resettable SET SESSION AUTHORISATION
Date: 2015-05-13 02:41:51
Message-ID: CAMsr+YENfbCjO3vCoMiFz5KA=sfxjOTsWqDn+gh2eSQOQLA76A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 13 May 2015 at 09:55, Stephen Frost <sfrost(at)snowman(dot)net> wrote:

> Craig,
>
> * Craig Ringer (craig(at)2ndquadrant(dot)com) wrote:
> > On 12 May 2015 at 21:10, Stephen Frost <sfrost(at)snowman(dot)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?
>

Yes, we must scrub it on that session, but that can be done with a dummy
stats report if nothing else.

>
> > > 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.
>

It was with regards to returning the commit LSN.

Thread begins here:
http://www.postgresql.org/message-id/53E2D346.9030806@2ndquadrant.com,
specific issue here:
http://www.postgresql.org/message-id/25297.1407459774@sss.pgh.pa.us . It's
WRT to using a GUC to enable/disable a protocol message, so it may simply
not apply here, since we can have a client-initiated message without which
the server behaviour isn't any different.

> > > 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"

It shouldn't be necessary to re-authenticate. Just change the active
authorizations to any that the current authentication permits.

Arguably that's what SET ROLE does already though.

> 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.
>

Fair point. A protocol level SET ROLE that cannot be RESET may be enough,
even if it's not totally transparent.

If making it possible with S.S.A. later is necessary we could look at
extending pg_authid but you're right that it's probably not truly needed.

> 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'll see if I can turn "vague memories" into something more useful.

> 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.

Yeah, fair.

So ... a protocol level SET ROLE that can't be reset by SQL-level SET or
RESET role then?

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2015-05-13 03:07:21 Re: Default Roles (was: Additional role attributes)
Previous Message Craig Ringer 2015-05-13 02:08:48 Re: proposal: contrib module - generic command scheduler