]> granicus.if.org Git - postgresql/commitdiff
Fix assorted issues in client host name lookup.
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 2 Apr 2014 21:11:31 +0000 (17:11 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 2 Apr 2014 21:11:31 +0000 (17:11 -0400)
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 c424d0d1052cb4053c8712ac44123f9b9a9aa3f2 and
1997f34db4687e671690ed054c8f30bb501b1168 into 9.2 and earlier.

Back-patch to 9.1 where the facility for hostnames in pg_hba.conf was
introduced.

src/backend/libpq/auth.c
src/backend/libpq/hba.c
src/backend/libpq/ip.c
src/backend/postmaster/postmaster.c
src/include/getaddrinfo.h
src/include/libpq/libpq-be.h
src/port/getaddrinfo.c

index 9cdee2bb3e7bd2e42a6de3e37b006786066fe543..c65427e53c49dad46790b84a6504ad6b77779a84 100644 (file)
@@ -440,15 +440,25 @@ ClientAuthentication(Port *port)
                                                                   NI_NUMERICHOST);
 
 #define HOSTNAME_LOOKUP_DETAIL(port) \
-                               (port->remote_hostname                            \
-                                ? (port->remote_hostname_resolv == +1                                  \
-                                       ? errdetail_log("Client IP address resolved to \"%s\", forward lookup matches.", port->remote_hostname) \
-                                       : (port->remote_hostname_resolv == 0                            \
-                                          ? errdetail_log("Client IP address resolved to \"%s\", forward lookup not checked.", port->remote_hostname) \
-                                          : (port->remote_hostname_resolv == -1                        \
-                                                 ? errdetail_log("Client IP address resolved to \"%s\", forward lookup does not match.", port->remote_hostname) \
-                                                 : 0)))                                                                                \
-                                : 0)
+                               (port->remote_hostname ? \
+                                (port->remote_hostname_resolv == +1 ? \
+                                 errdetail_log("Client IP address resolved to \"%s\", forward lookup matches.", \
+                                                               port->remote_hostname) : \
+                                 port->remote_hostname_resolv == 0 ? \
+                                 errdetail_log("Client IP address resolved to \"%s\", forward lookup not checked.", \
+                                                               port->remote_hostname) : \
+                                 port->remote_hostname_resolv == -1 ? \
+                                 errdetail_log("Client IP address resolved to \"%s\", forward lookup does not match.", \
+                                                               port->remote_hostname) : \
+                                 port->remote_hostname_resolv == -2 ? \
+                                 errdetail_log("Could not translate client host name \"%s\" to IP address: %s.", \
+                                                               port->remote_hostname, \
+                                                               gai_strerror(port->remote_hostname_errcode)) : \
+                                 0) \
+                                : (port->remote_hostname_resolv == -2 ? \
+                                       errdetail_log("Could not resolve client IP address to a host name: %s.", \
+                                                                 gai_strerror(port->remote_hostname_errcode)) : \
+                                       0))
 
                                if (am_walsender)
                                {
index 828f6dcc8e1653ad25f206f9d10b55006466b2e1..09e8715c798ec24c8596e78ef45f930f350109e6 100644 (file)
@@ -565,35 +565,47 @@ check_hostname(hbaPort *port, const char *hostname)
        int                     ret;
        bool            found;
 
+       /* Quick out if remote host name already known bad */
+       if (port->remote_hostname_resolv < 0)
+               return false;
+
        /* Lookup remote host name if not already done */
        if (!port->remote_hostname)
        {
                char            remote_hostname[NI_MAXHOST];
 
-               if (pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
-                                                          remote_hostname, sizeof(remote_hostname),
-                                                          NULL, 0,
-                                                          0) != 0)
+               ret = pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
+                                                                remote_hostname, sizeof(remote_hostname),
+                                                                NULL, 0,
+                                                                NI_NAMEREQD);
+               if (ret != 0)
+               {
+                       /* remember failure; don't complain in the postmaster log yet */
+                       port->remote_hostname_resolv = -2;
+                       port->remote_hostname_errcode = ret;
                        return false;
+               }
 
                port->remote_hostname = pstrdup(remote_hostname);
        }
 
+       /* Now see if remote host name matches this pg_hba line */
        if (!hostname_match(hostname, port->remote_hostname))
                return false;
 
-       /* Lookup IP from host name and check against original IP */
-
+       /* If we already verified the forward lookup, we're done */
        if (port->remote_hostname_resolv == +1)
                return true;
-       if (port->remote_hostname_resolv == -1)
-               return false;
 
+       /* Lookup IP from host name and check against original IP */
        ret = getaddrinfo(port->remote_hostname, NULL, NULL, &gai_result);
        if (ret != 0)
-               ereport(ERROR,
-                               (errmsg("could not translate host name \"%s\" to address: %s",
-                                               port->remote_hostname, gai_strerror(ret))));
+       {
+               /* remember failure; don't complain in the postmaster log yet */
+               port->remote_hostname_resolv = -2;
+               port->remote_hostname_errcode = ret;
+               return false;
+       }
 
        found = false;
        for (gai = gai_result; gai; gai = gai->ai_next)
index 550e11b68cf99683f33c9bb9dba1ab10b634c3c8..db3a5252efd4e21eb59a0168c1dfa75577c9d179 100644 (file)
@@ -247,11 +247,6 @@ getnameinfo_unix(const struct sockaddr_un * sa, int salen,
                (node == NULL && service == NULL))
                return EAI_FAIL;
 
-       /* We don't support those. */
-       if ((node && !(flags & NI_NUMERICHOST))
-               || (service && !(flags & NI_NUMERICSERV)))
-               return EAI_FAIL;
-
        if (node)
        {
                ret = snprintf(node, nodelen, "%s", "[local]");
index b96b505834a19e9888c2ae7d2ffb4d8341d43a6f..95640622bc1730c2064efb4a4e3f90b7a6024263 100644 (file)
@@ -3381,6 +3381,7 @@ static void
 BackendInitialize(Port *port)
 {
        int                     status;
+       int                     ret;
        char            remote_host[NI_MAXHOST];
        char            remote_port[NI_MAXSERV];
        char            remote_ps_data[NI_MAXHOST];
@@ -3442,21 +3443,13 @@ BackendInitialize(Port *port)
         */
        remote_host[0] = '\0';
        remote_port[0] = '\0';
-       if (pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
+       if ((ret = pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
                                                   remote_host, sizeof(remote_host),
                                                   remote_port, sizeof(remote_port),
-                                 (log_hostname ? 0 : NI_NUMERICHOST) | NI_NUMERICSERV) != 0)
-       {
-               int                     ret = pg_getnameinfo_all(&port->raddr.addr, port->raddr.salen,
-                                                                                        remote_host, sizeof(remote_host),
-                                                                                        remote_port, sizeof(remote_port),
-                                                                                        NI_NUMERICHOST | NI_NUMERICSERV);
-
-               if (ret != 0)
-                       ereport(WARNING,
-                                       (errmsg_internal("pg_getnameinfo_all() failed: %s",
-                                                                        gai_strerror(ret))));
-       }
+                                 (log_hostname ? 0 : NI_NUMERICHOST) | NI_NUMERICSERV)) != 0)
+               ereport(WARNING,
+                               (errmsg_internal("pg_getnameinfo_all() failed: %s",
+                                                                gai_strerror(ret))));
        if (remote_port[0] == '\0')
                snprintf(remote_ps_data, sizeof(remote_ps_data), "%s", remote_host);
        else
@@ -3480,8 +3473,23 @@ BackendInitialize(Port *port)
         */
        port->remote_host = strdup(remote_host);
        port->remote_port = strdup(remote_port);
-       if (log_hostname)
-               port->remote_hostname = port->remote_host;
+
+       /*
+        * If we did a reverse lookup to name, we might as well save the results
+        * rather than possibly repeating the lookup during authentication.
+        *
+        * Note that we don't want to specify NI_NAMEREQD above, because then we'd
+        * get nothing useful for a client without an rDNS entry.  Therefore, we
+        * must check whether we got a numeric IPv4 or IPv6 address, and not save
+        * it into remote_hostname if so.  (This test is conservative and might
+        * sometimes classify a hostname as numeric, but an error in that
+        * direction is safe; it only results in a possible extra lookup.)
+        */
+       if (log_hostname &&
+               ret == 0 &&
+               strspn(remote_host, "0123456789.") < strlen(remote_host) &&
+               strspn(remote_host, "0123456789ABCDEFabcdef:") < strlen(remote_host))
+               port->remote_hostname = strdup(remote_host);
 
        /*
         * Ready to begin client interaction.  We will give up and exit(1) after a
index 519a97975ef63e0c21f13893b3baacb1bde0e23f..9adceb4575ffe396a44ba5ea16c2ce5c245e0cec 100644 (file)
@@ -82,6 +82,9 @@
 #ifndef NI_NUMERICSERV
 #define NI_NUMERICSERV 2
 #endif
+#ifndef NI_NAMEREQD
+#define NI_NAMEREQD            4
+#endif
 
 #ifndef NI_MAXHOST
 #define NI_MAXHOST     1025
index 4d92c18974bc56b402445671e4ff84d7c7628daa..95b538c64db23fcac57ab39cbdfb6e587ba09e72 100644 (file)
@@ -99,6 +99,20 @@ typedef struct
  * still available when a backend is running (see MyProcPort). The data
  * it points to must also be malloc'd, or else palloc'd in TopMemoryContext,
  * so that it survives into PostgresMain execution!
+ *
+ * remote_hostname is set if we did a successful reverse lookup of the
+ * client's IP address during connection setup.
+ * remote_hostname_resolv tracks the state of hostname verification:
+ *     +1 = remote_hostname is known to resolve to client's IP address
+ *     -1 = remote_hostname is known NOT to resolve to client's IP address
+ *      0 = we have not done the forward DNS lookup yet
+ *     -2 = there was an error in name resolution
+ * If reverse lookup of the client IP address fails, remote_hostname will be
+ * left NULL while remote_hostname_resolv is set to -2.  If reverse lookup
+ * succeeds but forward lookup fails, remote_hostname_resolv is also set to -2
+ * (the case is distinguishable because remote_hostname isn't NULL).  In
+ * either of the -2 cases, remote_hostname_errcode saves the lookup return
+ * code for possible later use with gai_strerror.
  */
 
 typedef struct Port
@@ -111,12 +125,7 @@ typedef struct Port
        char       *remote_host;        /* name (or ip addr) of remote host */
        char       *remote_hostname;/* name (not ip addr) of remote host, if
                                                                 * available */
-       int                     remote_hostname_resolv; /* +1 = remote_hostname is known to
-                                                                                * resolve to client's IP address; -1
-                                                                                * = remote_hostname is known NOT to
-                                                                                * resolve to client's IP address; 0 =
-                                                                                * we have not done the forward DNS
-                                                                                * lookup yet */
+       int                     remote_hostname_resolv; /* see above */
        char       *remote_port;        /* text rep of remote port */
        CAC_state       canAcceptConnections;   /* postmaster connection status */
 
@@ -178,6 +187,9 @@ typedef struct Port
        char       *peer_cn;
        unsigned long count;
 #endif
+
+       /* This field will be in a saner place in 9.4 and up */
+       int                     remote_hostname_errcode;                /* see above */
 } Port;
 
 
index c117012ec7eadf8ea87c254a55c08a4372e3d9a2..3d23ffecaa33d56794758dcd2cb82099dc3dfd91 100644 (file)
@@ -182,7 +182,7 @@ getaddrinfo(const char *node, const char *service,
                else if (hints.ai_flags & AI_NUMERICHOST)
                {
                        if (!inet_aton(node, &sin.sin_addr))
-                               return EAI_FAIL;
+                               return EAI_NONAME;
                }
                else
                {
@@ -349,8 +349,8 @@ gai_strerror(int errcode)
 /*
  * Convert an ipv4 address to a hostname.
  *
- * Bugs:       - Only supports NI_NUMERICHOST and NI_NUMERICSERV
- *               It will never resolv a hostname.
+ * Bugs:       - Only supports NI_NUMERICHOST and NI_NUMERICSERV behavior.
+ *               It will never resolve a hostname.
  *             - No IPv6 support.
  */
 int
@@ -373,16 +373,15 @@ getnameinfo(const struct sockaddr * sa, int salen,
        if (sa == NULL || (node == NULL && service == NULL))
                return EAI_FAIL;
 
-       /* We don't support those. */
-       if ((node && !(flags & NI_NUMERICHOST))
-               || (service && !(flags & NI_NUMERICSERV)))
-               return EAI_FAIL;
-
 #ifdef HAVE_IPV6
        if (sa->sa_family == AF_INET6)
                return EAI_FAMILY;
 #endif
 
+       /* Unsupported flags. */
+       if (flags & NI_NAMEREQD)
+               return EAI_AGAIN;
+
        if (node)
        {
                if (sa->sa_family == AF_INET)
@@ -405,7 +404,7 @@ getnameinfo(const struct sockaddr * sa, int salen,
                        ret = snprintf(service, servicelen, "%d",
                                                   ntohs(((struct sockaddr_in *) sa)->sin_port));
                }
-               if (ret == -1 || ret > servicelen)
+               if (ret == -1 || ret >= servicelen)
                        return EAI_MEMORY;
        }