Skip to content

Commit fee476d

Browse files
tglsfdctglsfdc
committed
Improve libpq's error reporting for SSL failures.
In many cases, pqsecure_read/pqsecure_write set up useful error messages, which were then overwritten with useless ones by their callers. Fix this by defining the responsibility to set an error message to be entirely that of the lower-level function when using SSL. Back-patch to 8.3; the code is too different in 8.2 to be worth the trouble.
1 parent d0c2302 commit fee476d

File tree

2 files changed

+67
-18
lines changed

2 files changed

+67
-18
lines changed

‎src/interfaces/libpq/fe-misc.c‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,9 @@ pqReadData(PGconn *conn)
647647
if (SOCK_ERRNO == ECONNRESET)
648648
goto definitelyFailed;
649649
#endif
650-
printfPQExpBuffer(&conn->errorMessage,
650+
/* in SSL mode, pqsecure_read set the error message */
651+
if (conn->ssl == NULL)
652+
printfPQExpBuffer(&conn->errorMessage,
651653
libpq_gettext("could not receive data from server: %s\n"),
652654
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
653655
return -1;
@@ -737,7 +739,9 @@ pqReadData(PGconn *conn)
737739
if (SOCK_ERRNO == ECONNRESET)
738740
goto definitelyFailed;
739741
#endif
740-
printfPQExpBuffer(&conn->errorMessage,
742+
/* in SSL mode, pqsecure_read set the error message */
743+
if (conn->ssl == NULL)
744+
printfPQExpBuffer(&conn->errorMessage,
741745
libpq_gettext("could not receive data from server: %s\n"),
742746
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
743747
return -1;
@@ -753,8 +757,10 @@ pqReadData(PGconn *conn)
753757
* means the connection has been closed. Cope.
754758
*/
755759
definitelyFailed:
756-
printfPQExpBuffer(&conn->errorMessage,
757-
libpq_gettext(
760+
/* in SSL mode, pqsecure_read set the error message */
761+
if (conn->ssl == NULL)
762+
printfPQExpBuffer(&conn->errorMessage,
763+
libpq_gettext(
758764
"server closed the connection unexpectedly\n"
759765
"\tThis probably means the server terminated abnormally\n"
760766
"\tbefore or while processing the request.\n"));
@@ -831,8 +837,10 @@ pqSendSome(PGconn *conn, int len)
831837
#ifdef ECONNRESET
832838
case ECONNRESET:
833839
#endif
834-
printfPQExpBuffer(&conn->errorMessage,
835-
libpq_gettext(
840+
/* in SSL mode, pqsecure_write set the error message */
841+
if (conn->ssl == NULL)
842+
printfPQExpBuffer(&conn->errorMessage,
843+
libpq_gettext(
836844
"server closed the connection unexpectedly\n"
837845
"\tThis probably means the server terminated abnormally\n"
838846
"\tbefore or while processing the request.\n"));
@@ -849,7 +857,9 @@ pqSendSome(PGconn *conn, int len)
849857
return -1;
850858

851859
default:
852-
printfPQExpBuffer(&conn->errorMessage,
860+
/* in SSL mode, pqsecure_write set the error message */
861+
if (conn->ssl == NULL)
862+
printfPQExpBuffer(&conn->errorMessage,
853863
libpq_gettext("could not send data to server: %s\n"),
854864
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
855865
/* We don't assume it's a fatal error... */

‎src/interfaces/libpq/fe-secure.c‎

Lines changed: 50 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,11 @@ pqsecure_close(PGconn *conn)
302302

303303
/*
304304
* Read data from a secure connection.
305+
*
306+
* If SSL is in use, this function is responsible for putting a suitable
307+
* message into conn->errorMessage upon error; but the caller does that
308+
* when not using SSL. In either case, caller uses the returned errno
309+
* to decide whether to continue/retry after error.
305310
*/
306311
ssize_t
307312
pqsecure_read(PGconn *conn, void *ptr, size_t len)
@@ -325,6 +330,13 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
325330
switch (err)
326331
{
327332
case SSL_ERROR_NONE:
333+
if (n < 0)
334+
{
335+
printfPQExpBuffer(&conn->errorMessage,
336+
libpq_gettext("SSL_read failed but did not provide error information\n"));
337+
/* assume the connection is broken */
338+
SOCK_ERRNO_SET(ECONNRESET);
339+
}
328340
break;
329341
case SSL_ERROR_WANT_READ:
330342
n = 0;
@@ -342,7 +354,7 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
342354
{
343355
char sebuf[256];
344356

345-
if (n == -1)
357+
if (n < 0)
346358
{
347359
REMEMBER_EPIPE(spinfo, SOCK_ERRNO == EPIPE);
348360
printfPQExpBuffer(&conn->errorMessage,
@@ -353,29 +365,36 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
353365
{
354366
printfPQExpBuffer(&conn->errorMessage,
355367
libpq_gettext("SSL SYSCALL error: EOF detected\n"));
356-
368+
/* assume the connection is broken */
357369
SOCK_ERRNO_SET(ECONNRESET);
358370
n = -1;
359371
}
360372
break;
361373
}
362374
case SSL_ERROR_SSL:
363375
{
364-
char *err = SSLerrmessage();
376+
char *errm = SSLerrmessage();
365377

366378
printfPQExpBuffer(&conn->errorMessage,
367-
libpq_gettext("SSL error: %s\n"), err);
368-
SSLerrfree(err);
379+
libpq_gettext("SSL error: %s\n"), errm);
380+
SSLerrfree(errm);
381+
/* assume the connection is broken */
382+
SOCK_ERRNO_SET(ECONNRESET);
383+
n = -1;
384+
break;
369385
}
370-
/* fall through */
371386
case SSL_ERROR_ZERO_RETURN:
387+
printfPQExpBuffer(&conn->errorMessage,
388+
libpq_gettext("SSL connection has been closed unexpectedly\n"));
372389
SOCK_ERRNO_SET(ECONNRESET);
373390
n = -1;
374391
break;
375392
default:
376393
printfPQExpBuffer(&conn->errorMessage,
377394
libpq_gettext("unrecognized SSL error code: %d\n"),
378395
err);
396+
/* assume the connection is broken */
397+
SOCK_ERRNO_SET(ECONNRESET);
379398
n = -1;
380399
break;
381400
}
@@ -391,6 +410,11 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
391410

392411
/*
393412
* Write data to a secure connection.
413+
*
414+
* If SSL is in use, this function is responsible for putting a suitable
415+
* message into conn->errorMessage upon error; but the caller does that
416+
* when not using SSL. In either case, caller uses the returned errno
417+
* to decide whether to continue/retry after error.
394418
*/
395419
ssize_t
396420
pqsecure_write(PGconn *conn, const void *ptr, size_t len)
@@ -412,6 +436,13 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
412436
switch (err)
413437
{
414438
case SSL_ERROR_NONE:
439+
if (n < 0)
440+
{
441+
printfPQExpBuffer(&conn->errorMessage,
442+
libpq_gettext("SSL_write failed but did not provide error information\n"));
443+
/* assume the connection is broken */
444+
SOCK_ERRNO_SET(ECONNRESET);
445+
}
415446
break;
416447
case SSL_ERROR_WANT_READ:
417448

@@ -429,7 +460,7 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
429460
{
430461
char sebuf[256];
431462

432-
if (n == -1)
463+
if (n < 0)
433464
{
434465
REMEMBER_EPIPE(spinfo, SOCK_ERRNO == EPIPE);
435466
printfPQExpBuffer(&conn->errorMessage,
@@ -440,28 +471,36 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
440471
{
441472
printfPQExpBuffer(&conn->errorMessage,
442473
libpq_gettext("SSL SYSCALL error: EOF detected\n"));
474+
/* assume the connection is broken */
443475
SOCK_ERRNO_SET(ECONNRESET);
444476
n = -1;
445477
}
446478
break;
447479
}
448480
case SSL_ERROR_SSL:
449481
{
450-
char *err = SSLerrmessage();
482+
char *errm = SSLerrmessage();
451483

452484
printfPQExpBuffer(&conn->errorMessage,
453-
libpq_gettext("SSL error: %s\n"), err);
454-
SSLerrfree(err);
485+
libpq_gettext("SSL error: %s\n"), errm);
486+
SSLerrfree(errm);
487+
/* assume the connection is broken */
488+
SOCK_ERRNO_SET(ECONNRESET);
489+
n = -1;
490+
break;
455491
}
456-
/* fall through */
457492
case SSL_ERROR_ZERO_RETURN:
493+
printfPQExpBuffer(&conn->errorMessage,
494+
libpq_gettext("SSL connection has been closed unexpectedly\n"));
458495
SOCK_ERRNO_SET(ECONNRESET);
459496
n = -1;
460497
break;
461498
default:
462499
printfPQExpBuffer(&conn->errorMessage,
463500
libpq_gettext("unrecognized SSL error code: %d\n"),
464501
err);
502+
/* assume the connection is broken */
503+
SOCK_ERRNO_SET(ECONNRESET);
465504
n = -1;
466505
break;
467506
}

0 commit comments

Comments
 (0)