From 26139bb4a0b2792b4384deb8a82c4a03ffd49ead Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 16 Apr 1999 04:59:03 +0000 Subject: [PATCH] Improve error messages when a connection is rejected. --- src/backend/libpq/auth.c | 88 ++++++++++++++++++++++++++++++++++------ src/backend/libpq/hba.c | 69 ++++++++++++------------------- 2 files changed, 102 insertions(+), 55 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 7ef784e03d..6941934369 100644 --- a/src/backend/libpq/auth.c +++ b/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: diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 984b6da96d..cb0e9fc549 100644 --- a/src/backend/libpq/hba.c +++ b/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; } -- 2.40.0