From c1bd9a932a55d2efba64126118477de7055b1c2e Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Thu, 21 Dec 2023 20:12:02 +0000 Subject: [PATCH] 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 --- NEWS | 4 ++ ext/openssl/tests/gh10495.phpt | 116 +++++++++++++++++++++++++++++++++ ext/openssl/xp_ssl.c | 103 +++++++++++++++++++++++++---- 3 files changed, 209 insertions(+), 14 deletions(-) create mode 100644 ext/openssl/tests/gh10495.phpt diff --git a/NEWS b/NEWS index b3a9c4963ff..d7b492e6c37 100644 --- 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 new file mode 100644 index 00000000000..3ed5a3d384b --- /dev/null +++ b/ext/openssl/tests/gh10495.phpt @@ -0,0 +1,116 @@ +--TEST-- +GH-10495: feof hangs indefinitely +--EXTENSIONS-- +openssl +--SKIPIF-- + +--FILE-- + ['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-- + +--EXPECT-- +bool(false) +bool(true) +string(6) "warmup" diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 2e21c138d6b..e01d53656fe 100644 --- 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; - break; - default: - /* any other problem is a fatal error */ - alive = 0; + 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; + } + + /* 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;