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

Commita9d8644

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 parente1d88f9 commita9d8644

File tree

2 files changed

+102
-44
lines changed

2 files changed

+102
-44
lines changed

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

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ static void info_cb(const SSL *ssl, int type, int args);
8686
staticvoidinitialize_SSL(void);
8787
staticintopen_server_SSL(Port*);
8888
staticvoidclose_SSL(Port*);
89-
staticconstchar*SSLerrmessage(void);
89+
staticconstchar*SSLerrmessage(unsigned longecode);
9090
#endif
9191

9292
char*ssl_cert_file;
@@ -245,11 +245,14 @@ secure_read(Port *port, void *ptr, size_t len)
245245
if (port->ssl)
246246
{
247247
interr;
248+
unsigned longecode;
248249

249250
rloop:
250251
errno=0;
252+
ERR_clear_error();
251253
n=SSL_read(port->ssl,ptr,len);
252254
err=SSL_get_error(port->ssl,n);
255+
ecode= (err!=SSL_ERROR_NONE||n<0) ?ERR_get_error() :0;
253256
switch (err)
254257
{
255258
caseSSL_ERROR_NONE:
@@ -281,7 +284,7 @@ secure_read(Port *port, void *ptr, size_t len)
281284
caseSSL_ERROR_SSL:
282285
ereport(COMMERROR,
283286
(errcode(ERRCODE_PROTOCOL_VIOLATION),
284-
errmsg("SSL error: %s",SSLerrmessage())));
287+
errmsg("SSL error: %s",SSLerrmessage(ecode))));
285288
/* fall through */
286289
caseSSL_ERROR_ZERO_RETURN:
287290
errno=ECONNRESET;
@@ -321,6 +324,7 @@ secure_write(Port *port, void *ptr, size_t len)
321324
if (port->ssl)
322325
{
323326
interr;
327+
unsigned longecode;
324328

325329
if (ssl_renegotiation_limit&&port->count>ssl_renegotiation_limit*1024L)
326330
{
@@ -349,8 +353,10 @@ secure_write(Port *port, void *ptr, size_t len)
349353

350354
wloop:
351355
errno=0;
356+
ERR_clear_error();
352357
n=SSL_write(port->ssl,ptr,len);
353358
err=SSL_get_error(port->ssl,n);
359+
ecode= (err!=SSL_ERROR_NONE||n<0) ?ERR_get_error() :0;
354360
switch (err)
355361
{
356362
caseSSL_ERROR_NONE:
@@ -376,7 +382,7 @@ secure_write(Port *port, void *ptr, size_t len)
376382
caseSSL_ERROR_SSL:
377383
ereport(COMMERROR,
378384
(errcode(ERRCODE_PROTOCOL_VIOLATION),
379-
errmsg("SSL error: %s",SSLerrmessage())));
385+
errmsg("SSL error: %s",SSLerrmessage(ecode))));
380386
/* fall through */
381387
caseSSL_ERROR_ZERO_RETURN:
382388
errno=ECONNRESET;
@@ -536,7 +542,8 @@ load_dh_file(int keylength)
536542
{
537543
if (DH_check(dh,&codes)==0)
538544
{
539-
elog(LOG,"DH_check error (%s): %s",fnbuf,SSLerrmessage());
545+
elog(LOG,"DH_check error (%s): %s",fnbuf,
546+
SSLerrmessage(ERR_get_error()));
540547
returnNULL;
541548
}
542549
if (codes&DH_CHECK_P_NOT_PRIME)
@@ -576,7 +583,7 @@ load_dh_buffer(const char *buffer, size_t len)
576583
if (dh==NULL)
577584
ereport(DEBUG2,
578585
(errmsg_internal("DH load buffer: %s",
579-
SSLerrmessage())));
586+
SSLerrmessage(ERR_get_error()))));
580587
BIO_free(bio);
581588

582589
returndh;
@@ -746,7 +753,7 @@ initialize_SSL(void)
746753
if (!SSL_context)
747754
ereport(FATAL,
748755
(errmsg("could not create SSL context: %s",
749-
SSLerrmessage())));
756+
SSLerrmessage(ERR_get_error()))));
750757

751758
/*
752759
* Disable OpenSSL's moving-write-buffer sanity check, because it
@@ -762,7 +769,7 @@ initialize_SSL(void)
762769
ereport(FATAL,
763770
(errcode(ERRCODE_CONFIG_FILE_ERROR),
764771
errmsg("could not load server certificate file \"%s\": %s",
765-
ssl_cert_file,SSLerrmessage())));
772+
ssl_cert_file,SSLerrmessage(ERR_get_error()))));
766773

767774
if (stat(ssl_key_file,&buf)!=0)
768775
ereport(FATAL,
@@ -792,12 +799,12 @@ initialize_SSL(void)
792799
SSL_FILETYPE_PEM)!=1)
793800
ereport(FATAL,
794801
(errmsg("could not load private key file \"%s\": %s",
795-
ssl_key_file,SSLerrmessage())));
802+
ssl_key_file,SSLerrmessage(ERR_get_error()))));
796803

797804
if (SSL_CTX_check_private_key(SSL_context)!=1)
798805
ereport(FATAL,
799806
(errmsg("check of private key failed: %s",
800-
SSLerrmessage())));
807+
SSLerrmessage(ERR_get_error()))));
801808
}
802809

803810
/* set up ephemeral DH keys, and disallow SSL v2 while at it */
@@ -817,7 +824,7 @@ initialize_SSL(void)
817824
(root_cert_list=SSL_load_client_CA_file(ssl_ca_file))==NULL)
818825
ereport(FATAL,
819826
(errmsg("could not load root certificate file \"%s\": %s",
820-
ssl_ca_file,SSLerrmessage())));
827+
ssl_ca_file,SSLerrmessage(ERR_get_error()))));
821828
}
822829

823830
/*----------
@@ -848,7 +855,7 @@ initialize_SSL(void)
848855
else
849856
ereport(FATAL,
850857
(errmsg("could not load SSL certificate revocation list file \"%s\": %s",
851-
ssl_crl_file,SSLerrmessage())));
858+
ssl_crl_file,SSLerrmessage(ERR_get_error()))));
852859
}
853860
}
854861

@@ -884,6 +891,7 @@ open_server_SSL(Port *port)
884891
{
885892
intr;
886893
interr;
894+
unsigned longecode;
887895

888896
Assert(!port->ssl);
889897
Assert(!port->peer);
@@ -893,23 +901,43 @@ open_server_SSL(Port *port)
893901
ereport(COMMERROR,
894902
(errcode(ERRCODE_PROTOCOL_VIOLATION),
895903
errmsg("could not initialize SSL connection: %s",
896-
SSLerrmessage())));
904+
SSLerrmessage(ERR_get_error()))));
897905
return-1;
898906
}
899907
if (!my_SSL_set_fd(port->ssl,port->sock))
900908
{
901909
ereport(COMMERROR,
902910
(errcode(ERRCODE_PROTOCOL_VIOLATION),
903911
errmsg("could not set SSL socket: %s",
904-
SSLerrmessage())));
912+
SSLerrmessage(ERR_get_error()))));
905913
return-1;
906914
}
907915

908916
aloop:
917+
/*
918+
* Prepare to call SSL_get_error() by clearing thread's OpenSSL error
919+
* queue. In general, the current thread's error queue must be empty
920+
* before the TLS/SSL I/O operation is attempted, or SSL_get_error()
921+
* will not work reliably. An extension may have failed to clear the
922+
* per-thread error queue following another call to an OpenSSL I/O
923+
* routine.
924+
*/
925+
ERR_clear_error();
909926
r=SSL_accept(port->ssl);
910927
if (r <=0)
911928
{
912929
err=SSL_get_error(port->ssl,r);
930+
931+
/*
932+
* Other clients of OpenSSL in the backend may fail to call
933+
* ERR_get_error(), but we always do, so as to not cause problems
934+
* for OpenSSL clients that don't call ERR_clear_error()
935+
* defensively. Be sure that this happens by calling now.
936+
* SSL_get_error() relies on the OpenSSL per-thread error queue
937+
* being intact, so this is the earliest possible point
938+
* ERR_get_error() may be called.
939+
*/
940+
ecode=ERR_get_error();
913941
switch (err)
914942
{
915943
caseSSL_ERROR_WANT_READ:
@@ -935,7 +963,7 @@ open_server_SSL(Port *port)
935963
ereport(COMMERROR,
936964
(errcode(ERRCODE_PROTOCOL_VIOLATION),
937965
errmsg("could not accept SSL connection: %s",
938-
SSLerrmessage())));
966+
SSLerrmessage(ecode))));
939967
break;
940968
caseSSL_ERROR_ZERO_RETURN:
941969
ereport(COMMERROR,
@@ -1036,24 +1064,24 @@ close_SSL(Port *port)
10361064
/*
10371065
* Obtain reason string for last SSL error
10381066
*
1067+
* ERR_get_error() is used by caller to get errcode to pass here.
1068+
*
10391069
* Some caution is needed here since ERR_reason_error_string will
10401070
* return NULL if it doesn't recognize the error code. We don't
10411071
* want to return NULL ever.
10421072
*/
10431073
staticconstchar*
1044-
SSLerrmessage(void)
1074+
SSLerrmessage(unsigned longecode)
10451075
{
1046-
unsigned longerrcode;
10471076
constchar*errreason;
10481077
staticcharerrbuf[32];
10491078

1050-
errcode=ERR_get_error();
1051-
if (errcode==0)
1079+
if (ecode==0)
10521080
return_("no SSL error reported");
1053-
errreason=ERR_reason_error_string(errcode);
1081+
errreason=ERR_reason_error_string(ecode);
10541082
if (errreason!=NULL)
10551083
returnerrreason;
1056-
snprintf(errbuf,sizeof(errbuf),_("SSL error code %lu"),errcode);
1084+
snprintf(errbuf,sizeof(errbuf),_("SSL error code %lu"),ecode);
10571085
returnerrbuf;
10581086
}
10591087

0 commit comments

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