include host names in hba error messages

Started by Peter Eisentrautover 14 years ago4 messages
#1Peter Eisentraut
peter_e@gmx.net
1 attachment(s)

Since we are accepting host names in pg_hba.conf now, I figured it could
be useful to also show the host names in error message, e.g.,

no pg_hba.conf entry for host "localhost" (127.0.0.1), user "x", database "y"

Attached is an example patch. The question might be what criterion to
use for when to show the host name. It could be

if (port->remote_hostname_resolv == +1)

that is, we have done the reverse and forward lookup, or

if (port->remote_hostname_resolv >= 0)

that is, we have only done the reverse lookup (which is consistent with
log_hostname).

Although this whole thing could be quite weird, because the message that
a host name was rejected because the forward lookup didn't match the IP
address is at DEBUG2, so it's usually never shown. So if we tell
someone that there is 'no pg_hba.conf entry for host "foo"', even though
there is clearly a line saying "foo" in the file, it would be confusing.

Ideas?

Attachments:

hba-error-show-hostname.patchtext/x-patch; charset=UTF-8; name=hba-error-show-hostname.patchDownload
diff --git i/src/backend/libpq/auth.c w/src/backend/libpq/auth.c
index 7799111..3701672 100644
--- i/src/backend/libpq/auth.c
+++ w/src/backend/libpq/auth.c
@@ -442,33 +442,61 @@ ClientAuthentication(Port *port)
 				if (am_walsender)
 				{
 #ifdef USE_SSL
-					ereport(FATAL,
-					   (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\", %s",
-							   hostinfo, port->user_name,
-							   port->ssl ? _("SSL on") : _("SSL off"))));
+					if (port->remote_hostname_resolv == +1)
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for replication connection from host \"%s\" (%s), user \"%s\", %s",
+										port->remote_hostname, hostinfo, port->user_name,
+										port->ssl ? _("SSL on") : _("SSL off"))));
+					else
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\", %s",
+										hostinfo, port->user_name,
+										port->ssl ? _("SSL on") : _("SSL off"))));
 #else
-					ereport(FATAL,
-					   (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\"",
-							   hostinfo, port->user_name)));
+					if (port->remote_hostname_resolv == +1)
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for replication connection from host \"%s\" (%s), user \"%s\"",
+										port->remote_hostname, hostinfo, port->user_name)));
+					else
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\"",
+										hostinfo, port->user_name)));
 #endif
 				}
 				else
 				{
 #ifdef USE_SSL
-					ereport(FATAL,
-					   (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s",
-							   hostinfo, port->user_name,
-							   port->database_name,
-							   port->ssl ? _("SSL on") : _("SSL off"))));
+					if (port->remote_hostname_resolv == +1)
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for host \"%s\" (%s), user \"%s\", database \"%s\", %s",
+										port->remote_hostname, hostinfo, port->user_name,
+										port->database_name,
+										port->ssl ? _("SSL on") : _("SSL off"))));
+					else
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s",
+										hostinfo, port->user_name,
+										port->database_name,
+										port->ssl ? _("SSL on") : _("SSL off"))));
 #else
-					ereport(FATAL,
-					   (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
-						errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\"",
-							   hostinfo, port->user_name,
-							   port->database_name)));
+					if (port->remote_hostname_resolv == +1)
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for host \"%s\" (%s), user \"%s\", database \"%s\"",
+										port->remote_hostname, hostinfo, port->user_name,
+										port->database_name)));
+					else
+						ereport(FATAL,
+								(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+								 errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\"",
+										hostinfo, port->user_name,
+										port->database_name)));
 #endif
 				}
 				break;
#2Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#1)
Re: include host names in hba error messages

On Tue, Jul 19, 2011 at 2:18 AM, Peter Eisentraut <peter_e@gmx.net> wrote:

Since we are accepting host names in pg_hba.conf now, I figured it could
be useful to also show the host names in error message, e.g.,

   no pg_hba.conf entry for host "localhost" (127.0.0.1), user "x", database "y"

Attached is an example patch.  The question might be what criterion to
use for when to show the host name.  It could be

   if (port->remote_hostname_resolv == +1)

that is, we have done the reverse and forward lookup, or

   if (port->remote_hostname_resolv >= 0)

that is, we have only done the reverse lookup (which is consistent with
log_hostname).

Although this whole thing could be quite weird, because the message that
a host name was rejected because the forward lookup didn't match the IP
address is at DEBUG2, so it's usually never shown.  So if we tell
someone that there is 'no pg_hba.conf entry for host "foo"', even though
there is clearly a line saying "foo" in the file, it would be confusing.

Ideas?

I think it would be less confusing to write the IP address as the main
piece of information, and put the hostname in parentheses only if we
accepted it as valid (i.e. we did both lookups, and everything
matched).

ERROR: no pg_hba.conf entry for host 127.0.0.1 ("localhost"), user
"x", database "y"

As for the case where we the forward lookup and reverse lookup don't
match, could we add that as a DETAIL?

ERROR: no pg_hba.conf entry for host 127.0.0.1, user "x", database "y"
DETAIL: Forward and reverse DNS lookups do not match.

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

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#2)
1 attachment(s)
Re: include host names in hba error messages

On tis, 2011-07-19 at 14:17 -0400, Robert Haas wrote:

I think it would be less confusing to write the IP address as the main
piece of information, and put the hostname in parentheses only if we
accepted it as valid (i.e. we did both lookups, and everything
matched).

ERROR: no pg_hba.conf entry for host 127.0.0.1 ("localhost"), user
"x", database "y"

As for the case where we the forward lookup and reverse lookup don't
match, could we add that as a DETAIL?

ERROR: no pg_hba.conf entry for host 127.0.0.1, user "x", database "y"
DETAIL: Forward and reverse DNS lookups do not match.

On further reflection, the only way we would get a complete match host
name is if there actually were a line in pg_hba.conf with that host
name, but it didn't match because of other parameters. So that would be
quite rare, and so the error message would look one way or the other
depending on obscure circumstances, which would be confusing.

But picking up on your second suggestion, I propose instead that we put
a note in the detail about the host name and what we know about it, if
we know it, e.g.

ERROR: no pg_hba.conf entry for host 127.0.0.1, user "x", database "y"
DETAIL: Client IP address resolved to "localhost", forward lookup matches.

I chose to use errdetail_log(), which only goes into the server log, so
we don't expose too much about the server's DNS setup to the client.

Attachments:

hba-error-show-hostname.patchtext/x-patch; charset=UTF-8; name=hba-error-show-hostname.patchDownload
diff --git i/src/backend/libpq/auth.c w/src/backend/libpq/auth.c
index d153880..1b6399d 100644
--- i/src/backend/libpq/auth.c
+++ w/src/backend/libpq/auth.c
@@ -439,6 +439,17 @@ ClientAuthentication(Port *port)
 								   NULL, 0,
 								   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)
+
 				if (am_walsender)
 				{
 #ifdef USE_SSL
@@ -446,12 +457,14 @@ ClientAuthentication(Port *port)
 					   (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
 						errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\", %s",
 							   hostinfo, port->user_name,
-							   port->ssl ? _("SSL on") : _("SSL off"))));
+							   port->ssl ? _("SSL on") : _("SSL off")),
+						HOSTNAME_LOOKUP_DETAIL(port)));
 #else
 					ereport(FATAL,
 					   (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
 						errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\"",
-							   hostinfo, port->user_name)));
+							   hostinfo, port->user_name),
+						HOSTNAME_LOOKUP_DETAIL(port)));
 #endif
 				}
 				else
@@ -462,13 +475,15 @@ ClientAuthentication(Port *port)
 						errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s",
 							   hostinfo, port->user_name,
 							   port->database_name,
-							   port->ssl ? _("SSL on") : _("SSL off"))));
+							   port->ssl ? _("SSL on") : _("SSL off")),
+						HOSTNAME_LOOKUP_DETAIL(port)));
 #else
 					ereport(FATAL,
 					   (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
 						errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\"",
 							   hostinfo, port->user_name,
-							   port->database_name)));
+							   port->database_name),
+						HOSTNAME_LOOKUP_DETAIL(port)));
 #endif
 				}
 				break;
#4Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#3)
Re: include host names in hba error messages

On Fri, Jul 29, 2011 at 2:44 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On tis, 2011-07-19 at 14:17 -0400, Robert Haas wrote:

I think it would be less confusing to write the IP address as the main
piece of information, and put the hostname in parentheses only if we
accepted it as valid (i.e. we did both lookups, and everything
matched).

ERROR: no pg_hba.conf entry for host 127.0.0.1 ("localhost"), user
"x", database "y"

As for the case where we the forward lookup and reverse lookup don't
match, could we add that as a DETAIL?

ERROR: no pg_hba.conf entry for host 127.0.0.1, user "x", database "y"
DETAIL: Forward and reverse DNS lookups do not match.

On further reflection, the only way we would get a complete match host
name is if there actually were a line in pg_hba.conf with that host
name, but it didn't match because of other parameters.  So that would be
quite rare, and so the error message would look one way or the other
depending on obscure circumstances, which would be confusing.

But picking up on your second suggestion, I propose instead that we put
a note in the detail about the host name and what we know about it, if
we know it, e.g.

ERROR: no pg_hba.conf entry for host 127.0.0.1, user "x", database "y"
DETAIL: Client IP address resolved to "localhost", forward lookup matches.

I chose to use errdetail_log(), which only goes into the server log, so
we don't expose too much about the server's DNS setup to the client.

Seems reasonable.

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