Bizarre behavior in libpq's searching of ~/.pgpass

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Bizarre behavior in libpq's searching of ~/.pgpass
Date: 2018-07-28 03:38:57
Message-ID: 30805.1532749137@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I noticed that there's some strange coding in libpq's choice of
what hostname to use for searching ~/.pgpass for a password.
Historically (pre-v10), it just used the pghost parameter:

conn->pgpass = PasswordFromFile(conn->pghost, conn->pgport,
conn->dbName, conn->pguser);

no ifs, ands, or buts, except for the fact that PasswordFromFile
replaces its hostname parameter with "localhost" if it's null or
matches the default socket directory. This is per the documentation
(see sections 34.1.2 and 34.15).

Since v10 we've got this:

char *pwhost = conn->connhost[i].host;

if (conn->connhost[i].type == CHT_HOST_ADDRESS &&
conn->connhost[i].host != NULL &&
conn->connhost[i].host[0] != '\0')
pwhost = conn->connhost[i].hostaddr;

conn->connhost[i].password =
passwordFromFile(pwhost,
conn->connhost[i].port,
conn->dbName,
conn->pguser,
conn->pgpassfile);

Now that's just bizarre on its face: take hostaddr if it's specified,
but only if host is also specified? And it certainly doesn't match
the documentation.

It's easy to demonstrate by testing that if you specify both host and
hostaddr, the search behavior is different now than it was pre-v10.
Given the lack of documentation change to match, that seems like a bug.

Digging in the git history, it seems this was inadvertently broken by
commit 274bb2b38, which allowed multiple entries in pghost but not
pghostaddr, with some rather inconsistent redefinitions of what the
internal values were. Commit bdac9836d tried to repair it, but it was
dependent on said inconsistency, and got broken again in commit 7b02ba62e
which allowed multiple hostaddrs and removed the inconsistent internal
representation. At no point did anyone change the docs.

So my first thought was that we should go back to the pre-v10 behavior
of considering only the host parameter, which it looks like would only
require removing the "if" bit above.

But on second thought, I'm not clear that the pre-v10 behavior is really
all that sane either. What it means is that if you specify only hostaddr,
the code will happily grab your localhost password and send it off to
whatever server hostaddr references. This is unlikely to be helpful,
and it could even be painted as a security breach --- the remote server
could ask for your password in plaintext and then capture it.

What seems like a saner definition is "use host if it's specified
(nonempty), else use hostaddr if it's specified (nonempty), else
fall back to localhost". That avoids sending a password somewhere
it doesn't belong, and allows a useful ~/.pgpass lookup in cases
where only hostaddr is given -- you just need to make an entry
with the numeric IP address in the host column.

I think it's not too late to make v11 work that way, but I wonder
what we ought to do in v10. Comments?

regards, tom lane

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2018-07-28 06:14:11 Re: Explain buffers wrong counter with parallel plans
Previous Message Tatsuo Ishii 2018-07-28 03:03:07 Re: Removing useless \. at the end of copy in pgbench