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

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

From
Tom Lane
Date:
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


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

From
Tom Lane
Date:
I wrote
> I noticed that there's some strange coding in libpq's choice of
> what hostname to use for searching ~/.pgpass for a password.
> ...

> 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?

Here's a proposed patch to adopt that behavior.  I'm still of mixed
mind whether to push this into v10 ... but we definitely need some
change in v10, because it's not acting as per its docs.

            regards, tom lane

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d67212b..0bcaeb9 100644
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 1020,1026 ****
          </itemizedlist>
          Note that authentication is likely to fail if <literal>host</literal>
          is not the name of the server at network address <literal>hostaddr</literal>.
!         Also, note that <literal>host</literal> rather than <literal>hostaddr</literal>
          is used to identify the connection in a password file (see
          <xref linkend="libpq-pgpass"/>).
         </para>
--- 1020,1027 ----
          </itemizedlist>
          Note that authentication is likely to fail if <literal>host</literal>
          is not the name of the server at network address <literal>hostaddr</literal>.
!         Also, when both <literal>host</literal> and <literal>hostaddr</literal>
!         are specified, <literal>host</literal>
          is used to identify the connection in a password file (see
          <xref linkend="libpq-pgpass"/>).
         </para>
*************** myEventProc(PGEventId evtId, void *evtIn
*** 7521,7533 ****
     used.  (Therefore, put more-specific entries first when you are using
     wildcards.) If an entry needs to contain <literal>:</literal> or
     <literal>\</literal>, escape this character with <literal>\</literal>.
!    A host name of <literal>localhost</literal> matches both TCP (host name
!    <literal>localhost</literal>) and Unix domain socket (<literal>pghost</literal> empty
!    or the default socket directory) connections coming from the local
!    machine. In a standby server, a database name of <literal>replication</literal>
     matches streaming replication connections made to the master server.
!    The <literal>database</literal> field is of limited usefulness because
!    users have the same password for all databases in the same cluster.
    </para>

    <para>
--- 7522,7538 ----
     used.  (Therefore, put more-specific entries first when you are using
     wildcards.) If an entry needs to contain <literal>:</literal> or
     <literal>\</literal>, escape this character with <literal>\</literal>.
!    The host name field is matched to the <literal>host</literal> connection
!    parameter if that is specified, otherwise to
!    the <literal>hostaddr</literal> parameter if that is specified; if neither
!    are given then the host name <literal>localhost</literal> is searched for.
!    A host name field of <literal>localhost</literal> will also match
!    Unix-domain socket connections when the <literal>host</literal> parameter
!    equals the installation's default socket directory path.
!    In a standby server, a database name of <literal>replication</literal>
     matches streaming replication connections made to the master server.
!    The <literal>database</literal> field is of limited usefulness otherwise,
!    because users have the same password for all databases in the same cluster.
    </para>

    <para>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index bd7dac1..c4d27c4 100644
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** connectOptions2(PGconn *conn)
*** 1065,1072 ****
      }

      /*
!      * Supply default password if none given.  Note that the password might be
!      * different for each host/port pair.
       */
      if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
      {
--- 1065,1072 ----
      }

      /*
!      * If password was not given, try to look it up in password file.  Note
!      * that the result might be different for each host/port pair.
       */
      if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
      {
*************** connectOptions2(PGconn *conn)
*** 1094,1108 ****
              for (i = 0; i < conn->nconnhost; i++)
              {
                  /*
!                  * Try to get a password for this host from pgpassfile. We use
!                  * host name rather than host address in the same manner as
!                  * PQhost().
                   */
!                 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 =
--- 1094,1107 ----
              for (i = 0; i < conn->nconnhost; i++)
              {
                  /*
!                  * Try to get a password for this host from file.  We use host
!                  * if given, else hostaddr if given; if both are omitted then
!                  * the search will be for "localhost".  (That's handled inside
!                  * passwordFromFile.)
                   */
!                 const char *pwhost = conn->connhost[i].host;

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

                  conn->connhost[i].password =
*************** passwordFromFile(const char *hostname, c
*** 6385,6398 ****
  #define LINELEN NAMEDATALEN*5
      char        buf[LINELEN];

!     if (dbname == NULL || strlen(dbname) == 0)
          return NULL;

!     if (username == NULL || strlen(username) == 0)
          return NULL;

      /* 'localhost' matches pghost of '' or the default socket directory */
!     if (hostname == NULL)
          hostname = DefaultHost;
      else if (is_absolute_path(hostname))

--- 6384,6397 ----
  #define LINELEN NAMEDATALEN*5
      char        buf[LINELEN];

!     if (dbname == NULL || dbname[0] == '\0')
          return NULL;

!     if (username == NULL || username[0] == '\0')
          return NULL;

      /* 'localhost' matches pghost of '' or the default socket directory */
!     if (hostname == NULL || hostname[0] == '\0')
          hostname = DefaultHost;
      else if (is_absolute_path(hostname))

*************** passwordFromFile(const char *hostname, c
*** 6403,6409 ****
          if (strcmp(hostname, DEFAULT_PGSOCKET_DIR) == 0)
              hostname = DefaultHost;

!     if (port == NULL)
          port = DEF_PGPORT_STR;

      /* If password file cannot be opened, ignore it. */
--- 6402,6408 ----
          if (strcmp(hostname, DEFAULT_PGSOCKET_DIR) == 0)
              hostname = DefaultHost;

!     if (port == NULL || port[0] == '\0')
          port = DEF_PGPORT_STR;

      /* If password file cannot be opened, ignore it. */

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

From
Tomas Vondra
Date:
On 07/29/2018 11:15 PM, Tom Lane wrote:
> Here's a proposed patch to adopt that behavior.  I'm still of mixed
> mind whether to push this into v10 ... but we definitely need some
> change in v10, because it's not acting as per its docs.

Is there actually a useful use case working in v10 and broken by your
proposed patch? I can't think of any, and even if there is one it's
likely no one is aware of it as the docs were not updated. Considering
this could also be painted as a security issue (people generally don't
connect from servers to random machines, but still ...), I'd vote for
pushing this to v10.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

From
Robert Haas
Date:
On Fri, Jul 27, 2018 at 11:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.

Yeah, that's bad code.  The intent was that if you set host=a,b you
probably want to use either 'a' or 'b' as the thing to look up in
.pgpass, not 'a,b', but the implementation leaves something to be
desired.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

From
Tom Lane
Date:
Robert Haas <robertmhaas@gmail.com> writes:
> On Fri, Jul 27, 2018 at 11:38 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I noticed that there's some strange coding in libpq's choice of
>> what hostname to use for searching ~/.pgpass for a password.

> Yeah, that's bad code.  The intent was that if you set host=a,b you
> probably want to use either 'a' or 'b' as the thing to look up in
> .pgpass, not 'a,b', but the implementation leaves something to be
> desired.

Right.  Closely related to that is that the existing code in
passwordFromFile behaves differently for null vs. empty hostname.
This is bad on its face, because nowhere else in libpq do we treat
empty-string parameters differently from unset ones, but it's particularly
a mess for the comma-separated-list case because then it's impossible
for one of the alternatives to be NULL.  In the already-posted patch
I fixed that so that an empty alternative is replaced by DefaultHost
in passwordFromFile, and likewise for port (though I think the latter
case may be unreachable).

But now that I look at it, it seems like the code in connectOptions2
has also Gotten It Wrong.  Shouldn't the replacement of "unspecified"
cases by DEFAULT_PGSOCKET_DIR/DefaultHost also happen on an entry-by-
entry basis, so that "host=foo," would behave as though the empty
entry were "localhost"?

            regards, tom lane


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

From
Tom Lane
Date:
I wrote:
> But now that I look at it, it seems like the code in connectOptions2
> has also Gotten It Wrong.  Shouldn't the replacement of "unspecified"
> cases by DEFAULT_PGSOCKET_DIR/DefaultHost also happen on an entry-by-
> entry basis, so that "host=foo," would behave as though the empty
> entry were "localhost"?

Here's an updated patch that fixes that aspect too.  Although this
might seem independent, it's not really: this version of the patch
eliminates the corner case where we have neither a host or hostaddr
to search for.  The check for empty hostname in passwordFromFile is
thereby dead code, though it seemed better style to leave it in.

(Basically, the point here is to guarantee that passwordFromFile has
the same idea of what host/port we are going to connect to as the
actual connection code does.  That was not true before, either for
the host or the port :-()

            regards, tom lane

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d67212b..0b1ffb7 100644
*** a/doc/src/sgml/libpq.sgml
--- b/doc/src/sgml/libpq.sgml
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 938,945 ****
       <para>
         If a password file is used, you can have different passwords for
         different hosts. All the other connection options are the same for every
!        host, it is not possible to e.g. specify a different username for
!        different hosts.
       </para>
     </sect3>
    </sect2>
--- 938,945 ----
       <para>
         If a password file is used, you can have different passwords for
         different hosts. All the other connection options are the same for every
!        host in the list; it is not possible to e.g. specify different
!        usernames for different hosts.
       </para>
     </sect3>
    </sect2>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 961,967 ****
          name of the directory in which the socket file is stored.  If
          multiple host names are specified, each will be tried in turn in
          the order given.  The default behavior when <literal>host</literal> is
!         not specified is to connect to a Unix-domain
          socket<indexterm><primary>Unix domain socket</primary></indexterm> in
          <filename>/tmp</filename> (or whatever socket directory was specified
          when <productname>PostgreSQL</productname> was built). On machines without
--- 961,967 ----
          name of the directory in which the socket file is stored.  If
          multiple host names are specified, each will be tried in turn in
          the order given.  The default behavior when <literal>host</literal> is
!         not specified, or is empty, is to connect to a Unix-domain
          socket<indexterm><primary>Unix domain socket</primary></indexterm> in
          <filename>/tmp</filename> (or whatever socket directory was specified
          when <productname>PostgreSQL</productname> was built). On machines without
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 969,975 ****
         </para>
         <para>
          A comma-separated list of host names is also accepted, in which case
!         each host name in the list is tried in order. See
          <xref linkend="libpq-multiple-hosts"/> for details.
         </para>
        </listitem>
--- 969,976 ----
         </para>
         <para>
          A comma-separated list of host names is also accepted, in which case
!         each host name in the list is tried in order; an empty item in the
!         list selects the default behavior as explained above. See
          <xref linkend="libpq-multiple-hosts"/> for details.
         </para>
        </listitem>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 1020,1033 ****
          </itemizedlist>
          Note that authentication is likely to fail if <literal>host</literal>
          is not the name of the server at network address <literal>hostaddr</literal>.
!         Also, note that <literal>host</literal> rather than <literal>hostaddr</literal>
          is used to identify the connection in a password file (see
          <xref linkend="libpq-pgpass"/>).
         </para>

         <para>
          A comma-separated list of <literal>hostaddr</literal> values is also
!         accepted, in which case each host in the list is tried in order. See
          <xref linkend="libpq-multiple-hosts"/> for details.
         </para>
         <para>
--- 1021,1037 ----
          </itemizedlist>
          Note that authentication is likely to fail if <literal>host</literal>
          is not the name of the server at network address <literal>hostaddr</literal>.
!         Also, when both <literal>host</literal> and <literal>hostaddr</literal>
!         are specified, <literal>host</literal>
          is used to identify the connection in a password file (see
          <xref linkend="libpq-pgpass"/>).
         </para>

         <para>
          A comma-separated list of <literal>hostaddr</literal> values is also
!         accepted, in which case each host in the list is tried in order.
!         An empty item in the list causes the corresponding host name to be
!         used, or the default host name if that is empty as well. See
          <xref linkend="libpq-multiple-hosts"/> for details.
         </para>
         <para>
*************** postgresql://%2Fvar%2Flib%2Fpostgresql/d
*** 1047,1055 ****
          name extension for Unix-domain
          connections.<indexterm><primary>port</primary></indexterm>
          If multiple hosts were given in the <literal>host</literal> or
!         <literal>hostaddr</literal> parameters, this parameter may specify a list
!         of ports of equal length, or it may specify a single port number to
!         be used for all hosts.
         </para>
        </listitem>
       </varlistentry>
--- 1051,1062 ----
          name extension for Unix-domain
          connections.<indexterm><primary>port</primary></indexterm>
          If multiple hosts were given in the <literal>host</literal> or
!         <literal>hostaddr</literal> parameters, this parameter may specify a
!         comma-separated list of ports of the same length as the host list, or
!         it may specify a single port number to be used for all hosts.
!         An empty string, or an empty item in a comma-separated list,
!         specifies the default port number established
!         when <productname>PostgreSQL</productname> was built.
         </para>
        </listitem>
       </varlistentry>
*************** char *PQuser(const PGconn *conn);
*** 1683,1688 ****
--- 1690,1706 ----
  char *PQpass(const PGconn *conn);
  </synopsis>
        </para>
+
+       <para>
+        <function>PQpass</function> will return either the password specified
+        in the connection parameters, or if there was none and the password
+        was obtained from the <link linkend="libpq-pgpass">password
+        file</link>, it will return that.  In the latter case,
+        if multiple hosts were specified in the connection parameters, it is
+        not possible to rely on the result of <function>PQpass</function> until
+        the connection is established.  The status of the connection can be
+        checked using the function <function>PQstatus</function>.
+       </para>
       </listitem>
      </varlistentry>

*************** myEventProc(PGEventId evtId, void *evtIn
*** 7521,7533 ****
     used.  (Therefore, put more-specific entries first when you are using
     wildcards.) If an entry needs to contain <literal>:</literal> or
     <literal>\</literal>, escape this character with <literal>\</literal>.
!    A host name of <literal>localhost</literal> matches both TCP (host name
!    <literal>localhost</literal>) and Unix domain socket (<literal>pghost</literal> empty
!    or the default socket directory) connections coming from the local
!    machine. In a standby server, a database name of <literal>replication</literal>
     matches streaming replication connections made to the master server.
!    The <literal>database</literal> field is of limited usefulness because
!    users have the same password for all databases in the same cluster.
    </para>

    <para>
--- 7539,7556 ----
     used.  (Therefore, put more-specific entries first when you are using
     wildcards.) If an entry needs to contain <literal>:</literal> or
     <literal>\</literal>, escape this character with <literal>\</literal>.
!    The host name field is matched to the <literal>host</literal> connection
!    parameter if that is specified, otherwise to
!    the <literal>hostaddr</literal> parameter if that is specified; if neither
!    are given then the host name <literal>localhost</literal> is searched for.
!    The host name <literal>localhost</literal> is also searched for when
!    the connection is a Unix-domain socket connection in which
!    the <literal>host</literal> parameter equals the installation's default
!    socket directory path.
!    In a standby server, a database name of <literal>replication</literal>
     matches streaming replication connections made to the master server.
!    The <literal>database</literal> field is of limited usefulness otherwise,
!    because users have the same password for all databases in the same cluster.
    </para>

    <para>
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index bd7dac1..4b35994 100644
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
*************** parse_comma_separated_list(char **startp
*** 901,906 ****
--- 901,908 ----
  static bool
  connectOptions2(PGconn *conn)
  {
+     int            i;
+
      /*
       * Allocate memory for details about each host to which we might possibly
       * try to connect.  For that, count the number of elements in the hostaddr
*************** connectOptions2(PGconn *conn)
*** 920,930 ****

      /*
       * We now have one pg_conn_host structure per possible host.  Fill in the
!      * host details for each one.
       */
      if (conn->pghostaddr != NULL && conn->pghostaddr[0] != '\0')
      {
-         int            i;
          char       *s = conn->pghostaddr;
          bool        more = true;

--- 922,931 ----

      /*
       * We now have one pg_conn_host structure per possible host.  Fill in the
!      * host and hostaddr fields for each, by splitting the parameter strings.
       */
      if (conn->pghostaddr != NULL && conn->pghostaddr[0] != '\0')
      {
          char       *s = conn->pghostaddr;
          bool        more = true;

*************** connectOptions2(PGconn *conn)
*** 933,940 ****
              conn->connhost[i].hostaddr = parse_comma_separated_list(&s, &more);
              if (conn->connhost[i].hostaddr == NULL)
                  goto oom_error;
-
-             conn->connhost[i].type = CHT_HOST_ADDRESS;
          }

          /*
--- 934,939 ----
*************** connectOptions2(PGconn *conn)
*** 948,954 ****

      if (conn->pghost != NULL && conn->pghost[0] != '\0')
      {
-         int            i;
          char       *s = conn->pghost;
          bool        more = true;

--- 947,952 ----
*************** connectOptions2(PGconn *conn)
*** 957,973 ****
              conn->connhost[i].host = parse_comma_separated_list(&s, &more);
              if (conn->connhost[i].host == NULL)
                  goto oom_error;
-
-             /* Identify the type of host. */
-             if (conn->pghostaddr == NULL || conn->pghostaddr[0] == '\0')
-             {
-                 conn->connhost[i].type = CHT_HOST_NAME;
- #ifdef HAVE_UNIX_SOCKETS
-                 if (is_absolute_path(conn->connhost[i].host))
-                     conn->connhost[i].type = CHT_UNIX_SOCKET;
- #endif
-             }
          }
          if (more || i != conn->nconnhost)
          {
              conn->status = CONNECTION_BAD;
--- 955,963 ----
              conn->connhost[i].host = parse_comma_separated_list(&s, &more);
              if (conn->connhost[i].host == NULL)
                  goto oom_error;
          }
+
+         /* Check for wrong number of host items. */
          if (more || i != conn->nconnhost)
          {
              conn->status = CONNECTION_BAD;
*************** connectOptions2(PGconn *conn)
*** 979,1007 ****
      }

      /*
!      * If neither host or hostaddr options was given, connect to default host.
       */
!     if ((conn->pghostaddr == NULL || conn->pghostaddr[0] == '\0') &&
!         (conn->pghost == NULL || conn->pghost[0] == '\0'))
      {
!         Assert(conn->nconnhost == 1);
  #ifdef HAVE_UNIX_SOCKETS
!         conn->connhost[0].host = strdup(DEFAULT_PGSOCKET_DIR);
!         conn->connhost[0].type = CHT_UNIX_SOCKET;
  #else
!         conn->connhost[0].host = strdup(DefaultHost);
!         conn->connhost[0].type = CHT_HOST_NAME;
  #endif
!         if (conn->connhost[0].host == NULL)
!             goto oom_error;
      }

      /*
       * Next, work out the port number corresponding to each host name.
       */
      if (conn->pgport != NULL && conn->pgport[0] != '\0')
      {
-         int            i;
          char       *s = conn->pgport;
          bool        more = true;

--- 969,1016 ----
      }

      /*
!      * Now, for each host slot, identify the type of address spec, and fill in
!      * the default address if nothing was given.
       */
!     for (i = 0; i < conn->nconnhost; i++)
      {
!         pg_conn_host *ch = &conn->connhost[i];
!
!         if (ch->hostaddr != NULL && ch->hostaddr[0] != '\0')
!             ch->type = CHT_HOST_ADDRESS;
!         else if (ch->host != NULL && ch->host[0] != '\0')
!         {
!             ch->type = CHT_HOST_NAME;
  #ifdef HAVE_UNIX_SOCKETS
!             if (is_absolute_path(ch->host))
!                 ch->type = CHT_UNIX_SOCKET;
! #endif
!         }
!         else
!         {
!             if (ch->host)
!                 free(ch->host);
! #ifdef HAVE_UNIX_SOCKETS
!             ch->host = strdup(DEFAULT_PGSOCKET_DIR);
!             ch->type = CHT_UNIX_SOCKET;
  #else
!             ch->host = strdup(DefaultHost);
!             ch->type = CHT_HOST_NAME;
  #endif
!             if (ch->host == NULL)
!                 goto oom_error;
!         }
      }

      /*
       * Next, work out the port number corresponding to each host name.
+      *
+      * Note: unlike the above for host names, this could leave the port fields
+      * as null or empty strings.  We will substitute DEF_PGPORT whenever we
+      * read such a port field.
       */
      if (conn->pgport != NULL && conn->pgport[0] != '\0')
      {
          char       *s = conn->pgport;
          bool        more = true;

*************** connectOptions2(PGconn *conn)
*** 1065,1072 ****
      }

      /*
!      * Supply default password if none given.  Note that the password might be
!      * different for each host/port pair.
       */
      if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
      {
--- 1074,1081 ----
      }

      /*
!      * If password was not given, try to look it up in password file.  Note
!      * that the result might be different for each host/port pair.
       */
      if (conn->pgpass == NULL || conn->pgpass[0] == '\0')
      {
*************** connectOptions2(PGconn *conn)
*** 1089,1108 ****

          if (conn->pgpassfile != NULL && conn->pgpassfile[0] != '\0')
          {
-             int            i;
-
              for (i = 0; i < conn->nconnhost; i++)
              {
                  /*
!                  * Try to get a password for this host from pgpassfile. We use
!                  * host name rather than host address in the same manner as
!                  * PQhost().
                   */
!                 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 =
--- 1098,1113 ----

          if (conn->pgpassfile != NULL && conn->pgpassfile[0] != '\0')
          {
              for (i = 0; i < conn->nconnhost; i++)
              {
                  /*
!                  * Try to get a password for this host from file.  We use host
!                  * for the hostname search key if given, else hostaddr (at
!                  * least one of them is guaranteed nonempty by now).
                   */
!                 const char *pwhost = conn->connhost[i].host;

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

                  conn->connhost[i].password =
*************** passwordFromFile(const char *hostname, c
*** 6385,6398 ****
  #define LINELEN NAMEDATALEN*5
      char        buf[LINELEN];

!     if (dbname == NULL || strlen(dbname) == 0)
          return NULL;

!     if (username == NULL || strlen(username) == 0)
          return NULL;

      /* 'localhost' matches pghost of '' or the default socket directory */
!     if (hostname == NULL)
          hostname = DefaultHost;
      else if (is_absolute_path(hostname))

--- 6390,6403 ----
  #define LINELEN NAMEDATALEN*5
      char        buf[LINELEN];

!     if (dbname == NULL || dbname[0] == '\0')
          return NULL;

!     if (username == NULL || username[0] == '\0')
          return NULL;

      /* 'localhost' matches pghost of '' or the default socket directory */
!     if (hostname == NULL || hostname[0] == '\0')
          hostname = DefaultHost;
      else if (is_absolute_path(hostname))

*************** passwordFromFile(const char *hostname, c
*** 6403,6409 ****
          if (strcmp(hostname, DEFAULT_PGSOCKET_DIR) == 0)
              hostname = DefaultHost;

!     if (port == NULL)
          port = DEF_PGPORT_STR;

      /* If password file cannot be opened, ignore it. */
--- 6408,6414 ----
          if (strcmp(hostname, DEFAULT_PGSOCKET_DIR) == 0)
              hostname = DefaultHost;

!     if (port == NULL || port[0] == '\0')
          port = DEF_PGPORT_STR;

      /* If password file cannot be opened, ignore it. */