Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 36a3be6

Browse filesBrowse files
committed
Fix new and latent bugs with errno handling in secure_read/secure_write.
These functions must be careful that they return the intended value of errno to their callers. There were several scenarios where this might not happen: 1. The recent SSL renegotiation patch added a hunk of code that would execute after setting errno. In the first place, it's doubtful that we should consider renegotiation to be successfully completed after a failure, and in the second, there's no real guarantee that the called OpenSSL routines wouldn't clobber errno. Fix by not executing that hunk except during success exit. 2. errno was left in an unknown state in case of an unrecognized return code from SSL_get_error(). While this is a "can't happen" case, it seems like a good idea to be sure we know what would happen, so reset errno to ECONNRESET in such cases. (The corresponding code in libpq's fe-secure.c already did this.) 3. There was an (undocumented) assumption that client_read_ended() wouldn't change errno. While true in the current state of the code, this seems less than future-proof. Add explicit saving/restoring of errno to make sure that changes in the called functions won't break things. I see no need to back-patch, since #1 is new code and the other two issues are mostly hypothetical. Per discussion with Amit Kapila.
1 parent 08d1b22 commit 36a3be6
Copy full SHA for 36a3be6

File tree

2 files changed

+27
-16
lines changed
Filter options

2 files changed

+27
-16
lines changed

‎src/backend/libpq/be-secure.c

Copy file name to clipboardExpand all lines: src/backend/libpq/be-secure.c
+21-16Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,7 @@ secure_read(Port *port, void *ptr, size_t len)
295295
(errcode(ERRCODE_PROTOCOL_VIOLATION),
296296
errmsg("unrecognized SSL error code: %d",
297297
err)));
298+
errno = ECONNRESET;
298299
n = -1;
299300
break;
300301
}
@@ -416,28 +417,32 @@ secure_write(Port *port, void *ptr, size_t len)
416417
(errcode(ERRCODE_PROTOCOL_VIOLATION),
417418
errmsg("unrecognized SSL error code: %d",
418419
err)));
420+
errno = ECONNRESET;
419421
n = -1;
420422
break;
421423
}
422424

423-
/* is renegotiation complete? */
424-
if (in_ssl_renegotiation &&
425-
SSL_num_renegotiations(port->ssl) >= 1)
425+
if (n >= 0)
426426
{
427-
in_ssl_renegotiation = false;
428-
port->count = 0;
429-
}
427+
/* is renegotiation complete? */
428+
if (in_ssl_renegotiation &&
429+
SSL_num_renegotiations(port->ssl) >= 1)
430+
{
431+
in_ssl_renegotiation = false;
432+
port->count = 0;
433+
}
430434

431-
/*
432-
* if renegotiation is still ongoing, and we've gone beyond the limit,
433-
* kill the connection now -- continuing to use it can be considered a
434-
* security problem.
435-
*/
436-
if (in_ssl_renegotiation &&
437-
port->count > ssl_renegotiation_limit * 1024L)
438-
ereport(FATAL,
439-
(errcode(ERRCODE_PROTOCOL_VIOLATION),
440-
errmsg("SSL failed to renegotiate connection before limit expired")));
435+
/*
436+
* if renegotiation is still ongoing, and we've gone beyond the
437+
* limit, kill the connection now -- continuing to use it can be
438+
* considered a security problem.
439+
*/
440+
if (in_ssl_renegotiation &&
441+
port->count > ssl_renegotiation_limit * 1024L)
442+
ereport(FATAL,
443+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
444+
errmsg("SSL failed to renegotiate connection before limit expired")));
445+
}
441446
}
442447
else
443448
#endif

‎src/backend/tcop/postgres.c

Copy file name to clipboardExpand all lines: src/backend/tcop/postgres.c
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,16 +526,22 @@ prepare_for_client_read(void)
526526

527527
/*
528528
* client_read_ended -- get out of the client-input state
529+
*
530+
* This is called just after low-level reads. It must preserve errno!
529531
*/
530532
void
531533
client_read_ended(void)
532534
{
533535
if (DoingCommandRead)
534536
{
537+
int save_errno = errno;
538+
535539
ImmediateInterruptOK = false;
536540

537541
DisableNotifyInterrupt();
538542
DisableCatchupInterrupt();
543+
544+
errno = save_errno;
539545
}
540546
}
541547

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.