Skip to content

Commit 8330ca3

Browse files
committed
Fix StreamPollHandle changing unread_bytes of a TLS stream when added
StreamPollHandle took the descriptor through the select cast. On a TLS stream that cast reads: it moves what OpenSSL holds decrypted into the stream buffer so that stream_select() can report it. Adding a watcher to an Io\Poll\Context therefore read from the stream, filled the buffer of a stream set unbuffered with stream_set_read_buffer() and changed unread_bytes. That is not how the poll API should behave. Adding a handle is a registration and must leave the stream as it found it, and the data that moved was not reported by the watcher anyway, as bytes held by the stream layer are not readiness of the descriptor. A new cast PHP_STREAM_AS_FD_FOR_POLL returns the descriptor and nothing else. The socket, TLS, plain and pgsql wrappers answer it, a userspace wrapper sees it as STREAM_CAST_FOR_SELECT and the stream it returns is cast the same way, and a filtered stream allows it like the select cast. StreamPollHandle uses it, so adding a TLS stream leaves its buffer and unread_bytes as they were. The select cast and stream_select() are unchanged. This is fixed in 8.6 because the API is new there and this is the behaviour it should ship with. Keeping a read inside the registration would complicate things later: once reads on a TLS stream can suspend, the cast would become a nested read on a stream with an operation in flight, and changing the cast then would change the visible behaviour of a released API.
1 parent 9d2d793 commit 8330ca3

10 files changed

Lines changed: 97 additions & 5 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,8 @@ PHP NEWS
130130
output handler. (Ilia Alshanetsky)
131131
. Fixed proc_open() leaking descriptor zero when descriptor setup fails.
132132
(Ilia Alshanetsky)
133+
. Fixed StreamPollHandle changing unread_bytes of a TLS stream when it is
134+
added to an Io\Poll\Context. (Jakub Zelenka)
133135

134136
- Tidy:
135137
. Fixed a use-after-free when a tidyNode is used after its document is
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
--TEST--
2+
A stream poll handle takes the descriptor of a TLS stream without touching its buffers
3+
--EXTENSIONS--
4+
openssl
5+
--SKIPIF--
6+
<?php
7+
if (!function_exists("proc_open")) die("skip no proc_open");
8+
?>
9+
--FILE--
10+
<?php
11+
$certFile = __DIR__ . DIRECTORY_SEPARATOR . 'stream_poll_handle_cast.pem.tmp';
12+
13+
$serverCode = <<<'CODE'
14+
$serverCtx = stream_context_create(['ssl' => ['local_cert' => '%s']]);
15+
$sock = stream_socket_server("tls://127.0.0.1:0", $errno, $errstr,
16+
STREAM_SERVER_BIND | STREAM_SERVER_LISTEN, $serverCtx);
17+
phpt_notify_server_start($sock);
18+
19+
$link = stream_socket_accept($sock);
20+
/* One record of 100 bytes */
21+
fwrite($link, str_repeat("x", 100));
22+
phpt_wait();
23+
fclose($link);
24+
CODE;
25+
$serverCode = sprintf($serverCode, $certFile);
26+
27+
$clientCode = <<<'CODE'
28+
$clientCtx = stream_context_create(['ssl' => [
29+
'verify_peer' => false,
30+
'verify_peer_name' => false,
31+
]]);
32+
$sock = stream_socket_client("tls://{{ ADDR }}", $errno, $errstr, 2, STREAM_CLIENT_CONNECT, $clientCtx);
33+
34+
/* 10 of 100 bytes read, 90 stay inside OpenSSL */
35+
stream_set_read_buffer($sock, 0);
36+
var_dump(strlen(fread($sock, 10)));
37+
38+
/* Adding the handle must not move them into the stream buffer */
39+
$ctx = new Io\Poll\Context();
40+
$w = $ctx->add(new StreamPollHandle($sock), [Io\Poll\Event::Read]);
41+
var_dump(stream_get_meta_data($sock)['unread_bytes']);
42+
43+
/* Not readiness of the socket */
44+
var_dump(count($ctx->wait(Time\Duration::fromMilliseconds(100))));
45+
var_dump(strlen(fread($sock, 90)));
46+
47+
/* The peer's close is */
48+
phpt_notify();
49+
$fired = $ctx->wait(Time\Duration::fromSeconds(2));
50+
var_dump(count($fired), $fired[0] === $w);
51+
var_dump(fread($sock, 10), feof($sock));
52+
CODE;
53+
54+
include 'CertificateGenerator.inc';
55+
(new CertificateGenerator())->saveNewCertAsFileWithKey('stream_poll_handle_cast', $certFile);
56+
57+
include 'ServerClientTestCase.inc';
58+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
59+
?>
60+
--CLEAN--
61+
<?php
62+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'stream_poll_handle_cast.pem.tmp');
63+
?>
64+
--EXPECT--
65+
int(10)
66+
int(0)
67+
int(0)
68+
int(90)
69+
int(1)
70+
bool(true)
71+
string(0) ""
72+
bool(true)

‎ext/openssl/xp_ssl.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3909,6 +3909,13 @@ static int php_openssl_sockop_cast(php_stream *stream, int castas, void **ret)
39093909
}
39103910
return SUCCESS;
39113911

3912+
case PHP_STREAM_AS_FD_FOR_POLL:
3913+
/* Descriptor only, OpenSSL pending bytes stay put */
3914+
if (ret) {
3915+
*(php_socket_t *)ret = sslsock->s.socket;
3916+
}
3917+
return SUCCESS;
3918+
39123919
case PHP_STREAM_AS_FD:
39133920
case PHP_STREAM_AS_SOCKETD:
39143921
if (sslsock->ssl_active) {

‎ext/pgsql/pgsql.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4419,6 +4419,7 @@ static int php_pgsql_fd_cast(php_stream *stream, int cast_as, void **ret) /* {{{
44194419

44204420
switch (cast_as) {
44214421
case PHP_STREAM_AS_FD_FOR_SELECT:
4422+
case PHP_STREAM_AS_FD_FOR_POLL:
44224423
case PHP_STREAM_AS_FD:
44234424
case PHP_STREAM_AS_SOCKETD: {
44244425
int fd_number = PQsocket(pgsql);

‎ext/standard/io_poll.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ static php_socket_t php_stream_poll_handle_get_fd(php_poll_handle_object *handle
205205
return SOCK_ERR;
206206
}
207207

208-
if (php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL,
208+
if (php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_POLL | PHP_STREAM_CAST_INTERNAL,
209209
(void *) &fd, 1)
210210
!= SUCCESS
211211
|| fd == -1) {

‎main/php_streams.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,6 +536,8 @@ END_EXTERN_C()
536536
#define PHP_STREAM_AS_FD_FOR_SELECT 3
537537
/* cast as fd/socket for copy purposes */
538538
#define PHP_STREAM_AS_FD_FOR_COPY 4
539+
/* cast as fd/socket for polling, buffers untouched */
540+
#define PHP_STREAM_AS_FD_FOR_POLL 5
539541

540542
/* try really, really hard to make sure the cast happens (avoid using this flag if possible) */
541543
#define PHP_STREAM_CAST_TRY_HARD 0x80000000

‎main/streams/cast.c‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,8 @@ PHPAPI zend_result php_stream_cast(php_stream *stream, int castas, void **ret, i
194194
castas &= ~PHP_STREAM_CAST_MASK;
195195

196196
/* synchronize our buffer (if possible) */
197-
if (ret && castas != PHP_STREAM_AS_FD_FOR_SELECT && castas != PHP_STREAM_AS_FD_FOR_COPY) {
197+
if (ret && castas != PHP_STREAM_AS_FD_FOR_SELECT && castas != PHP_STREAM_AS_FD_FOR_COPY
198+
&& castas != PHP_STREAM_AS_FD_FOR_POLL) {
198199
php_stream_flush(stream);
199200
if (stream->ops->seek && (stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) {
200201
zend_off_t dummy;
@@ -304,7 +305,8 @@ PHPAPI zend_result php_stream_cast(php_stream *stream, int castas, void **ret, i
304305
}
305306
}
306307

307-
if (php_stream_is_filtered(stream) && castas != PHP_STREAM_AS_FD_FOR_SELECT) {
308+
if (php_stream_is_filtered(stream) && castas != PHP_STREAM_AS_FD_FOR_SELECT
309+
&& castas != PHP_STREAM_AS_FD_FOR_POLL) {
308310
if (show_err) {
309311
php_stream_warn(stream, CastNotSupported,
310312
"Cannot cast a filtered stream on this system");
@@ -316,11 +318,13 @@ PHPAPI zend_result php_stream_cast(php_stream *stream, int castas, void **ret, i
316318

317319
if (show_err) {
318320
/* these names depend on the values of the PHP_STREAM_AS_XXX defines in php_streams.h */
319-
static const char *cast_names[4] = {
321+
static const char *cast_names[6] = {
320322
"STDIO FILE*",
321323
"File Descriptor",
322324
"Socket Descriptor",
323-
"select()able descriptor"
325+
"select()able descriptor",
326+
"copyable descriptor",
327+
"pollable descriptor"
324328
};
325329

326330
php_stream_warn(stream, CastNotSupported,

‎main/streams/plain_wrapper.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,7 @@ static int php_stdiop_cast(php_stream *stream, int castas, void **ret)
689689
return SUCCESS;
690690

691691
case PHP_STREAM_AS_FD_FOR_SELECT:
692+
case PHP_STREAM_AS_FD_FOR_POLL:
692693
PHP_STDIOP_GET_FD(fd, data);
693694
if (SOCK_ERR == fd) {
694695
return FAILURE;

‎main/streams/userspace.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1435,6 +1435,8 @@ static int php_userstreamop_cast(php_stream *stream, int castas, void **retptr)
14351435

14361436
switch(castas) {
14371437
case PHP_STREAM_AS_FD_FOR_SELECT:
1438+
case PHP_STREAM_AS_FD_FOR_POLL:
1439+
/* Userland only knows STREAM_CAST_FOR_SELECT */
14381440
ZVAL_LONG(&args[0], PHP_STREAM_AS_FD_FOR_SELECT);
14391441
break;
14401442
default:

‎main/streams/xp_socket.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,7 @@ static int php_sockop_cast(php_stream *stream, int castas, void **ret)
518518
}
519519
return SUCCESS;
520520
case PHP_STREAM_AS_FD_FOR_SELECT:
521+
case PHP_STREAM_AS_FD_FOR_POLL:
521522
case PHP_STREAM_AS_FD:
522523
case PHP_STREAM_AS_SOCKETD:
523524
if (ret)

0 commit comments

Comments
 (0)