php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #45808 [PATCH] stream_socket_enable_crypto() blocks and eats CPU
Submitted: 2008-08-13 17:41 UTC Modified: 2010-06-20 18:33 UTC
Votes:24
Avg. Score:4.8 ± 0.5
Reproduced:22 of 22 (100.0%)
Same Version:14 (63.6%)
Same OS:20 (90.9%)
From: six at aegis-corp dot org Assigned: pajoye (profile)
Status: Closed Package: Streams related
PHP Version: 5.*, 6 OS: Linux 2.6
Private report: No CVE-ID: None
 [2008-08-13 17:41 UTC] six at aegis-corp dot org
Description:
------------
The documentation says about stream_socket_enable_crypto :

Returns TRUE on success, FALSE if negotiation has failed or 0 if there isn't enough data and you should try again (only for non-blocking sockets). 

In practice, if you feed a non blocking server socket to it, it will block and consume lots of CPU until the SSL/TLS handshake is done or the client connection is dropped.

Reproduce code:
---------------
<?php

$s = stream_socket_server("tcp://127.0.0.1:8888");
$c = stream_socket_accept($s);
stream_set_blocking($c, false);

$ret = stream_socket_enable_crypto($c, true, STREAM_CRYPTO_METHOD_TLS_SERVER);

var_dump($ret);

?>

then just "telnet localhost 8888" from another term

Expected result:
----------------
script should print "int(0)" and exit

Actual result:
--------------
script blocks at the stream_socket_enable_crypto() call and is stuck in a CPU consuming loop until the client connection is either handshaked or dropped.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2008-09-25 10:06 UTC] six at aegis-corp dot org
the bug is still present in php5.3-200809232030
 [2008-09-25 16:07 UTC] nasam at mailvault dot com
Bug is in ext/openssl/xp_ssl.c
Function handle_ssl_error: (line 107)
case SSL_ERROR_WANT_READ:
case SSL_ERROR_WANT_WRITE:
       /* re-negotiation, or perhaps the SSL layer needs more
       * packets: retry in next iteration */
       errno = EAGAIN;
       retry = is_init ? 1 : sslsock->s.is_blocked; //BUG
       break;

it sets retry to 1 in php_openssl_enable_crypto no matter if socket is blocking or not.
 [2008-09-25 17:59 UTC] singularity_control at rcpt dot at
This makes a serious security issue. It is a very effective DoS on
all single process PHP servers with SSL and a slightly less bad DoS on multi-process PHP servers.
 [2008-10-30 11:03 UTC] xl269 at cam dot ac dot uk
just to confirm that this bug still exists in php5.3-200810292330
 [2009-08-18 16:15 UTC] garretts@php.net
FYI: 
I can't repro this on Windows with the build off the snaps' box (VC9 x86 Non Thread Safe (2009-Aug-18 16:00:00)). 

It: 
  blocks until connection using telnet[expected]
  doens't consume any CPU[expected]
  and returns 'bool(false)' [expected -- I assume the same as 'int(0)']
  and exits[expected]  

G
 [2009-10-12 20:50 UTC] vincent at optilian dot com
Here is a patch to fix this issue (diff against 5.3.0)

As far as I have tested, everything works as expected with this patch applied.

--- xp_ssl.c.orig	2009-10-12 19:34:31.000000000 +0200
+++ xp_ssl.c	2009-10-12 20:39:19.000000000 +0200
@@ -299,8 +299,12 @@
 	SSL_METHOD *method;
 	
 	if (sslsock->ssl_handle) {
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "SSL/TLS already set-up for this stream");
-		return -1;
+		if (sslsock->is_client) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "SSL/TLS already set-up for this stream");
+			return -1;
+		} else {
+			return 0;
+		}
 	}
 
 	/* need to do slightly different things, based on client/server method,
@@ -415,7 +419,7 @@
 			}
 
 			if (n <= 0) {
-				retry = handle_ssl_error(stream, n, 1 TSRMLS_CC);
+				retry = handle_ssl_error(stream, n, sslsock->is_client TSRMLS_CC);
 			} else {
 				break;
 			}
 [2009-10-13 10:45 UTC] vincent at optilian dot com
Actually I fixed some things in the patch, see below ...

It makes more sense to test whether the socket is in blocking mode, even if a client ssl socket doesn't need multiple calls to stream_socket_enable_crypto()

--- xp_ssl.c.orig	2009-10-12 19:34:31.000000000 +0200
+++ xp_ssl.c	2009-10-13 12:30:24.000000000 +0200
@@ -299,8 +299,12 @@
 	SSL_METHOD *method;
 	
 	if (sslsock->ssl_handle) {
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "SSL/TLS already set-up for this stream");
-		return -1;
+		if (sslsock->s.is_blocked) {
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "SSL/TLS already set-up for this stream");
+			return -1;
+		} else {
+			return 0;
+		}
 	}
 
 	/* need to do slightly different things, based on client/server method,
@@ -415,7 +419,7 @@
 			}
 
 			if (n <= 0) {
-				retry = handle_ssl_error(stream, n, 1 TSRMLS_CC);
+				retry = handle_ssl_error(stream, n, sslsock->is_client || sslsock->s.is_blocked TSRMLS_CC);
 			} else {
 				break;
 			}
 [2010-06-13 16:20 UTC] felipe@php.net
-Summary: stream_socket_enable_crypto() blocks and eats CPU +Summary: [PATCH] stream_socket_enable_crypto() blocks and eats CPU
 [2010-06-13 16:20 UTC] felipe@php.net
Pierre, what about this patch?
 [2010-06-20 18:33 UTC] pajoye@php.net
Automatic comment from SVN on behalf of pajoye
Revision: http://svn.php.net/viewvc/?view=revision&amp;revision=300617
Log: - #45808, stream_socket_enable_crypto() blocks and eats CPU
 [2010-06-20 18:33 UTC] pajoye@php.net
-Status: Assigned +Status: Closed
 [2010-06-20 18:33 UTC] pajoye@php.net
This bug has been fixed in SVN.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
 
Thank you for the report, and for helping us make PHP better.


 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 08:01:29 2024 UTC