pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Bizarre behavior in libpq's searching of ~/.pgpass
Date: Fri, 27 Jul 2018 23:38:57 -0400
Message-ID: <30805.1532749137@sss.pgh.pa.us> (raw)

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




view thread (6+ messages)  latest in thread

Message-ID: <30805.1532749137@sss.pgh.pa.us>
Permalink:  ../30805.1532749137@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/30805.1532749137@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
  Subject: Re: Bizarre behavior in libpq's searching of ~/.pgpass
  In-Reply-To: <30805.1532749137@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

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