From 32be79dcfa1bc5af8682d9f512da68c5b3e2cbf3 Mon Sep 17 00:00:00 2001 From: Chris Wright Date: Sat, 23 Aug 2014 01:40:19 +0100 Subject: [PATCH] Fix stream_select() issue with OpenSSL buffer Ensure data from OpenSSL internal buffer has been transfered to PHP stream buffer before a select() emulation operation is performed Addresses bug #65137 https://bugs.php.net/bug.php?id=65137 Conflicts: ext/openssl/xp_ssl.c --- ext/openssl/xp_ssl.c | 13 +++++++++++++ main/php_streams.h | 3 +++ main/streams/streams.c | 8 ++++---- 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index b7b8690..956ffd0 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -835,6 +881,19 @@ case PHP_STREAM_AS_FD_FOR_SELECT: if (ret) { + if (sslsock->ssl_active) { + /* OpenSSL has an internal buffer which select() cannot see. If we don't + fetch it into the stream's buffer, no activity will be reported on the + stream even though there is data waiting to be read - but we only fetch + the number of bytes OpenSSL has ready to give us since we weren't asked + for any data at this stage. This is only likely to cause issues with + non-blocking streams, but it's harmless to always do it. */ + int bytes; + while ((bytes = SSL_pending(sslsock->ssl_handle)) > 0) { + php_stream_fill_read_buffer(stream, (size_t)bytes); + } + } + *(php_socket_t *)ret = sslsock->s.socket; } return SUCCESS; diff --git a/main/php_streams.h b/main/php_streams.h index 2e4a3a2..89b877f 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -297,6 +297,9 @@ PHPAPI size_t _php_stream_write(php_stream *stream, const char *buf, size_t coun #define php_stream_write_string(stream, str) _php_stream_write(stream, str, strlen(str) TSRMLS_CC) #define php_stream_write(stream, buf, count) _php_stream_write(stream, (buf), (count) TSRMLS_CC) +PHPAPI void _php_stream_fill_read_buffer(php_stream *stream, size_t size TSRMLS_DC); +#define php_stream_fill_read_buffer(stream, size) _php_stream_fill_read_buffer((stream), (size) TSRMLS_CC) + #ifdef ZTS PHPAPI size_t _php_stream_printf(php_stream *stream TSRMLS_DC, const char *fmt, ...) PHP_ATTRIBUTE_FORMAT(printf, 3, 4); #else diff --git a/main/streams/streams.c b/main/streams/streams.c index 3fd4ab3..fbcc1ca 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -568,7 +568,7 @@ fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remov /* {{{ generic stream operations */ -static void php_stream_fill_read_buffer(php_stream *stream, size_t size TSRMLS_DC) +PHPAPI void _php_stream_fill_read_buffer(php_stream *stream, size_t size TSRMLS_DC) { /* allocate/fill the buffer */ @@ -736,7 +736,7 @@ PHPAPI size_t _php_stream_read(php_stream *stream, char *buf, size_t size TSRMLS break; } } else { - php_stream_fill_read_buffer(stream, size TSRMLS_CC); + php_stream_fill_read_buffer(stream, size); toread = stream->writepos - stream->readpos; if (toread > size) { @@ -972,7 +972,7 @@ PHPAPI char *_php_stream_get_line(php_stream *stream, char *buf, size_t maxlen, } } - php_stream_fill_read_buffer(stream, toread TSRMLS_CC); + php_stream_fill_read_buffer(stream, toread); if (stream->writepos - stream->readpos == 0) { break; @@ -1047,7 +1047,7 @@ PHPAPI char *php_stream_get_record(php_stream *stream, size_t maxlen, size_t *re to_read_now = MIN(maxlen - buffered_len, stream->chunk_size); - php_stream_fill_read_buffer(stream, buffered_len + to_read_now TSRMLS_CC); + php_stream_fill_read_buffer(stream, buffered_len + to_read_now); just_read = STREAM_BUFFERED_AMOUNT(stream) - buffered_len; -- 1.9.2 From 84a4041ba47e92e7a0ba03938d0ebf88b5fcf6cf Mon Sep 17 00:00:00 2001 From: Anatol Belski Date: Thu, 7 Aug 2014 19:49:59 +0200 Subject: [PATCH] fix TS build --- ext/openssl/xp_ssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 672070e..b7b8690 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -204,7 +204,7 @@ static size_t php_openssl_sockop_write(php_stream *stream, const char *buf, size return didwrite; } -static void php_openssl_stream_wait_for_data(php_netstream_data_t *sock TSRMLS_DC) +static void php_openssl_stream_wait_for_data(php_netstream_data_t *sock) { int retval; struct timeval *ptimeout; From 6569db88081562f68a4f79e52cba83482bdf05fc Mon Sep 17 00:00:00 2001 From: Daniel Lowrey Date: Thu, 7 Aug 2014 11:47:42 -0400 Subject: [PATCH] Bug #41631: Observe socket read timeouts in SSL streams --- ext/openssl/xp_ssl.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 3082c83..672070e 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -204,13 +204,59 @@ static size_t php_openssl_sockop_write(php_stream *stream, const char *buf, size return didwrite; } +static void php_openssl_stream_wait_for_data(php_netstream_data_t *sock TSRMLS_DC) +{ + int retval; + struct timeval *ptimeout; + + if (sock->socket == -1) { + return; + } + + sock->timeout_event = 0; + + if (sock->timeout.tv_sec == -1) + ptimeout = NULL; + else + ptimeout = &sock->timeout; + + while(1) { + retval = php_pollfd_for(sock->socket, PHP_POLLREADABLE, ptimeout); + + if (retval == 0) + sock->timeout_event = 1; + + if (retval >= 0) + break; + + if (php_socket_errno() != EINTR) + break; + } +} + static size_t php_openssl_sockop_read(php_stream *stream, char *buf, size_t count TSRMLS_DC) { php_openssl_netstream_data_t *sslsock = (php_openssl_netstream_data_t*)stream->abstract; + php_netstream_data_t *sock; int nr_bytes = 0; if (sslsock->ssl_active) { int retry = 1; + sock = (php_netstream_data_t*)stream->abstract; + + /* The SSL_read() function will block indefinitely waiting for data on a blocking + socket. If we don't poll for readability first this operation has the potential + to hang forever. To avoid this scenario we poll with a timeout before performing + the actual read. If it times out we're finished. + */ + if (sock->is_blocked) { + php_openssl_stream_wait_for_data(sock); + if (sock->timeout_event) { + stream->eof = 1; + php_error_docref(NULL TSRMLS_CC, E_WARNING, "SSL read operation timed out"); + return nr_bytes; + } + } do { nr_bytes = SSL_read(sslsock->ssl_handle, buf, count); -- 1.9.2