Skip to content

Commit c1bd9a9

Browse files
bukkabukka
committed
Fix phpGH-10495: feof on OpenSSL stream hangs indefinitely
This fixes the issue with unbounded waiting on SSL_peek which can happen when only part of the record is fetched. It makes socket non blocking so it is possible to verify if OpenSSL is expecting some more data or if there is an error. This also fixes bug #79501 Closes phpGH-13487
1 parent c087398 commit c1bd9a9

File tree

3 files changed

+208
-13
lines changed

3 files changed

+208
-13
lines changed

‎NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ PHP NEWS
66
. Fixed bug GH-13563 (Setting bool values via env in FPM config fails).
77
(Jakub Zelenka)
88

9+
- OpenSSL:
10+
. Fixed bug GH-10495 (feof on OpenSSL stream hangs indefinitely).
11+
(Jakub Zelenka)
12+
913
- Streams:
1014
. Fixed bug GH-13264 (Part 1 - Memory leak on stream filter failure).
1115
(Jakub Zelenka)

‎ext/openssl/tests/gh10495.phpt

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
--TEST--
2+
GH-10495: feof hangs indefinitely
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 . 'gh10495.pem.tmp';
12+
$cacertFile = __DIR__ . DIRECTORY_SEPARATOR . 'gh10495-ca.pem.tmp';
13+
14+
$peerName = 'gh10495';
15+
$clientCode = <<<'CODE'
16+
$context = stream_context_create(['ssl' => ['verify_peer' => false, 'peer_name' => '%s']]);
17+
18+
phpt_wait('server');
19+
phpt_notify('proxy');
20+
21+
phpt_wait('proxy');
22+
$fp = stream_socket_client("tlsv1.2://127.0.0.1:10012", $errornum, $errorstr, 1, STREAM_CLIENT_CONNECT, $context);
23+
24+
phpt_wait('proxy');
25+
26+
$time = microtime(true);
27+
var_dump(feof($fp));
28+
var_dump(microtime(true) - $time < 0.5);
29+
30+
var_dump(stream_get_contents($fp, 6));
31+
32+
phpt_notify('server');
33+
phpt_notify('proxy');
34+
CODE;
35+
$clientCode = sprintf($clientCode, $peerName);
36+
37+
$serverCode = <<<'CODE'
38+
$context = stream_context_create(['ssl' => ['local_cert' => '%s']]);
39+
40+
$flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
41+
$fp = stream_socket_server("tlsv1.2://127.0.0.1:10011", $errornum, $errorstr, $flags, $context);
42+
phpt_notify();
43+
44+
$conn = stream_socket_accept($fp);
45+
fwrite($conn, 'warmup');
46+
47+
phpt_wait();
48+
fclose($conn);
49+
CODE;
50+
$serverCode = sprintf($serverCode, $certFile);
51+
52+
$proxyCode = <<<'CODE'
53+
phpt_wait();
54+
55+
$upstream = stream_socket_client("tcp://127.0.0.1:10011", $errornum, $errorstr, 3000, STREAM_CLIENT_CONNECT);
56+
stream_set_blocking($upstream, false);
57+
58+
$flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
59+
$server = stream_socket_server("tcp://127.0.0.1:10012", $errornum, $errorstr, $flags);
60+
phpt_notify();
61+
$conn = stream_socket_accept($server);
62+
stream_set_blocking($conn, false);
63+
64+
$read = [$upstream, $conn];
65+
$applicationData = false;
66+
$i = 1;
67+
while (stream_select($read, $write, $except, 1)) {
68+
foreach ($read as $fp) {
69+
$data = stream_get_contents($fp);
70+
if ($fp === $conn) {
71+
fwrite($upstream, $data);
72+
} else {
73+
if ($data !== '' && $data[0] === chr(23)) {
74+
if (!$applicationData) {
75+
$applicationData = true;
76+
fwrite($conn, $data[0]);
77+
phpt_notify();
78+
sleep(1);
79+
fwrite($conn, substr($data, 1));
80+
} else {
81+
fwrite($conn, $data);
82+
}
83+
} else {
84+
fwrite($conn, $data);
85+
}
86+
}
87+
}
88+
if (feof($upstream)) {
89+
break;
90+
}
91+
$read = [$upstream, $conn];
92+
}
93+
94+
phpt_wait();
95+
CODE;
96+
97+
include 'CertificateGenerator.inc';
98+
$certificateGenerator = new CertificateGenerator();
99+
$certificateGenerator->saveCaCert($cacertFile);
100+
$certificateGenerator->saveNewCertAsFileWithKey($peerName, $certFile);
101+
102+
include 'ServerClientTestCase.inc';
103+
ServerClientTestCase::getInstance()->run($clientCode, [
104+
'server' => $serverCode,
105+
'proxy' => $proxyCode,
106+
]);
107+
?>
108+
--CLEAN--
109+
<?php
110+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh10495.pem.tmp');
111+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh10495-ca.pem.tmp');
112+
?>
113+
--EXPECT--
114+
bool(false)
115+
bool(true)
116+
string(6) "warmup"

‎ext/openssl/xp_ssl.c

Lines changed: 88 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2484,21 +2484,96 @@ static int php_openssl_sockop_set_option(php_stream *stream, int option, int val
24842484
/* the poll() call was skipped if the socket is non-blocking (or MSG_DONTWAIT is available) and if the timeout is zero */
24852485
/* additionally, we don't use this optimization if SSL is active because in that case, we're not using MSG_DONTWAIT */
24862486
if (sslsock->ssl_active) {
2487-
int n = SSL_peek(sslsock->ssl_handle, &buf, sizeof(buf));
2488-
if (n <= 0) {
2489-
int err = SSL_get_error(sslsock->ssl_handle, n);
2490-
switch (err) {
2491-
case SSL_ERROR_SYSCALL:
2492-
alive = php_socket_errno() == EAGAIN;
2493-
break;
2494-
case SSL_ERROR_WANT_READ:
2495-
case SSL_ERROR_WANT_WRITE:
2496-
alive = 1;
2487+
int retry = 1;
2488+
struct timeval start_time;
2489+
struct timeval *timeout = NULL;
2490+
int began_blocked = sslsock->s.is_blocked;
2491+
int has_timeout = 0;
2492+
2493+
/* never use a timeout with non-blocking sockets */
2494+
if (began_blocked) {
2495+
timeout = &tv;
2496+
}
2497+
2498+
if (timeout && php_set_sock_blocking(sslsock->s.socket, 0) == SUCCESS) {
2499+
sslsock->s.is_blocked = 0;
2500+
}
2501+
2502+
if (!sslsock->s.is_blocked && timeout && (timeout->tv_sec > 0 || (timeout->tv_sec == 0 && timeout->tv_usec))) {
2503+
has_timeout = 1;
2504+
/* gettimeofday is not monotonic; using it here is not strictly correct */
2505+
gettimeofday(&start_time, NULL);
2506+
}
2507+
2508+
/* Main IO loop. */
2509+
do {
2510+
struct timeval cur_time, elapsed_time, left_time;
2511+
2512+
/* If we have a timeout to check, figure out how much time has elapsed since we started. */
2513+
if (has_timeout) {
2514+
gettimeofday(&cur_time, NULL);
2515+
2516+
/* Determine how much time we've taken so far. */
2517+
elapsed_time = php_openssl_subtract_timeval(cur_time, start_time);
2518+
2519+
/* and return an error if we've taken too long. */
2520+
if (php_openssl_compare_timeval(elapsed_time, *timeout) > 0 ) {
2521+
/* If the socket was originally blocking, set it back. */
2522+
if (began_blocked) {
2523+
php_set_sock_blocking(sslsock->s.socket, 1);
2524+
sslsock->s.is_blocked = 1;
2525+
}
2526+
sslsock->s.timeout_event = 1;
2527+
return PHP_STREAM_OPTION_RETURN_ERR;
2528+
}
2529+
}
2530+
2531+
int n = SSL_peek(sslsock->ssl_handle, &buf, sizeof(buf));
2532+
/* If we didn't do anything on the last loop (or an error) check to see if we should retry or exit. */
2533+
if (n <= 0) {
2534+
/* Now, do the IO operation. Don't block if we can't complete... */
2535+
int err = SSL_get_error(sslsock->ssl_handle, n);
2536+
switch (err) {
2537+
case SSL_ERROR_SYSCALL:
2538+
retry = php_socket_errno() == EAGAIN;
2539+
break;
2540+
case SSL_ERROR_WANT_READ:
2541+
case SSL_ERROR_WANT_WRITE:
2542+
retry = 1;
2543+
break;
2544+
default:
2545+
/* any other problem is a fatal error */
2546+
retry = 0;
2547+
}
2548+
2549+
/* Don't loop indefinitely in non-blocking mode if no data is available */
2550+
if (began_blocked == 0 || !has_timeout) {
2551+
alive = retry;
24972552
break;
2498-
default:
2499-
/* any other problem is a fatal error */
2500-
alive = 0;
2553+
}
2554+
2555+
/* Now, if we have to wait some time, and we're supposed to be blocking, wait for the socket to become
2556+
* available. Now, php_pollfd_for uses select to wait up to our time_left value only...
2557+
*/
2558+
if (retry) {
2559+
/* Now, how much time until we time out? */
2560+
left_time = php_openssl_subtract_timeval(*timeout, elapsed_time);
2561+
if (php_pollfd_for(sslsock->s.socket, PHP_POLLREADABLE|POLLPRI|POLLOUT, has_timeout ? &left_time : NULL) <= 0) {
2562+
retry = 0;
2563+
alive = 0;
2564+
};
2565+
}
2566+
} else {
2567+
retry = 0;
2568+
alive = 1;
25012569
}
2570+
/* Finally, we keep going until there are any data or there is no time to wait. */
2571+
} while (retry);
2572+
2573+
if (began_blocked && !sslsock->s.is_blocked) {
2574+
// Set it back to blocking
2575+
php_set_sock_blocking(sslsock->s.socket, 1);
2576+
sslsock->s.is_blocked = 1;
25022577
}
25032578
} else if (0 == recv(sslsock->s.socket, &buf, sizeof(buf), MSG_PEEK|MSG_DONTWAIT) && php_socket_errno() != EAGAIN) {
25042579
alive = 0;

0 commit comments

Comments
 (0)