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. This also fixes bug #79501 Closes GH-13487
php · Mar 29, 2024 · c1bd9a9 · c1bd9a9
1 parent c087398
commit c1bd9a9
Show file tree

Hide file tree

Showing 3 changed files with 208 additions and 13 deletions.
diff --git a/NEWS b/NEWS
@@ -6,6 +6,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-13563 (Setting bool values via env in FPM config fails).
     (Jakub Zelenka)
 
+- OpenSSL:
+  . Fixed bug GH-10495 (feof on OpenSSL stream hangs indefinitely).
+    (Jakub Zelenka)
+
 - Streams:
   . Fixed bug GH-13264 (Part 1 - Memory leak on stream filter failure).
     (Jakub Zelenka)

diff --git a/ext/openssl/tests/gh10495.phpt b/ext/openssl/tests/gh10495.phpt
@@ -0,0 +1,116 @@
+--TEST--
+GH-10495: feof hangs indefinitely
+--EXTENSIONS--
+openssl
+--SKIPIF--
+<?php
+if (!function_exists("proc_open")) die("skip no proc_open");
+?>
+--FILE--
+<?php
+$certFile = __DIR__ . DIRECTORY_SEPARATOR . 'gh10495.pem.tmp';
+$cacertFile = __DIR__ . DIRECTORY_SEPARATOR . 'gh10495-ca.pem.tmp';
+
+$peerName = 'gh10495';
+$clientCode = <<<'CODE'
+    $context = stream_context_create(['ssl' => ['verify_peer' => false, 'peer_name' => '%s']]);
+
+    phpt_wait('server');
+    phpt_notify('proxy');
+
+    phpt_wait('proxy');
+    $fp = stream_socket_client("tlsv1.2://127.0.0.1:10012", $errornum, $errorstr, 1, STREAM_CLIENT_CONNECT, $context);
+
+    phpt_wait('proxy');
+
+    $time = microtime(true);
+    var_dump(feof($fp));
+    var_dump(microtime(true) - $time < 0.5);
+
+    var_dump(stream_get_contents($fp, 6));
+
+    phpt_notify('server');
+    phpt_notify('proxy');
+CODE;
+$clientCode = sprintf($clientCode, $peerName);
+
+$serverCode = <<<'CODE'
+    $context = stream_context_create(['ssl' => ['local_cert' => '%s']]);
+
+    $flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
+    $fp = stream_socket_server("tlsv1.2://127.0.0.1:10011", $errornum, $errorstr, $flags, $context);
+    phpt_notify();
+
+    $conn = stream_socket_accept($fp);
+    fwrite($conn, 'warmup');
+
+    phpt_wait();
+    fclose($conn);
+CODE;
+$serverCode = sprintf($serverCode, $certFile);
+
+$proxyCode = <<<'CODE'
+    phpt_wait();
+
+    $upstream = stream_socket_client("tcp://127.0.0.1:10011", $errornum, $errorstr, 3000, STREAM_CLIENT_CONNECT);
+    stream_set_blocking($upstream, false);
+
+    $flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
+    $server = stream_socket_server("tcp://127.0.0.1:10012", $errornum, $errorstr, $flags);
+    phpt_notify();
+    $conn = stream_socket_accept($server);
+    stream_set_blocking($conn, false);
+
+    $read = [$upstream, $conn];
+    $applicationData = false;
+    $i = 1;
+    while (stream_select($read, $write, $except, 1)) {
+        foreach ($read as $fp) {
+            $data = stream_get_contents($fp);
+            if ($fp === $conn) {
+                fwrite($upstream, $data);
+            } else {
+                if ($data !== '' && $data[0] === chr(23)) {
+                    if (!$applicationData) {
+                        $applicationData = true;
+                        fwrite($conn, $data[0]);
+                        phpt_notify();
+                        sleep(1);
+                        fwrite($conn, substr($data, 1));
+                    } else {
+                        fwrite($conn, $data);
+                    }
+                } else {
+                    fwrite($conn, $data);
+                }
+            }
+        }
+        if (feof($upstream)) {
+            break;
+        }
+        $read = [$upstream, $conn];
+    }
+
+    phpt_wait();
+CODE;
+
+include 'CertificateGenerator.inc';
+$certificateGenerator = new CertificateGenerator();
+$certificateGenerator->saveCaCert($cacertFile);
+$certificateGenerator->saveNewCertAsFileWithKey($peerName, $certFile);
+
+include 'ServerClientTestCase.inc';
+ServerClientTestCase::getInstance()->run($clientCode, [
+    'server' => $serverCode,
+    'proxy' => $proxyCode,
+]);
+?>
+--CLEAN--
+<?php
+@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh10495.pem.tmp');
+@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh10495-ca.pem.tmp');
+?>
+--EXPECT--
+bool(false)
+bool(true)
+string(6) "warmup"
diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c
@@ -2484,21 +2484,96 @@ static int php_openssl_sockop_set_option(php_stream *stream, int option, int val
 					/* the poll() call was skipped if the socket is non-blocking (or MSG_DONTWAIT is available) and if the timeout is zero */
 					/* additionally, we don't use this optimization if SSL is active because in that case, we're not using MSG_DONTWAIT */
 					if (sslsock->ssl_active) {
-						int n = SSL_peek(sslsock->ssl_handle, &buf, sizeof(buf));
-						if (n <= 0) {
-							int err = SSL_get_error(sslsock->ssl_handle, n);
-							switch (err) {
-								case SSL_ERROR_SYSCALL:
-									alive = php_socket_errno() == EAGAIN;
-									break;
-								case SSL_ERROR_WANT_READ:
-								case SSL_ERROR_WANT_WRITE:
-									alive = 1;
+						int retry = 1;
+						struct timeval start_time;
+						struct timeval *timeout = NULL;
+						int began_blocked = sslsock->s.is_blocked;
+						int has_timeout = 0;
+
+						/* never use a timeout with non-blocking sockets */
+						if (began_blocked) {
+							timeout = &tv;
+						}
+
+						if (timeout && php_set_sock_blocking(sslsock->s.socket, 0) == SUCCESS) {
+							sslsock->s.is_blocked = 0;
+						}
+
+						if (!sslsock->s.is_blocked && timeout && (timeout->tv_sec > 0 || (timeout->tv_sec == 0 && timeout->tv_usec))) {
+							has_timeout = 1;
+							/* gettimeofday is not monotonic; using it here is not strictly correct */
+							gettimeofday(&start_time, NULL);
+						}
+
+						/* Main IO loop. */
+						do {
+							struct timeval cur_time, elapsed_time, left_time;
+
+							/* If we have a timeout to check, figure out how much time has elapsed since we started. */
+							if (has_timeout) {
+								gettimeofday(&cur_time, NULL);
+
+								/* Determine how much time we've taken so far. */
+								elapsed_time = php_openssl_subtract_timeval(cur_time, start_time);
+
+								/* and return an error if we've taken too long. */
+								if (php_openssl_compare_timeval(elapsed_time, *timeout) > 0 ) {
+									/* If the socket was originally blocking, set it back. */
+									if (began_blocked) {
+										php_set_sock_blocking(sslsock->s.socket, 1);
+										sslsock->s.is_blocked = 1;
+									}
+									sslsock->s.timeout_event = 1;
+									return PHP_STREAM_OPTION_RETURN_ERR;
+								}
+							}
+
+							int n = SSL_peek(sslsock->ssl_handle, &buf, sizeof(buf));
+							/* If we didn't do anything on the last loop (or an error) check to see if we should retry or exit. */
+							if (n <= 0) {
+								/* Now, do the IO operation. Don't block if we can't complete... */
+								int err = SSL_get_error(sslsock->ssl_handle, n);
+								switch (err) {
+									case SSL_ERROR_SYSCALL:
+										retry = php_socket_errno() == EAGAIN;
+										break;
+									case SSL_ERROR_WANT_READ:
+									case SSL_ERROR_WANT_WRITE:
+										retry = 1;
+										break;
+									default:
+										/* any other problem is a fatal error */
+										retry = 0;
+								}
+
+								/* Don't loop indefinitely in non-blocking mode if no data is available */
+								if (began_blocked == 0 || !has_timeout) {
+									alive = retry;
 									break;
-								default:
-									/* any other problem is a fatal error */
-									alive = 0;
+								}
+
+								/* Now, if we have to wait some time, and we're supposed to be blocking, wait for the socket to become
+								* available. Now, php_pollfd_for uses select to wait up to our time_left value only...
+								*/
+								if (retry) {
+									/* Now, how much time until we time out? */
+									left_time = php_openssl_subtract_timeval(*timeout, elapsed_time);
+									if (php_pollfd_for(sslsock->s.socket, PHP_POLLREADABLE|POLLPRI|POLLOUT, has_timeout ? &left_time : NULL) <= 0) {
+										retry = 0;
+										alive = 0;
+									};
+								}
+							} else {
+								retry = 0;
+								alive = 1;
 							}
+							/* Finally, we keep going until there are any data or there is no time to wait. */
+						} while (retry);
+
+						if (began_blocked && !sslsock->s.is_blocked) {
+							// Set it back to blocking
+							php_set_sock_blocking(sslsock->s.socket, 1);
+							sslsock->s.is_blocked = 1;
 						}
 					} else if (0 == recv(sslsock->s.socket, &buf, sizeof(buf), MSG_PEEK|MSG_DONTWAIT) && php_socket_errno() != EAGAIN) {
 						alive = 0;