Skip to content

Commit 26139bb

Browse files
tglsfdctglsfdc
committed
Improve error messages when a connection is rejected.
1 parent 1d1cf38 commit 26139bb

File tree

2 files changed

+102
-55
lines changed

2 files changed

+102
-55
lines changed

‎src/backend/libpq/auth.c

Lines changed: 75 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/libpq/auth.c,v 1.34 1999/03/14 16:06:42 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/libpq/auth.c,v 1.35 1999/04/16 04:59:03 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -390,13 +390,53 @@ pg_passwordv0_recvauth(void *arg, PacketLen len, void *pkt)
390390

391391

392392
/*
393-
* Tell the user the authentication failed, but not why.
393+
* Tell the user the authentication failed, but not (much about) why.
394+
*
395+
* There is a tradeoff here between security concerns and making life
396+
* unnecessarily difficult for legitimate users. We would not, for example,
397+
* want to report the password we were expecting to receive...
398+
* But it seems useful to report the username and authorization method
399+
* in use, and these are items that must be presumed known to an attacker
400+
* anyway.
401+
* Note that many sorts of failure report additional information in the
402+
* postmaster log, which we hope is only readable by good guys.
394403
*/
395404

396405
void
397406
auth_failed(Port *port)
398407
{
399-
PacketSendError(&port->pktInfo, "User authentication failed");
408+
char buffer[512];
409+
const char *authmethod = "Unknown auth method:";
410+
411+
switch (port->auth_method)
412+
{
413+
case uaReject:
414+
authmethod = "Rejected host:";
415+
break;
416+
case uaKrb4:
417+
authmethod = "Kerberos4";
418+
break;
419+
case uaKrb5:
420+
authmethod = "Kerberos5";
421+
break;
422+
case uaTrust:
423+
authmethod = "Trusted";
424+
break;
425+
case uaIdent:
426+
authmethod = "IDENT";
427+
break;
428+
case uaPassword:
429+
authmethod = "Password";
430+
break;
431+
case uaCrypt:
432+
authmethod = "Password";
433+
break;
434+
}
435+
436+
sprintf(buffer, "%s authentication failed for user '%s'",
437+
authmethod, port->user);
438+
439+
PacketSendError(&port->pktInfo, buffer);
400440
}
401441

402442

@@ -409,12 +449,15 @@ be_recvauth(Port *port)
409449

410450
/*
411451
* Get the authentication method to use for this frontend/database
412-
* combination.
452+
* combination. Note: a failure return indicates a problem with
453+
* the hba config file, not with the request. hba.c should have
454+
* dropped an error message into the postmaster logfile if it failed.
413455
*/
414456

415457
if (hba_getauthmethod(&port->raddr, port->user, port->database,
416458
port->auth_arg, &port->auth_method) != STATUS_OK)
417-
PacketSendError(&port->pktInfo, "Missing or mis-configured pg_hba.conf file");
459+
PacketSendError(&port->pktInfo,
460+
"Missing or erroneous pg_hba.conf file, see postmaster log for details");
418461

419462
else if (PG_PROTOCOL_MAJOR(port->proto) == 0)
420463
{
@@ -425,20 +468,39 @@ be_recvauth(Port *port)
425468
}
426469
else
427470
{
428-
AuthRequest areq;
429-
PacketDoneProc auth_handler;
430-
431-
/* Keep the compiler quiet. */
432-
433-
areq = AUTH_REQ_OK;
434-
435471
/* Handle new style authentication. */
436472

437-
auth_handler = NULL;
473+
AuthRequest areq = AUTH_REQ_OK;
474+
PacketDoneProc auth_handler = NULL;
438475

439476
switch (port->auth_method)
440477
{
441478
case uaReject:
479+
/*
480+
* This could have come from an explicit "reject" entry
481+
* in pg_hba.conf, but more likely it means there was no
482+
* matching entry. Take pity on the poor user and issue
483+
* a helpful error message. NOTE: this is not a security
484+
* breach, because all the info reported here is known
485+
* at the frontend and must be assumed known to bad guys.
486+
* We're merely helping out the less clueful good guys.
487+
* NOTE 2: libpq-be.h defines the maximum error message
488+
* length as 99 characters. It probably wouldn't hurt
489+
* anything to increase it, but there might be some
490+
* client out there that will fail. So, be terse.
491+
*/
492+
{
493+
char buffer[512];
494+
const char *hostinfo = "localhost";
495+
496+
if (port->raddr.sa.sa_family == AF_INET)
497+
hostinfo = inet_ntoa(port->raddr.in.sin_addr);
498+
sprintf(buffer,
499+
"No pg_hba.conf entry for host %s, user %s, database %s",
500+
hostinfo, port->user, port->database);
501+
PacketSendError(&port->pktInfo, buffer);
502+
return;
503+
}
442504
break;
443505

444506
case uaKrb4:

‎src/backend/libpq/hba.c

Lines changed: 27 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* wherein you authenticate a user by seeing what IP address the system
66
* says he comes from and possibly using ident).
77
*
8-
* $Id: hba.c,v 1.39 1999/02/13 23:15:43 momjian Exp $
8+
* $Id: hba.c,v 1.40 1999/04/16 04:59:03 tgl Exp $
99
*
1010
*-------------------------------------------------------------------------
1111
*/
@@ -298,81 +298,66 @@ process_hba_record(FILE *file, SockAddr *raddr, const char *user,
298298

299299
static void
300300
process_open_config_file(FILE *file, SockAddr *raddr, const char *user,
301-
const char *database, bool *host_ok_p,
301+
const char *database, bool *hba_ok_p,
302302
UserAuth *userauth_p, char *auth_arg)
303303
{
304304
/*---------------------------------------------------------------------------
305305
This function does the same thing as find_hba_entry, only with
306306
the config file already open on stream descriptor "file".
307307
----------------------------------------------------------------------------*/
308-
bool found_entry;
308+
bool found_entry = false; /* found an applicable entry? */
309+
bool error = false; /* found an erroneous entry? */
310+
bool eof = false; /* end of hba file */
309311

310-
/* We've processed a record that applies to our connection */
311-
bool error;
312-
313-
/* Said record has invalid syntax. */
314-
bool eof; /* We've reached the end of the file we're
315-
* reading */
316-
317-
found_entry = false; /* initial value */
318-
error = false; /* initial value */
319-
eof = false; /* initial value */
320312
while (!eof && !found_entry && !error)
321313
{
322314
/* Process a line from the config file */
323-
324-
int c; /* a character read from the file */
325-
326-
c = getc(file);
327-
ungetc(c, file);
315+
int c = getc(file);
328316
if (c == EOF)
329317
eof = true;
330318
else
331319
{
320+
ungetc(c, file);
332321
if (c == '#')
333322
read_through_eol(file);
334323
else
335-
{
336324
process_hba_record(file, raddr, user, database,
337325
&found_entry, &error, userauth_p, auth_arg);
338-
}
339326
}
340327
}
341328

342329
if (!error)
343330
{
344-
/* If no entry was found then force a rejection. */
331+
/* If no matching entry was found, synthesize 'reject' entry. */
345332

346333
if (!found_entry)
347334
*userauth_p = uaReject;
348335

349-
*host_ok_p = true;
336+
*hba_ok_p = true;
350337
}
351338
}
352339

353340

354341

355342
static void
356343
find_hba_entry(SockAddr *raddr, const char *user, const char *database,
357-
bool *host_ok_p, UserAuth *userauth_p, char *auth_arg)
344+
bool *hba_ok_p, UserAuth *userauth_p, char *auth_arg)
358345
{
359346
/*
360347
* Read the config file and find an entry that allows connection from
361-
* host "*raddr" to database "database". If found, return *host_ok_p == true
362-
* and *userauth_p and *auth_arg representing the contents of that entry.
363-
*
364-
* When a record has invalid syntax, we either ignore it or reject the
365-
* connection (depending on where it's invalid). No message or anything.
366-
* We need to fix that some day.
348+
* host "raddr", user "user", to database "database". If found,
349+
* return *hba_ok_p = true and *userauth_p and *auth_arg representing
350+
* the contents of that entry. If there is no matching entry, we
351+
* set *hba_ok_p = true, *userauth_p = uaReject.
367352
*
368-
* If we don't find or can't access the config file, we issue an error
369-
* message and deny the connection.
353+
* If the config file is unreadable or contains invalid syntax, we
354+
* issue a diagnostic message to stderr (ie, the postmaster log file)
355+
* and return without changing *hba_ok_p.
370356
*
371357
* If we find a file by the old name of the config file (pg_hba), we issue
372358
* an error message because it probably needs to be converted. He didn't
373359
* follow directions and just installed his old hba file in the new database
374360
* system.
375-
*
376361
*/
377362

378363
int fd,
@@ -431,14 +416,13 @@ find_hba_entry(SockAddr *raddr, const char *user, const char *database,
431416
}
432417
else
433418
{
434-
process_open_config_file(file, raddr, user, database, host_ok_p,
419+
process_open_config_file(file, raddr, user, database, hba_ok_p,
435420
userauth_p, auth_arg);
436421
FreeFile(file);
437422
}
438423
pfree(conf_file);
439424
}
440425
pfree(old_conf_file);
441-
return;
442426
}
443427

444428

@@ -1079,20 +1063,21 @@ GetCharSetByHost(char *TableName, int host, const char *DataDir)
10791063

10801064
#endif
10811065

1082-
extern int
1066+
int
10831067
hba_getauthmethod(SockAddr *raddr, char *user, char *database,
10841068
char *auth_arg, UserAuth *auth_method)
10851069
{
10861070
/*---------------------------------------------------------------------------
10871071
Determine what authentication method should be used when accessing database
1088-
"database" from frontend "raddr". Return the method, an optional argument,
1089-
and STATUS_OK.
1072+
"database" from frontend "raddr", user "user". Return the method,
1073+
an optional argument, and STATUS_OK.
1074+
Note that STATUS_ERROR indicates a problem with the hba config file.
1075+
If the file is OK but does not contain any entry matching the request,
1076+
we return STATUS_OK and method = uaReject.
10901077
----------------------------------------------------------------------------*/
1091-
bool host_ok;
1092-
1093-
host_ok = false;
1078+
bool hba_ok = false;
10941079

1095-
find_hba_entry(raddr, user, database, &host_ok, auth_method, auth_arg);
1080+
find_hba_entry(raddr, user, database, &hba_ok, auth_method, auth_arg);
10961081

1097-
return host_ok ? STATUS_OK : STATUS_ERROR;
1082+
return hba_ok ? STATUS_OK : STATUS_ERROR;
10981083
}

0 commit comments

Comments
 (0)