Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 65183fb

Browse files
committed
Fix assorted issues in client host name lookup.
The code for matching clients to pg_hba.conf lines that specify host names (instead of IP address ranges) failed to complain if reverse DNS lookup failed; instead it silently didn't match, so that you might end up getting a surprising "no pg_hba.conf entry for ..." error, as seen in bug #9518 from Mike Blackwell. Since we don't want to make this a fatal error in situations where pg_hba.conf contains a mixture of host names and IP addresses (clients matching one of the numeric entries should not have to have rDNS data), remember the lookup failure and mention it as DETAIL if we get to "no pg_hba.conf entry". Apply the same approach to forward-DNS lookup failures, too, rather than treating them as immediate hard errors. Along the way, fix a couple of bugs that prevented us from detecting an rDNS lookup error reliably, and make sure that we make only one rDNS lookup attempt; formerly, if the lookup attempt failed, the code would try again for each host name entry in pg_hba.conf. Since more or less the whole point of this design is to ensure there's only one lookup attempt not one per entry, the latter point represents a performance bug that seems sufficient justification for back-patching. Also, adjust src/port/getaddrinfo.c so that it plays as well as it can with this code. Which is not all that well, since it does not have actual support for rDNS lookup, but at least it should return the expected (and required by spec) error codes so that the main code correctly perceives the lack of functionality as a lookup failure. It's unlikely that PG is still being used in production on any machines that require our getaddrinfo.c, so I'm not excited about working harder than this. To keep the code in the various branches similar, this includes back-patching commits c424d0d and 1997f34 into 9.2 and earlier. Back-patch to 9.1 where the facility for hostnames in pg_hba.conf was introduced.
1 parent 6cb229d commit 65183fb

File tree

6 files changed

+88
-32
lines changed

6 files changed

+88
-32
lines changed

src/backend/libpq/auth.c

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -447,15 +447,25 @@ ClientAuthentication(Port *port)
447447
NI_NUMERICHOST);
448448

449449
#define HOSTNAME_LOOKUP_DETAIL(port) \
450-
(port->remote_hostname \
451-
? (port->remote_hostname_resolv == +1 \
452-
? errdetail_log("Client IP address resolved to \"%s\", forward lookup matches.", port->remote_hostname) \
453-
: (port->remote_hostname_resolv == 0 \
454-
? errdetail_log("Client IP address resolved to \"%s\", forward lookup not checked.", port->remote_hostname) \
455-
: (port->remote_hostname_resolv == -1 \
456-
? errdetail_log("Client IP address resolved to \"%s\", forward lookup does not match.", port->remote_hostname) \
457-
: 0))) \
458-
: 0)
450+
(port->remote_hostname ? \
451+
(port->remote_hostname_resolv == +1 ? \
452+
errdetail_log("Client IP address resolved to \"%s\", forward lookup matches.", \
453+
port->remote_hostname) : \
454+
port->remote_hostname_resolv == 0 ? \
455+
errdetail_log("Client IP address resolved to \"%s\", forward lookup not checked.", \
456+
port->remote_hostname) : \
457+
port->remote_hostname_resolv == -1 ? \
458+
errdetail_log("Client IP address resolved to \"%s\", forward lookup does not match.", \
459+
port->remote_hostname) : \
460+
port->remote_hostname_resolv == -2 ? \
461+
errdetail_log("Could not translate client host name \"%s\" to IP address: %s.", \
462+
port->remote_hostname, \
463+
gai_strerror(port->remote_hostname_errcode)) : \
464+
0) \
465+
: (port->remote_hostname_resolv == -2 ? \
466+
errdetail_log("Could not resolve client IP address to a host name: %s.", \
467+
gai_strerror(port->remote_hostname_errcode)) : \
468+
0))
459469

460470
if (am_walsender)
461471
{

src/backend/libpq/hba.c

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -592,35 +592,47 @@ check_hostname(hbaPort *port, const char *hostname)
592592
int ret;
593593
bool found;
594594

595+
/* Quick out if remote host name already known bad */
596+
if (port->remote_hostname_resolv < 0)
597+
return false;
598+
595599
/* Lookup remote host name if not already done */
596600
if (!port->remote_hostname)
597601
{
598602
char remote_hostname[NI_MAXHOST];
599603

600-
if (pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
601-
remote_hostname, sizeof(remote_hostname),
602-
NULL, 0,
603-
0) != 0)
604+
ret = pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
605+
remote_hostname, sizeof(remote_hostname),
606+
NULL, 0,
607+
NI_NAMEREQD);
608+
if (ret != 0)
609+
{
610+
/* remember failure; don't complain in the postmaster log yet */
611+
port->remote_hostname_resolv = -2;
612+
port->remote_hostname_errcode = ret;
604613
return false;
614+
}
605615

606616
port->remote_hostname = pstrdup(remote_hostname);
607617
}
608618

619+
/* Now see if remote host name matches this pg_hba line */
609620
if (!hostname_match(hostname, port->remote_hostname))
610621
return false;
611622

612-
/* Lookup IP from host name and check against original IP */
613-
623+
/* If we already verified the forward lookup, we're done */
614624
if (port->remote_hostname_resolv == +1)
615625
return true;
616-
if (port->remote_hostname_resolv == -1)
617-
return false;
618626

627+
/* Lookup IP from host name and check against original IP */
619628
ret = getaddrinfo(port->remote_hostname, NULL, NULL, &gai_result);
620629
if (ret != 0)
621-
ereport(ERROR,
622-
(errmsg("could not translate host name \"%s\" to address: %s",
623-
port->remote_hostname, gai_strerror(ret))));
630+
{
631+
/* remember failure; don't complain in the postmaster log yet */
632+
port->remote_hostname_resolv = -2;
633+
port->remote_hostname_errcode = ret;
634+
return false;
635+
}
624636

625637
found = false;
626638
for (gai = gai_result; gai; gai = gai->ai_next)

src/backend/postmaster/postmaster.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3859,8 +3859,23 @@ BackendInitialize(Port *port)
38593859
*/
38603860
port->remote_host = strdup(remote_host);
38613861
port->remote_port = strdup(remote_port);
3862-
if (log_hostname)
3863-
port->remote_hostname = port->remote_host;
3862+
3863+
/*
3864+
* If we did a reverse lookup to name, we might as well save the results
3865+
* rather than possibly repeating the lookup during authentication.
3866+
*
3867+
* Note that we don't want to specify NI_NAMEREQD above, because then we'd
3868+
* get nothing useful for a client without an rDNS entry. Therefore, we
3869+
* must check whether we got a numeric IPv4 or IPv6 address, and not save
3870+
* it into remote_hostname if so. (This test is conservative and might
3871+
* sometimes classify a hostname as numeric, but an error in that
3872+
* direction is safe; it only results in a possible extra lookup.)
3873+
*/
3874+
if (log_hostname &&
3875+
ret == 0 &&
3876+
strspn(remote_host, "0123456789.") < strlen(remote_host) &&
3877+
strspn(remote_host, "0123456789ABCDEFabcdef:") < strlen(remote_host))
3878+
port->remote_hostname = strdup(remote_host);
38643879

38653880
/*
38663881
* Ready to begin client interaction. We will give up and exit(1) after a

src/include/getaddrinfo.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,9 @@
8282
#ifndef NI_NUMERICSERV
8383
#define NI_NUMERICSERV 2
8484
#endif
85+
#ifndef NI_NAMEREQD
86+
#define NI_NAMEREQD 4
87+
#endif
8588

8689
#ifndef NI_MAXHOST
8790
#define NI_MAXHOST 1025

src/include/libpq/libpq-be.h

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,20 @@ typedef struct
9999
* still available when a backend is running (see MyProcPort). The data
100100
* it points to must also be malloc'd, or else palloc'd in TopMemoryContext,
101101
* so that it survives into PostgresMain execution!
102+
*
103+
* remote_hostname is set if we did a successful reverse lookup of the
104+
* client's IP address during connection setup.
105+
* remote_hostname_resolv tracks the state of hostname verification:
106+
* +1 = remote_hostname is known to resolve to client's IP address
107+
* -1 = remote_hostname is known NOT to resolve to client's IP address
108+
* 0 = we have not done the forward DNS lookup yet
109+
* -2 = there was an error in name resolution
110+
* If reverse lookup of the client IP address fails, remote_hostname will be
111+
* left NULL while remote_hostname_resolv is set to -2. If reverse lookup
112+
* succeeds but forward lookup fails, remote_hostname_resolv is also set to -2
113+
* (the case is distinguishable because remote_hostname isn't NULL). In
114+
* either of the -2 cases, remote_hostname_errcode saves the lookup return
115+
* code for possible later use with gai_strerror.
102116
*/
103117

104118
typedef struct Port
@@ -111,12 +125,7 @@ typedef struct Port
111125
char *remote_host; /* name (or ip addr) of remote host */
112126
char *remote_hostname;/* name (not ip addr) of remote host, if
113127
* available */
114-
int remote_hostname_resolv; /* +1 = remote_hostname is known to
115-
* resolve to client's IP address; -1
116-
* = remote_hostname is known NOT to
117-
* resolve to client's IP address; 0 =
118-
* we have not done the forward DNS
119-
* lookup yet */
128+
int remote_hostname_resolv; /* see above */
120129
char *remote_port; /* text rep of remote port */
121130
CAC_state canAcceptConnections; /* postmaster connection status */
122131

@@ -178,6 +187,9 @@ typedef struct Port
178187
char *peer_cn;
179188
unsigned long count;
180189
#endif
190+
191+
/* This field will be in a saner place in 9.4 and up */
192+
int remote_hostname_errcode; /* see above */
181193
} Port;
182194

183195

src/port/getaddrinfo.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ getaddrinfo(const char *node, const char *service,
182182
else if (hints.ai_flags & AI_NUMERICHOST)
183183
{
184184
if (!inet_aton(node, &sin.sin_addr))
185-
return EAI_FAIL;
185+
return EAI_NONAME;
186186
}
187187
else
188188
{
@@ -349,8 +349,8 @@ gai_strerror(int errcode)
349349
/*
350350
* Convert an ipv4 address to a hostname.
351351
*
352-
* Bugs: - Only supports NI_NUMERICHOST and NI_NUMERICSERV
353-
* It will never resolv a hostname.
352+
* Bugs: - Only supports NI_NUMERICHOST and NI_NUMERICSERV behavior.
353+
* It will never resolve a hostname.
354354
* - No IPv6 support.
355355
*/
356356
int
@@ -378,6 +378,10 @@ getnameinfo(const struct sockaddr * sa, int salen,
378378
return EAI_FAMILY;
379379
#endif
380380

381+
/* Unsupported flags. */
382+
if (flags & NI_NAMEREQD)
383+
return EAI_AGAIN;
384+
381385
if (node)
382386
{
383387
if (sa->sa_family == AF_INET)
@@ -400,7 +404,7 @@ getnameinfo(const struct sockaddr * sa, int salen,
400404
ret = snprintf(service, servicelen, "%d",
401405
ntohs(((struct sockaddr_in *) sa)->sin_port));
402406
}
403-
if (ret == -1 || ret > servicelen)
407+
if (ret == -1 || ret >= servicelen)
404408
return EAI_MEMORY;
405409
}
406410

0 commit comments

Comments
 (0)