php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #72333 fwrite() on non-blocking SSL sockets doesn't work
Submitted: 2016-06-04 09:45 UTC Modified: 2017-03-14 18:16 UTC
Votes:12
Avg. Score:5.0 ± 0.0
Reproduced:12 of 12 (100.0%)
Same Version:3 (25.0%)
Same OS:9 (75.0%)
From: webmaster_20160604 at cubiclesoft dot com Assigned: bukka (profile)
Status: Closed Package: OpenSSL related
PHP Version: 5.6.22 OS: All
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: webmaster_20160604 at cubiclesoft dot com
New email:
PHP Version: OS:

 

 [2016-06-04 09:45 UTC] webmaster_20160604 at cubiclesoft dot com
Description:
------------
The userland test script demonstrates the problem.  A large fwrite() call ends up returning a $result smaller than strlen($str) when non-blocking sockets are used.  For regular TCP sockets, this isn't a huge deal - just lop off the bytes sent and try again later with the remaining data when stream_select() returns.  However, that becomes a serious problem when a SSL-enabled non-blocking socket is used to attempt to send a large amount of data.

Behind the scenes in the stream handler function php_openssl_sockop_write() (ext\openssl\xp_ssl.c), SSL_write()/SSL_get_error() returns an error code of SSL_ERROR_WANT_READ or SSL_ERROR_WANT_WRITE when the first fwrite() function is called.  When the socket is non-blocking, the number of retries returned is 0 and therefore the function immediately returns.  Unfortunately, this has the unintended effect of breaking the underlying stream even though I don't precisely know how that happens - from the userland perspective, $str never changes so how the pointers change is a bit of a mystery.  I digress.

Ultimately, the real culprit is SSL_write().  By default, it wants the same EXACT buffer (i.e. pointer) and buffer length when it is called again.  When blocking sockets are used, the php_openssl_sockop_write() function simply retries the SSL_write() call continuously until it succeeds and therefore there is no issue.  However, when non-blocking sockets are used, the function returns to the caller but effectively destroys the socket since there is no apparent way to pass the same buffer to SSL_write() ever again.  Attempting to call fwrite() with the same input as before results in a return value of 0 and this PHP warning is emitted:

PHP Warning:  fwrite(): SSL operation failed with code 1. OpenSSL Error messages:
error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry in [FILE] on line [LINE].

Even though the value of $str never changes from the userland perspective in the example, the 'const char *buf' pointer is effectively different by the time it reaches the second SSL_write() call, which subsequently fails when it detects the difference, which is typically where the 'bad write retry' error comes from.

One possible solution could be to call:

SSL_CTX_set_mode(ctx, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);

Or:

SSL_set_mode(ssl, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);

During setup of the SSL context or later after the SSL object is initialized.  Even if $str relocates elsewhere in memory, it won't be a problem with the moving write buffer option and the partial write option allows for the data that was sent to be dropped just like non-SSL non-blocking sockets.  But you might want to read the manpages on those options as the authors have written a few "caveats".

To avoid conflicts with the current code that works fine for blocking SSL sockets, those two options should probably only be set when non-blocking SSL sockets are in use and adjusted when the blocking mode changes to reduce the potential for severe breakage with blocking SSL sockets.

It would also be extremely helpful to know which error type (i.e. WANT_READ/WANT_WRITE) was returned from SSL_write()/SSL_get_error() on failure so the socket can be passed into the correct stream_select() array.  It's a related bug in that the details about the error aren't available to userland (EAGAIN isn't sufficient to determine read vs. write) and therefore fwrite() subsequently reduces the performance of stream_select() for non-blocking SSL sockets (i.e. not knowing wastes CPU cycles in a busy loop).

Test script:
---------------
<?php
	$context = stream_context_create();
	$fp = stream_socket_client("tls://www.google.com:443", $errornum, $errorstr, 300, STREAM_CLIENT_CONNECT, $context);
var_dump($fp);
var_dump($errornum);
var_dump($errorstr);

	stream_set_blocking($fp, 0);

	$str = "GET / HTTP/1.1\r\n";
	$str .= str_repeat("a", 1048576);

	$result = fwrite($fp, $str);
var_dump($result);

	$result = fwrite($fp, $str);
var_dump($result);
?>

Expected result:
----------------
The first fwrite() to not break the underlying stream.

The second fwrite() to not fail with a PHP Warning.

Actual result:
--------------
The first fwrite() breaks the underlying stream due to the loss of a valid pointer to pass to SSL_write() later to resume operations.

The second fwrite() subsequently fails with the PHP Warning:

PHP Warning:  fwrite(): SSL operation failed with code 1. OpenSSL Error messages:
error:1409F07F:SSL routines:SSL3_WRITE_PENDING:bad write retry in [FILE] on line [LINE].


Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-06-04 23:48 UTC] webmaster_20160604 at cubiclesoft dot com
Minor correction.  I was looking at the source code of 5.6.8 when I posted my original message (it's what I had on hand at the time, sorry).  I'm now looking at the source code of 5.6.22.  The same problem still applies - it has just migrated to php_openssl_sockop_io() instead of php_openssl_sockop_write().  This new function has a couple of minor additional issues I noticed:

if (nr_bytes <= 0) {
  /* Get the error code from SSL, and check to see if it's an error or not. */
  int err = SSL_get_error(sslsock->ssl_handle, nr_bytes );
  retry = handle_ssl_error(stream, nr_bytes, 0 TSRMLS_CC);

  /* If we get this (the above doesn't check) then we'll retry as well. */
  if (errno == EAGAIN && err == SSL_ERROR_WANT_READ && read) {
    retry = 1;
  }
  if (errno == EAGAIN && err == SSL_ERROR_WANT_WRITE && read == 0) {
    retry = 1;
  }

...

SSL_write() can return both SSL_ERROR_WANT_WRITE *and* SSL_ERROR_WANT_READ.  It's counter-intuitive, but if the client or the server are in the process of renegotiating something, the caller can get SSL_ERROR_WANT_READ from SSL_write().  Shouldn't 'retry' be set to 1 if either SSL_ERROR_WANT_WRITE or SSL_ERROR_WANT_READ is received regardless of the value of 'read'?

...

  if (retry) {
    if (read) {
      php_pollfd_for(sslsock->s.socket, (err == SSL_ERROR_WANT_WRITE) ?
        (POLLOUT|POLLPRI) : (POLLIN|POLLPRI), has_timeout ? &left_time : NULL);
    } else {
      php_pollfd_for(sslsock->s.socket, (err == SSL_ERROR_WANT_READ) ?
        (POLLIN|POLLPRI) : (POLLOUT|POLLPRI), has_timeout ? &left_time : NULL);
    }
  }

...

The inner if statement also appears to contain two logically identical function calls to php_pollfd_for().  The value of 'err' can only be SSL_ERROR_WANT_WRITE or SSL_ERROR_WANT_READ at that point in the code.

At any rate, that's not why I went to go get the latest source code.  Here's a possible implementation to the main SSL_write() issue with non-blocking sockets:

Current code:

  } else {
    nr_bytes = SSL_write(sslsock->ssl_handle, buf, count);
  }

Change to:

  } else {
    if (began_blocked == 0)  SSL_set_mode(sslsock->ssl_handle, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);

    nr_bytes = SSL_write(sslsock->ssl_handle, buf, count);
  }

And document that non-blocking SSL sockets may break on fwrite() if they are switched back from non-blocking to blocking mode with stream_set_blocking() after fwrite() has been called while in non-blocking mode (an even rarer case where the person probably has absolutely no idea what they doing with non-blocking sockets/files/etc).  There doesn't appear to be a way to turn off a SSL write mode for a context once it has been turned on, so it is imperative to wait until the absolute last moment before SSL_write() to enable those options.  I'm okay with whatever performance loss there is for calling SSL_set_mode() repeatedly in exchange for fully functional non-blocking SSL sockets.  That's the simplest one-line fix I can come up with.

An alternative approach, and one that would involve a rather significant rewrite, would be to copy input data to an internal queue and directly maintain those pointers so that SSL_write() always sees the same inputs.  That sort of solution would require allocating RAM and tracking pointers, which would create a mess of logic to properly maintain.  It was my original thought until I stumbled upon the SSL_set_mode() call.
 [2016-11-13 15:11 UTC] bukka@php.net
-Package: Streams related +Package: OpenSSL related
 [2016-12-05 13:52 UTC] bogdan at yurov dot me
Hi,

Are there any workarounds or news on this?
 [2016-12-07 15:09 UTC] webmaster_20160604 at cubiclesoft dot com
Nothing has changed (yet).  The only userland "workaround" (hack) is to temporarily switch a non-blocking SSL socket to blocking, send a smaller amount of data (e.g. 4KB), and then re-enable non-blocking mode.  This effectively makes reading data non-blocking but writing data will potentially block.  You just have to hope that no one attempts to exploit this (e.g. a SSL enabled server written in PHP).

You can also try compiling PHP after patching php_openssl_sockop_io() with the suggested fix.
 [2017-01-02 20:32 UTC] bukka@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: bukka
 [2017-01-02 20:32 UTC] bukka@php.net
First of all, really nice and helpful description.

I played a bit with the changed mode and finally put together my local server and client tests that can be used to recreate the issue locally:

https://github.com/bukka/php-util/tree/78f448a87b2cd557144dc3155e647df8ed67632b/tests/openssl/asyncw

It seems to work fine but will need to think about it a bit more and test it a bit more as well.
 [2017-03-14 18:17 UTC] bukka@php.net
-Summary: fwrite() on non-blocking SSL sockets doesn't work after SSL_ERROR_WANT_... +Summary: fwrite() on non-blocking SSL sockets doesn't work
 [2017-03-14 18:44 UTC] bukka@php.net
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=17e9fc9bfeaf575b25782a42937a56e809155858
Log: Fix bug #72333 (fwrite() on non-blocking SSL sockets does not work)
 [2017-03-14 18:44 UTC] bukka@php.net
-Status: Assigned +Status: Closed
 [2017-03-14 18:46 UTC] bukka@php.net
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=17e9fc9bfeaf575b25782a42937a56e809155858
Log: Fix bug #72333 (fwrite() on non-blocking SSL sockets does not work)
 [2017-03-14 18:48 UTC] bukka@php.net
Automatic comment on behalf of bukka
Revision: http://git.php.net/?p=php-src.git;a=commit;h=17e9fc9bfeaf575b25782a42937a56e809155858
Log: Fix bug #72333 (fwrite() on non-blocking SSL sockets does not work)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 13:01:29 2024 UTC