libpq should not look up all host addresses at once

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: libpq should not look up all host addresses at once
Date: 2018-08-09 15:05:02
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Whilst fooling with the patch for CVE-2018-10915, I got annoyed by
the fact that connectDBStart() does the DNS lookups for all supplied
hostnames at once, and fails if any of them are bad. It was reasonable
to do the lookup there when we only allowed one hostname, but now that
"host" can be a list, this is really pretty stupid. The whole point
of allowing multiple hostnames is redundancy and avoiding a single
point of failure; but the way this is written, *each* of your servers'
DNS servers is a single point of failure all by itself. If any one of
them is down, you don't connect. Plus, in the normal case where you
successfully connect to something before the very last host in the list,
the extra DNS lookups are wasted --- and DNS lookups aren't that cheap,
since they typically involve a network round trip.

So I think what this code should do is (1) look up each hostname as it
needs it, not all at once, and (2) proceed on to the next hostname
if it gets a DNS lookup failure, not fail the whole connection attempt
immediately. As attached.

I'm tempted to call this a back-patchable bug fix, because the existing
behavior basically negates the entire value of the multi-hostname feature
once you consider the possibility of DNS server failures. But given the
lack of field complaints, maybe that's an overreaction.

I'll put this in the September fest for review.

regards, tom lane

Attachment Content-Type Size
libpq-delay-getaddrinfo-1.patch text/x-diff 12.9 KB


Browse pgsql-hackers by date

  From Date Subject
Next Message Marina Polyakova 2018-08-09 15:17:22 Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Previous Message Emre Hasegeli 2018-08-09 14:44:05 Re: Optimizer misses big in 10.4 with BRIN index