Edit report at https://bugs.php.net/bug.php?id=79501&edit=1
ID: 79501
Updated by: bukka@php.net
Reported by: imnieves at gmail dot com
Summary: TLS connections freezing on 7.4 (all versions after
7.3.17)
Status: Re-Opened
Type: Bug
Package: OpenSSL related
Operating System: Linux
PHP Version: 7.4.5
Assigned To: bukka
Block user comment: N
Private report: N
New Comment:
As I just mentioned in the Redis issue thread
( https://github.com/phpredis/phpredis/issues/1726#issuecomment-1367875600
) and as was correctly mentioned here before by the OP, there is an issue with the liveness checking
as the poll is only readable. The problem is the freezing and not respecting the socket timeout.
Currently the only solution that I can think of is to do a switch to non blocking for eof check
which we do in other places. We can't really know when SSL_peek blocks otherwise - neither
SSL_pending nor poll on read is reliable here. I will need to think about it and also think how we
could cover it by test as it will be challenging too.
Previous Comments:
------------------------------------------------------------------------
[2022-12-29 15:53:42] bukka@php.net
I have done some investigation and updated the linked issue in phpredis as this might potentially be
a problem with API usage there - it calls php_stream_eof before php_stream_write.
------------------------------------------------------------------------
[2022-02-06 07:57:21] imnieves at gmail dot com
This bug has also noticed the same underlying problem:
https://github.com/reactphp/socket/issues/184
The problem is that when a server has sent no data, a client side call to feof or stream_eof will
block when TLS 1.3 is used and will not block when TLS 1.2 is used.
I am quite confident my fix/workaround above is not correct.
------------------------------------------------------------------------
[2022-02-02 22:29:45] imnieves at gmail dot com
Here is the second part of the message.
I was able to get TLS 1.3 working in PHP 7.4 by wrapping the SSL_peek function call and the logic
immediately following that call with a call to SSL_pending. SSL_pending returns the count of data
bytes that can be read, and these are application data bytes not the non-application data records,
so if SSL_pending > 0 then SSL_peek will not block. Beyond that, SSL_pending itself will not
block, so it is safe to call.
To show what I did, I converted these lines from:
https://github.com/php/php-src/blob/96f753a2b56b7c4927f1a64253ca60bb481ee2c3/ext/openssl/xp_ssl.c#L2466
int n = SSL_peek(sslsock->ssl_handle, &buf, sizeof(buf));
if (n <= 0) {
int err = SSL_get_error(sslsock->ssl_handle, n);
switch (err) {
case SSL_ERROR_SYSCALL:
alive = php_socket_errno() == EAGAIN;
break;
case SSL_ERROR_WANT_READ:
case SSL_ERROR_WANT_WRITE:
alive = 1;
break;
default:
/* any other problem is a fatal error */
alive = 0;
}
}
to these lines:
if(SSL_pending(sslsock->ssl_handle)){
int n = SSL_peek(sslsock->ssl_handle, &buf, sizeof(buf));
if (n <= 0) {
int err = SSL_get_error(sslsock->ssl_handle, n);
switch (err) {
case SSL_ERROR_SYSCALL:
alive = php_socket_errno() == EAGAIN;
break;
case SSL_ERROR_WANT_READ:
case SSL_ERROR_WANT_WRITE:
alive = 1;
break;
default:
/* any other problem is a fatal error */
alive = 0;
}
}
}
I am not claiming this fixes the problem entirely or that there are no side-effects, as I did not
run any automated tests or any other manual tests. But this seemingly improved behavior in our
particular use case seems to increase the likelihood of the location of the flawed code.
In summary, I am thinking the issue is related how PHP streams wraps OpenSSL. The issue seems to be
the incorrect assumption that if records are available to read then SSL_peek will not block.
Finally, although this issue seems to arise mostly in TLS 1.3, this is quite likely also an issue in
PHP streams on TLS 1.2, although very (possibly very very) rare.
For reference on that last sentence, see the first comment by mattcaswell at:
https://github.com/openssl/openssl/issues/7327
Further reference:
https://www.openssl.org/docs/man1.1.1/man3/SSL_pending.htmlhttps://www.openssl.org/docs/man1.1.1/man3/SSL_read.html
------------------------------------------------------------------------
[2022-02-02 22:28:54] imnieves at gmail dot com
I will break this message into two parts, hopefully that will help this message to not be detected
as SPAM.
I was able to determine the exact line that is blocking in PHP series 7.4 (7.4..27 precisely) when
TLS 1.3 is being used.
The offending line is:
https://github.com/php/php-src/blob/96f753a2b56b7c4927f1a64253ca60bb481ee2c3/ext/openssl/xp_ssl.c#L2466
int n = SSL_peek(sslsock->ssl_handle, &buf, sizeof(buf));
For reference, the PHP method that calls this SSL_peek is itself called at:
https://github.com/php/php-src/blob/96f753a2b56b7c4927f1a64253ca60bb481ee2c3/main/streams/streams.c#L790
if (!stream->eof && PHP_STREAM_OPTION_RETURN_ERR ==
php_stream_set_option(stream, PHP_STREAM_OPTION_CHECK_LIVENESS,
0, NULL)) {
stream->eof = 1;
}
After reading about so-called SSL non-application data records:
https://github.com/openssl/openssl/blob/6e94b5aecd619afd25e3dc25902952b1b3194edf/CHANGES#L237https://wiki.openssl.org/index.php/TLS1.3#Non-application_data_records
openssl/openssl#7327
I believe the blocking is occurring because PHP 7.4 is assuming (in xp_ssl.c @ L2466, the first code
snippet above) that if there is a record that can be read, then the record must be a data record and
therefore the SSL_peek will not block. This assumption is false in both TLS 1.2 and TLS 1.3, however
the impact of this assumption is felt rarely (if at all) in TLS 1.2 but it arises frequently in TLS
1.3, in fact it arises instantly in TLS 1.3.
In TLS 1.2, this line:
https://github.com/php/php-src/blob/96f753a2b56b7c4927f1a64253ca60bb481ee2c3/ext/openssl/xp_ssl.c#L2464
} else if (php_pollfd_for(sslsock->s.socket, PHP_POLLREADABLE|POLLPRI, &tv) > 0) {
evaluates to false.
But in TLS 1.3 it evaluates to true, which enables SSL_peek to be called, and block.
------------------------------------------------------------------------
[2020-08-08 17:58:30] imnieves at gmail dot com
According to comment from a maintainer of PhpRedis[1], they feel that the issue is not coming from
PhpRedis itself:
| I'm going to close this issue because it is actually not a bug in phpredis.
[1] https://github.com/phpredis/phpredis/issues/1726#issuecomment-652563196
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=79501
--
Edit this bug report at https://bugs.php.net/bug.php?id=79501&edit=1