Re: [patch] fix dblink security hole

Joe Conway <mail(at)joeconway(dot)com> writes:
> I took a look and can partially see Marko's point. The scenario exists
> within this context:

> 1. "superuser" installs dblink on db1, running on postgres server
> under the "superuser" account
> 2. "superuser" has .pgpass file
> 3. the "superuser" .pgpass file is set up with wildcards, e.g.
> *:*:*:postgres:mypassword
> 4. "superuser" creates login for "luser" in db1

> This depends on "superuser" to not only make use of .pgpass, but
> specifically to use it in an insecure way, i.e. using wildcards to
> specify that the login credentials should be sent to any arbitrary
> Postgres installation.

It seems to me that this is a pretty far-fetched scenario; someone
who'd set up his .pgpass that way would be at risk from his own typos,
not just from nefarious users. I'm not sure how far out of our way we
need to go to protect stupid DBAs. But anyway:

The main thing that bothers me about the proposed patch is that it takes
away the security mechanism that existed before. Now you have either no
trust or 100% trust, you don't have the option to trust people who know
a password. That's less secure, not more, if you ask me. Marko's
original patch is just as bad.

If I understand the complaint correctly, it is not that a luser can make
a connection, it is that the password will be sent before dblink rejects
the connection. So really this problem is not specific to dblink ---
what it's saying is that PQconnectionUsedPassword is broken by design
and we should deprecate using that for security purposes.

I think there is an alternative solution, if we are only going to patch
this in 8.4 and up: provide a new libpq conninfo-string option saying
not to use .pgpass, and have dblink add that to the passed-in conninfo
string instead of trying to check after the fact. Then we aren't
changing dblink's API at all, only replacing a leaky security check
with a better one.

regards, tom lane