Skip to content

Commit 7150b38

Browse files
committed
Fix GH-22081: Memory leak in php_openssl_enable_crypto
This was leaking because php_openssl_enable_crypto can be called multiple times. The reneg was moved there after the session changes so it needs to only happen once there. The fix moves it (and some other parts that should be done just once) inside state_set block where it can run only once.
1 parent f2b371e commit 7150b38

2 files changed

Lines changed: 88 additions & 17 deletions

File tree

ext/openssl/tests/gh22081.phpt

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
--TEST--
2+
GH-22081: server reneg limit not reallocated across non-blocking retries
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 . 'gh22081-server-reneg-nonblocking.pem.tmp';
12+
13+
$serverCode = <<<'CODE'
14+
$flags = STREAM_SERVER_BIND|STREAM_SERVER_LISTEN;
15+
$ctx = stream_context_create(['ssl' => [
16+
'local_cert' => '%s',
17+
]]);
18+
19+
/* Plain TCP listener so the TLS handshake is driven manually below. */
20+
$server = stream_socket_server('tcp://127.0.0.1:0', $errno, $errstr, $flags, $ctx);
21+
phpt_notify_server_start($server);
22+
23+
/* Complete each handshake in non-blocking mode so that php_openssl_enable_crypto() is
24+
* re-entered across multiple WANT_READ/WANT_WRITE rounds per connection. */
25+
for ($i = 0; $i < 5; $i++) {
26+
$conn = @stream_socket_accept($server, 30);
27+
if (!$conn) {
28+
continue;
29+
}
30+
stream_set_blocking($conn, false);
31+
do {
32+
$r = stream_socket_enable_crypto($conn, true, STREAM_CRYPTO_METHOD_TLS_SERVER);
33+
} while ($r === 0);
34+
35+
if ($r === true) {
36+
fwrite($conn, "ok $i\n");
37+
}
38+
fclose($conn);
39+
}
40+
CODE;
41+
$serverCode = sprintf($serverCode, $certFile);
42+
43+
$clientCode = <<<'CODE'
44+
$flags = STREAM_CLIENT_CONNECT;
45+
$ctx = stream_context_create(['ssl' => [
46+
'verify_peer' => false,
47+
'verify_peer_name' => false,
48+
]]);
49+
50+
for ($i = 0; $i < 5; $i++) {
51+
$client = stream_socket_client("tls://{{ ADDR }}", $errno, $errstr, 30, $flags, $ctx);
52+
if ($client) {
53+
echo trim(fgets($client)) . "\n";
54+
fclose($client);
55+
}
56+
}
57+
CODE;
58+
59+
include 'CertificateGenerator.inc';
60+
$certificateGenerator = new CertificateGenerator();
61+
$certificateGenerator->saveNewCertAsFileWithKey('gh22081-server-reneg-nonblocking', $certFile);
62+
63+
include 'ServerClientTestCase.inc';
64+
ServerClientTestCase::getInstance()->run($clientCode, $serverCode);
65+
?>
66+
--CLEAN--
67+
<?php
68+
@unlink(__DIR__ . DIRECTORY_SEPARATOR . 'gh22081-server-reneg-nonblocking.pem.tmp');
69+
?>
70+
--EXPECT--
71+
ok 0
72+
ok 1
73+
ok 2
74+
ok 3
75+
ok 4
76+

ext/openssl/xp_ssl.c

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2688,28 +2688,23 @@ static int php_openssl_enable_crypto(php_stream *stream,
26882688
struct timeval start_time, *timeout;
26892689
bool blocked = sslsock->s.is_blocked, has_timeout = false;
26902690

2691-
if (sslsock->is_client) {
2692-
/* Set session data for client */
2693-
if ( php_openssl_apply_client_session_data(stream, sslsock)) {
2694-
return FAILURE;
2695-
}
2696-
#ifdef HAVE_TLS_SNI
2697-
php_openssl_enable_client_sni(stream, sslsock);
2698-
#endif
2699-
} else {
2700-
php_openssl_init_server_reneg_limit(stream, sslsock);
2701-
}
2702-
2691+
if (!sslsock->state_set) {
27032692
#ifdef PHP_OPENSSL_TLS_DEBUG
2704-
BIO *b_out = BIO_new_fp(stdout, BIO_NOCLOSE | BIO_FP_TEXT);
2705-
SSL_set_msg_callback(sslsock->ssl_handle, SSL_trace);
2706-
SSL_set_msg_callback_arg(sslsock->ssl_handle, b_out);
2693+
BIO *b_out = BIO_new_fp(stdout, BIO_NOCLOSE | BIO_FP_TEXT);
2694+
SSL_set_msg_callback(sslsock->ssl_handle, SSL_trace);
2695+
SSL_set_msg_callback_arg(sslsock->ssl_handle, b_out);
27072696
#endif
2708-
2709-
if (!sslsock->state_set) {
27102697
if (sslsock->is_client) {
2698+
/* Set session data for client */
2699+
if (php_openssl_apply_client_session_data(stream, sslsock) == FAILURE) {
2700+
return -1;
2701+
}
2702+
#ifdef HAVE_TLS_SNI
2703+
php_openssl_enable_client_sni(stream, sslsock);
2704+
#endif
27112705
SSL_set_connect_state(sslsock->ssl_handle);
27122706
} else {
2707+
php_openssl_init_server_reneg_limit(stream, sslsock);
27132708
SSL_set_accept_state(sslsock->ssl_handle);
27142709
}
27152710
sslsock->state_set = 1;

0 commit comments

Comments
 (0)