Обсуждение: Proposal: Role Sandboxing for Secure Impersonation

Поиск
Список
Период
Сортировка

Proposal: Role Sandboxing for Secure Impersonation

От
Eric Hanson
Дата:
Hi all,

I'd like to revisit a previously discussed feature [1] that PostgreSQL could benefit from a "role sandbox", a feature that would build on SET [LOCAL] ROLE, and prevent or restrict RESET ROLE.

Rationale:  Connection pooling is widely used to optimize database performance by reducing use of memory, process creation, etc.  However, connection pools typically operate on a "pool-per-role" basis, because each connection is bound to a single role and can't be reused by another role.  For systems that make use of many roles, this limits the effectiveness of connection pooling because each role has their own "pool space" and max_connections puts a hard limit on how many connections can exist.

To work around this, projects (e.g. PostgREST) employ the "user impersonation" pattern:

- All connections use a shared "authenticator" role

- When a user (e.g. Alice) sends a request to the connection pooler, it temporarily sets the role using:

    SET [LOCAL] ROLE alice;

- After processing Alice's request, the session resets the role back to the "authenticator" role by either issuing a "RESET ROLE" or ending the "local" transaction.

This approach works well in theory, but poses a significant security concern:

RESET ROLE allows a client to reset the role back to the "authenticator" role, *before* handing the session back to the pooler.  Any SQL injection vulnerability or anything else that allows arbitrary SQL allows the client to issue a `RESET ROLE; SET ROLE anybody_else;`, bypassing authentication.  Depending on the privileges of the "authenticator" role, the client can become any other user, or worse.

Proposal:  What if PostgreSQL had a "role sandbox", a state where RESET ROLE was prohibited or restricted?  If PostgreSQL could guarantee that RESET ROLE was not allowed, even SQL injection vulnerabilities would not allow a client to bypass database privileges and RLS when using user impersonation.  Systems with many roles could safely and efficiently use many roles in parallel with connection pooling.  The feature probably has other applications as well.

Sandboxing could happen at the session level, or the transaction level; both seem to have benefits.  Here are some syntax ideas floating around:

SET ROLE IDEAS

a) Transaction ("local") Sandbox:
- SET LOCAL ROLE alice NO RESET;
- SET LOCAL ROLE alice WITHOUT RESET;
- BEGIN AS ROLE alice;

Transaction-level sandboxes have the benefit that a pooler can simply start a new sandboxed transaction for each request and never have to worry about resetting or reusing them.

b) Session Sandbox:
- SET ROLE alice NO RESET;
- SET ROLE alice WITHOUT RESET;
- SET UNRESETTABLE ROLE alice; --veto

Session-level sandboxes have the benefit that they can do things that can't be done inside a transaction (e.g. create extensions, vacuum, analyze, etc.)  It's a fully functional session.  However if RESET ROLE is prohibited for the rest of the session, a connection pooler couldn't reuse it.

c) "Guarded" Transaction/Session
- SET [LOCAL] ROLE alice GUARDED BY reset_token;
- RESET ROLE WITH TOKEN reset_token;

Guarded sandboxes are nice because the session can also exit the sandbox if it has the token.

Another aspect of this is SET SESSION AUTHORIZATION.  I don't see preventing reset as particularly useful at least for connection poolers, since it then couldn't be reused.  However, the GUARDED BY token idea would make it restricted but not prevented, which could be useful.

I'd love to hear your thoughts on this feature.  If we can finalize the design, I would be willing to try implementing this.  I haven't coded C for years though so I will probably need some help depending on how complex it is.  SET ROLE is intertwined with the rest of the SET variable grammar but doesn't seem too hard to extend, if we go that route.  Steve Chavez of PostgREST said he'd be willing to help, and could use the feature in PostgREST if it existed.  I think other poolers could benefit from it as well.

Thanks,
Eric

Re: Proposal: Role Sandboxing for Secure Impersonation

От
Wolfgang Walther
Дата:
Eric Hanson:
> a) Transaction ("local") Sandbox:
> - SET LOCAL ROLE alice NO RESET;
> - SET LOCAL ROLE alice WITHOUT RESET;
> - BEGIN AS ROLE alice;
> 
> Transaction-level sandboxes have the benefit that a pooler can simply 
> start a new sandboxed transaction for each request and never have to 
> worry about resetting or reusing them.
> 
> b) Session Sandbox:
> - SET ROLE alice NO RESET;
> - SET ROLE alice WITHOUT RESET;
> - SET UNRESETTABLE ROLE alice; --veto
> 
> Session-level sandboxes have the benefit that they can do things that 
> can't be done inside a transaction (e.g. create extensions, vacuum, 
> analyze, etc.)  It's a fully functional session.  However if RESET ROLE 
> is prohibited for the rest of the session, a connection pooler couldn't 
> reuse it.
> 
> c) "Guarded" Transaction/Session
> - SET [LOCAL] ROLE alice GUARDED BY reset_token;
> - RESET ROLE WITH TOKEN reset_token;
> 
> Guarded sandboxes are nice because the session can also exit the sandbox 
> if it has the token.
d) SET [LOCAL] ROLE alice WITH <password>;

Which would allow you to change to a role for which you don't have any 
grants, yet. Then, the "authenticator pattern" could be implemented by 
having an authenticator role without any privileges to start with.

The client would provide the password to elevate their privileges. RESET 
ROLE would not be problematic anymore. This would be much cheaper than 
having those roles do full logins on a new connection and could be used 
with connection poolers nicely.

Possibly, this could also be extended by supporting alternatives to just 
a password, for example Json Web Tokens. Maybe building on top of the 
ongoing OAuth effort? Those tokens would then contain a claim for the 
role they are allowed to set.

Best,

Wolfgang



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Joe Conway
Дата:
On 12/2/24 08:41, Eric Hanson wrote:
> Hi all,
> 
> I'd like to revisit a previously discussed feature [1] that PostgreSQL 
> could benefit from a "role sandbox", a feature that would build on SET 
> [LOCAL] ROLE, and prevent or restrict RESET ROLE.
> 
> Rationale:  Connection pooling is widely used to optimize database 
> performance by reducing use of memory, process creation, etc.  However, 
> connection pools typically operate on a "pool-per-role" basis, because 
> each connection is bound to a single role and can't be reused by another 
> role.  For systems that make use of many roles, this limits the 
> effectiveness of connection pooling because each role has their own 
> "pool space" and max_connections puts a hard limit on how many 
> connections can exist.
> 
> To work around this, projects (e.g. PostgREST) employ the "user 
> impersonation" pattern:
> 
> - All connections use a shared "authenticator" role
> 
> - When a user (e.g. Alice) sends a request to the connection pooler, it 
> temporarily sets the role using:
> 
>      SET [LOCAL] ROLE alice;
> 
> - After processing Alice's request, the session resets the role back to 
> the "authenticator" role by either issuing a "RESET ROLE" or ending the 
> "local" transaction.
> 
> This approach works well in theory, but poses a significant security 
> concern:
> 
> RESET ROLE allows a client to reset the role back to the "authenticator" 
> role, *before* handing the session back to the pooler.  Any SQL 
> injection vulnerability or anything else that allows arbitrary SQL 
> allows the client to issue a `RESET ROLE; SET ROLE anybody_else;`, 
> bypassing authentication.  Depending on the privileges of the 
> "authenticator" role, the client can become any other user, or worse.
> 
> Proposal:  What if PostgreSQL had a "role sandbox", a state where RESET 
> ROLE was prohibited or restricted?  If PostgreSQL could guarantee that 
> RESET ROLE was not allowed, even SQL injection vulnerabilities would not 
> allow a client to bypass database privileges and RLS when using user 
> impersonation.  Systems with many roles could safely and efficiently use 
> many roles in parallel with connection pooling.  The feature probably 
> has other applications as well.
> 
> Sandboxing could happen at the session level, or the transaction level; 
> both seem to have benefits.  Here are some syntax ideas floating around:
> 
> SET ROLE IDEAS
> 
> a) Transaction ("local") Sandbox:
> - SET LOCAL ROLE alice NO RESET;
> - SET LOCAL ROLE alice WITHOUT RESET;
> - BEGIN AS ROLE alice;
> 
> Transaction-level sandboxes have the benefit that a pooler can simply 
> start a new sandboxed transaction for each request and never have to 
> worry about resetting or reusing them.
> 
> b) Session Sandbox:
> - SET ROLE alice NO RESET;
> - SET ROLE alice WITHOUT RESET;
> - SET UNRESETTABLE ROLE alice; --veto
> 
> Session-level sandboxes have the benefit that they can do things that 
> can't be done inside a transaction (e.g. create extensions, vacuum, 
> analyze, etc.)  It's a fully functional session.  However if RESET ROLE 
> is prohibited for the rest of the session, a connection pooler couldn't 
> reuse it.
> 
> c) "Guarded" Transaction/Session
> - SET [LOCAL] ROLE alice GUARDED BY reset_token;
> - RESET ROLE WITH TOKEN reset_token;
> 
> Guarded sandboxes are nice because the session can also exit the sandbox 
> if it has the token.
> 
> Another aspect of this is SET SESSION AUTHORIZATION.  I don't see 
> preventing reset as particularly useful at least for connection poolers, 
> since it then couldn't be reused.  However, the GUARDED BY token idea 
> would make it restricted but not prevented, which could be useful.
> 
> I'd love to hear your thoughts on this feature.


I am very much in favor of functionality of this sort being built in to 
the core database. Very similar functionality is available in an 
extension I wrote years ago (without the SQL grammar support) -- see 
https://github.com/pgaudit/set_user

I have never proposed it (or maybe I did years ago, don't actually 
remember) because I did not think the community was interested in this 
approach, but perhaps the time is ripe to discuss it.

-- 
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Michał Kłeczek
Дата:

On 2 Dec 2024, at 18:39, Joe Conway <mail@joeconway.com> wrote:

I am very much in favor of functionality of this sort being built in to the core database. Very similar functionality is available in an extension I wrote years ago (without the SQL grammar support) -- see https://github.com/pgaudit/set_user

Coincidentally, I’ve created https://github.com/pgaudit/set_user/issues/87

BTW thanks for set_user - really needed and appreciated.

Michal 

Re: Proposal: Role Sandboxing for Secure Impersonation

От
Eric Hanson
Дата:
On Mon, Dec 2, 2024 at 10:31 AM Wolfgang Walther <walther@technowledgy.de> wrote:
Eric Hanson:
> a) Transaction ("local") Sandbox:
> - SET LOCAL ROLE alice NO RESET;
> - SET LOCAL ROLE alice WITHOUT RESET;
> - BEGIN AS ROLE alice;
>
> Transaction-level sandboxes have the benefit that a pooler can simply
> start a new sandboxed transaction for each request and never have to
> worry about resetting or reusing them.
>
> b) Session Sandbox:
> - SET ROLE alice NO RESET;
> - SET ROLE alice WITHOUT RESET;
> - SET UNRESETTABLE ROLE alice; --veto
>
> Session-level sandboxes have the benefit that they can do things that
> can't be done inside a transaction (e.g. create extensions, vacuum,
> analyze, etc.)  It's a fully functional session.  However if RESET ROLE
> is prohibited for the rest of the session, a connection pooler couldn't
> reuse it.
>
> c) "Guarded" Transaction/Session
> - SET [LOCAL] ROLE alice GUARDED BY reset_token;
> - RESET ROLE WITH TOKEN reset_token;
>
> Guarded sandboxes are nice because the session can also exit the sandbox
> if it has the token.
d) SET [LOCAL] ROLE alice WITH <password>;

Which would allow you to change to a role for which you don't have any
grants, yet. Then, the "authenticator pattern" could be implemented by
having an authenticator role without any privileges to start with.

The client would provide the password to elevate their privileges. RESET
ROLE would not be problematic anymore. This would be much cheaper than
having those roles do full logins on a new connection and could be used
with connection poolers nicely.

Possibly, this could also be extended by supporting alternatives to just
a password, for example Json Web Tokens. Maybe building on top of the
ongoing OAuth effort? Those tokens would then contain a claim for the
role they are allowed to set.

Thanks all for the input so far.  I think we are the "usual suspects" of advocating for this feature. :)

set_user is a great extension.  I think the functionality belongs in core though, because it can't be used in hosted environments that don't support it.  Hopefully the cloud will wake up to the issue and start supporting set_user in the meantime.

SET ROLE WITH <password> is really intriguing.  If the authenticator role has no privileges, then the pooler only elevates permissions, and auth is integrated with the whole systems auth mechanism.  Supporting other auth mechanisms would be amazing. Big +1 from me.

I dug into existing poolers, to see where things stand.  AFAICT, neither pgbouncer nor pgpool-II do any user impersonation out of the box, so they both have the "pool-per-role" bottleneck.

pgbouncer has three `pool_mode` options, defaulting to `session`.

;; When server connection is released back to pool:
;;   session      - after client disconnects (default)
;;   transaction  - after transaction finishes
;;   statement    - after statement finishes
;pool_mode = session

It also has a `disable_pqexec` config option:

;; Hackish security feature.  Helps against SQL injection: when PQexec
;; is disabled, multi-statement cannot be made.
;disable_pqexec = 0

This effectively makes SET ROLE useless because the session is reset after each single statement.

Pgpool-II has a `reset_query_list` config parameter, a set of commands run each time a new client connects.  For pg 8.3 and later they recommend 'ABORT; DISCARD ALL'.

AFAICT, it's left up to the developer to build user impersonation on top of these poolers, which short of having a sandboxed session (or being flawless w.r.t SQL injection) is inherently insecure.

As far as priorities, any thoughts on what would be the most beneficial feature to add?  I am not sure.  SET SESSION AUTHORIZATION GUARDED BY seems most powerful, but since it requires superuser, that's not ideal either.

Regards,
Eric

Re: Proposal: Role Sandboxing for Secure Impersonation

От
Joe Conway
Дата:
On 12/4/24 11:13, Eric Hanson wrote:
> Thanks all for the input so far.  I think we are the "usual suspects" of 
> advocating for this feature. :)

Yeah, I looked at the old thread and came to the same conclusion ;-)

However on that thread[1] Jelte and Robert expressed a preference to 
accomplishing the goal via protocol changes. That is not my preference, 
but it would be worth hearing from them how firm they are in their 
resolve -- i.e. if we went down the path of adding grammar and support 
along the lines discussed here will they seek to block it from being 
committed? And similarly for others that have not spoken up at all.

I don't want to put a bunch of time and effort into something which is 
ultimately a dead end due to fundamental objections (which is why I made 
set_user an extension in the first place).

On the other hand, if there is a reasonable chance we can get buy in 
given a high enough quality implementation, I would be excited to work 
on it.


[1] 
https://postgr.es/m/flat/CACA6kxgdzt-oForijaxfXHHhnZ1WBoVGMXVwFrJqUu-Hg3C-jA%40mail.gmail.com
-- 
Joe Conway
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Michał Kłeczek
Дата:


On 4 Dec 2024, at 17:13, Eric Hanson <eric@aquameta.com> wrote:

On Mon, Dec 2, 2024 at 10:31 AM Wolfgang Walther <walther@technowledgy.de> wrote:
Eric Hanson:
> a) Transaction ("local") Sandbox:
> - SET LOCAL ROLE alice NO RESET;
> - SET LOCAL ROLE alice WITHOUT RESET;
[snip]
> c) "Guarded" Transaction/Session
> - SET [LOCAL] ROLE alice GUARDED BY reset_token;
> - RESET ROLE WITH TOKEN reset_token;

These are preferable options for PostgREST (at least as long as JWT based impersonation is implemented in Postgres).

> 
> Guarded sandboxes are nice because the session can also exit the sandbox 
> if it has the token.
d) SET [LOCAL] ROLE alice WITH <password>;


PostgREST does not know alice's password as it performs JWT based authentication.

Regards

Michal

Re: Proposal: Role Sandboxing for Secure Impersonation

От
Robert Haas
Дата:
On Wed, Dec 4, 2024 at 2:02 PM Joe Conway <mail@joeconway.com> wrote:
> However on that thread[1] Jelte and Robert expressed a preference to
> accomplishing the goal via protocol changes. That is not my preference,
> but it would be worth hearing from them how firm they are in their
> resolve -- i.e. if we went down the path of adding grammar and support
> along the lines discussed here will they seek to block it from being
> committed? And similarly for others that have not spoken up at all.

I do think the protocol change is better. I think we'd likely have it
already if Jelte hadn't switched employers, but oh well.

I wouldn't oppose a command that does an absolutely irrevocable SET
ROLE -- i.e. once you execute it, it is as if you logged in as the
target role originally, and the only way to get your privileges back
is a new connection.

I am extremely skeptical of something like SET ROLE WITH <password>.
To me, that just seems under-engineered -- why would anyone prefer
that over a protocol-level facility, which seems so much more secure
and less hacky? If it turns out anyone can guess or steal the secret,
then that's a CVE, which is no fun at all. And there's lots of vectors
for trying to steal that secret -- logfiles, pg_stat_activity,
probably others.

--
Robert Haas
EDB: http://www.enterprisedb.com



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Jelte Fennema-Nio
Дата:
On Wed, 4 Dec 2024 at 22:05, Robert Haas <robertmhaas@gmail.com> wrote:
> I do think the protocol change is better.

Fully agreed. But I now also know the herculean amount of effort
that's necessary for that to happen, and I personally don't have the
bandwidth to push that through anymore. So I'm definitely open to a
SQL way if there's a safe way to achieve that.

> I think we'd likely have it
> already if Jelte hadn't switched employers, but oh well.

That's definitely possible, but I'm not so sure. For now I'm trying to
focus on other/smaller things in the community that allow for more
immediate impact.

> I wouldn't oppose a command that does an absolutely irrevocable SET
> ROLE -- i.e. once you execute it, it is as if you logged in as the
> target role originally, and the only way to get your privileges back
> is a new connection.

Agreed, that seems fine to me.

> I am extremely skeptical of something like SET ROLE WITH <password>.

Totally agreed on the security concerns here. We don't want to provide
passwords in a SQL command. For the same reasons explained by Robert,
we also tell people not to set user passwords using SQL, but to use
the \password command instead which generates hashes client side.

I think there's a fairly easy safe alternative though, let's call this
option e):
Instead of letting the client/user provide a secret, the server could
generate it:

SET ROLE jelte WITH GUARD;
-> returns a single row with a 'random-token-abc'
RESET ROLE WITH TOKEN 'random-token-abc';

Such an approach would be totally usable by connection poolers to
multiplex different users on the same connection. All they'd have to
do is run the SET ROLE ... WITH GUARD command and save the token. Then
before it transfers the connection to another role it would need to
run:

DISCARD ALL; -- to clear any scary session state
RESET ROLE WITH TOKEN 'the-token';
SET ROLE some_other_user WITH GUARD;

A full response on all the options proposed so far:
a): This would not be usable by transaction poolers. Because the
design seems to be based on a common misunderstanding of how
transaction pooling works. PgBouncer does not parse the COMMIT, it
only knows that a transaction is finished because postgres will tell
it. Since poolers allow pipelining of messages for performance
reasons, a client can trivially bypass this by quickly sending another
command after the COMMIT message.
b) Fine, details explained above.
c) Very scary security wise, explained above.
d) Even scarier than c, now actual user passwords will start to end up
in logs, not just session local passwords. I don't think there's a
safe way to do this without extending the protocol. We don't want
users to send their plaintext passwords to Postgres, that's why we
have SCRAM auth. If we want something like this, we'd want to allow
users to re-trigger SCRAM authentication. Which clearly requires a
protocol change.

P.S. If we're going to move forward in this direction, then SET
SESSION AUTHORIZATION should have the same functionality. Poolers
probably would want to lock that instead of ROLE, that way users can
still use SET ROLE to change the role that the current SESSION
AUTHORIZATION is allowed to change to.



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Wolfgang Walther
Дата:
Michał Kłeczek:
> PostgREST does not know alice's password as it performs JWT based 
> authentication.

Yes, that's why I proposed an extension to support JWTs in the next 
sentence. Then PostgREST would not need to do any auth at all anymore.

Best,

Wolfgang



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Wolfgang Walther
Дата:
Jelte Fennema-Nio:
>> I am extremely skeptical of something like SET ROLE WITH <password>.
> 
> Totally agreed on the security concerns here. We don't want to provide
> passwords in a SQL command. For the same reasons explained by Robert,
> we also tell people not to set user passwords using SQL, but to use
> the \password command instead which generates hashes client side.

Right, I should have clarified: My proposal wasn't mean to be taken 
literally as an SQL command. Passwords should not be sent as plain text, 
no question. This needs to happen on the protocol level.

What my proposal is about is this: All other suggestions just seem to 
fight the symptoms of an underlying problem. Which is, that connection 
poolers / PostgREST need to run with a very high privileged role, 
because they need to be able to switch into all possible roles that 
could come in via that connection.

Of course, authentication should still happen - but it doesn't happen 
with PostgreSQL anymore. It has to be implemented in the application 
layer / pooler. That kind of defeats some of the purpose of using the 
PostgreSQL role system for users' roles.

I don't want to give any privileges to the connection pooler / 
application and I don't want to outsource authentication.

Once the role to connect with is unprivileged, all the other problems 
become obsolete. RESET ROLE is just fine - you can't win anything.

> If we want something like this, we'd want to allow
> users to re-trigger SCRAM authentication. Which clearly requires a
> protocol change.

Yes. This. Re-authenticating without re-connecting.

I'd hope that this would also work around problems building up the role 
cache when doing SET ROLE with a lot of granted roles [1].

Best,

Wolfgang

[1]: 
https://www.postgresql.org/message-id/7d32e088-34a7-421a-9398-80958acb3f64%40technowledgy.de



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Jelte Fennema-Nio
Дата:
On Thu, 5 Dec 2024 at 09:47, Wolfgang Walther <walther@technowledgy.de> wrote:
> Right, I should have clarified: My proposal wasn't mean to be taken
> literally as an SQL command. Passwords should not be sent as plain text,
> no question. This needs to happen on the protocol level.

Thanks for clarifying.

> I don't want to give any privileges to the connection pooler /
> application and I don't want to outsource authentication.

I understand the security consideration and I think it's valid. But
I'd like to call out for completeness that such an approach (when
using scram) would require two roundtrips, instead of just one like
for option e). So for an admin this is a tradeoff (security vs perf),
not simply better.



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Robert Haas
Дата:
On Wed, Dec 4, 2024 at 5:54 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> On Wed, 4 Dec 2024 at 22:05, Robert Haas <robertmhaas@gmail.com> wrote:
> > I do think the protocol change is better.
>
> Fully agreed. But I now also know the herculean amount of effort
> that's necessary for that to happen, and I personally don't have the
> bandwidth to push that through anymore. So I'm definitely open to a
> SQL way if there's a safe way to achieve that.

I'm not convinced it has to be a Herculean amount of effort. Why do
you think otherwise?

--
Robert Haas
EDB: http://www.enterprisedb.com



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Eric Hanson
Дата:
On Wed, Dec 4, 2024 at 4:54 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> > I wouldn't oppose a command that does an absolutely irrevocable SET
> > ROLE -- i.e. once you execute it, it is as if you logged in as the
> > target role originally, and the only way to get your privileges back
> > is a new connection.
>
> Agreed, that seems fine to me.


Irrevocable SET ROLE would only help with poolers in transaction mode,
via SET LOCAL ROLE x NO RESET (or whatever), right?  (I kind of like
the syntax `BEGIN AS ROLE alice`).

Irrevocably setting the session/connection's role (non-local) could be
generally useful but doesn't seem to help with poolers, as I think
others have mentioned.

>
> e) SET ROLE jelte WITH GUARD;
> -> returns a single row with a 'random-token-abc'
> RESET ROLE WITH TOKEN 'random-token-abc';

Whoa, letting PostgreSQL generate the token is great!

Is there any issue with this being a SET, since SET commands don't
typically return results?  How would you call it from say plpgsql?

> a): This would not be usable by transaction poolers. Because the
> design seems to be based on a common misunderstanding of how
> transaction pooling works. PgBouncer does not parse the COMMIT, it
> only knows that a transaction is finished because postgres will tell
> it. Since poolers allow pipelining of messages for performance
> reasons, a client can trivially bypass this by quickly sending another
> command after the COMMIT message.

When pgbouncer is in transaction mode, the pipeline doesn't stop when
the transaction ends?  Mayhaps I have the common misunderstanding.  So
guarded/unresettable transactions are not at all helpful for security
in pgbouncer?  Is this generally true for others?

> P.S. If we're going to move forward in this direction, then SET
>
> SESSION AUTHORIZATION should have the same functionality. Poolers
> probably would want to lock that instead of ROLE, that way users can
> still use SET ROLE to change the role that the current SESSION
> AUTHORIZATION is allowed to change to.

"should have" for consistency and general usefulness?  At least for
poolers, this would require the authenticator role to be a superuser,
which is scary to me but maybe people do it.  But as far as bringing
sandboxing to PostgreSQL in general, I see the point.

Something along the lines of `SET [LOCAL] ROLE x WITH GUARD` fulfills
all my hopes and dreams.  Probably should get out more. :)

Regards,
Eric



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Jacob Champion
Дата:
On Thu, Dec 5, 2024 at 12:47 AM Wolfgang Walther
<walther@technowledgy.de> wrote:
> > If we want something like this, we'd want to allow
> > users to re-trigger SCRAM authentication. Which clearly requires a
> > protocol change.
>
> Yes. This. Re-authenticating without re-connecting.

The ability to reauthenticate would be useful for the OAUTHBEARER
mechanism as well. (Specifically, the ability to perform a new SASL
exchange on the connection after the first one has failed.) And it
would probably have overlap with the recent discussion around
pass-through SCRAM [1].

--Jacob

[1] https://postgr.es/m/27b29a35-9b96-46a9-bc1a-914140869dac%40gmail.com



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Jelte Fennema-Nio
Дата:
On Thu, 5 Dec 2024 at 16:35, Eric Hanson <eric@aquameta.com> wrote:
> When pgbouncer is in transaction mode, the pipeline doesn't stop when
> the transaction ends?  Mayhaps I have the common misunderstanding.

When PgBouncer is in transaction mode, the server connection will only
be unlinked, when PgBouncer receives a ReadyForQuery with the "idle"
flag from the server **and** there are no messages from the client in
flight anymore. It's totally valid for a client to send multiple
transactions in a single pipeline without waiting for their result.

> So
> guarded/unresettable transactions are not at all helpful for security
> in pgbouncer?

Correct.

> Is this generally true for others?

I'm not sure whether all poolers implement this correctly (pgbouncer
definitely had some recent bugs in this area), but none that I know
parse the COMMIT message. So they will happily forward commands to the
server after the COMMIT is sent if they haven't received a
ReadyForQuery with "idle" back yet.

> > P.S. If we're going to move forward in this direction, then SET
> >
> > SESSION AUTHORIZATION should have the same functionality. Poolers
> > probably would want to lock that instead of ROLE, that way users can
> > still use SET ROLE to change the role that the current SESSION
> > AUTHORIZATION is allowed to change to.
>
> "should have" for consistency and general usefulness?  At least for
> poolers, this would require the authenticator role to be a superuser,
> which is scary to me but maybe people do it.  But as far as bringing
> sandboxing to PostgreSQL in general, I see the point.

Hmm, I didn't realize that SET SESSION AUTHORIZATION required
superuser. I had expected you could set it to any roles that you are
part of. That seems like a fixable problem at least, we could add some
new role that would allow that, like pg_session_authorization.



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Eric Hanson
Дата:
On Thu, Dec 5, 2024 at 4:29 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
> When PgBouncer is in transaction mode, the server connection will only
> be unlinked, when PgBouncer receives a ReadyForQuery with the "idle"
> flag from the server **and** there are no messages from the client in
> flight anymore. It's totally valid for a client to send multiple
> transactions in a single pipeline without waiting for their result.
>
> > So
> > guarded/unresettable transactions are not at all helpful for security
> > in pgbouncer?
>
> Correct.
>
> > Is this generally true for others?
>
> I'm not sure whether all poolers implement this correctly (pgbouncer
> definitely had some recent bugs in this area), but none that I know
> parse the COMMIT message. So they will happily forward commands to the
> server after the COMMIT is sent if they haven't received a
> ReadyForQuery with "idle" back yet.

Got it.

Would you agree that pipelines and connection pooling are somewhat
orthogonal anyway?  At least in the abstract.  One can pool
connections without pipelining and still at least avoid the
max_connections bottleneck.  I would think that, had guarded
sessions/transactions existed when PgBouncer was invented, they might
have added another config mode that pooled and reused the
authenticator role for multiple client roles, but somehow some way
made sure that client requests couldn't spill over after the COMMIT.
Reasonable?

> > > P.S. If we're going to move forward in this direction, then SET
> > >
> > > SESSION AUTHORIZATION should have the same functionality. Poolers
> > > probably would want to lock that instead of ROLE, that way users can
> > > still use SET ROLE to change the role that the current SESSION
> > > AUTHORIZATION is allowed to change to.
> >
> > "should have" for consistency and general usefulness?  At least for
> > poolers, this would require the authenticator role to be a superuser,
> > which is scary to me but maybe people do it.  But as far as bringing
> > sandboxing to PostgreSQL in general, I see the point.
>
> Hmm, I didn't realize that SET SESSION AUTHORIZATION required
> superuser. I had expected you could set it to any roles that you are
> part of. That seems like a fixable problem at least, we could add some
> new role that would allow that, like pg_session_authorization.

I don't even know why SET SESSION AUTH exists.  To me, session auth is
the role that started the session, which is immutable.  The docs say

> Using this command, it is possible, for example, to temporarily become
> an unprivileged user and later switch back to being a superuser.

Sort of?  It's not really "unprivileged" when the client can just
reset it back to privileged.  There must be good reasons, but it seems
like another "we don't have a sandbox" seam.

Regards,
Eric



Re: Proposal: Role Sandboxing for Secure Impersonation

От
Tomas Vondra
Дата:
On 12/5/24 16:12, Robert Haas wrote:
> On Wed, Dec 4, 2024 at 5:54 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>> On Wed, 4 Dec 2024 at 22:05, Robert Haas <robertmhaas@gmail.com> wrote:
>>> I do think the protocol change is better.
>>
>> Fully agreed. But I now also know the herculean amount of effort
>> that's necessary for that to happen, and I personally don't have the
>> bandwidth to push that through anymore. So I'm definitely open to a
>> SQL way if there's a safe way to achieve that.
> 
> I'm not convinced it has to be a Herculean amount of effort. Why do
> you think otherwise?
> 

I'm also interested in why would this be a Herculean effort.

I know next to nothing about the protocol code, but I'd imagine the
amount of new code to handle this new operation (change role + return
reset token) would be relatively modest. I assume the complexity would
come from having to do this in a backwards-compatible way, right?

Could the recent protocol versioning improvements help with this? Or am
I missing some fundamental issue here?


In any case, I think it'd be very useful to have some sort of protected
SET ROLE command, safe for use cases like connection poolers. Be it a
new protocol message, or the SET ROLE variant returning a token. It's a
common pattern we recommend, and a fair fraction of users is unaware of
this weakness.

I agree handling this at the protocol level seems better, but if that
not happened yet. And "something" beats "nothing".



Now, let me go off on a tangent for a bit ...

I started looking at SET ROLE not because of connection pooling, but
because of RLS (row-level security). Which does not really need to be
tied to roles, but the canonical way to write policies is to reference
current_user - just look at [1]).

So in a way, RLS is also affected by this, because if an application
relies on RLS for data protection, but gets connections from a pool that
user SET ROLE, it's vulnerable to the same RESET ROLE thing. A fair
number of developers do not realize this.

But even if they were aware of it, what could they do about it? Stop
using the connection pool, or use something like set_user(). Not always
possible / practical.

But I also realized they often don't *want* to use roles. Managing a
huge number of roles (one for each user) is no fun, really. They'd often
choose something like TENANTID, or something like that. But roles are
the only "trusted" context they have access to, so they use that (and
map it to the features they actually need).

ACAIFS there's no good way to define stuff the policies could rely on.
GUCs access control is quite crude, and it's hard to ensure users can't
change a GUC to something arbitrary (a bit like the RESET ROLE thing).

But what if we provided a way to define such trusted context?

Imagine a context that could be "installed" in a controlled way, either
by the user or by the connection pool. But it could not be modified (at
any point), only "reset". I suspected this might be doable by digital
signing, so I gave it a try and I wrote signed_context [2].

The basic idea is that the user is handed a "context", which is pretty
much just a string encoding key/value pairs. And it's signed by a key
unknown to the user (or to the DB), so that the user can't modify it.
But the public key is known, so the DB can verify that the context is
valid when "setting" it for the session.

It's backed by a GUC, so the user can do:

  SET signed_context.context = '... signed context ...';

and magic happens, and the context is defined. The connection pool could
do this on behalf of the user, and the user wouldn't even know the
signed value. The RLS policies then can call signed_context_ge('X') to
lookup keys in the context, etc.


The reason why I'm mentioning it here is that it seems to face some of
the same challenges mentioned in this thread for the SET ROLE changes.

For example, the signed context is a bit of a sensitive value - in the
simplest form it's a bit like a password. If I learn your token (e.g.
because it leaked into server log), I can impersonate you. There are
ways to mitigate this (could be tied to session, allows to be used only
once, have built-in expiration, ...).


But it occurred to me maybe this is something we could handle at the
protocol level too. Possibly by making the SET ROLE protocol "thing"
generic enough to cover this kind of use case too.

The other thing is that out-of-core extensions are not great for managed
systems (e.g. set_user is nice, but how many systems can use that?).
While that's really a problem of providers of those systems, I wonder if
we should have something like this built-in.



regards

[1] https://www.postgresql.org/docs/current/ddl-rowsecurity.html

[2] https://github.com/tvondra/signed_context

-- 
Tomas Vondra




Re: Proposal: Role Sandboxing for Secure Impersonation

От
Tomas Vondra
Дата:
On 12/9/24 18:27, Eric Hanson wrote:
> On Thu, Dec 5, 2024 at 4:29 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>> When PgBouncer is in transaction mode, the server connection will only
>> be unlinked, when PgBouncer receives a ReadyForQuery with the "idle"
>> flag from the server **and** there are no messages from the client in
>> flight anymore. It's totally valid for a client to send multiple
>> transactions in a single pipeline without waiting for their result.
>>
>>> So
>>> guarded/unresettable transactions are not at all helpful for security
>>> in pgbouncer?
>>
>> Correct.
>>
>>> Is this generally true for others?
>>
>> I'm not sure whether all poolers implement this correctly (pgbouncer
>> definitely had some recent bugs in this area), but none that I know
>> parse the COMMIT message. So they will happily forward commands to the
>> server after the COMMIT is sent if they haven't received a
>> ReadyForQuery with "idle" back yet.
> 
> Got it.
> 
> Would you agree that pipelines and connection pooling are somewhat
> orthogonal anyway?  At least in the abstract.  One can pool
> connections without pipelining and still at least avoid the
> max_connections bottleneck.  I would think that, had guarded
> sessions/transactions existed when PgBouncer was invented, they might
> have added another config mode that pooled and reused the
> authenticator role for multiple client roles, but somehow some way
> made sure that client requests couldn't spill over after the COMMIT.
> Reasonable?
> 

I'm not sure those features really are entirely orthogonal. Sure, you
could "disable" pipelining, but that could significantly limit
throughput, causing the connection to pile on ...


The pooler is meant to be transparent, and we don't start transactions
when connected directly to the DB, so this would surprise users.

Or what if the user really wants to do something that can't happen in a
transaction, like CREATE INDEX CONCURRENTLY? Or what if the user wants
to start a transaction with other features (e.g. read-only or a
different isolation level)?

It'd also interfere with procedures, which can commit/rollback and chain
transactions, but only when not in an explicit transaction. If the
pooler starts wrapping everything in a transaction, this would break.

Not sure if this might cause some issues with "idle in transaction"
sessions. Maybe not ...

So I don't think the pooler should be starting transactions, unless the
user actually started a transaction.



regards

-- 
Tomas Vondra