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

Commit6f782a2

Browse files
Avoid mixing custom and OpenSSL BIO functions
PostgreSQL has for a long time mixed two BIO implementations, which canlead to subtle bugs and inconsistencies. This cleans up our BIO by justjust setting up the methods we need. This patch does not introduce anyfunctionality changes.The following methods are no longer defined due to not being needed: - gets: Not used by libssl - puts: Not used by libssl - create: Sets up state not used by libpq - destroy: Not used since libpq use BIO_NOCLOSE, if it was used it close the socket from underneath libpq - callback_ctrl: Not implemented by socketsThe following methods are defined for our BIO: - read: Used for reading arbitrary length data from the BIO. No change in functionality from the previous implementation. - write: Used for writing arbitrary length data to the BIO. No change in functionality from the previous implementation. - ctrl: Used for processing ctrl messages in the BIO (similar to ioctl). The only ctrl message which matters is BIO_CTRL_FLUSH used for writing out buffered data (or signal EOF and that no more data will be written). BIO_CTRL_FLUSH is mandatory to implement and is implemented as a no-op since there is no intermediate buffer to flush. BIO_CTRL_EOF is the out-of-band method for signalling EOF to read_ex based BIO's. Our BIO is not read_ex based but someone could accidentally call BIO_CTRL_EOF on us so implement mainly for completeness sake.As the implementation is no longer related to BIO_s_socket or callingSSL_set_fd, methods have been renamed to reference the PGconn and Porttypes instead.This also reverts back to using BIO_set_data, with our fallback, as a smalloptimization as BIO_set_app_data require the ex_data mechanism in OpenSSL.Author: David Benjamin <davidben@google.com>Reviewed-by: Andres Freund <andres@anarazel.de>Reviewed-by: Daniel Gustafsson <daniel@yesql.se>Discussion:https://postgr.es/m/CAF8qwaCZ97AZWXtg_y359SpOHe+HdJ+p0poLCpJYSUxL-8Eo8A@mail.gmail.com
1 parent4e1fad3 commit6f782a2

File tree

4 files changed

+122
-86
lines changed

4 files changed

+122
-86
lines changed

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

Lines changed: 62 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@
5757
staticvoiddefault_openssl_tls_init(SSL_CTX*context,boolisServerStart);
5858
openssl_tls_init_hook_typopenssl_tls_init_hook=default_openssl_tls_init;
5959

60-
staticintmy_sock_read(BIO*h,char*buf,intsize);
61-
staticintmy_sock_write(BIO*h,constchar*buf,intsize);
62-
staticBIO_METHOD*my_BIO_s_socket(void);
63-
staticintmy_SSL_set_fd(Port*port,intfd);
60+
staticintport_bio_read(BIO*h,char*buf,intsize);
61+
staticintport_bio_write(BIO*h,constchar*buf,intsize);
62+
staticBIO_METHOD*port_bio_method(void);
63+
staticintssl_set_port_bio(Port*port);
6464

6565
staticDH*load_dh_file(char*filename,boolisServerStart);
6666
staticDH*load_dh_buffer(constchar*buffer,size_tlen);
@@ -453,7 +453,7 @@ be_tls_open_server(Port *port)
453453
SSLerrmessage(ERR_get_error()))));
454454
return-1;
455455
}
456-
if (!my_SSL_set_fd(port,port->sock))
456+
if (!ssl_set_port_bio(port))
457457
{
458458
ereport(COMMERROR,
459459
(errcode(ERRCODE_PROTOCOL_VIOLATION),
@@ -890,17 +890,19 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
890890
* see sock_read() and sock_write() in OpenSSL's crypto/bio/bss_sock.c.
891891
*/
892892

893-
staticBIO_METHOD*my_bio_methods=NULL;
893+
staticBIO_METHOD*port_bio_method_ptr=NULL;
894894

895895
staticint
896-
my_sock_read(BIO*h,char*buf,intsize)
896+
port_bio_read(BIO*h,char*buf,intsize)
897897
{
898898
intres=0;
899+
Port*port= (Port*)BIO_get_data(h);
899900

900901
if (buf!=NULL)
901902
{
902-
res=secure_raw_read(((Port*)BIO_get_app_data(h)),buf,size);
903+
res=secure_raw_read(port,buf,size);
903904
BIO_clear_retry_flags(h);
905+
port->last_read_was_eof=res==0;
904906
if (res <=0)
905907
{
906908
/* If we were interrupted, tell caller to retry */
@@ -915,11 +917,11 @@ my_sock_read(BIO *h, char *buf, int size)
915917
}
916918

917919
staticint
918-
my_sock_write(BIO*h,constchar*buf,intsize)
920+
port_bio_write(BIO*h,constchar*buf,intsize)
919921
{
920922
intres=0;
921923

922-
res=secure_raw_write(((Port*)BIO_get_app_data(h)),buf,size);
924+
res=secure_raw_write(((Port*)BIO_get_data(h)),buf,size);
923925
BIO_clear_retry_flags(h);
924926
if (res <=0)
925927
{
@@ -933,66 +935,81 @@ my_sock_write(BIO *h, const char *buf, int size)
933935
returnres;
934936
}
935937

938+
staticlong
939+
port_bio_ctrl(BIO*h,intcmd,longnum,void*ptr)
940+
{
941+
longres;
942+
Port*port= (Port*)BIO_get_data(h);
943+
944+
switch (cmd)
945+
{
946+
caseBIO_CTRL_EOF:
947+
948+
/*
949+
* This should not be needed. port_bio_read already has a way to
950+
* signal EOF to OpenSSL. However, OpenSSL made an undocumented,
951+
* backwards-incompatible change and now expects EOF via BIO_ctrl.
952+
* See https://github.com/openssl/openssl/issues/8208
953+
*/
954+
res=port->last_read_was_eof;
955+
break;
956+
caseBIO_CTRL_FLUSH:
957+
/* libssl expects all BIOs to support BIO_flush. */
958+
res=1;
959+
break;
960+
default:
961+
res=0;
962+
break;
963+
}
964+
965+
returnres;
966+
}
967+
936968
staticBIO_METHOD*
937-
my_BIO_s_socket(void)
969+
port_bio_method(void)
938970
{
939-
if (!my_bio_methods)
971+
if (!port_bio_method_ptr)
940972
{
941-
BIO_METHOD*biom= (BIO_METHOD*)BIO_s_socket();
942973
intmy_bio_index;
943974

944975
my_bio_index=BIO_get_new_index();
945976
if (my_bio_index==-1)
946977
returnNULL;
947-
my_bio_index |=(BIO_TYPE_DESCRIPTOR |BIO_TYPE_SOURCE_SINK);
948-
my_bio_methods=BIO_meth_new(my_bio_index,"PostgreSQL backend socket");
949-
if (!my_bio_methods)
978+
my_bio_index |=BIO_TYPE_SOURCE_SINK;
979+
port_bio_method_ptr=BIO_meth_new(my_bio_index,"PostgreSQL backend socket");
980+
if (!port_bio_method_ptr)
950981
returnNULL;
951-
if (!BIO_meth_set_write(my_bio_methods,my_sock_write)||
952-
!BIO_meth_set_read(my_bio_methods,my_sock_read)||
953-
!BIO_meth_set_gets(my_bio_methods,BIO_meth_get_gets(biom))||
954-
!BIO_meth_set_puts(my_bio_methods,BIO_meth_get_puts(biom))||
955-
!BIO_meth_set_ctrl(my_bio_methods,BIO_meth_get_ctrl(biom))||
956-
!BIO_meth_set_create(my_bio_methods,BIO_meth_get_create(biom))||
957-
!BIO_meth_set_destroy(my_bio_methods,BIO_meth_get_destroy(biom))||
958-
!BIO_meth_set_callback_ctrl(my_bio_methods,BIO_meth_get_callback_ctrl(biom)))
982+
if (!BIO_meth_set_write(port_bio_method_ptr,port_bio_write)||
983+
!BIO_meth_set_read(port_bio_method_ptr,port_bio_read)||
984+
!BIO_meth_set_ctrl(port_bio_method_ptr,port_bio_ctrl))
959985
{
960-
BIO_meth_free(my_bio_methods);
961-
my_bio_methods=NULL;
986+
BIO_meth_free(port_bio_method_ptr);
987+
port_bio_method_ptr=NULL;
962988
returnNULL;
963989
}
964990
}
965-
returnmy_bio_methods;
991+
returnport_bio_method_ptr;
966992
}
967993

968-
/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
969994
staticint
970-
my_SSL_set_fd(Port*port,intfd)
995+
ssl_set_port_bio(Port*port)
971996
{
972-
intret=0;
973997
BIO*bio;
974998
BIO_METHOD*bio_method;
975999

976-
bio_method=my_BIO_s_socket();
1000+
bio_method=port_bio_method();
9771001
if (bio_method==NULL)
978-
{
979-
SSLerr(SSL_F_SSL_SET_FD,ERR_R_BUF_LIB);
980-
gotoerr;
981-
}
982-
bio=BIO_new(bio_method);
1002+
return0;
9831003

1004+
bio=BIO_new(bio_method);
9841005
if (bio==NULL)
985-
{
986-
SSLerr(SSL_F_SSL_SET_FD,ERR_R_BUF_LIB);
987-
gotoerr;
988-
}
989-
BIO_set_app_data(bio,port);
1006+
return0;
1007+
1008+
BIO_set_data(bio,port);
1009+
BIO_set_init(bio,1);
9901010

991-
BIO_set_fd(bio,fd,BIO_NOCLOSE);
9921011
SSL_set_bio(port->ssl,bio,bio);
993-
ret=1;
994-
err:
995-
returnret;
1012+
return1;
9961013
}
9971014

9981015
/*

‎src/include/libpq/libpq-be.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ typedef struct Port
204204
char*peer_dn;
205205
boolpeer_cert_valid;
206206
boolalpn_used;
207+
boollast_read_was_eof;
207208

208209
/*
209210
* OpenSSL structures. (Keep these last so that the locations of other

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

Lines changed: 58 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ static char *SSLerrmessage(unsigned long ecode);
7777
staticvoidSSLerrfree(char*buf);
7878
staticintPQssl_passwd_cb(char*buf,intsize,intrwflag,void*userdata);
7979

80-
staticintmy_sock_read(BIO*h,char*buf,intsize);
81-
staticintmy_sock_write(BIO*h,constchar*buf,intsize);
82-
staticBIO_METHOD*my_BIO_s_socket(void);
83-
staticintmy_SSL_set_fd(PGconn*conn,intfd);
80+
staticintpgconn_bio_read(BIO*h,char*buf,intsize);
81+
staticintpgconn_bio_write(BIO*h,constchar*buf,intsize);
82+
staticBIO_METHOD*pgconn_bio_method(void);
83+
staticintssl_set_pgconn_bio(PGconn*conn);
8484

8585
staticpthread_mutex_tssl_config_mutex=PTHREAD_MUTEX_INITIALIZER;
8686

@@ -989,7 +989,7 @@ initialize_SSL(PGconn *conn)
989989
*/
990990
if (!(conn->ssl=SSL_new(SSL_context))||
991991
!SSL_set_app_data(conn->ssl,conn)||
992-
!my_SSL_set_fd(conn,conn->sock))
992+
!ssl_set_pgconn_bio(conn))
993993
{
994994
char*err=SSLerrmessage(ERR_get_error());
995995

@@ -1670,16 +1670,17 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
16701670
*/
16711671

16721672
/* protected by ssl_config_mutex */
1673-
staticBIO_METHOD*my_bio_methods;
1673+
staticBIO_METHOD*pgconn_bio_method_ptr;
16741674

16751675
staticint
1676-
my_sock_read(BIO*h,char*buf,intsize)
1676+
pgconn_bio_read(BIO*h,char*buf,intsize)
16771677
{
1678-
PGconn*conn= (PGconn*)BIO_get_app_data(h);
1678+
PGconn*conn= (PGconn*)BIO_get_data(h);
16791679
intres;
16801680

16811681
res=pqsecure_raw_read(conn,buf,size);
16821682
BIO_clear_retry_flags(h);
1683+
conn->last_read_was_eof=res==0;
16831684
if (res<0)
16841685
{
16851686
/* If we were interrupted, tell caller to retry */
@@ -1707,11 +1708,11 @@ my_sock_read(BIO *h, char *buf, int size)
17071708
}
17081709

17091710
staticint
1710-
my_sock_write(BIO*h,constchar*buf,intsize)
1711+
pgconn_bio_write(BIO*h,constchar*buf,intsize)
17111712
{
17121713
intres;
17131714

1714-
res=pqsecure_raw_write((PGconn*)BIO_get_app_data(h),buf,size);
1715+
res=pqsecure_raw_write((PGconn*)BIO_get_data(h),buf,size);
17151716
BIO_clear_retry_flags(h);
17161717
if (res<0)
17171718
{
@@ -1736,25 +1737,54 @@ my_sock_write(BIO *h, const char *buf, int size)
17361737
returnres;
17371738
}
17381739

1740+
staticlong
1741+
pgconn_bio_ctrl(BIO*h,intcmd,longnum,void*ptr)
1742+
{
1743+
longres;
1744+
PGconn*conn= (PGconn*)BIO_get_data(h);
1745+
1746+
switch (cmd)
1747+
{
1748+
caseBIO_CTRL_EOF:
1749+
1750+
/*
1751+
* This should not be needed. pgconn_bio_read already has a way to
1752+
* signal EOF to OpenSSL. However, OpenSSL made an undocumented,
1753+
* backwards-incompatible change and now expects EOF via BIO_ctrl.
1754+
* See https://github.com/openssl/openssl/issues/8208
1755+
*/
1756+
res=conn->last_read_was_eof;
1757+
break;
1758+
caseBIO_CTRL_FLUSH:
1759+
/* libssl expects all BIOs to support BIO_flush. */
1760+
res=1;
1761+
break;
1762+
default:
1763+
res=0;
1764+
break;
1765+
}
1766+
1767+
returnres;
1768+
}
1769+
17391770
staticBIO_METHOD*
1740-
my_BIO_s_socket(void)
1771+
pgconn_bio_method(void)
17411772
{
17421773
BIO_METHOD*res;
17431774

17441775
if (pthread_mutex_lock(&ssl_config_mutex))
17451776
returnNULL;
17461777

1747-
res=my_bio_methods;
1778+
res=pgconn_bio_method_ptr;
17481779

1749-
if (!my_bio_methods)
1780+
if (!pgconn_bio_method_ptr)
17501781
{
1751-
BIO_METHOD*biom= (BIO_METHOD*)BIO_s_socket();
17521782
intmy_bio_index;
17531783

17541784
my_bio_index=BIO_get_new_index();
17551785
if (my_bio_index==-1)
17561786
gotoerr;
1757-
my_bio_index |=(BIO_TYPE_DESCRIPTOR |BIO_TYPE_SOURCE_SINK);
1787+
my_bio_index |=BIO_TYPE_SOURCE_SINK;
17581788
res=BIO_meth_new(my_bio_index,"libpq socket");
17591789
if (!res)
17601790
gotoerr;
@@ -1763,20 +1793,15 @@ my_BIO_s_socket(void)
17631793
* As of this writing, these functions never fail. But check anyway,
17641794
* like OpenSSL's own examples do.
17651795
*/
1766-
if (!BIO_meth_set_write(res,my_sock_write)||
1767-
!BIO_meth_set_read(res,my_sock_read)||
1768-
!BIO_meth_set_gets(res,BIO_meth_get_gets(biom))||
1769-
!BIO_meth_set_puts(res,BIO_meth_get_puts(biom))||
1770-
!BIO_meth_set_ctrl(res,BIO_meth_get_ctrl(biom))||
1771-
!BIO_meth_set_create(res,BIO_meth_get_create(biom))||
1772-
!BIO_meth_set_destroy(res,BIO_meth_get_destroy(biom))||
1773-
!BIO_meth_set_callback_ctrl(res,BIO_meth_get_callback_ctrl(biom)))
1796+
if (!BIO_meth_set_write(res,pgconn_bio_write)||
1797+
!BIO_meth_set_read(res,pgconn_bio_read)||
1798+
!BIO_meth_set_ctrl(res,pgconn_bio_ctrl))
17741799
{
17751800
gotoerr;
17761801
}
17771802
}
17781803

1779-
my_bio_methods=res;
1804+
pgconn_bio_method_ptr=res;
17801805
pthread_mutex_unlock(&ssl_config_mutex);
17811806
returnres;
17821807

@@ -1787,33 +1812,25 @@ my_BIO_s_socket(void)
17871812
returnNULL;
17881813
}
17891814

1790-
/* This should exactly match OpenSSL's SSL_set_fd except for using my BIO */
17911815
staticint
1792-
my_SSL_set_fd(PGconn*conn,intfd)
1816+
ssl_set_pgconn_bio(PGconn*conn)
17931817
{
1794-
intret=0;
17951818
BIO*bio;
17961819
BIO_METHOD*bio_method;
17971820

1798-
bio_method=my_BIO_s_socket();
1821+
bio_method=pgconn_bio_method();
17991822
if (bio_method==NULL)
1800-
{
1801-
SSLerr(SSL_F_SSL_SET_FD,ERR_R_BUF_LIB);
1802-
gotoerr;
1803-
}
1823+
return0;
1824+
18041825
bio=BIO_new(bio_method);
18051826
if (bio==NULL)
1806-
{
1807-
SSLerr(SSL_F_SSL_SET_FD,ERR_R_BUF_LIB);
1808-
gotoerr;
1809-
}
1810-
BIO_set_app_data(bio,conn);
1827+
return0;
1828+
1829+
BIO_set_data(bio,conn);
1830+
BIO_set_init(bio,1);
18111831

18121832
SSL_set_bio(conn->ssl,bio,bio);
1813-
BIO_set_fd(bio,fd,BIO_NOCLOSE);
1814-
ret=1;
1815-
err:
1816-
returnret;
1833+
return1;
18171834
}
18181835

18191836
/*

‎src/interfaces/libpq/libpq-int.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ struct pg_conn
581581
boolssl_handshake_started;
582582
boolssl_cert_requested;/* Did the server ask us for a cert? */
583583
boolssl_cert_sent;/* Did we send one in reply? */
584+
boollast_read_was_eof;
584585

585586
#ifdefUSE_SSL
586587
#ifdefUSE_OPENSSL

0 commit comments

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