Improve error messages when a connection is rejected.
authorTom Lane
Fri, 16 Apr 1999 04:59:03 +0000 (04:59 +0000)
committerTom Lane
Fri, 16 Apr 1999 04:59:03 +0000 (04:59 +0000)
src/backend/libpq/auth.c
src/backend/libpq/hba.c

index 7ef784e03d08f7a449217dd61d26b26cc9655053..6941934369c22f3ba21e52abfa458081d0b8f9f1 100644 (file)
@@ -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:
index 984b6da96d6a78590f6929b61dec766c4ec9aba6..cb0e9fc549381cbb7ac56b88e49dc8e909cb2502 100644 (file)
@@ -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;
 }