Improve error messages when a connection is rejected.

src/backend/libpq/auth.c

@@ -7,7 +7,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/libpq/auth.c,v 1.34 1999/03/14 16:06:42 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/libpq/auth.c,v 1.35 1999/04/16 04:59:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -390,13 +390,53 @@ pg_passwordv0_recvauth(void *arg, PacketLen len, void *pkt)
 
 
 /*
- * Tell the user the authentication failed, but not why.
+ * Tell the user the authentication failed, but not (much about) why.
+ *
+ * There is a tradeoff here between security concerns and making life
+ * unnecessarily difficult for legitimate users.  We would not, for example,
+ * want to report the password we were expecting to receive...
+ * But it seems useful to report the username and authorization method
+ * in use, and these are items that must be presumed known to an attacker
+ * anyway.
+ * Note that many sorts of failure report additional information in the
+ * postmaster log, which we hope is only readable by good guys.
  */
 
 void
 auth_failed(Port *port)
 {
-	PacketSendError(&port->pktInfo, "User authentication failed");
+	char	buffer[512];
+	const char *authmethod = "Unknown auth method:";
+
+	switch (port->auth_method)
+	{
+		case uaReject:
+			authmethod = "Rejected host:";
+			break;
+		case uaKrb4:
+			authmethod = "Kerberos4";
+			break;
+		case uaKrb5:
+			authmethod = "Kerberos5";
+			break;
+		case uaTrust:
+			authmethod = "Trusted";
+			break;
+		case uaIdent:
+			authmethod = "IDENT";
+			break;
+		case uaPassword:
+			authmethod = "Password";
+			break;
+		case uaCrypt:
+			authmethod = "Password";
+			break;
+	}
+
+	sprintf(buffer, "%s authentication failed for user '%s'",
+			authmethod, port->user);
+
+	PacketSendError(&port->pktInfo, buffer);
 }
 
 
@@ -409,12 +449,15 @@ be_recvauth(Port *port)
 
 	/*
 	 * Get the authentication method to use for this frontend/database
-	 * combination.
+	 * combination.  Note: a failure return indicates a problem with
+	 * the hba config file, not with the request.  hba.c should have
+	 * dropped an error message into the postmaster logfile if it failed.
 	 */
 
 	if (hba_getauthmethod(&port->raddr, port->user, port->database,
 						port->auth_arg, &port->auth_method) != STATUS_OK)
-		PacketSendError(&port->pktInfo, "Missing or mis-configured pg_hba.conf file");
+		PacketSendError(&port->pktInfo,
+						"Missing or erroneous pg_hba.conf file, see postmaster log for details");
 
 	else if (PG_PROTOCOL_MAJOR(port->proto) == 0)
 	{
@@ -425,20 +468,39 @@ be_recvauth(Port *port)
 	}
 	else
 	{
-		AuthRequest areq;
-		PacketDoneProc auth_handler;
-
-		/* Keep the compiler quiet. */
-
-		areq = AUTH_REQ_OK;
-
 		/* Handle new style authentication. */
 
-		auth_handler = NULL;
+		AuthRequest		areq = AUTH_REQ_OK;
+		PacketDoneProc	auth_handler = NULL;
 
 		switch (port->auth_method)
 		{
 			case uaReject:
+				/*
+				 * This could have come from an explicit "reject" entry
+				 * in pg_hba.conf, but more likely it means there was no
+				 * matching entry.  Take pity on the poor user and issue
+				 * a helpful error message.  NOTE: this is not a security
+				 * breach, because all the info reported here is known
+				 * at the frontend and must be assumed known to bad guys.
+				 * We're merely helping out the less clueful good guys.
+				 * NOTE 2: libpq-be.h defines the maximum error message
+				 * length as 99 characters.  It probably wouldn't hurt
+				 * anything to increase it, but there might be some
+				 * client out there that will fail.  So, be terse.
+				 */
+				{
+					char buffer[512];
+					const char *hostinfo = "localhost";
+
+					if (port->raddr.sa.sa_family == AF_INET)
+						hostinfo = inet_ntoa(port->raddr.in.sin_addr);
+					sprintf(buffer,
+							"No pg_hba.conf entry for host %s, user %s, database %s",
+							hostinfo, port->user, port->database);
+					PacketSendError(&port->pktInfo, buffer);
+					return;
+				}
 				break;
 
 			case uaKrb4:

src/backend/libpq/hba.c

@@ -5,7 +5,7 @@
  *	  wherein you authenticate a user by seeing what IP address the system
  *	  says he comes from and possibly using ident).
  *
- *  $Id: hba.c,v 1.39 1999/02/13 23:15:43 momjian Exp $
+ *  $Id: hba.c,v 1.40 1999/04/16 04:59:03 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -298,55 +298,42 @@ syntax:
 
 static void
 process_open_config_file(FILE *file, SockAddr *raddr, const char *user,
-						 const char *database, bool *host_ok_p,
+						 const char *database, bool *hba_ok_p,
 						 UserAuth *userauth_p, char *auth_arg)
 {
 /*---------------------------------------------------------------------------
   This function does the same thing as find_hba_entry, only with
   the config file already open on stream descriptor "file".
 ----------------------------------------------------------------------------*/
-	bool		found_entry;
+	bool	found_entry = false; /* found an applicable entry? */
+	bool	error = false;		/* found an erroneous entry? */
+	bool	eof = false;		/* end of hba file */
 
-	/* We've processed a record that applies to our connection */
-	bool		error;
-
-	/* Said record has invalid syntax. */
-	bool		eof;			/* We've reached the end of the file we're
-								 * reading */
-
-	found_entry = false;		/* initial value */
-	error = false;				/* initial value */
-	eof = false;				/* initial value */
 	while (!eof && !found_entry && !error)
 	{
 		/* Process a line from the config file */
-
-		int			c;			/* a character read from the file */
-
-		c = getc(file);
-		ungetc(c, file);
+		int		c = getc(file);
 		if (c == EOF)
 			eof = true;
 		else
 		{
+			ungetc(c, file);
 			if (c == '#')
 				read_through_eol(file);
 			else
-			{
 				process_hba_record(file, raddr, user, database,
 							 &found_entry, &error, userauth_p, auth_arg);
 		}
 	}
-	}
 
 	if (!error)
 	{
-		/* If no entry was found then force a rejection. */
+		/* If no matching entry was found, synthesize 'reject' entry. */
 
 		if (!found_entry)
 			*userauth_p = uaReject;
 
-		*host_ok_p = true;
+		*hba_ok_p = true;
 	}
 }
 
@@ -354,25 +341,23 @@ process_open_config_file(FILE *file, SockAddr *raddr, const char *user,
 
 static void
 find_hba_entry(SockAddr *raddr, const char *user, const char *database,
-			   bool *host_ok_p, UserAuth *userauth_p, char *auth_arg)
+			   bool *hba_ok_p, UserAuth *userauth_p, char *auth_arg)
 {
 /*
  * Read the config file and find an entry that allows connection from
- * host "*raddr" to database "database".  If found, return *host_ok_p == true
- * and *userauth_p and *auth_arg representing the contents of that entry.
- *
- * When a record has invalid syntax, we either ignore it or reject the
- * connection (depending on where it's invalid).  No message or anything.
- * We need to fix that some day.
+ * host "raddr", user "user", to database "database".  If found,
+ * return *hba_ok_p = true and *userauth_p and *auth_arg representing
+ * the contents of that entry.  If there is no matching entry, we
+ * set *hba_ok_p = true, *userauth_p = uaReject.
  *
- * If we don't find or can't access the config file, we issue an error
- * message and deny the connection.
+ * If the config file is unreadable or contains invalid syntax, we
+ * issue a diagnostic message to stderr (ie, the postmaster log file)
+ * and return without changing *hba_ok_p.
  *
  * If we find a file by the old name of the config file (pg_hba), we issue
  * an error message because it probably needs to be converted.  He didn't
  * follow directions and just installed his old hba file in the new database
  * system.
- *
  */
 
 	int		fd,
@@ -431,14 +416,13 @@ find_hba_entry(SockAddr *raddr, const char *user, const char *database,
 		}
 		else
 		{
-			process_open_config_file(file, raddr, user, database, host_ok_p, 
+			process_open_config_file(file, raddr, user, database, hba_ok_p, 
 					userauth_p, auth_arg);
 			FreeFile(file);
 		}
 		pfree(conf_file);
 	}
 	pfree(old_conf_file);
-	return;
 }
 
 
@@ -1079,20 +1063,21 @@ GetCharSetByHost(char *TableName, int host, const char *DataDir)
 
 #endif
 
-extern int
+int
 hba_getauthmethod(SockAddr *raddr, char *user, char *database,
 				  char *auth_arg, UserAuth *auth_method)
 {
 /*---------------------------------------------------------------------------
   Determine what authentication method should be used when accessing database
-  "database" from frontend "raddr".  Return the method, an optional argument,
-  and STATUS_OK.
+  "database" from frontend "raddr", user "user".  Return the method,
+  an optional argument, and STATUS_OK.
+  Note that STATUS_ERROR indicates a problem with the hba config file.
+  If the file is OK but does not contain any entry matching the request,
+  we return STATUS_OK and method = uaReject.
 ----------------------------------------------------------------------------*/
-	bool		host_ok;
-
-	host_ok = false;
+	bool		hba_ok = false;
 
-	find_hba_entry(raddr, user, database, &host_ok, auth_method, auth_arg);
+	find_hba_entry(raddr, user, database, &hba_ok, auth_method, auth_arg);
 
-	return host_ok ? STATUS_OK : STATUS_ERROR;
+	return hba_ok ? STATUS_OK : STATUS_ERROR;
 }