Improve DNS lookup error handling. (#615)

Author: DimCitus

src/bin/pg_autoctl/cli_create_drop_node.c

@@ -1397,8 +1397,8 @@ check_hostname(const char *hostname)
 	}
 	else
 	{
-		char cidr[BUFSIZE];
-		char ipaddr[BUFSIZE];
+		char cidr[BUFSIZE] = { 0 };
+		char ipaddr[BUFSIZE] = { 0 };
 
 		if (!fetchLocalCIDR(hostname, cidr, BUFSIZE))
 		{
@@ -1407,7 +1407,9 @@ check_hostname(const char *hostname)
 					 hostname);
 		}
 
+		bool useHostname = false;
+
 		/* use pghba_check_hostname for log diagnostics */
-		(void) pghba_check_hostname(hostname, ipaddr, sizeof(ipaddr));
+		(void) pghba_check_hostname(hostname, ipaddr, BUFSIZE, &useHostname);
 	}
 }

src/bin/pg_autoctl/cli_do_show.c

@@ -307,7 +307,7 @@ static void
 cli_show_reverse(int argc, char **argv)
 {
 	char ipaddr[BUFSIZE] = { 0 };
-
+	bool foundHostnameFromAddress = false;
 
 	if (argc != 1)
 	{
@@ -324,7 +324,9 @@ cli_show_reverse(int argc, char **argv)
 		exit(EXIT_CODE_BAD_ARGS);
 	}
 
-	if (!resolveHostnameForwardAndReverse(hostname, ipaddr, sizeof(ipaddr)))
+	if (!resolveHostnameForwardAndReverse(hostname, ipaddr, sizeof(ipaddr),
+										  &foundHostnameFromAddress) ||
+		!foundHostnameFromAddress)
 	{
 		log_fatal("Failed to find an IP address for hostname \"%s\" that "
 				  "matches hostname again in a reverse-DNS lookup.",

src/bin/pg_autoctl/ipaddr.c

@@ -679,13 +679,15 @@ findHostnameFromLocalIpAddress(char *localIpAddress, char *hostname, int size)
  * of the IP addresses that our hostname forward-DNS query returns.
  */
 bool
-resolveHostnameForwardAndReverse(const char *hostname, char *ipaddr, int size)
+resolveHostnameForwardAndReverse(const char *hostname, char *ipaddr, int size,
+								 bool *foundHostnameFromAddress)
 {
 	struct addrinfo *lookup, *ai;
 
-	bool foundHostnameFromAddress = false;
+	*foundHostnameFromAddress = false;
 
 	int error = getaddrinfo(hostname, NULL, 0, &lookup);
+
 	if (error != 0)
 	{
 		log_warn("Failed to resolve DNS name \"%s\": %s",
@@ -733,13 +735,13 @@ resolveHostnameForwardAndReverse(const char *hostname, char *ipaddr, int size)
 		/* compare reverse-DNS lookup result with our hostname */
 		if (strcmp(hbuf, hostname) == 0)
 		{
-			foundHostnameFromAddress = true;
+			*foundHostnameFromAddress = true;
 			break;
 		}
 	}
 	freeaddrinfo(lookup);
 
-	return foundHostnameFromAddress;
+	return true;
 }
 
 

src/bin/pg_autoctl/ipaddr.h

@@ -31,7 +31,8 @@ bool findHostnameFromLocalIpAddress(char *localIpAddress,
 									char *hostname, int size);
 
 bool resolveHostnameForwardAndReverse(const char *hostname,
-									  char *ipaddr, int size);
+									  char *ipaddr, int size,
+									  bool *foundHostnameFromAddress);
 
 bool ipaddrGetLocalHostname(char *hostname, size_t size);
 

src/bin/pg_autoctl/pghba.c

@@ -130,7 +130,12 @@ pghba_ensure_host_rule_exists(const char *hbaFilePath,
 	 *
 	 * HBA & DNS is hard.
 	 */
-	bool useHostname = pghba_check_hostname(host, ipaddr, sizeof(ipaddr));
+	bool useHostname = false;
+
+	if (!pghba_check_hostname(host, ipaddr, sizeof(ipaddr), &useHostname))
+	{
+		/* errors have already been logged (DNS failure) */
+	}
 
 	if (!useHostname)
 	{
@@ -331,8 +336,11 @@ pghba_ensure_host_rules_exist(const char *hbaFilePath,
 			 *
 			 * HBA & DNS is hard.
 			 */
-			useHostname =
-				pghba_check_hostname(node->host, ipaddr, sizeof(ipaddr));
+			if (!pghba_check_hostname(node->host, ipaddr, sizeof(ipaddr),
+									  &useHostname))
+			{
+				/* errors have already been logged (DNS failure) */
+			}
 
 			if (!useHostname)
 			{
@@ -709,7 +717,8 @@ pghba_enable_lan_cidr(PGSQL *pgsql,
  * resolve an IP address.)
  */
 bool
-pghba_check_hostname(const char *hostname, char *ipaddr, size_t size)
+pghba_check_hostname(const char *hostname,
+					 char *ipaddr, size_t size, bool *useHostname)
 {
 	/*
 	 * IP addresses do not require any DNS properties/lookups. Also hostname
@@ -720,27 +729,43 @@ pghba_check_hostname(const char *hostname, char *ipaddr, size_t size)
 	 */
 	if (strchr(hostname, '/') || ip_address_type(hostname) != IPTYPE_NONE)
 	{
+		*useHostname = true;
 		return true;
 	}
 
-	if (!resolveHostnameForwardAndReverse(hostname, ipaddr, size))
+	bool foundHostnameFromAddress = false;
+
+	if (!resolveHostnameForwardAndReverse(hostname, ipaddr, size,
+										  &foundHostnameFromAddress))
 	{
-		/* warn users about possible DNS misconfiguration */
-		log_warn("Failed to resolve hostname \"%s\" to an IP address that "
-				 "resolves back to the hostname on a reverse DNS lookup.",
-				 hostname);
+		/* errors have already been logged (DNS failure) */
+		*useHostname = true;
+		return false;
+	}
 
-		log_warn("Postgres might deny connection attempts from \"%s\", "
-				 "even with the new HBA rules.",
-				 hostname);
+	if (foundHostnameFromAddress)
+	{
+		*useHostname = true;
 
-		log_warn("Hint: correct setup of HBA with host names requires proper "
-				 "reverse DNS setup. You might want to use IP addresses.");
+		log_debug("pghba_check_hostname: \"%s\" <-> %s", hostname, ipaddr);
 
-		return false;
+		return true;
 	}
 
-	log_debug("pghba_check_hostname: \"%s\" <-> %s", hostname, ipaddr);
+	*useHostname = false;
+
+	/* warn users about possible DNS misconfiguration */
+	log_warn("Failed to resolve hostname \"%s\" to an IP address that "
+			 "resolves back to the hostname on a reverse DNS lookup.",
+			 hostname);
+
+	log_warn("Postgres might deny connection attempts from \"%s\", "
+			 "even with the new HBA rules.",
+			 hostname);
+
+	log_warn("Hint: correct setup of HBA with host names requires proper "
+			 "reverse DNS setup. You might want to use IP addresses.");
 
+	/* we could successfully check that we should not use the hostname */
 	return true;
 }

src/bin/pg_autoctl/pghba.h

@@ -49,6 +49,7 @@ bool pghba_enable_lan_cidr(PGSQL *pgsql,
 						   HBAEditLevel hbaLevel,
 						   const char *pgdata);
 
-bool pghba_check_hostname(const char *hostname, char *ipaddr, size_t size);
+bool pghba_check_hostname(const char *hostname, char *ipaddr, size_t size,
+						  bool *useHostname);
 
 #endif /* PGHBA_H */