Edit report at https://bugs.php.net/bug.php?id=72333&edit=1
ID: 72333
User updated by: webmaster_20160604 at cubiclesoft dot com
Reported by: webmaster_20160604 at cubiclesoft dot com
Summary: fwrite() on non-blocking SSL sockets doesn't work
after SSL_ERROR_WANT_...
Status: Open
Type: Bug
Package: Streams related
Operating System: All
PHP Version: 5.6.22
Block user comment: N
Private report: N
New Comment:
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.
Previous Comments:
------------------------------------------------------------------------
[2016-06-04 09:45:18] 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].
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=72333&edit=1