Обсуждение: Proposal: Role Sandboxing for Secure Impersonation
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.
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;
- 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 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
Eric
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
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
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
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
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
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
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
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.
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
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
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.
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
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
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
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.
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
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
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