Skip to content

Commit 5097b83

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 551458b commit 5097b83

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
@@ -625,7 +625,9 @@ pqReadData(PGconn *conn)
625625
if (SOCK_ERRNO == ECONNRESET)
626626
goto definitelyFailed;
627627
#endif
628-
printfPQExpBuffer(&conn->errorMessage,
628+
/* in SSL mode, pqsecure_read set the error message */
629+
if (conn->ssl == NULL)
630+
printfPQExpBuffer(&conn->errorMessage,
629631
libpq_gettext("could not receive data from server: %s\n"),
630632
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
631633
return -1;
@@ -715,7 +717,9 @@ pqReadData(PGconn *conn)
715717
if (SOCK_ERRNO == ECONNRESET)
716718
goto definitelyFailed;
717719
#endif
718-
printfPQExpBuffer(&conn->errorMessage,
720+
/* in SSL mode, pqsecure_read set the error message */
721+
if (conn->ssl == NULL)
722+
printfPQExpBuffer(&conn->errorMessage,
719723
libpq_gettext("could not receive data from server: %s\n"),
720724
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
721725
return -1;
@@ -731,8 +735,10 @@ pqReadData(PGconn *conn)
731735
* means the connection has been closed. Cope.
732736
*/
733737
definitelyFailed:
734-
printfPQExpBuffer(&conn->errorMessage,
735-
libpq_gettext(
738+
/* in SSL mode, pqsecure_read set the error message */
739+
if (conn->ssl == NULL)
740+
printfPQExpBuffer(&conn->errorMessage,
741+
libpq_gettext(
736742
"server closed the connection unexpectedly\n"
737743
"\tThis probably means the server terminated abnormally\n"
738744
"\tbefore or while processing the request.\n"));
@@ -808,8 +814,10 @@ pqSendSome(PGconn *conn, int len)
808814
#ifdef ECONNRESET
809815
case ECONNRESET:
810816
#endif
811-
printfPQExpBuffer(&conn->errorMessage,
812-
libpq_gettext(
817+
/* in SSL mode, pqsecure_write set the error message */
818+
if (conn->ssl == NULL)
819+
printfPQExpBuffer(&conn->errorMessage,
820+
libpq_gettext(
813821
"server closed the connection unexpectedly\n"
814822
"\tThis probably means the server terminated abnormally\n"
815823
"\tbefore or while processing the request.\n"));
@@ -826,7 +834,9 @@ pqSendSome(PGconn *conn, int len)
826834
return -1;
827835

828836
default:
829-
printfPQExpBuffer(&conn->errorMessage,
837+
/* in SSL mode, pqsecure_write set the error message */
838+
if (conn->ssl == NULL)
839+
printfPQExpBuffer(&conn->errorMessage,
830840
libpq_gettext("could not send data to server: %s\n"),
831841
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
832842
/* 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
@@ -301,6 +301,11 @@ pqsecure_close(PGconn *conn)
301301

302302
/*
303303
* Read data from a secure connection.
304+
*
305+
* If SSL is in use, this function is responsible for putting a suitable
306+
* message into conn->errorMessage upon error; but the caller does that
307+
* when not using SSL. In either case, caller uses the returned errno
308+
* to decide whether to continue/retry after error.
304309
*/
305310
ssize_t
306311
pqsecure_read(PGconn *conn, void *ptr, size_t len)
@@ -322,6 +327,13 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
322327
switch (err)
323328
{
324329
case SSL_ERROR_NONE:
330+
if (n < 0)
331+
{
332+
printfPQExpBuffer(&conn->errorMessage,
333+
libpq_gettext("SSL_read failed but did not provide error information\n"));
334+
/* assume the connection is broken */
335+
SOCK_ERRNO_SET(ECONNRESET);
336+
}
325337
break;
326338
case SSL_ERROR_WANT_READ:
327339
n = 0;
@@ -339,7 +351,7 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
339351
{
340352
char sebuf[256];
341353

342-
if (n == -1)
354+
if (n < 0)
343355
{
344356
REMEMBER_EPIPE(SOCK_ERRNO == EPIPE);
345357
printfPQExpBuffer(&conn->errorMessage,
@@ -350,29 +362,36 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
350362
{
351363
printfPQExpBuffer(&conn->errorMessage,
352364
libpq_gettext("SSL SYSCALL error: EOF detected\n"));
353-
365+
/* assume the connection is broken */
354366
SOCK_ERRNO_SET(ECONNRESET);
355367
n = -1;
356368
}
357369
break;
358370
}
359371
case SSL_ERROR_SSL:
360372
{
361-
char *err = SSLerrmessage();
373+
char *errm = SSLerrmessage();
362374

363375
printfPQExpBuffer(&conn->errorMessage,
364-
libpq_gettext("SSL error: %s\n"), err);
365-
SSLerrfree(err);
376+
libpq_gettext("SSL error: %s\n"), errm);
377+
SSLerrfree(errm);
378+
/* assume the connection is broken */
379+
SOCK_ERRNO_SET(ECONNRESET);
380+
n = -1;
381+
break;
366382
}
367-
/* fall through */
368383
case SSL_ERROR_ZERO_RETURN:
384+
printfPQExpBuffer(&conn->errorMessage,
385+
libpq_gettext("SSL connection has been closed unexpectedly\n"));
369386
SOCK_ERRNO_SET(ECONNRESET);
370387
n = -1;
371388
break;
372389
default:
373390
printfPQExpBuffer(&conn->errorMessage,
374391
libpq_gettext("unrecognized SSL error code: %d\n"),
375392
err);
393+
/* assume the connection is broken */
394+
SOCK_ERRNO_SET(ECONNRESET);
376395
n = -1;
377396
break;
378397
}
@@ -388,6 +407,11 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)
388407

389408
/*
390409
* Write data to a secure connection.
410+
*
411+
* If SSL is in use, this function is responsible for putting a suitable
412+
* message into conn->errorMessage upon error; but the caller does that
413+
* when not using SSL. In either case, caller uses the returned errno
414+
* to decide whether to continue/retry after error.
391415
*/
392416
ssize_t
393417
pqsecure_write(PGconn *conn, const void *ptr, size_t len)
@@ -407,6 +431,13 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
407431
switch (err)
408432
{
409433
case SSL_ERROR_NONE:
434+
if (n < 0)
435+
{
436+
printfPQExpBuffer(&conn->errorMessage,
437+
libpq_gettext("SSL_write failed but did not provide error information\n"));
438+
/* assume the connection is broken */
439+
SOCK_ERRNO_SET(ECONNRESET);
440+
}
410441
break;
411442
case SSL_ERROR_WANT_READ:
412443

@@ -424,7 +455,7 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
424455
{
425456
char sebuf[256];
426457

427-
if (n == -1)
458+
if (n < 0)
428459
{
429460
REMEMBER_EPIPE(SOCK_ERRNO == EPIPE);
430461
printfPQExpBuffer(&conn->errorMessage,
@@ -435,28 +466,36 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t len)
435466
{
436467
printfPQExpBuffer(&conn->errorMessage,
437468
libpq_gettext("SSL SYSCALL error: EOF detected\n"));
469+
/* assume the connection is broken */
438470
SOCK_ERRNO_SET(ECONNRESET);
439471
n = -1;
440472
}
441473
break;
442474
}
443475
case SSL_ERROR_SSL:
444476
{
445-
char *err = SSLerrmessage();
477+
char *errm = SSLerrmessage();
446478

447479
printfPQExpBuffer(&conn->errorMessage,
448-
libpq_gettext("SSL error: %s\n"), err);
449-
SSLerrfree(err);
480+
libpq_gettext("SSL error: %s\n"), errm);
481+
SSLerrfree(errm);
482+
/* assume the connection is broken */
483+
SOCK_ERRNO_SET(ECONNRESET);
484+
n = -1;
485+
break;
450486
}
451-
/* fall through */
452487
case SSL_ERROR_ZERO_RETURN:
488+
printfPQExpBuffer(&conn->errorMessage,
489+
libpq_gettext("SSL connection has been closed unexpectedly\n"));
453490
SOCK_ERRNO_SET(ECONNRESET);
454491
n = -1;
455492
break;
456493
default:
457494
printfPQExpBuffer(&conn->errorMessage,
458495
libpq_gettext("unrecognized SSL error code: %d\n"),
459496
err);
497+
/* assume the connection is broken */
498+
SOCK_ERRNO_SET(ECONNRESET);
460499
n = -1;
461500
break;
462501
}

0 commit comments

Comments
 (0)