Skip to content

Commit d4a2a73

Browse files
bukkabukka
committed
Fix GH-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.
1 parent 6f11cc4 commit d4a2a73

File tree

3 files changed

+234
-13
lines changed

3 files changed

+234
-13
lines changed

‎ext/openssl/tests/ServerClientTestCase.inc

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,35 @@ function phpt_has_sslv3() {
2626
return $result;
2727
}
2828

29+
function phpt_extract_tls_records($rawData) {
30+
$records = [];
31+
$offset = 0;
32+
$dataLength = strlen($rawData);
33+
34+
while ($offset < $dataLength) {
35+
// Ensure there's enough data left for the header.
36+
if ($offset + 5 > $dataLength) {
37+
break;
38+
}
39+
40+
// Extract the length of the current record.
41+
$length = unpack("n", substr($rawData, $offset + 3, 2))[1];
42+
43+
// Check if the total length is within the bounds of the rawData.
44+
if ($offset + 5 + $length > $dataLength) {
45+
break;
46+
}
47+
48+
// Extract the record and add it to the records array.
49+
$records[] = substr($rawData, $offset, 5 + $length);
50+
51+
// Move the offset past the current record.
52+
$offset += 5 + $length;
53+
}
54+
55+
return $records;
56+
}
57+
2958
/**
3059
* This is a singleton to let the wait/notify functions work
3160
* I know it's horrible, but it's a means to an end

‎ext/openssl/tests/gh10495.phpt

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
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+
while (stream_select($read, $write, $except, 1)) {
67+
foreach ($read as $fp) {
68+
$data = stream_get_contents($fp);
69+
if ($fp === $conn) {
70+
fwrite($upstream, $data);
71+
} else {
72+
foreach (phpt_extract_tls_records($data) as $record) {
73+
if ($record !== '' && $record[0] === chr(23)) {
74+
if (!$applicationData) {
75+
$applicationData = true;
76+
fwrite($conn, $record[0]);
77+
phpt_notify();
78+
sleep(1);
79+
fwrite($conn, substr($record, 1));
80+
} else {
81+
fwrite($conn, $record);
82+
}
83+
} else {
84+
fwrite($conn, $record);
85+
}
86+
}
87+
}
88+
}
89+
if (feof($upstream)) {
90+
break;
91+
}
92+
$read = [$upstream, $conn];
93+
}
94+
95+
phpt_wait();
96+
CODE;
97+
98+
include 'CertificateGenerator.inc';
99+
$certificateGenerator = new CertificateGenerator();
100+
$certificateGenerator->saveCaCert($cacertFile);
101+
$certificateGenerator->saveNewCertAsFileWithKey($peerName, $certFile);
102+
103+
include 'ServerClientTestCase.inc';
104+
ServerClientTestCase::getInstance()->run($clientCode, [
105+
'server' => $serverCode,
106+
'proxy' => $proxyCode,
107+
]);
108+
?>
109+
--CLEAN--
110+
<?php
111+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh10495.pem.tmp');
112+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh10495-ca.pem.tmp');
113+
?>
114+
--EXPECT--
115+
bool(false)
116+
bool(true)
117+
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)