Skip to content

Commit 614c628

Browse files
authored
Fix TLS stream EOF detection after close_notify with stale errno (#24132)
php_openssl_handle_ssl_error() sets errno to EAGAIN on SSL_ERROR_WANT_READ and SSL_ERROR_WANT_WRITE, and php_openssl_sockop_io() reads it back to avoid marking a non-blocking read that needs to wait as EOF. OpenSSL resets errno before every recv() on POSIX systems, but on Windows it uses the Winsock error state instead and never touches errno, so the EAGAIN stays set across every later successful read. A close_notify received afterwards returned SSL_ERROR_ZERO_RETURN but did not set stream->eof, and feof() stayed false while the TCP connection was still open. Decide EOF from the SSL error code, which already says whether the operation just needs to wait, instead of from errno.
1 parent aaa8707 commit 614c628

3 files changed

Lines changed: 64 additions & 2 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ PHP NEWS
120120
- OpenSSL:
121121
. Fixed stream_socket_enable_crypto() leaving the socket non-blocking
122122
after a handshake timeout. (Ilia Alshanetsky)
123+
. Fixed feof() on a TLS stream staying false after a close_notify on Windows.
124+
(Jakub Zelenka)
123125

124126
- PCNTL:
125127
. Fixed pcntl_signal_dispatch() dropping the queued signals when it runs while
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
--TEST--
2+
feof() is true after a TLS close_notify even if an earlier read left errno set to EAGAIN
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_eof_after_close_notify.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+
/* Let the client block in fread() first, so its SSL_read() sees WANT_READ */
21+
phpt_wait();
22+
usleep(100000);
23+
fwrite($link, "data");
24+
/* close_notify only, the TCP connection stays open */
25+
stream_socket_enable_crypto($link, false);
26+
phpt_wait();
27+
fclose($link);
28+
CODE;
29+
$serverCode = sprintf($serverCode, $certFile);
30+
31+
$clientCode = <<<'CODE'
32+
$clientCtx = stream_context_create(['ssl' => [
33+
'verify_peer' => false,
34+
'verify_peer_name' => false,
35+
]]);
36+
$sock = stream_socket_client("tls://{{ ADDR }}", $errno, $errstr, 2, STREAM_CLIENT_CONNECT, $clientCtx);
37+
38+
phpt_notify();
39+
var_dump(fread($sock, 4));
40+
var_dump(fread($sock, 4));
41+
var_dump(feof($sock));
42+
phpt_notify();
43+
CODE;
44+
45+
include 'CertificateGenerator.inc';
46+
(new CertificateGenerator())->saveNewCertAsFileWithKey('stream_eof_after_close_notify', $certFile);
47+
48+
include 'ServerClientTestCase.inc';
49+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
50+
?>
51+
--CLEAN--
52+
<?php
53+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'stream_eof_after_close_notify.pem.tmp');
54+
?>
55+
--EXPECT--
56+
string(4) "data"
57+
string(0) ""
58+
bool(true)

‎ext/openssl/xp_ssl.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2186,9 +2186,11 @@ static ssize_t php_openssl_sockop_io(int read, php_stream *stream, char *buf, si
21862186
retry = 1;
21872187
}
21882188

2189-
/* Also, on reads, we may get this condition on an EOF. We should check properly. */
21902189
if (read) {
2191-
stream->eof = (retry == 0 && errno != EAGAIN && !SSL_pending(sslsock->ssl_handle));
2190+
/* EOF unless the SSL layer just needs to wait. */
2191+
stream->eof = (retry == 0
2192+
&& err != SSL_ERROR_WANT_READ && err != SSL_ERROR_WANT_WRITE
2193+
&& !SSL_pending(sslsock->ssl_handle));
21922194
}
21932195

21942196
/* Don't loop indefinitely in non-blocking mode if no data is available */

0 commit comments

Comments
 (0)