public inbox for [email protected]  
help / color / mirror / Atom feed
Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
11+ messages / 4 participants
[nested] [flat]

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
@ 2023-03-08 22:44 Jacob Champion <[email protected]>
  2023-03-09 14:12 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  0 siblings, 2 replies; 11+ messages in thread

From: Jacob Champion @ 2023-03-08 22:44 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On Wed, Mar 8, 2023 at 11:40 AM Robert Haas <[email protected]> wrote:
> On Wed, Mar 8, 2023 at 2:30 PM Jacob Champion <[email protected]> wrote:
> > I don't think I necessarily like that option better than SASL-style,
> > but hopefully that clarifies it somewhat?
>
> Hmm, yeah, I guess that's OK.

Okay, cool.

> I still don't love it, though. It feels
> more solid to me if the proxy can actually block the connections
> before they even happen, without having to rely on a server
> interaction to figure out what is permissible.

Sure. I don't see a way for the proxy to figure that out by itself,
though, going back to my asymmetry argument from before. Only the
server truly knows, at time of HBA processing, whether the proxy
itself has authority. If the proxy knew, it wouldn't be confused.

> I don't know what you mean by SASL-style, exactly.

That's the one where the server explicitly names all forms of
authentication, including the ambient ones (ANONYMOUS, EXTERNAL,
etc.), and requires the client to choose one before running any
actions on their behalf. That lets the require_auth machinery work for
this case, too.

--Jacob






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
@ 2023-03-09 14:12 ` Robert Haas <[email protected]>
  1 sibling, 0 replies; 11+ messages in thread

From: Robert Haas @ 2023-03-09 14:12 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On Wed, Mar 8, 2023 at 5:44 PM Jacob Champion <[email protected]> wrote:
>
> On Wed, Mar 8, 2023 at 11:40 AM Robert Haas <[email protected]> wrote:
> > On Wed, Mar 8, 2023 at 2:30 PM Jacob Champion <[email protected]> wrote:
> > > I don't think I necessarily like that option better than SASL-style,
> > > but hopefully that clarifies it somewhat?
> >
> > Hmm, yeah, I guess that's OK.
>
> Okay, cool.
>
> > I still don't love it, though. It feels
> > more solid to me if the proxy can actually block the connections
> > before they even happen, without having to rely on a server
> > interaction to figure out what is permissible.
>
> Sure. I don't see a way for the proxy to figure that out by itself,
> though, going back to my asymmetry argument from before. Only the
> server truly knows, at time of HBA processing, whether the proxy
> itself has authority. If the proxy knew, it wouldn't be confused.
>
> > I don't know what you mean by SASL-style, exactly.
>
> That's the one where the server explicitly names all forms of
> authentication, including the ambient ones (ANONYMOUS, EXTERNAL,
> etc.), and requires the client to choose one before running any
> actions on their behalf. That lets the require_auth machinery work for
> this case, too.
>
> --Jacob



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






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
@ 2023-03-09 14:17 ` Robert Haas <[email protected]>
  2023-03-11 00:00   ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  1 sibling, 1 reply; 11+ messages in thread

From: Robert Haas @ 2023-03-09 14:17 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On Wed, Mar 8, 2023 at 5:44 PM Jacob Champion <[email protected]> wrote:
> Sure. I don't see a way for the proxy to figure that out by itself,
> though, going back to my asymmetry argument from before. Only the
> server truly knows, at time of HBA processing, whether the proxy
> itself has authority. If the proxy knew, it wouldn't be confused.

That seems like a circular argument. If you call the problem the
confused deputy problem then the issue must indeed be that the deputy
is confused, and needs to talk to someone else to get un-confused. But
why is the deputy necessarily confused in the first place? Our deputy
is confused because our code to decide whether to proxy a connection
or not is super-dumb, but if there's an intrinsic reason it can't be
smarter, I don't understand what it is.

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






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
@ 2023-03-11 00:00   ` Jacob Champion <[email protected]>
  2023-03-20 16:32     ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Jacob Champion @ 2023-03-11 00:00 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On Thu, Mar 9, 2023 at 6:17 AM Robert Haas <[email protected]> wrote:
> That seems like a circular argument. If you call the problem the
> confused deputy problem then the issue must indeed be that the deputy
> is confused, and needs to talk to someone else to get un-confused. But
> why is the deputy necessarily confused in the first place? Our deputy
> is confused because our code to decide whether to proxy a connection
> or not is super-dumb,

No, I think our proxy is confused because it doesn't know what power
it has, and it can't tell the server what power it wants to use. That
problem is independent of the decision to proxy. You're suggesting
strengthening the code that makes that decision -- adding an oracle
(in the form of a DBA) that knows about the confusion and actively
mitigates it. That's guaranteed to work if the oracle is perfect,
because "perfect" is somewhat tautologically defined as "whatever
ensures secure operation". But the oracle doesn't reduce the
confusion, and DBAs aren't perfect.

If you want to add a Sheriff Andy to hold Barney Fife's hand [1], that
will absolutely make Barney less of a problem, and I'd like to have
Andy around regardless. But Barney still doesn't know what's going on,
and when Andy makes a mistake, there will still be trouble. I'd like
to teach Barney some useful stuff.

> but if there's an intrinsic reason it can't be
> smarter, I don't understand what it is.

Well... I'm not well-versed enough in this to prove non-existence of a
solution. Can you find a solution, using the current protocol, that
doesn't make use of perfect out-of-band knowledge? We have a client
that will authenticate using any method the server asks it to, even if
its user intended to use something else. And we have a server that can
eagerly skip client authentication, and then eagerly run code on its
behalf, without first asking the client what it's even trying to do.
That would be an inherently hostile environment for *any* proxy, not
just ours.

Thanks,
--Jacob

[1] https://en.wikipedia.org/wiki/The_Andy_Griffith_Show#Premise_and_characters






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-11 00:00   ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
@ 2023-03-20 16:32     ` Robert Haas <[email protected]>
  2023-03-24 21:47       ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  0 siblings, 1 reply; 11+ messages in thread

From: Robert Haas @ 2023-03-20 16:32 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On Fri, Mar 10, 2023 at 7:00 PM Jacob Champion <[email protected]> wrote:
> On Thu, Mar 9, 2023 at 6:17 AM Robert Haas <[email protected]> wrote:
> > That seems like a circular argument. If you call the problem the
> > confused deputy problem then the issue must indeed be that the deputy
> > is confused, and needs to talk to someone else to get un-confused. But
> > why is the deputy necessarily confused in the first place? Our deputy
> > is confused because our code to decide whether to proxy a connection
> > or not is super-dumb,
>
> No, I think our proxy is confused because it doesn't know what power
> it has, and it can't tell the server what power it wants to use. That
> problem is independent of the decision to proxy. You're suggesting
> strengthening the code that makes that decision -- adding an oracle
> (in the form of a DBA) that knows about the confusion and actively
> mitigates it. That's guaranteed to work if the oracle is perfect,
> because "perfect" is somewhat tautologically defined as "whatever
> ensures secure operation". But the oracle doesn't reduce the
> confusion, and DBAs aren't perfect.

I think this is the root of our disagreement. My understanding of the
previous discussion is that people think that the major problem here
is the wraparound-to-superuser attack. That is, in general, we expect
that when we connect to a database over the network, we expect it to
do some kind of active authentication, like asking us for a password,
or asking us for an SSL certificate that isn't just lying around for
anyone to use. However, in the specific case of a local connection, we
have a reliable way of knowing who the remote user is without any kind
of active authentication, namely 'peer' authentication or perhaps even
'trust' if we trust all the local users, and so we don't judge it
unreasonable to allow local connections without any form of active
authentication. There can be some scenarios where even over a network
we can know the identity of the person connecting with complete
certainty, e.g. if endpoints are locked down such that the source IP
address is a reliable indicator of who is initiating the connection,
but in general when there's a network involved you don't know who the
person making the connection is and need to do something extra to
figure it out.

If you accept this characterization of the problem, then I don't think
the oracle is that hard to design. We simply set it up not to allow
wraparound connections, or maybe even more narrowly to not allow
wraparound connections to superuser. If the DBA has some weird network
topology where that's not the correct rule, either because they want
to allow wraparound connections or they want to disallow other things,
then yeah they have to tell us what to allow, but I don't really see
why that's an unreasonable expectation. I'd expect the correct
configuration of the proxy facility to fall naturally out of what's
allowed in pg_hba.conf. If machine A is configured to accept
connections from machines B and C based on environmental factors, then
machines B and C should be configured not to proxy connections to A.
If machines B and C aren't under our control such that we can
configure them that way, then the configuration is fundamentally
insecure in a way that we can't really fix.

I think that what you're proposing is that B and C can just be allowed
to proxy to A and A can say "hey, by the way, I'm just gonna let you
in without asking for anything else" and B and C can, when proxying,
react to that by disconnecting before the connection actually goes
through. That's simpler, in a sense. It doesn't require us to set up
the proxy configuration on B and C in a way that matches what
pg_hba.conf allows on A. Instead, B and C can automatically deduce
what connections they should refuse to proxy. I guess that's nice, but
it feels pretty magical to me. It encourages the DBA not to think
about what B and C should actually be allowed to proxy, and instead
just trust that the automatics are going to prevent any security
disasters. I'm not sure that they always will, and I fear cultivating
too much reliance on them. I think that if you're setting up a network
topology where the correct rule is something more complex than "don't
allow wraparound connections to superuser," maybe you ought to be
forced to spell that rule out instead of letting the system deduce one
that you hope will be right.

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






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-11 00:00   ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-20 16:32     ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
@ 2023-03-24 21:47       ` Jacob Champion <[email protected]>
  2023-03-30 12:58         ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-30 18:13         ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Stephen Frost <[email protected]>
  0 siblings, 2 replies; 11+ messages in thread

From: Jacob Champion @ 2023-03-24 21:47 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On 3/20/23 09:32, Robert Haas wrote:
> I think this is the root of our disagreement.

Agreed.

> My understanding of the
> previous discussion is that people think that the major problem here
> is the wraparound-to-superuser attack. That is, in general, we expect
> that when we connect to a database over the network, we expect it to
> do some kind of active authentication, like asking us for a password,
> or asking us for an SSL certificate that isn't just lying around for
> anyone to use. However, in the specific case of a local connection, we
> have a reliable way of knowing who the remote user is without any kind
> of active authentication, namely 'peer' authentication or perhaps even
> 'trust' if we trust all the local users, and so we don't judge it
> unreasonable to allow local connections without any form of active
> authentication. There can be some scenarios where even over a network
> we can know the identity of the person connecting with complete
> certainty, e.g. if endpoints are locked down such that the source IP
> address is a reliable indicator of who is initiating the connection,
> but in general when there's a network involved you don't know who the
> person making the connection is and need to do something extra to
> figure it out.

Okay, but this is walking back from the network example you just
described upthread. Do you still consider that in scope, or...?

> If you accept this characterization of the problem,

I'm not going to say yes or no just yet, because I don't understand your
rationale for where to draw the lines.

If you just want the bare minimum thing that will solve the localhost
case, require_auth landed this week. Login triggers are not yet a thing,
so `require_auth=password,md5,scram-sha-256` ensures active
authentication. You don't even have to disallow localhost connections,
as far as I can tell; they'll work as intended.

If you think login triggers will get in for PG16, my bigger proposal
can't help in time. But if you're drawing the line at "environmental
HBAs are fundamentally unsafe and you shouldn't use them if you have a
proxy," why can't I instead draw the line at "login triggers are
fundamentally unsafe and you shouldn't use them if you have a proxy"?

And if you want to handle the across-the-network case, too, then I don't
accept the characterization of the problem.

> then I don't think
> the oracle is that hard to design. We simply set it up not to allow
> wraparound connections, or maybe even more narrowly to not allow
> wraparound connections to superuser. If the DBA has some weird network
> topology where that's not the correct rule, either because they want
> to allow wraparound connections or they want to disallow other things,
> then yeah they have to tell us what to allow, but I don't really see
> why that's an unreasonable expectation.

This seems like a security model that has been carefully gerrymandered
around the existing implementation. My argument is that the "weird
network topology" isn't weird at all, and it's only dangerous because of
decisions we made (and can unmake).

I feel pretty strongly that the design arrow needs to be pointed in the
opposite direction. The model needs to be chosen first, to prevent us
from saying, "We defend against whatever the implementation lets us
defend against today. Good luck, DBAs."

> If machines B and C aren't under our control such that we can
> configure them that way, then the configuration is fundamentally
> insecure in a way that we can't really fix.

Here's probably our biggest point of contention. You're unlikely to
convince me that this is the DBA's fault.

If machines B and C aren't under our control, then our *protocol* is
fundamentally insecure in a way that we have the ability to fix, in a
way that's already been characterized in security literature.

> I think that what you're proposing is that B and C can just be allowed
> to proxy to A and A can say "hey, by the way, I'm just gonna let you
> in without asking for anything else" and B and C can, when proxying,
> react to that by disconnecting before the connection actually goes
> through. That's simpler, in a sense. It doesn't require us to set up
> the proxy configuration on B and C in a way that matches what
> pg_hba.conf allows on A. Instead, B and C can automatically deduce
> what connections they should refuse to proxy.

Right. It's meant to take the "localhost/wraparound connection" out of a
class of special things we have to worry about, and make it completely
boring.

> I guess that's nice, but
> it feels pretty magical to me. It encourages the DBA not to think
> about what B and C should actually be allowed to proxy, and instead
> just trust that the automatics are going to prevent any security
> disasters.

I agree magical behavior is dangerous, if what you think it can do
doesn't match up with what it can actually do. Bugs are always possible,
and maybe I'm just not seeing a corner case yet, because I'm talking too
much and not coding it -- but is this really a case where I'm
overpromising? Or does it just feel magical because it's meant to fix
the root issue?

(Remember, I'm not arguing against your proxy filter; I just want both.
They complement each other.)

> I'm not sure that they always will, and I fear cultivating
> too much reliance on them.

I can't really argue against this... but I'm not really sure anyone could.

My strawman rephrasing of that is, "we have to make the feature crappy
enough that we can blame the DBA when things go wrong." And even that
strawman could be perfectly reasonable, in situations where the DBA
necessarily has more information than the machine. In this case, though,
it seems to me that the two machines have all the information necessary
to make a correct decision between them.

Thanks!
--Jacob






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-11 00:00   ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-20 16:32     ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-24 21:47       ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
@ 2023-03-30 12:58         ` Robert Haas <[email protected]>
  2023-04-12 18:23           ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  1 sibling, 1 reply; 11+ messages in thread

From: Robert Haas @ 2023-03-30 12:58 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On Fri, Mar 24, 2023 at 5:47 PM Jacob Champion <[email protected]> wrote:
> Okay, but this is walking back from the network example you just
> described upthread. Do you still consider that in scope, or...?

Sorry, I don't know which example you mean.

> > If machines B and C aren't under our control such that we can
> > configure them that way, then the configuration is fundamentally
> > insecure in a way that we can't really fix.
>
> Here's probably our biggest point of contention. You're unlikely to
> convince me that this is the DBA's fault.
>
> If machines B and C aren't under our control, then our *protocol* is
> fundamentally insecure in a way that we have the ability to fix, in a
> way that's already been characterized in security literature.

I guess I wouldn't have a problem blaming the DBA here, but you seem
to be telling me that the security literature has settled on another
kind of approach, and I'm not in a position to dispute that. It still
feels weird to me, though.

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






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-11 00:00   ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-20 16:32     ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-24 21:47       ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-30 12:58         ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
@ 2023-04-12 18:23           ` Jacob Champion <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Jacob Champion @ 2023-04-12 18:23 UTC (permalink / raw)
  To: Robert Haas <[email protected]>; +Cc: Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On 3/30/23 05:58, Robert Haas wrote:
> On Fri, Mar 24, 2023 at 5:47 PM Jacob Champion <[email protected]> wrote:
>> Okay, but this is walking back from the network example you just
>> described upthread. Do you still consider that in scope, or...?
> 
> Sorry, I don't know which example you mean.

The symmetrical proxy situation you described, where all the proxies are
mutually trusting. While it's easier to secure that setup than the
asymmetrical ones, it's also not a localhost-only situation anymore, and
the moment you open up to other machines is where I think your
characterization runs into trouble.

> I guess I wouldn't have a problem blaming the DBA here, but you seem
> to be telling me that the security literature has settled on another
> kind of approach, and I'm not in a position to dispute that. It still
> feels weird to me, though.

If it helps, [1] is a paper that helped me wrap my head around some of
it. It's focused on capability systems and an academic audience, but the
"Avoiding Confused Deputy Problems" section starting on page 11 is a
good place to jump to for the purposes of this discussion.

--Jacob

[1] https://srl.cs.jhu.edu/pubs/SRL2003-02.pdf






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-11 00:00   ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-20 16:32     ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-24 21:47       ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
@ 2023-03-30 18:13         ` Stephen Frost <[email protected]>
  2023-04-12 18:24           ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  1 sibling, 1 reply; 11+ messages in thread

From: Stephen Frost @ 2023-03-30 18:13 UTC (permalink / raw)
  To: Jacob Champion <[email protected]>; +Cc: Robert Haas <[email protected]>; Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

Greetings,

* Jacob Champion ([email protected]) wrote:
> On 3/20/23 09:32, Robert Haas wrote:
> > I think this is the root of our disagreement.
> 
> Agreed.

I've read all the way back to the $SUBJECT change to try and get an
understanding of the questions here and it's not been easy, in part, I
think, due to the verbiage but also the perhaps lack of concrete
examples and instead references to other systems and protocols.

> > My understanding of the
> > previous discussion is that people think that the major problem here
> > is the wraparound-to-superuser attack. That is, in general, we expect
> > that when we connect to a database over the network, we expect it to
> > do some kind of active authentication, like asking us for a password,
> > or asking us for an SSL certificate that isn't just lying around for
> > anyone to use. However, in the specific case of a local connection, we
> > have a reliable way of knowing who the remote user is without any kind
> > of active authentication, namely 'peer' authentication or perhaps even
> > 'trust' if we trust all the local users, and so we don't judge it
> > unreasonable to allow local connections without any form of active
> > authentication. There can be some scenarios where even over a network
> > we can know the identity of the person connecting with complete
> > certainty, e.g. if endpoints are locked down such that the source IP
> > address is a reliable indicator of who is initiating the connection,
> > but in general when there's a network involved you don't know who the
> > person making the connection is and need to do something extra to
> > figure it out.
> 
> Okay, but this is walking back from the network example you just
> described upthread. Do you still consider that in scope, or...?

The concern around the network certainly needs to be in-scope overall.

> > If you accept this characterization of the problem,
> 
> I'm not going to say yes or no just yet, because I don't understand your
> rationale for where to draw the lines.
> 
> If you just want the bare minimum thing that will solve the localhost
> case, require_auth landed this week. Login triggers are not yet a thing,
> so `require_auth=password,md5,scram-sha-256` ensures active
> authentication. You don't even have to disallow localhost connections,
> as far as I can tell; they'll work as intended.

I do think require_auth helps us move in a positive direction.  As I
mentioned elsewhere, I don't think we highlight it nearly enough in the
postgres_fdw documentation.  Let's look at that in a bit more depth with
concrete examples and perhaps everyone will be able to get a bit more
understanding of the issues.

Client is psql
Proxy is some PG server that's got postgres_fdw
Target is another PG server, that is being connected to from Proxy
Authentication is via GSS/Kerberos with proxied credentials

What do we want to require the user to configure to make this secure?

Proxy's pg_hba configured to require GSS auth from Client.
Target's pg_hba configured to require GSS auth from Proxy.

Who are we trusting with what?  In particular, I'd argue that the user
who is able to install the postgres_fdw extension and the user who is
able to issue the CREATE SERVER are largely trusted; at least in so far
as the user doing CREATE SERVER is allowed to create the server and
through that allowed to make outbound connections from the Proxy.

Therefore, the Proxy is configured with postgres_fdw and with a trusted
user performing the CREATE SERVER.

What doesn't this handle today?  Connection side-effects are one
problem- once the CREATE SERVER is done, any user with USAGE rights on
the server can create a USER MAPPING for themselves, either with a
password or without one (if they're able to proxy GSS credentials to the
system).  They aren't able to set password_required though, which
defaults to true.  However, without having require_auth set, they're
able to cause the Proxy to reach an authentication stage with the Target
that might not match what credentials they're supposed to be providing.

We attempt to address this by checking post-auth to Target that we used
the credentials to connect that we expected to- if GSS credentials were
proxied, then we expect to use those.  If a password was provided then
we expect to use a password to auth (only checked after we see if GSS
credentials were proxied and used).  The issue here is 'post-auth' bit,
we'd prefer to fail the connection pre-auth if it isn't what we're
expecting.  Should we then explicit set require_auth=gss when GSS
credentials are proxied?  Also, if a password is provided, then
explicitly set require_auth=scram-sha-256?  Or default to these, at
least, and allow the CREATE SERVER user to override our choices?  Or
should it be a USER MAPPING option that's restricted?  Or not?

> > I think that what you're proposing is that B and C can just be allowed
> > to proxy to A and A can say "hey, by the way, I'm just gonna let you
> > in without asking for anything else" and B and C can, when proxying,
> > react to that by disconnecting before the connection actually goes
> > through. That's simpler, in a sense. It doesn't require us to set up
> > the proxy configuration on B and C in a way that matches what
> > pg_hba.conf allows on A. Instead, B and C can automatically deduce
> > what connections they should refuse to proxy.
> 
> Right. It's meant to take the "localhost/wraparound connection" out of a
> class of special things we have to worry about, and make it completely
> boring.

Again, trying to get at a more concrete example- the concern here is a
user with CREATE SERVER ability could leverage that access to become a
superuser if the system is configured with 'peer' access, right?  A
non-superuser is already prevented from being able to set
"password_required=false", perhaps we shouldn't allow them to set
"require_auth=none" (or have that effect) either?  Perhaps the system
should simply forcibly set require_auth based on the credentials
provided in the USER MAPPING or on the connection and have require_auth
otherwise restricted to superuser (who could override it if they'd
really like to)?  Perhaps if password_required=false we implicitly
un-set require_auth, to avoid having to make superusers change their
existing configurations where they've clearly already accepted that
credential-less connections are allowed.

Automatically setting require_auth and restricting the ability of it to
be set on user mappings to superusers doesn't strike me as terribly
difficult to do and seems like it'd prevent this concern.

Just to make sure I'm following- Robert's up-thread suggestion of an
'outbound pg_hba' would be an additional restriction when it comes to
what a user who can use CREATE SERVER is allowed to do?  I'm not against
the idea of having a way to lock that down.. but it's another level of
complication certainly and I'm not sure that some external config file
or such is the best way to try and deal with that, though I do see how
it can have some appeal for certain environments.  It does overall
strike me as something we've not tried to address in any way thus far
and a pretty large effort that's not likely to make it into PG16, unlike
the possibility of auto-setting require_auth, now that it exists.

Thanks!

Stephen


Attachments:

  [application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security
  2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-09 14:17 ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-11 00:00   ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-20 16:32     ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Robert Haas <[email protected]>
  2023-03-24 21:47       ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
  2023-03-30 18:13         ` Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Stephen Frost <[email protected]>
@ 2023-04-12 18:24           ` Jacob Champion <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Jacob Champion @ 2023-04-12 18:24 UTC (permalink / raw)
  To: Stephen Frost <[email protected]>; +Cc: Robert Haas <[email protected]>; Andres Freund <[email protected]>; Mark Dilger <[email protected]>; Jeff Davis <[email protected]>; Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers

On 3/30/23 11:13, Stephen Frost wrote:
>> Okay, but this is walking back from the network example you just
>> described upthread. Do you still consider that in scope, or...?
> 
> The concern around the network certainly needs to be in-scope overall.

Sounds good!

> Who are we trusting with what?  In particular, I'd argue that the user
> who is able to install the postgres_fdw extension and the user who is
> able to issue the CREATE SERVER are largely trusted; at least in so far
> as the user doing CREATE SERVER is allowed to create the server and
> through that allowed to make outbound connections from the Proxy.
> 
> Therefore, the Proxy is configured with postgres_fdw and with a trusted
> user performing the CREATE SERVER.
> 
> What doesn't this handle today?  Connection side-effects are one
> problem- once the CREATE SERVER is done, any user with USAGE rights on
> the server can create a USER MAPPING for themselves, either with a
> password or without one (if they're able to proxy GSS credentials to the
> system).  They aren't able to set password_required though, which
> defaults to true.  However, without having require_auth set, they're
> able to cause the Proxy to reach an authentication stage with the Target
> that might not match what credentials they're supposed to be providing.
> 
> We attempt to address this by checking post-auth to Target that we used
> the credentials to connect that we expected to- if GSS credentials were
> proxied, then we expect to use those.  If a password was provided then
> we expect to use a password to auth (only checked after we see if GSS
> credentials were proxied and used).  The issue here is 'post-auth' bit,
> we'd prefer to fail the connection pre-auth if it isn't what we're
> expecting.

Right. Keep in mind that require_auth is post-auth, though; it can't fix
that issue, so it doesn't fix any connection side-effect problems at all.

> Should we then explicit set require_auth=gss when GSS
> credentials are proxied?  Also, if a password is provided, then
> explicitly set require_auth=scram-sha-256?  Or default to these, at
> least, and allow the CREATE SERVER user to override our choices?  Or
> should it be a USER MAPPING option that's restricted?  Or not?
IMO, yes -- whatever credentials the proxy is forwarding from the user,
the proxy should be checking that the server has actually used them. The
person with the ability to create a USER MAPPING should probably not
have the ability to override that check.

>>> I think that what you're proposing is that B and C can just be allowed
>>> to proxy to A and A can say "hey, by the way, I'm just gonna let you
>>> in without asking for anything else" and B and C can, when proxying,
>>> react to that by disconnecting before the connection actually goes
>>> through. That's simpler, in a sense. It doesn't require us to set up
>>> the proxy configuration on B and C in a way that matches what
>>> pg_hba.conf allows on A. Instead, B and C can automatically deduce
>>> what connections they should refuse to proxy.
>>
>> Right. It's meant to take the "localhost/wraparound connection" out of a
>> class of special things we have to worry about, and make it completely
>> boring.
> 
> Again, trying to get at a more concrete example- the concern here is a
> user with CREATE SERVER ability could leverage that access to become a
> superuser if the system is configured with 'peer' access, right?

Or 'trust localhost', or 'ident [postgres user]', yes.

> A
> non-superuser is already prevented from being able to set
> "password_required=false", perhaps we shouldn't allow them to set
> "require_auth=none" (or have that effect) either?

I think that sounds reasonable.

> Perhaps the system
> should simply forcibly set require_auth based on the credentials
> provided in the USER MAPPING or on the connection and have require_auth
> otherwise restricted to superuser (who could override it if they'd
> really like to)?  Perhaps if password_required=false we implicitly
> un-set require_auth, to avoid having to make superusers change their
> existing configurations where they've clearly already accepted that
> credential-less connections are allowed.

Mm, I think I like the first idea better. If you've set a password,
wouldn't you like to know if the server ignored it? If password_required
is false, *and* you don't have a password, then we can drop require_auth
without issue.

> Automatically setting require_auth and restricting the ability of it to
> be set on user mappings to superusers doesn't strike me as terribly
> difficult to do and seems like it'd prevent this concern.
> 
> Just to make sure I'm following- Robert's up-thread suggestion of an
> 'outbound pg_hba' would be an additional restriction when it comes to
> what a user who can use CREATE SERVER is allowed to do?

Yes. That can provide additional safety in the case where you really
need to take the require_auth checks away for whatever reason. I think
it's just a good in-depth measure, and if we don't extend the protocol
in some way to do a pre-auth check, it's also the way for the DBA to
bless known-good connection paths.

Thanks,
--Jacob






^ permalink  raw  reply  [nested|flat] 11+ messages in thread

* [PATCH v49 2/7] Rename cluster.c/h -> repack.c/h
@ 2026-03-31 16:55 Álvaro Herrera <[email protected]>
  0 siblings, 0 replies; 11+ messages in thread

From: Álvaro Herrera @ 2026-03-31 16:55 UTC (permalink / raw)

---
 src/backend/commands/Makefile                |  2 +-
 src/backend/commands/matview.c               |  2 +-
 src/backend/commands/meson.build             |  2 +-
 src/backend/commands/{cluster.c => repack.c} |  6 +++---
 src/backend/commands/tablecmds.c             |  2 +-
 src/backend/commands/vacuum.c                |  6 +++---
 src/backend/storage/ipc/procsignal.c         |  1 +
 src/backend/tcop/postgres.c                  |  1 +
 src/backend/tcop/utility.c                   |  2 +-
 src/include/commands/{cluster.h => repack.h} | 12 ++++++------
 10 files changed, 19 insertions(+), 17 deletions(-)
 rename src/backend/commands/{cluster.c => repack.c} (99%)
 rename src/include/commands/{cluster.h => repack.h} (90%)

diff --git a/src/backend/commands/Makefile b/src/backend/commands/Makefile
index c10fdba2bbb..fe1bba3a9b9 100644
--- a/src/backend/commands/Makefile
+++ b/src/backend/commands/Makefile
@@ -18,7 +18,6 @@ OBJS = \
 	amcmds.o \
 	analyze.o \
 	async.o \
-	cluster.o \
 	collationcmds.o \
 	comment.o \
 	constraint.o \
@@ -51,6 +50,7 @@ OBJS = \
 	proclang.o \
 	propgraphcmds.o \
 	publicationcmds.o \
+	repack.o \
 	schemacmds.o \
 	seclabel.o \
 	sequence.o \
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index d3be8939011..5db4fe75dce 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -24,8 +24,8 @@
 #include "catalog/namespace.h"
 #include "catalog/pg_am.h"
 #include "catalog/pg_opclass.h"
-#include "commands/cluster.h"
 #include "commands/matview.h"
+#include "commands/repack.h"
 #include "commands/tablecmds.h"
 #include "commands/tablespace.h"
 #include "executor/executor.h"
diff --git a/src/backend/commands/meson.build b/src/backend/commands/meson.build
index 90c7e37a429..f624aae74af 100644
--- a/src/backend/commands/meson.build
+++ b/src/backend/commands/meson.build
@@ -6,7 +6,6 @@ backend_sources += files(
   'amcmds.c',
   'analyze.c',
   'async.c',
-  'cluster.c',
   'collationcmds.c',
   'comment.c',
   'constraint.c',
@@ -39,6 +38,7 @@ backend_sources += files(
   'proclang.c',
   'propgraphcmds.c',
   'publicationcmds.c',
+  'repack.c',
   'schemacmds.c',
   'seclabel.c',
   'sequence.c',
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/repack.c
similarity index 99%
rename from src/backend/commands/cluster.c
rename to src/backend/commands/repack.c
index f241e18b153..20f0a572236 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/repack.c
@@ -1,6 +1,6 @@
 /*-------------------------------------------------------------------------
  *
- * cluster.c
+ * repack.c
  *    REPACK a table; formerly known as CLUSTER.  VACUUM FULL also uses
  *    parts of this code.
  *
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  src/backend/commands/cluster.c
+ *	  src/backend/commands/repack.c
  *
  *-------------------------------------------------------------------------
  */
@@ -33,9 +33,9 @@
 #include "catalog/pg_am.h"
 #include "catalog/pg_inherits.h"
 #include "catalog/toasting.h"
-#include "commands/cluster.h"
 #include "commands/defrem.h"
 #include "commands/progress.h"
+#include "commands/repack.h"
 #include "commands/tablecmds.h"
 #include "commands/vacuum.h"
 #include "miscadmin.h"
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0ce2e81f9c2..e2882a50b3b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -57,10 +57,10 @@
 #include "catalog/storage.h"
 #include "catalog/storage_xlog.h"
 #include "catalog/toasting.h"
-#include "commands/cluster.h"
 #include "commands/comment.h"
 #include "commands/defrem.h"
 #include "commands/event_trigger.h"
+#include "commands/repack.h"
 #include "commands/sequence.h"
 #include "commands/tablecmds.h"
 #include "commands/tablespace.h"
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 0ed363d1c85..b179b62b5c8 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -9,7 +9,7 @@
  *
  * VACUUM for heap AM is implemented in vacuumlazy.c, parallel vacuum in
  * vacuumparallel.c, ANALYZE in analyze.c, and VACUUM FULL is a variant of
- * CLUSTER, handled in cluster.c.
+ * REPACK, handled in repack.c.
  *
  *
  * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
@@ -38,9 +38,9 @@
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
 #include "commands/async.h"
-#include "commands/cluster.h"
 #include "commands/defrem.h"
 #include "commands/progress.h"
+#include "commands/repack.h"
 #include "commands/vacuum.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -2293,7 +2293,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams params,
 			if ((params.options & VACOPT_VERBOSE) != 0)
 				cluster_params.options |= CLUOPT_VERBOSE;
 
-			/* VACUUM FULL is a variant of REPACK; see cluster.c */
+			/* VACUUM FULL is a variant of REPACK; see repack.c */
 			cluster_rel(REPACK_COMMAND_VACUUMFULL, rel, InvalidOid,
 						&cluster_params);
 			/* cluster_rel closes the relation, but keeps lock */
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 7e017c8d53b..7cef6e43661 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -19,6 +19,7 @@
 
 #include "access/parallel.h"
 #include "commands/async.h"
+#include "commands/repack.h"
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "port/pg_bitutils.h"
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 10be60011ad..9fbaa5c00f0 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -39,6 +39,7 @@
 #include "commands/event_trigger.h"
 #include "commands/explain_state.h"
 #include "commands/prepare.h"
+#include "commands/repack.h"
 #include "common/pg_prng.h"
 #include "jit/jit.h"
 #include "libpq/libpq.h"
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 2b609bfc824..5f8c766c4be 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -26,7 +26,6 @@
 #include "catalog/toasting.h"
 #include "commands/alter.h"
 #include "commands/async.h"
-#include "commands/cluster.h"
 #include "commands/collationcmds.h"
 #include "commands/comment.h"
 #include "commands/conversioncmds.h"
@@ -46,6 +45,7 @@
 #include "commands/proclang.h"
 #include "commands/propgraphcmds.h"
 #include "commands/publicationcmds.h"
+#include "commands/repack.h"
 #include "commands/schemacmds.h"
 #include "commands/seclabel.h"
 #include "commands/sequence.h"
diff --git a/src/include/commands/cluster.h b/src/include/commands/repack.h
similarity index 90%
rename from src/include/commands/cluster.h
rename to src/include/commands/repack.h
index d6b62c747e8..85061158b0c 100644
--- a/src/include/commands/cluster.h
+++ b/src/include/commands/repack.h
@@ -1,17 +1,17 @@
 /*-------------------------------------------------------------------------
  *
- * cluster.h
- *	  header file for postgres cluster command stuff
+ * repack.h
+ *	  header file for the REPACK command
  *
  * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994-5, Regents of the University of California
  *
- * src/include/commands/cluster.h
+ * src/include/commands/repack.h
  *
  *-------------------------------------------------------------------------
  */
-#ifndef CLUSTER_H
-#define CLUSTER_H
+#ifndef REPACK_H
+#define REPACK_H
 
 #include "nodes/parsenodes.h"
 #include "parser/parse_node.h"
@@ -52,4 +52,4 @@ extern void finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
 							 MultiXactId cutoffMulti,
 							 char newrelpersistence);
 
-#endif							/* CLUSTER_H */
+#endif							/* REPACK_H */
-- 
2.47.3


--3n3vqr5y2jhknrqj
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="v49-0003-Add-CONCURRENTLY-option-to-REPACK-command.patch"



^ permalink  raw  reply  [nested|flat] 11+ messages in thread


end of thread, other threads:[~2026-03-31 16:55 UTC | newest]

Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-03-08 22:44 Re: postgres_fdw, dblink, and CREATE SUBSCRIPTION security Jacob Champion <[email protected]>
2023-03-09 14:12 ` Robert Haas <[email protected]>
2023-03-09 14:17 ` Robert Haas <[email protected]>
2023-03-11 00:00   ` Jacob Champion <[email protected]>
2023-03-20 16:32     ` Robert Haas <[email protected]>
2023-03-24 21:47       ` Jacob Champion <[email protected]>
2023-03-30 12:58         ` Robert Haas <[email protected]>
2023-04-12 18:23           ` Jacob Champion <[email protected]>
2023-03-30 18:13         ` Stephen Frost <[email protected]>
2023-04-12 18:24           ` Jacob Champion <[email protected]>
2026-03-31 16:55 [PATCH v49 2/7] Rename cluster.c/h -> repack.c/h Álvaro Herrera <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox