Movatterモバイル変換

[0]ホーム
URL:

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

 Sign up
Appearance settings

Commita3c17b2

Browse files
committed
Distrust external OpenSSL clients; clear err queue
OpenSSL has an unfortunate tendency to mix per-session state errorhandling with per-thread error handling. This can cause problems whenprograms that link to libpq with OpenSSL enabled have some other use ofOpenSSL; without care, one caller of OpenSSL may cause problems for theother caller. Backend code might similarly be affected, for examplewhen a third party extension independently uses OpenSSL without takingthe appropriate precautions.To fix, don't trust other users of OpenSSL to clear the per-thread errorqueue. Instead, clear the entire per-thread queue ahead of certain I/Ooperations when it appears that there might be trouble (these I/Ooperations mostly need to call SSL_get_error() to check for success,which relies on the queue being empty). This is slightly aggressive,but it's pretty clear that the other callers have a very dubious claimto ownership of the per-thread queue. Do this is both frontend andbackend code.Finally, be more careful about clearing our own error queue, so as tonot cause these problems ourself. It's possibly that control previouslydid not always reach SSLerrmessage(), where ERR_get_error() was supposedto be called to clear the queue's earliest code. Make sureERR_get_error() is always called, so as to spare other users of OpenSSLthe possibility of similar problems caused by libpq (as opposed toproblems caused by a third party OpenSSL library like PHP's OpenSSLextension). Again, do this is both frontend and backend code.See bug #12799 andhttps://bugs.php.net/bug.php?id=68276Based on patches by Dave Vitek and Peter Eisentraut.From: Peter Geoghegan <pg@bowt.ie>
1 parentab32a40 commita3c17b2

File tree

2 files changed

+102
-45
lines changed

2 files changed

+102
-45
lines changed

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

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ static DH *tmp_dh_cb(SSL *s, int is_export, int keylength);
7878
staticintverify_cb(int,X509_STORE_CTX*);
7979
staticvoidinfo_cb(constSSL*ssl,inttype,intargs);
8080
staticvoidinitialize_ecdh(void);
81-
staticconstchar*SSLerrmessage(void);
81+
staticconstchar*SSLerrmessage(unsigned longecode);
8282

8383
staticchar*X509_NAME_to_cstring(X509_NAME*name);
8484

@@ -182,7 +182,7 @@ be_tls_init(void)
182182
if (!SSL_context)
183183
ereport(FATAL,
184184
(errmsg("could not create SSL context: %s",
185-
SSLerrmessage())));
185+
SSLerrmessage(ERR_get_error()))));
186186

187187
/*
188188
* Disable OpenSSL's moving-write-buffer sanity check, because it
@@ -198,7 +198,7 @@ be_tls_init(void)
198198
ereport(FATAL,
199199
(errcode(ERRCODE_CONFIG_FILE_ERROR),
200200
errmsg("could not load server certificate file \"%s\": %s",
201-
ssl_cert_file,SSLerrmessage())));
201+
ssl_cert_file,SSLerrmessage(ERR_get_error()))));
202202

203203
if (stat(ssl_key_file,&buf)!=0)
204204
ereport(FATAL,
@@ -228,12 +228,12 @@ be_tls_init(void)
228228
SSL_FILETYPE_PEM)!=1)
229229
ereport(FATAL,
230230
(errmsg("could not load private key file \"%s\": %s",
231-
ssl_key_file,SSLerrmessage())));
231+
ssl_key_file,SSLerrmessage(ERR_get_error()))));
232232

233233
if (SSL_CTX_check_private_key(SSL_context)!=1)
234234
ereport(FATAL,
235235
(errmsg("check of private key failed: %s",
236-
SSLerrmessage())));
236+
SSLerrmessage(ERR_get_error()))));
237237
}
238238

239239
/* set up ephemeral DH keys, and disallow SSL v2/v3 while at it */
@@ -262,7 +262,7 @@ be_tls_init(void)
262262
(root_cert_list=SSL_load_client_CA_file(ssl_ca_file))==NULL)
263263
ereport(FATAL,
264264
(errmsg("could not load root certificate file \"%s\": %s",
265-
ssl_ca_file,SSLerrmessage())));
265+
ssl_ca_file,SSLerrmessage(ERR_get_error()))));
266266
}
267267

268268
/*----------
@@ -293,7 +293,7 @@ be_tls_init(void)
293293
else
294294
ereport(FATAL,
295295
(errmsg("could not load SSL certificate revocation list file \"%s\": %s",
296-
ssl_crl_file,SSLerrmessage())));
296+
ssl_crl_file,SSLerrmessage(ERR_get_error()))));
297297
}
298298
}
299299

@@ -330,6 +330,7 @@ be_tls_open_server(Port *port)
330330
intr;
331331
interr;
332332
intwaitfor;
333+
unsigned longecode;
333334

334335
Assert(!port->ssl);
335336
Assert(!port->peer);
@@ -339,24 +340,44 @@ be_tls_open_server(Port *port)
339340
ereport(COMMERROR,
340341
(errcode(ERRCODE_PROTOCOL_VIOLATION),
341342
errmsg("could not initialize SSL connection: %s",
342-
SSLerrmessage())));
343+
SSLerrmessage(ERR_get_error()))));
343344
return-1;
344345
}
345346
if (!my_SSL_set_fd(port,port->sock))
346347
{
347348
ereport(COMMERROR,
348349
(errcode(ERRCODE_PROTOCOL_VIOLATION),
349350
errmsg("could not set SSL socket: %s",
350-
SSLerrmessage())));
351+
SSLerrmessage(ERR_get_error()))));
351352
return-1;
352353
}
353354
port->ssl_in_use= true;
354355

355356
aloop:
357+
/*
358+
* Prepare to call SSL_get_error() by clearing thread's OpenSSL error
359+
* queue. In general, the current thread's error queue must be empty
360+
* before the TLS/SSL I/O operation is attempted, or SSL_get_error()
361+
* will not work reliably. An extension may have failed to clear the
362+
* per-thread error queue following another call to an OpenSSL I/O
363+
* routine.
364+
*/
365+
ERR_clear_error();
356366
r=SSL_accept(port->ssl);
357367
if (r <=0)
358368
{
359369
err=SSL_get_error(port->ssl,r);
370+
371+
/*
372+
* Other clients of OpenSSL in the backend may fail to call
373+
* ERR_get_error(), but we always do, so as to not cause problems
374+
* for OpenSSL clients that don't call ERR_clear_error()
375+
* defensively. Be sure that this happens by calling now.
376+
* SSL_get_error() relies on the OpenSSL per-thread error queue
377+
* being intact, so this is the earliest possible point
378+
* ERR_get_error() may be called.
379+
*/
380+
ecode=ERR_get_error();
360381
switch (err)
361382
{
362383
caseSSL_ERROR_WANT_READ:
@@ -390,7 +411,7 @@ be_tls_open_server(Port *port)
390411
ereport(COMMERROR,
391412
(errcode(ERRCODE_PROTOCOL_VIOLATION),
392413
errmsg("could not accept SSL connection: %s",
393-
SSLerrmessage())));
414+
SSLerrmessage(ecode))));
394415
break;
395416
caseSSL_ERROR_ZERO_RETURN:
396417
ereport(COMMERROR,
@@ -499,10 +520,13 @@ be_tls_read(Port *port, void *ptr, size_t len, int *waitfor)
499520
{
500521
ssize_tn;
501522
interr;
523+
unsigned longecode;
502524

503525
errno=0;
526+
ERR_clear_error();
504527
n=SSL_read(port->ssl,ptr,len);
505528
err=SSL_get_error(port->ssl,n);
529+
ecode= (err!=SSL_ERROR_NONE||n<0) ?ERR_get_error() :0;
506530
switch (err)
507531
{
508532
caseSSL_ERROR_NONE:
@@ -529,7 +553,7 @@ be_tls_read(Port *port, void *ptr, size_t len, int *waitfor)
529553
caseSSL_ERROR_SSL:
530554
ereport(COMMERROR,
531555
(errcode(ERRCODE_PROTOCOL_VIOLATION),
532-
errmsg("SSL error: %s",SSLerrmessage())));
556+
errmsg("SSL error: %s",SSLerrmessage(ecode))));
533557
/* fall through */
534558
caseSSL_ERROR_ZERO_RETURN:
535559
errno=ECONNRESET;
@@ -556,10 +580,13 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
556580
{
557581
ssize_tn;
558582
interr;
583+
unsigned longecode;
559584

560585
errno=0;
586+
ERR_clear_error();
561587
n=SSL_write(port->ssl,ptr,len);
562588
err=SSL_get_error(port->ssl,n);
589+
ecode= (err!=SSL_ERROR_NONE||n<0) ?ERR_get_error() :0;
563590
switch (err)
564591
{
565592
caseSSL_ERROR_NONE:
@@ -586,7 +613,7 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
586613
caseSSL_ERROR_SSL:
587614
ereport(COMMERROR,
588615
(errcode(ERRCODE_PROTOCOL_VIOLATION),
589-
errmsg("SSL error: %s",SSLerrmessage())));
616+
errmsg("SSL error: %s",SSLerrmessage(ecode))));
590617
/* fall through */
591618
caseSSL_ERROR_ZERO_RETURN:
592619
errno=ECONNRESET;
@@ -742,7 +769,8 @@ load_dh_file(int keylength)
742769
{
743770
if (DH_check(dh,&codes)==0)
744771
{
745-
elog(LOG,"DH_check error (%s): %s",fnbuf,SSLerrmessage());
772+
elog(LOG,"DH_check error (%s): %s",fnbuf,
773+
SSLerrmessage(ERR_get_error()));
746774
returnNULL;
747775
}
748776
if (codes&DH_CHECK_P_NOT_PRIME)
@@ -782,7 +810,7 @@ load_dh_buffer(const char *buffer, size_t len)
782810
if (dh==NULL)
783811
ereport(DEBUG2,
784812
(errmsg_internal("DH load buffer: %s",
785-
SSLerrmessage())));
813+
SSLerrmessage(ERR_get_error()))));
786814
BIO_free(bio);
787815

788816
returndh;
@@ -948,26 +976,26 @@ initialize_ecdh(void)
948976
}
949977

950978
/*
951-
* Obtain reason string for last SSL error
979+
* Obtain reason string for passed SSL errcode
980+
*
981+
* ERR_get_error() is used by caller to get errcode to pass here.
952982
*
953983
* Some caution is needed here since ERR_reason_error_string will
954984
* return NULL if it doesn't recognize the error code. We don't
955985
* want to return NULL ever.
956986
*/
957987
staticconstchar*
958-
SSLerrmessage(void)
988+
SSLerrmessage(unsigned longecode)
959989
{
960-
unsigned longerrcode;
961990
constchar*errreason;
962991
staticcharerrbuf[32];
963992

964-
errcode=ERR_get_error();
965-
if (errcode==0)
993+
if (ecode==0)
966994
return_("no SSL error reported");
967-
errreason=ERR_reason_error_string(errcode);
995+
errreason=ERR_reason_error_string(ecode);
968996
if (errreason!=NULL)
969997
returnerrreason;
970-
snprintf(errbuf,sizeof(errbuf),_("SSL error code %lu"),errcode);
998+
snprintf(errbuf,sizeof(errbuf),_("SSL error code %lu"),ecode);
971999
returnerrbuf;
9721000
}
9731001

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp